I'd rather not bikeshed this url placement atm. I'll move the path when I talk with FE. I know the mvp for endpoints, and will change them in another PR.

Really just trying to get some footing for roles so I can start enforcing them without breaking things.

Could we reuse theadmin andmember strings? From what I can tell, it'll be scoped to an organization anyways.

We totally can, they are scoped. I was adding theorganization prefix because I imagine in the UI they will strip the "scopeID" when assigning roles. I kinda like the fact org roles have an org prefix to prevent any misunderstanding.

Eg: A dropdown list of roles you can add to a org member, will sayorganization-member, notorganization-member:00000000-00000000-00000000-00000000.

We probably wouldn't have a single dropdown to show organization and site-level roles though, because that'd easily be misinterpreted by users IMO.

Doesn't seem this is used anywhere right now!

It's not, but it will be when I add the endpoints for retrieving roles that can applied. I wrote this to show how to get the roles for that endpoint in this design.

When we go to a database model, these won't be dynamic lists, so all this looping goes away. I'll drop this and bring it back in my next pr

That makes sense, thanks for the context!

We might not want to update this data in a migration right now. I'm concerned that changing this behavior will be difficult down the road, so I'd prefer if we just straight-up broke the schema right now (eg. add therbac_roles directly to thebase.up migration. I'm not strict on this though, and it's just a hunch. Any thoughts?

I think we should try to not break migrations, lest we become comfortable in the habit and it bite us squarely where we sit down the line.

Is anyone running a persistent V2? If we are only running in dev mode, I'll drop all this.
I just didn't want to break any current deployments. But if that's cool, I'm more than happy to drop this code to assign roles in the migration 👍

Apparently we have a dev and QA instance, so this will be required to not totally break those.

Fair enough fair enough

TIL postgres supports native text arrays 😄

Neat trick! For posterity here,UNNEST turns an array into a one-row temporary table,SELECT DISTINCT removes duplicates, and then it just becomes an array again.

nit: formatting, also "only be able to change your own roles"?

Yea, sinceauthorize is not yet used, I let you change your roles to w/e you want. This is what we are doing for a lot of user endpoints atm

clarification: if a user has roles{A, B, C} and we callUpdateUserRoles with{D}, does this mean the user ends up with just{D} and not{A, B, C, D}? That seems like a sharp edge.

This is correct. I actually had it asgrant D before, but if we match the current UI, it actually makes sense to pass{A, B, C, D}.

t

The alternative is to run the diff from the UI as 2 requests.

PUT{D}
DELETE{A, B, C}

I think we can implement this logic better in the BE. I could add a route/query param to "Add" the role vs update for example. I think this is something we can make easier once the UI needs are better understood too. But w/e the end result is, the database call is ok to do anALL, and let the Golang figure out the diffs.