- Notifications
You must be signed in to change notification settings - Fork927
chore: merge organization member db queries#13542
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Emyrk commentedJun 11, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
c739650 tob8eb28aCompareMerge into 1 that also joins in the user table for username.Required to list organization members on UI/cli
b8eb28a to4e1d469Comparee2b5ca2 to13ed01dCompare13ed01d toe2b5ca2Comparee2b5ca2 to8b1bc7fCompare| func (q *querier) GetOrganizationMemberByUserID(ctx context.Context, arg database.GetOrganizationMemberByUserIDParams) (database.OrganizationMember, error) { | ||
| return fetch(q.log, q.auth, q.db.GetOrganizationMemberByUserID)(ctx, arg) | ||
| } | ||
| func (q *querier) GetOrganizationMembershipsByUserID(ctx context.Context, userID uuid.UUID) ([]database.OrganizationMember, error) { | ||
| return fetchWithPostFilter(q.auth, policy.ActionRead, q.db.GetOrganizationMembershipsByUserID)(ctx, userID) | ||
| } |
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.
Removed these, replaced with 1OrganizationMembers.
| // Optional override of the default error checks. | ||
| // By default, we search for the expected error strings. | ||
| // If these strings are present, these strings will be searched | ||
| // instead. | ||
| notAuthorizedExpect string | ||
| cancelledCtxExpect string |
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 used to be done with a static list:
coder/coderd/database/dbauthz/setup_test.go
Lines 164 to 167 ine8aa9ac
| slice.Contains([]string{ | |
| "GetAuthorizedWorkspaces", | |
| "GetAuthorizedTemplates", | |
| },methodName) { |
We should move to this new approach, which is much closer to the actual writing of the tests.
ade54e1 tob98d06bCompare| AND user_id = $2 | ||
| LIMIT | ||
| 1; | ||
| true |
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.
just for my understanding, why do we need to dowhere true and case instead of awhere case?
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.
You are right, we don't need thetrue and 👍
b98d06b to2542dcdComparede9e688 intomainUh oh!
There was an error while loading.Please reload this page.
Merge into 1 that also joins in the user table for username.
Required to list organization members on UI/cli