Warn users about unrestricted roles being assignable by a restricted role #34499
Conversation
| @@ -0,0 +1,3 @@ | |||
| .role-modal-warning { // Used in combination with Bootstrap's 'help-block'/'alert' classes | |||
| margin-bottom: 0px; | |||
| } | |||
There was a problem hiding this comment.
This feature seems like an example of doing field-level help: basically using help-block but not the alert classes. Docs are here: https://www.commcarehq.org/styleguide/b5/organisms/forms/#field-errors
There was a problem hiding this comment.
I see. I think the red text is still appropriate, but when I use help-block with text-danger it overrides the text color:
So I tried using my own specifier: link.
Or would it be better to use help-block and then add a custom specifier that overrides the text color? Does it matter? Trying to get a feel for best practices.
Also, didn't even realize I am working in Bootstrap 3 territory.
There was a problem hiding this comment.
Oops, I sent the B5 link by mistake. The B3 docs are here: you get the red text by applying has-error to an ancestor of the help-block.
Also, didn't even realize I am working in Bootstrap 3 territory.
Yeah, the vast majority of HQ is on B3, although that is rapidly changing.
There was a problem hiding this comment.
Ah, gotcha. Thanks Jenny.
| data.manageRoleAssignments = { | ||
| all: data.is_non_admin_editable, | ||
| specific: ko.utils.arrayMap(o.nonAdminRoles, function (role) { | ||
| return { | ||
| path: role._id, | ||
| name: role.name, | ||
| value: data.assignable_by.indexOf(role._id) !== -1, | ||
| value: ko.observable(data.assignable_by.indexOf(role._id) !== -1), | ||
| access_all_locations: role.permissions.access_all_locations, |
There was a problem hiding this comment.
This is an observable, right?
There was a problem hiding this comment.
I don't think it is, actually. Seems likeo is just a dictionary that was passed straight through.
There was a problem hiding this comment.
huh, I see. I had thought it would need to be in order for the new markup to appear/disappear as needed when someone checks/unchecks full org access. You've confirmed that works, right?
There was a problem hiding this comment.
I'm aware that it does not update without reloading the page.
There was a problem hiding this comment.
I kind of accepted that as a defect--if role's name is changed in a role's model that isn't updated in other modals, either. Generally I think that's true for other modal data. I had trouble figuring out a reasonable high-level change for that.
There was a problem hiding this comment.
The js already rewraps the saved role with the data sent back from the server. I think you'd need to rewrap the other roles and/or recreate the RolesViewModel based on the new data.
That said, I checked out the branch and played around with the interaction, and I think it's fine to call that out of scope.
There was a problem hiding this comment.
Oh, that makes sense. Also maybe I could create a new array on each model that points to the self.permissions.access_all_locations observables on other models?
But yeah, was also thinking it might be out of scope here.
There was a problem hiding this comment.
I wouldn't think you'd need to add any data - each role has the proper info about the other roles on page load, that data just isn't getting refreshed properly somewhere.
But yeah, out of scope is fine too.
corehq/apps/users/templates/users/partials/edit_role_modal.html
Outdated
Show resolved
Hide resolved
corehq/apps/users/templates/users/partials/edit_role_modal.html
Outdated
Show resolved
Hide resolved
…ocation-restricted-roles-assigning-unrestricted
|
@orangejenny it's been 2 weeks so merged in master. Could you give final thumbs up? (we are trying to merge today) |
|
Thanks! |
AddisonDunn
deleted the
ad/warn-location-restricted-roles-assigning-unrestricted
branch
Product Description
USH-4414
A role that does not have "full organization access" being able to assign a role that does increases security risk. This PR implements a warning about this sort of configuration in the role modal.
See this doc for UI pics/an example.
Feature Flag
No FF, but location restriction is only available at certain subscription levels.
Safety Assurance
Safety story
QA Plan
None
Rollback instructions
Labels & Review