Add Admin UI #30
Add Admin UI #30
Conversation
The admin UI allows setting of user info and permissions. The user info is the user's name, email, job title and their region. Permissions are modeled to be flexible. A user permission is made up of a region and a scope. A scope represents the ability to do something specific, like READ_REPORTS. Combined with the region the permission system should be flexible enough to handle user's that need permissions that cross regions. Not yet done: * Any API work, including saving/modeling/REST endpoints * Checking/usage of permissions/user info in any other part of the TTAHUB
|
Very nice! 👍 🎉 Couple of minor items I noticed: We might want to make "adding validations", e.g. email address to the list of out of scope items. Per Ryan, the name field will most likely involve just one field with first, middle, last names. I think it would make sense to adjust the UI with that in mind. The current permissions change what is displayed based on which user is selected 👍 - except Harry Potter. Interestingly, the current permissions show region 1, but the top region field has region 4. Is this expected? |
👍
👍
"Harry Potter" should have permissions that look like this for region 1: "Harry Potter" current permissions include both the |
|
I am seeing this, which might be correct, but check the region box showing Atlanta
|
Oh yes that is the region "Harry Potter" is assigned. It is basic user info, has nothing to do with permissions. I imagine it will be used to default any lists to items from the user's region. Do you think I should update the admin UI to make that region dropdown clearer? |
|
I see. It could be a good idea to match the default region with the read-write permission shown, unless we want to highlight what you just pointed out above. I am thinking at the demo others might think it's a bug, while in fact it isn't :-) |
This is to closer match the current permissions for the user
| fullName: 'Ron Weasley', | ||
| permissions: [ | ||
| { | ||
| region: 0, |
There was a problem hiding this comment.
Is there literally going to be a region called 0 or is this a null value?
There was a problem hiding this comment.
Good question, I got so used to region 0 that it stopped bothering me until you pointed it out. I haven't given much thought on if the region is 0 or null, seems like something that will get nailed down in HHS#50. Having region be 0 or null doesn't feel that great to me, maybe in HHS#50 we decide to have a table for regional permissions (with a foreign key to region) and a table for global permissions (with no foreign key to region). Maybe we only have a single global permission so it makes sense to add a column directly to the users table. Regardless I'll add a comment to all the places where region 0 is being used explaining it isn't a final decision.
Description of change
The admin UI allows setting of user info and permissions. The user info is the user's name, email, job title and their region. Permissions are modeled to be flexible. A user permission is made up of a region and a scope. A scope represents the ability to do something specific, like
READ_REPORTS. Combined with the region the permission system should be flexible enough to handle user's that need permissions that cross regions.This is not the final version of the admin UI, although I feel like the UI itself won't be updated much. I expect the schema of the data that drives the admin UI will be updated and will require the UI to be updated as well. But I feel this is a good base for those changes to be applied when needed once the permission model has been finalized, and when we have a more definitive list of scopes.
Out of scope:
TTAHUB
Sorry for the big PR, not sure how it could have been broken up smaller. Let me know if anyone wants to hope on a call to walk through it!
How to test
yarn deps(yarn docker:deps)yarn start:local(yarn docker:start)localhost:3000/adminIssue(s)
Checklist