-
-
Notifications
You must be signed in to change notification settings - Fork 658
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
feat: ensure at least one owner on remove user/group access #5085
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Ignored Deployments
|
858641c
to
e6c4f3c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this wants a test. I'm guessing this originally regressed because there wasn't one
@@ -491,31 +499,6 @@ test('should remove user from the project', async () => { | |||
expect(memberUsers).toHaveLength(0); | |||
}); | |||
|
|||
test('should not remove user from the project', async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to line 1006
d9de330
to
b5b4fb9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -508,6 +500,11 @@ export default class ProjectService { | |||
userId, | |||
); | |||
|
|||
const ownerRole = await this.accessService.getRoleByName( | |||
RoleName.OWNER, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this seems to be a bug since we're getting a role for the owner not the user we're deleting
About the changes
This makes sure that projects have at least one owner, either a group or a user. This is to prevent accidentally losing access to a project.
We check this when removing a user/group or when changing the role of a user/group
Note: We can still leave a group empty as the only owner of the project, but that's okay because we can still add more users to the group