Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove userId from MembershipResource interface #4570

Merged
merged 4 commits into from
Apr 4, 2024

Conversation

spolu
Copy link
Contributor

@spolu spolu commented Apr 4, 2024

Description

We cut the MembershipResource with userId(s) (number) in its interface which breaks our invariants.

This PR refactors parts of the code to use UserType everywhere.

Behavior change:

  • When rendering the authors of an assistant we don't filter along membership, showing authors that may have been revoked.
  • When deleting memberships of a workspace (GDPR scrubbing) the logic is changed to properly delete users if they only belonged to that workspace (otherwise they would be dangling with no membership, I think that's what we want) (r? @PopDaph)

Risk

Risky as it touches many parts of the code, will run tests on all touched parts (esp. agent author display)

Test plan:

  • poke revoke
  • Login
  • recent authors display
  • Invite user / revoke user
  • general use of the app

Deploy Plan

  • deploy front

@spolu spolu requested review from fontanierh and PopDaph April 4, 2024 13:26
Copy link
Contributor

@fontanierh fontanierh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good

@spolu spolu merged commit 7e62018 into main Apr 4, 2024
7 checks passed
@spolu spolu deleted the spolu-remove_userId_from_membership_resource branch April 4, 2024 14:13
flvndvd pushed a commit that referenced this pull request May 26, 2024
* Remove userId from MembershipResource interface

* lint

* fix poke endpoint call

* lint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants