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

CRDCDH-2105 PBAC – Individual Permissions Control of User Accounts #572

Merged
merged 23 commits into from
Dec 31, 2024

Conversation

amattu2
Copy link
Member

@amattu2 amattu2 commented Dec 27, 2024

Overview

This PR introduces the ability to configure individual PBAC permissions for an account.

Warning

Some known issues / expected things from the backend implementation:

  • You can adjust a Submitter or User's permissions, but no changes will actually be saved. This is expected, but BE should be locking all permissions, and they currently aren't
  • The backend seems to reject some permissions and/or notification selections and returns an error for it. Just uncheck them in the FE for now
  • The visual permission names don't really match the user stories anymore (they keep getting changed)
  • The order of actual permissions in the nested groups may not match the user stories (see below)

Note

The frontend sorts the groups based on the User Story, but we don't sort the nested permissions themselves since that's somewhat impractical (way too many options).

Change Details (Specifics)

  • Implement a PermissionsPanel component, which handles the PBAC checkboxes, defaults, and form management
  • Update the useProfileFields hook to conditionally render permissions/notifications fields
  • Update remaining queries to pull the notification/permission assignments
  • Update the Apollo memory cache to stop it from normalizing the PBAC data
    • It was discarding the relationship between the roles and their specific defaults causing serious issues
  • Move AuthPermissions to a separate file (PBAC.d.ts) and create AuthNotifications type for consistency
    • I'm also fine with just leaving it in Auth.d.ts if that's preferred, it was just getting cluttered
  • Update the ProfileView action buttons to be consistent with design (matching DS and SR theme)

Related Ticket(s)

CRDCDH-2105 (Task)
... A lot of stories

@amattu2 amattu2 added the 🚧 Do Not Merge This PR is not ready for merging label Dec 27, 2024
@amattu2 amattu2 added this to the 3.2.0 (PMVP-M3) milestone Dec 27, 2024
@amattu2 amattu2 changed the title CRDCDH-2105 CRDCDH-2105 PBAC – Edit User Individual Permissions Control Dec 27, 2024
@amattu2 amattu2 changed the title CRDCDH-2105 PBAC – Edit User Individual Permissions Control CRDCDH-2105 PBAC – Individual Permissions Control of User Accounts Dec 30, 2024
@amattu2 amattu2 marked this pull request as ready for review December 30, 2024 17:59
@amattu2 amattu2 removed the 🚧 Do Not Merge This PR is not ready for merging label Dec 30, 2024
Copy link
Collaborator

@Alejandro-Vega Alejandro-Vega left a comment

Choose a reason for hiding this comment

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

Code looks solid, and I like the changes you made with separating the PBAC types to their own file. I left a few super small and optional suggestions. Feel free to disregard them.

src/utils/profileUtils.ts Outdated Show resolved Hide resolved
src/components/PermissionPanel/index.tsx Outdated Show resolved Hide resolved
src/components/PermissionPanel/index.tsx Outdated Show resolved Hide resolved
@Alejandro-Vega Alejandro-Vega added the 📝 Change Requested This PR has requested changes label Dec 31, 2024
@amattu2 amattu2 removed the 📝 Change Requested This PR has requested changes label Dec 31, 2024
@Alejandro-Vega Alejandro-Vega merged commit 421d99a into pbac-update-user-roles Dec 31, 2024
7 checks passed
@Alejandro-Vega Alejandro-Vega deleted the CRDCDH-2105 branch December 31, 2024 20:11
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