DT-3098: (Re-)Add PI Email to Study#2918
Open
rushtong wants to merge 7 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request restores support for a top-level Study piEmail field by reintroducing it into the dataset registration schema, persisting it in the study table, and threading it through the service/DAO layers so it can be created, updated, patched, and returned in Study objects.
Changes:
- Add nullable
pi_emailcolumn to thestudytable via Liquibase and wire it intoStudyDAOinsert/update and study-loading queries. - Reintroduce
piEmailinto the dataset registration JSON schema and the generatedDatasetRegistrationSchemaV1model, mapping it to/fromStudy. - Extend patch/update flows and tests to cover
piEmailpropagation (plus additional service/DAO test additions included in this PR).
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/org/broadinstitute/consent/http/db/DatasetDAO.java | Select s.pi_email AS s_pi_email so nested Study hydration includes PI email. |
| src/main/java/org/broadinstitute/consent/http/db/StudyDAO.java | Add pi_email to insert/update and adjust study query used for hydration. |
| src/main/java/org/broadinstitute/consent/http/models/Study.java | Add piEmail field with getter/setter. |
| src/main/java/org/broadinstitute/consent/http/models/StudyConversion.java | Add piEmail and include it when creating a study stub. |
| src/main/java/org/broadinstitute/consent/http/models/StudyPatch.java | Add piEmail patch field and patchability check. |
| src/main/java/org/broadinstitute/consent/http/models/dataset_registration_v1/DatasetRegistrationSchemaV1.java | Add piEmail to the registration schema model (JSON mapping, equals/hashCode/toString). |
| src/main/java/org/broadinstitute/consent/http/models/dataset_registration_v1/builder/SchemaFromStudy.java | Map Study.piEmail into the outgoing registration schema. |
| src/main/java/org/broadinstitute/consent/http/service/DatasetRegistrationService.java | Include piEmail in study insert/update objects built from registration. |
| src/main/java/org/broadinstitute/consent/http/service/DatasetService.java | Include piEmail when inserting/updating studies during dataset→study conversion. |
| src/main/java/org/broadinstitute/consent/http/service/dao/DatasetServiceDAO.java | Add piEmail to StudyInsert/StudyUpdate records and patch conversion logic. |
| src/main/resources/changelog-master.xml | Include the new Liquibase changeset. |
| src/main/resources/changesets/changelog-consent-2026-06-02-study-pi-email.xml | New changeset adding study.pi_email. |
| src/main/resources/dataset-registration-schema_v1.json | Re-add piEmail as an (optional) email-formatted field. |
| src/test/java/org/broadinstitute/consent/http/db/DatasetDAOTest.java | Update test inserts to account for the new pi_email column. |
| src/test/java/org/broadinstitute/consent/http/db/StudyDAOTest.java | Update study insert/update tests to include piEmail parameter position. |
| src/test/java/org/broadinstitute/consent/http/models/StudyConversionTest.java | New unit tests covering StudyConversion behavior including PI email. |
| src/test/java/org/broadinstitute/consent/http/models/StudyPatchTest.java | Update patch tests for new constructor signature and add PI email patchability coverage. |
| src/test/java/org/broadinstitute/consent/http/service/DatasetRegistrationServiceTest.java | Add assertions that PI email is captured in study insert/update and add dataset patch tests. |
| src/test/java/org/broadinstitute/consent/http/service/DatasetServiceTest.java | Add/extend service tests (includes dataset→study conversion and patch exception coverage). |
| src/test/java/org/broadinstitute/consent/http/service/dao/DatasetServiceDAOTest.java | Update DAO tests to include PI email in study insert/update/patch flows. |
|
otchet-broad
approved these changes
Jun 2, 2026
Contributor
otchet-broad
left a comment
There was a problem hiding this comment.
Nice work. Great catch.
| LEFT JOIN dataset d ON d.study_id = s.study_id | ||
| WHERE s.study_id = :studyId | ||
| """) | ||
| fso.file_storage_object_id AS fso_file_storage_object_id, |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Addresses
https://broadworkbench.atlassian.net/browse/DT-3098
Summary
This PR fixes a bug where the study PI Email that is sent from the UI is not saved on the back end.
Originally, there was a PI Email as a top level study property in the dataset registration schema json file. It was also present in the corresponding java file and in the Swagger documentation. It was removed soon afterward from the schema and from java, but left behind in the documentation. The UI functionality to bring that back relied on the swagger contract, but the code did not have support for it.
This PR brings that field back and allows for it to be populated and updated based on the schema. It is queried, mapped, and returned in study objects as a top level field.
Since the UI was expecting this to be the case, there are no required changes there.
Have you read CONTRIBUTING.md lately? If not, do that first.