This repository was archived by the owner on Sep 30, 2024. It is now read-only.
WIP: remove dependency on users.site_admin column when checking for site_admin status#58114
Draft
BolajiOlajide wants to merge 3 commits into
Draft
WIP: remove dependency on users.site_admin column when checking for site_admin status#58114BolajiOlajide wants to merge 3 commits into
users.site_admin column when checking for site_admin status#58114BolajiOlajide wants to merge 3 commits into
Conversation
Member
|
@BolajiOlajide where are we at on this PR? IIRC, it was pretty close where we left offa |
Contributor
Author
I just need to fix the tests. I'll prioritize this before I leave for vacation. |
1bb5820 to
eef8ee6
Compare
eef8ee6 to
63015b8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #51153
#43276 introduced a better way to assign user roles on a Sourcegraph instance. Before #43276, we only had two types of uses:
With a proper RBAC system in place, we can have different types of roles assigned to a user; however, we've been relying on the
site_admincolumn on theuserstable, which isn't the right way to determine the SiteAdmin status of a user.In this PR, I've removed all references to the
users.site_admincolumn and ensured that site_admin status is computed based on the user's roles. This PR doesn't remove thesite_admincolumn from theuserstable (this will be done in a follow-up PR).The query for fetching user information has been updated to reference the
user_rolesandrolestable to figure out the site_admin status of a user. That means the query for fetching a user changed fromto
This ensures we remove the dependency on
users.site_admin.Test plan
users.site_admincolumn and ensured tests were passing.Follow-up:
There'll be a follow-up PR to remove the
users.site_admincolumn in the next non-patch release.