-
Notifications
You must be signed in to change notification settings - Fork 937
feat: add organization_ids in the user(s) response #1184
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
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1184 +/- ##
==========================================
- Coverage 66.27% 66.01% -0.26%
==========================================
Files 265 265
Lines 16681 16751 +70
Branches 157 157
==========================================
+ Hits 11055 11059 +4
- Misses 4484 4532 +48
- Partials 1142 1160 +18
Continue to review full report at Codecov.
|
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.
We should probably cover the case of fetching multiple users before merging this. Otherwise we'll have a struct that our API only serves on some routes.
@kylecarbs this PR is covering the scenario of fetching multiple users. https://github.com/coder/coder/pull/1184/files#diff-b4e10e23e473674139719f634b3ad7e760467574e342916c4bbb42df7a613722R151 this is the GET /users endpoint. Or are you talking about something different? |
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. I'm happy this just adds a single database call!
|
||
-- name: GetOrganizationIDsByMemberIDs :many | ||
SELECT | ||
user_id, array_agg(organization_id) :: uuid [ ] AS "organization_IDs" |
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.
Can we remove AS "organization_IDs"
?
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.
Unfortunately not because sqlc is generating the field name as "organization_ids" not forcing the "ID" uppercase stuff 😕
Reason:
FE needs to access the user org id to perform some actions like creating a new user in the same org. You can see the full conversation here https://codercom.slack.com/archives/C01A9SEKFEE/p1650907824531999.
Possible solutions