Skip to content

Conversation

@tw4l
Copy link
Member

@tw4l tw4l commented Jan 31, 2023

Connected to #525

Adds the user's role for each org in GET /api/users/me-with-orgs and a unit test.

FYI for superusers this will be None/null as they don't have defined roles for the orgs.

@tw4l tw4l requested review from SuaYoo and ikreymer January 31, 2023 18:35
Copy link
Member

@SuaYoo SuaYoo left a comment

Choose a reason for hiding this comment

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

That was lightning fast 💯

@ikreymer
Copy link
Member

FYI for superusers this will be None/null as they don't have defined roles for the orgs.

Hm, is this what we want there, or should we not include superuser at all OR list them as 'owner' or something else?

@tw4l
Copy link
Member Author

tw4l commented Jan 31, 2023

Hm, is this what we want there, or should we not include superuser at all OR list them as 'owner' or something else?

@ikreymer Good questions! I think we should include superuser (this endpoint is necessary for rendering the UI). We could list them as owners, but it might be worth maintaining the distinction? Superusers currently also can't e.g. edit member settings for an org unless they've been explicitly invited as an Owner. My two cents is either we should leave it as-is or in a separate issue/PR modify the backend to automatically give Owner permissions to each org to superusers.

@ikreymer
Copy link
Member

Hm, is this what we want there, or should we not include superuser at all OR list them as 'owner' or something else?

@ikreymer Good questions! I think we should include superuser (this endpoint is necessary for rendering the UI). We could list them as owners, but it might be worth maintaining the distinction? Superusers currently also can't e.g. edit member settings for an org unless they've been explicitly invited as an Owner. My two cents is either we should leave it as-is or in a separate issue/PR modify the backend to automatically give Owner permissions to each org to superusers.

Hm, what if we add a new role SUPERADMIN = 100 in https://github.com/webrecorder/browsertrix-cloud/blob/main/backend/btrixcloud/invites.py#L16 and return that for this API?
Would be a bit clearer than just returning null?

@tw4l
Copy link
Member Author

tw4l commented Jan 31, 2023

Hm, what if we add a new role SUPERADMIN = 100 in https://github.com/webrecorder/browsertrix-cloud/blob/main/backend/btrixcloud/invites.py#L16 and return that for this API?

That makes sense to me! And then can determine whether to remove or continue to use the new role when working on #538.

@tw4l tw4l merged commit 7d25565 into main Jan 31, 2023
@tw4l tw4l deleted the issue-525-add-role-to-me-with-orgs branch January 31, 2023 21:27
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.

4 participants