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-1501 Create/Edit Pre-Approved Study Page #469

Merged
merged 27 commits into from
Oct 7, 2024
Merged

CRDCDH-1501 Create/Edit Pre-Approved Study Page #469

merged 27 commits into from
Oct 7, 2024

Conversation

Alejandro-Vega
Copy link
Collaborator

@Alejandro-Vega Alejandro-Vega commented Sep 18, 2024

Overview

Created page for creating and editing approved studies. This will be in the Admin dashboard and can be accessed through the Manage Studies page.

Change Details (Specifics)

  • Added new mutations and queries for creating, retrieving, and updating approved studies
  • Created a new Study View for creating new or updating existing approved studies
  • Before submission, it will validate ORCID and make sure at least one Access Type is selected, otherwise it will show an error.
  • Ensured after saving/canceling it returns back to the Manage Studies page with the previous table state
  • Added formatting as you type to ORCID like in Submission Request
  • The dbGaP input is only required if "Controlled Access" is selected

Related Ticket(s)

CRDCDH-1501 (Create Study Task)
CRDCDH-1488 (Create Study User Story)
CRDCDH-1502 (Edit Study Task)
CRDCDH-1485 (Edit Study User Story)

@Alejandro-Vega Alejandro-Vega added the 🚧 Do Not Merge This PR is not ready for merging label Sep 18, 2024
@Alejandro-Vega Alejandro-Vega added this to the 3.1.0 (PMVP-M2) milestone Sep 18, 2024
Base automatically changed from CRDCDH-1500 to 3.1.0 October 2, 2024 13:21
@Alejandro-Vega Alejandro-Vega removed the 🚧 Do Not Merge This PR is not ready for merging label Oct 3, 2024
@Alejandro-Vega Alejandro-Vega marked this pull request as ready for review October 3, 2024 17:26
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.

Really nice job putting this together. Great to see the test coverage for the StudyView, I'm sure that was a large effort. I annotated some issues and also some recommendations below.

FYI,
The backend isn't updating any external references to the study. e.g. If you change the abbreviation, it doesn't update any Data Submissions that use the study. The requirements don't explicitly call for this, but I would think it's assumed? Could be wrong though.

src/content/studies/StudyView.tsx Outdated Show resolved Hide resolved
src/content/studies/StudyView.tsx Show resolved Hide resolved
src/content/studies/StudyView.tsx Outdated Show resolved Hide resolved
src/content/studies/StudyView.tsx Outdated Show resolved Hide resolved
src/content/studies/StudyView.tsx Show resolved Hide resolved
src/content/studies/StudyView.tsx Outdated Show resolved Hide resolved
src/content/studies/Controller.test.tsx Show resolved Hide resolved
src/content/studies/StudyView.tsx Outdated Show resolved Hide resolved
@amattu2 amattu2 added the 📝 Change Requested This PR has requested changes label Oct 4, 2024
@Alejandro-Vega Alejandro-Vega removed the 📝 Change Requested This PR has requested changes label Oct 7, 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 amattu2 merged commit 0d7389c into 3.1.0 Oct 7, 2024
5 checks passed
@amattu2 amattu2 deleted the CRDCDH-1501 branch October 7, 2024 17:05
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