Skip to content
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): Add user role scope #554

Merged
merged 4 commits into from
Jan 21, 2025

Conversation

Ogollah
Copy link
Contributor

@Ogollah Ogollah commented Jan 17, 2025

Requirements

  • This PR has a title that briefly describes the work done, including the ticket number if there is a ticket.
  • My work conforms to the OpenMRS 3.0 Styleguide.
  • I checked for feature overlap with existing widgets.

Summary

What does this PR do?

  • Adds user role scope to the user management.
  • Pick one of the selected roles to add operations and locations to add the user role scope.
  • Refactor inventory roles category to additional roles

Screenshots

userScope.webm

None.

Related Issue

None.

Other

None.

@Ogollah Ogollah requested a review from makombe January 17, 2025 07:19
Copy link
Collaborator

@makombe makombe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Ogollah left some few suggestions

Comment on lines 91 to 93
licenseNumber: providerAttributeType.find((type) => type.name === 'Practising License Number')?.uuid || '',
licenseExpiry: providerAttributeType.find((type) => type.name === 'License Expiry Date')?.uuid || '',
primaryFacility: providerAttributeType.find((type) => type.name === 'Primary Facility')?.uuid || '',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ogollah Can we check for the uuids here instead of name? Names are always volatile and can be changed any time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted

display: role.display,
description: role.description,
})) || [],
gender: initialUserValue.person?.gender || 'M',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the gender or here explicitly M ? should it be empty?

@Ogollah Ogollah requested a review from makombe January 20, 2025 09:27
Copy link
Collaborator

@makombe makombe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Ogollah

@makombe makombe merged commit 7cf8418 into palladiumkenya:main Jan 21, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants