fix(eco): Ensures user mappings are deleted alongside their associated organization membership#109249
fix(eco): Ensures user mappings are deleted alongside their associated organization membership#109249GabeVillalobos wants to merge 3 commits intomasterfrom
Conversation
github-actions
bot
added
the
Scope: Backend
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
GabeVillalobos
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
|
|
||
| if instance.user_id is not None: | ||
| # We need to clean up external actors (user mappings for integrations), | ||
| # but only if the org member is not an invite. |
There was a problem hiding this comment.
What does it mean to be an invited member and how are we filtering those out? I see the test for it but I'm still a little confused.
There was a problem hiding this comment.
OrganizationMember objects have 2 major states:
- Active organization user in the Sentry org, has a fk to user, roles, etc...
- Invited user, not yet active, which only contains an email address, no user fk, and a separate invite status to track whether they've accepted the invite or not, along with the usual role information.
It's sort of confusing that we do it this way, but the lack of a user ID on the org member is a way to check whether or not the "org member" is actually an active user.
Check the invite code for an example of this:
| relations.append( | ||
| ModelRelation( | ||
| ExternalActor, | ||
| {"user_id": instance.user_id, "organization_id": instance.organization_id}, | ||
| ) | ||
| ) |
There was a problem hiding this comment.
This will work well when members are removed as part of an organization being deleted. However, the simple case of sending a DELETE request to OrganizationMemberDetails won't hit this path, as it uses the django ORM directly.
I think we would also need to remove related actors in the model.delete() situation.
There was a problem hiding this comment.
Hmm could I just update that path to use the scheduled deletion, or synchronously execute the deletion task in that code? This reminds me of the previous challenges we hit in HC where we need consistency in ways the ORM lets you bypass in weird ways 😓
I guess the heavyweight approach could be to use outboxes to drive this instead?
3 participants
Currently, when
OrganizationMembersare deleted, we don't clean up the assocaitedExternalActorobjects that reference their user and organization IDs. As a result, we orphan these mappings, causing collisions in the future when that same user attempts to re-add their external mapping.This ensures these deletions are done in an eventually consistent manner, post-deletion.
Fixes ISWF-2122