Unsubscribe original territory owner on transfer or unarchive #1791
Conversation
|
|
||
| // unsubscribe the old user | ||
| const oldSubscription = await tx.subSubscription.findUnique({ where: { userId_subName: { userId: oldUserId, subName: name } } }) | ||
| if (oldSubscription) await tx.subSubscription.delete({ where: { userId_subName: { subName: name, userId: oldUserId } } }) |
There was a problem hiding this comment.
do you prefer this or deleteMany?
There was a problem hiding this comment.
I think delete should be fine. But do you need to check first if it exists?
There was a problem hiding this comment.
yes, delete will error if the record does not exist (eg. if the user unsubscribed manually before transferring the territory) the workaround is to use deleteMany that will not error.
But in this case, since it is already inside a transaction, maybe this is better?
| // unsubscribe the old user | ||
| const oldSubscription = await tx.subSubscription.findUnique({ where: { userId_subName: { userId: oldUserId, subName } } }) | ||
| if (oldSubscription) await tx.subSubscription.delete({ where: { userId_subName: { subName, userId: oldUserId } } }) | ||
| return updatedSub |
There was a problem hiding this comment.
Should it also subscribe the new user if they aren't already?
On one hand, I think it would be expected to do so, but on the other hand, you can transfer territories without the receiver's consent, so maybe it shouldn’t subscribe automatically?
There was a problem hiding this comment.
Nah, I'd say let's not automatically subscribe them for the reason you mentioned
riccardobl
changed the title
Description
Fixes #1517
Checklist
Are your changes backwards compatible? Please answer below:
yes
On a scale of 1-10 how well and how have you QA'd this change and any features it might affect? Please answer below:
For frontend changes: Tested on mobile, light and dark mode? Please answer below:
Did you introduce any new environment variables? If so, call them out explicitly here:
no