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-2098 PBAC - Data Submission Button Visibility #573

Merged
merged 18 commits into from
Dec 31, 2024

Conversation

Alejandro-Vega
Copy link
Collaborator

Overview

Implemented PBAC to Data Submission related buttons.

Change Details (Specifics)

  • Updated Download API Token in the Navigation to only display if the user has "Create" permission

  • Updated “Create a Data Submission” button to only display if the user has "Create" permission

  • The following buttons below are only visible if they have the required permissions as well as some role specific requirements such as:

    • Submitter role - apply to their own data submissions
    • Fed Lead role - apply to the data submission with their assigned study
    • Data Commons Personnel role - apply to the data submission with their assigned data commons
    • Admin - apply to any Data Submission
    • User - None
  • Only displayed the following buttons if user has “Create” permission or is collaborator as well as role requirements described above

    • “Upload”
    • “Submit”
    • “Withdraw”
    • “Cancel”
  • Only displayed the "Validate" button if the user has "Validate" permission or is collaborator and submission has been Submitted, as well as role requirements described above

  • Only displayed the following buttons if the user has "Review" permission or is collaborator and submission has been Submitted, as well as role requirements described above

    • "Validate"
    • “Cross Validate”
    • “Release”
    • “Reject”
  • Only displayed the "Admin Submit" button if the user has "Admin Submit" permission and it is available, as well as role requirements described above

  • Only displayed the "Confirm" button if the user has "Confirm" permission, submission has been Released, as well as role requirements described above

    • “Complete”
    • “Reject”
  • Revised existing tests to match new PBAC

  • Uncomment role-specific tests from ticket

Related Ticket(s)

CRDCDH-2098 (Task)
CRDCDH-2023 (US)

@Alejandro-Vega Alejandro-Vega added the 🚧 Do Not Merge This PR is not ready for merging label Dec 30, 2024
@Alejandro-Vega Alejandro-Vega added this to the 3.2.0 (PMVP-M3) milestone Dec 30, 2024
@Alejandro-Vega Alejandro-Vega marked this pull request as ready for review December 30, 2024 23:37
@Alejandro-Vega Alejandro-Vega removed the 🚧 Do Not Merge This PR is not ready for merging label Dec 30, 2024
@Alejandro-Vega Alejandro-Vega changed the title CRDCDH-2098 PBAC - Permission Based Access Control Button Visibility CRDCDH-2098 PBAC - Data Submission Button Visibility Dec 30, 2024
@amattu2 amattu2 added the 🚧 Do Not Merge This PR is not ready for merging label Dec 31, 2024
Copy link
Member

@amattu2 amattu2 left a comment

Choose a reason for hiding this comment

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

Code changes LGTM. I annotated two super minor things (memo dependencies), and one question that doesn't necessarily require action.

I'm going to label this as Do Not Merge until we just get confirmation that there is a data_submission:validate permission. I'm not sure if this story is outdated or not.

src/components/APITokenDialog/index.tsx Outdated Show resolved Hide resolved
src/components/DataSubmissions/DataUpload.tsx Outdated Show resolved Hide resolved
src/components/DataSubmissions/ValidationControls.tsx Outdated Show resolved Hide resolved
@amattu2 amattu2 added the Not Run This PR was not tested locally label Dec 31, 2024
Copy link
Member

@amattu2 amattu2 left a comment

Choose a reason for hiding this comment

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

LGTM.

@amattu2
Copy link
Member

amattu2 commented Dec 31, 2024

@Alejandro-Vega I don't think the data_submission:validate permission actually exists, but I don't want to make any changes until we know for sure.

Would you prefer to merge this, and just make another PR to remove this permission if we need to, or wait for confirmation? If the former, feel free to go ahead and merge it. I'm fine either way

@Alejandro-Vega
Copy link
Collaborator Author

@Alejandro-Vega I don't think the data_submission:validate permission actually exists, but I don't want to make any changes until we know for sure.

Would you prefer to merge this, and just make another PR to remove this permission if we need to, or wait for confirmation? If the former, feel free to go ahead and merge it. I'm fine either way

We can hold off until we get confirmation unless this branch ever becomes a blocker then I'd say we go ahead and merge and fix later. As of now, I think it won't hurt to wait.

@Alejandro-Vega
Copy link
Collaborator Author

PR has been updated to remove any mention of the "data_submission:validate" permission in ee86b10.

@Alejandro-Vega Alejandro-Vega removed the 🚧 Do Not Merge This PR is not ready for merging label Dec 31, 2024
@amattu2 amattu2 merged commit 29463e1 into pbac-update-user-roles Dec 31, 2024
7 checks passed
@amattu2 amattu2 deleted the CRDCDH-2098 branch December 31, 2024 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Not Run This PR was not tested locally
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants