-
Notifications
You must be signed in to change notification settings - Fork 9
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: help users with user role assignment [DHIS2-18422] [DHIS2-18446] #1506
base: master
Are you sure you want to change the base?
Conversation
🚀 Deployed on https://pr-1506--dhis2-user.netlify.app |
}) | ||
}) | ||
|
||
it.skip('does not show a warning if user cannot assign role because role is assigned to user and keyCanGrantOwnUserAuthorityGroups:true ', async () => { |
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.
I'm skipping this test because the mocking of useSystemInformation is not working (and I don't know why 😢). The original mocked value (with keyCanGrantOwnUserAuthorityGroups:false
is being used in the but in other files the values that I mock within the test get used 🤔)
@flaminic and I talked through this high-level and we noticed that when removing legacy authorities on debug/dev with (also a general note: e2e is failing because fixtures are not updated, but we are waiting on a backend fix to DHIS2-18646) |
userRoleIds.includes(roleId) | ||
|
||
// check if role has authorities that user does not have | ||
const authoritiesUserDoesNotHave = hasAllAuthority |
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 is super small, but i really struggled with reading the const name here. Maybe missingAutoritiesForUser or something of that sort?
Background
This PR covers two tickets:
These are conceptually linked because both tickets were created to help users understand and troubleshoot why they were not able to assign user roles from the users app.
Changes
user roles - legacy authorities
You will now see a list of "legacy" authorities (or orphan authorities) when looking at a user role (if applicable).
"Facility Tracker" role on Sierra Leone v42
Antenatal care program (no legacy authorities)
user role edit page - assignability warning
(Note, none of these warnings appear for superusers (e.g.
system
user))-If the role has an authority you do not have, you will see a message explaining that you cannot assign it (by default the list of authorities is collapsed)
-If you set
Allow users to grant own roles
to false in Settings app, you will get a warning about roles that you are assigned to:user role view page - assignability warning
user edit/create page - warnings about unassignable roles
Testing
Automated tests
data
I pass toCustomDataProvider
. It does seem to be called, it's just not resolving for some reason. Anyway, that made me structure the tests slightly different than I would have liked.General code notes