- Notifications
You must be signed in to change notification settings - Fork1k
chore: implement api for creating custom roles#13298
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 commentedMay 16, 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.
This stack of pull requests is managed by Graphite.Learn more about stacking. |
ba10185 toef8c950Comparead4023b toc1c3891Compare4060c07 toc58a6fdComparec1c3891 toaf25ebbComparec58a6fd to88ca559Compareaf25ebb to8abb57eCompare88ca559 to96d1527Comparef1c61bd toe5613b2CompareUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
| // UpsertCustomSiteRole will upsert a custom site wide role | ||
| func (c*Client)UpsertCustomSiteRole(ctx context.Context,reqRole) (Role,error) { | ||
| res,err:=c.Request(ctx,http.MethodPatch,"/api/v2/users/roles",req) |
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.
/api/v2/users/roles seems like a weird place to mount this 🤔
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.
Yea? That is where the roles api currently is. Any suggestions?
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.
Wouldn't they be org-scoped?
EDIT: Ah no, they can have user, org, and site perms.
Honestly I'd put them under a separate/api/v2/roles endpoint but I think that's definitely out of scope of this PR.
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.
These roles are site scoped.
There is a/roles under/organizations/{organization}/members/roles for org scoped roles.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
4259691 toe0cdcddCompare| // Verify the role exists in the list | ||
| // TODO: Turn this assertion back on when the cli api experience is created. | ||
| //allRoles, err := tmplAdmin.ListSiteRoles(ctx) | ||
| //require.NoError(t, err) | ||
| // | ||
| //require.True(t, slices.ContainsFunc(allRoles, func(selected codersdk.AssignableRoles) bool { | ||
| //return selected.Name == role.Name | ||
| //}), "role missing from site role list") |
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.
To prevent this PR from growing anymore, I am going to push this to the next PR. Essentially, from a UI perspective, our existing role endpoints do not fetch custom roles.
Since there is no UI or cli in this PR and this is already large, I am going to intentionally not solve this here (and this is gated behind an experiment).
96d1527 to992c845Compare07d531d to28477baCompare992c845 toa6996f6Compare75e7bd8 to747fc79Compare801e054 to68807acCompare
No description provided.