- Notifications
You must be signed in to change notification settings - Fork928
refactor: Add roles into the user response#1347
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
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.
Frontend looks good!
Uh oh!
There was an error while loading.Please reload this page.
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 one tiny change!
Uh oh!
There was an error while loading.Please reload this page.
codecovbot commentedMay 9, 2022 • 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.
Codecov Report
@@ Coverage Diff @@## main #1347 +/- ##==========================================- Coverage 66.44% 66.43% -0.02%========================================== Files 284 280 -4 Lines 18584 18393 -191 Branches 235 235 ==========================================- Hits 12349 12219 -130+ Misses 4967 4923 -44+ Partials 1268 1251 -17
Continue to review full report at Codecov.
|
Interesting to see the JS tests failing because an API change 👏 |
Uh oh!
There was an error while loading.Please reload this page.
coderd/roles.go Outdated
| } | ||
| } | ||
| func ConvertRoles(roles []rbac.Role) []codersdk.Role { |
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 shouldn't be exported either!
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 are using it for some tests here
Line 88 in87c474b
| ExpectedRoles:coderd.ConvertRoles(rbac.OrganizationRoles(admin.OrganizationID)), |
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 never need to use internalcoderd APIs to check API responses, otherwise, our customers will have to as well!
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.
Thinking on:
- Do not export the convert functions
- Copy the convert functions into the role tests
- See if we need restructure the tests at all
Since the roles are already returned by the User model, we just need to parse it from a rbac.Role to codersdk.Role.
Closes#1346