Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

CRDCDH-49 Form Auth Update #90

Merged
merged 9 commits into from
Aug 14, 2023
Merged

CRDCDH-49 Form Auth Update #90

merged 9 commits into from
Aug 14, 2023

Conversation

Alejandro-Vega
Copy link
Collaborator

@Alejandro-Vega Alejandro-Vega commented Aug 10, 2023

OVERVIEW
This PR aims to expand the amount of variables to calculate the form mode, rather than just "User" and "FederalLead" roles and form status. I separated the logic for calculating the form mode into its own utility file where I could create tests for it easier and also simplify the useFormMode hook itself.

  • Added organization to getMyUser query and User type
  • Separated useFormMode hook logic into separate utility file to make it testable
  • Created detailed tests for each role, orgRole, formStatus, who submitted the form, which organization the submission belongs to, which organization the user belongs to (if any), etc.
  • Updated Approve/Reject form dialog to maintain internal state for review comment and only pass the value onSubmit. Previously, it was causing the parent to re-render due to state changes.

TICKETS
CRDCDH-49

NOTE
This implementation assumes a user will only belong to a single organization.

…ty file to make testing easier. Added detailed tests for each role/orgRole to ensure the correct formMode is being calculated
@Alejandro-Vega Alejandro-Vega added the 🚧 Do Not Merge This PR is not ready for merging label Aug 10, 2023
@Alejandro-Vega Alejandro-Vega removed the 🚧 Do Not Merge This PR is not ready for merging label Aug 11, 2023
@Alejandro-Vega Alejandro-Vega marked this pull request as ready for review August 11, 2023 15:35
@Alejandro-Vega Alejandro-Vega removed the request for review from jonkiky August 11, 2023 15:47
@Alejandro-Vega Alejandro-Vega added the 🚧 Do Not Merge This PR is not ready for merging label Aug 11, 2023
… Review mode. Also updated test to skip Review section as it was causing it to break
@Alejandro-Vega Alejandro-Vega removed the 🚧 Do Not Merge This PR is not ready for merging label Aug 11, 2023
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.

@Alejandro-Vega Very solid work here with the refactoring and comprehensive test cases. I added some comments which are optional, and one change required for the Fed lead role.

@amattu2 amattu2 added the 📝 Change Requested This PR has requested changes label Aug 11, 2023
…ole, regardless if they have an orgRole. Made FederalLead default to view only for any other status. Moved OrgInfo type
@Alejandro-Vega Alejandro-Vega removed the 📝 Change Requested This PR has requested changes label Aug 11, 2023
@Alejandro-Vega Alejandro-Vega requested a review from amattu2 August 11, 2023 21:41
@amattu2 amattu2 merged commit 2049d0f into mvp-1.0.0 Aug 14, 2023
@amattu2 amattu2 deleted the CRDCDH-49-Form-Auth-Update branch August 14, 2023 13:29
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants