[DT-4061] Add PI institution, ORCID, and profile links to studies

[otchet-broad]

Base: branch 1 · 20 files, +1165/-22 · Migration: yes (2 changesets) · UI impact: additive only

Adds pi_institution_id, pi_orcid, pi_linkedin_url, pi_website_url to
the study table and surfaces them on Study, StudyPatch, and the study
registration read path.

Registration already collected the PI institution as the numeric
piInstitution study property, so createStudy now records the column too,
and a backfill changeset copies the property into the column for studies
registered earlier. Without the backfill every existing study shows a blank
PI Institution until someone re-PATCHes it. The backfill skips values that
are not plain integers and ids with no matching institution row, which the
foreign key would otherwise reject.

Review focus: these are columns, not study properties, so they deliberately
do not follow the "blank string deletes" convention the patchable
properties use. They follow the JSON convention instead — absent is a no-op,
explicit null clears, blank normalizes to a clear. Jackson cannot tell an
absent field from an explicit null, so StudyPatch.fromJson re-reads the
body to record which field names arrived as literal nulls.

A registration PUT carries the PI institution but not the profile links,
which are PATCH-only, so DatasetRegistrationService carries the stored
links forward rather than dropping them; a study conversion does the same.
executeUpdateStudy now raises NotFoundException for a missing study
instead of dereferencing null.

Additive on the wire: four new fields on study reads, accepted on PATCH, no
existing field changes shape.

Depends on: branch 1 — shares hunks in DatasetService.java and
DatasetServiceTest. That file overlap is the only reason it is stacked here.

---

Where this sits in the DT-4061 stack

otchet-dt-3990-study-ratings-pi-details was too large to review meaningfully
(95 files, +8168), so it was split into eight PRs. This PR targets otchet-dt-4061-study-visibility-authz,
not develop, so its diff shows only its own work.

#	Branch	Base
1	otchet-dt-4061-study-visibility-authz	develop
1b	otchet-dt-4061-study-patch-ownership	otchet-dt-4061-study-visibility-authz
2	otchet-dt-4061-study-pi-details ← this PR	otchet-dt-4061-study-visibility-authz
3	otchet-dt-4061-study-comments	otchet-dt-4061-study-pi-details
4	otchet-dt-4061-study-dar-metrics	otchet-dt-4061-study-comments
5	otchet-dt-4061-study-recommendations	otchet-dt-4061-study-dar-metrics
6	otchet-dt-4061-study-asset-fields	otchet-dt-4061-study-recommendations
7	otchet-dt-4061-study-asset-endpoints	otchet-dt-4061-study-asset-fields

Branch 1b is a sibling rather than a link in the chain: nothing depends on it,
so it can be held or dropped without blocking the others. Land the numbered
chain in order — merging out of order, or squash-merging, will require rebasing
the descendants.

The split is verified lossless: branch 7 plus 1b reproduces the original branch
exactly, apart from five scripts/verify-study-*.sh helper scripts that were
dropped at the author's request. ./mvnw test-compile passes on each branch
independently.

🤖 Generated with Claude Code

[kevinmarete]

Here are some findings:

1. P1 — PI institution can be silently reverted.
   PATCH updates the new column, but the registration response builder still reads the legacy study property at SchemaFromStudy.java#L89.

   Example: institution 1 is backfilled, PATCH changes it to 2, registration GET still returns 1, and a subsequent registration PUT writes 1 back into the column. The builder should treat study.piInstitution as authoritative, with a PATCH → registration GET → PUT regression test.

2. P2 — Study responses fabricate an institution creation date.
   StudyReducer.java#L26 constructs a partial Institution, whose constructor initializes createDate to the current time at Institution.java#L44. Every study read therefore returns a false, changing piInstitution.createDate. The query should map the actual institution metadata, or the API should use a dedicated ID/name summary model.

[fboulnois]

Some issues I identified with Claude and Codex:

1. An out-of-range value aborts the backfill

I ran the shipped SQL against postgres:16.10-alpine with piInstitution = 99999999999. It passes ^[0-9]+$, then fails the integer cast:

ERROR:  value "99999999999" is out of range for type integer

This fails the entire changeset, including the backfill of valid rows. Limiting the digit count avoids the error:

AND btrim(sp.value) ~ '^[0-9]{1,9}$'

I confirmed this backfills the valid row and skips the out-of-range value. It also excludes valid 10-digit integer IDs, so a full range check would be needed to support those.

2. Both write paths need to validate piInstitutionId

PATCH: DatasetService.java:826-830 catches every exception and throws InternalServerErrorException, so {"piInstitutionId": 999999} returns 500 instead of 400 when the institution doesn't exist.

Registration: checkPiInstitutionRequired only checks for null. Previously, a nonexistent ID was stored as a text property and registration succeeded. Now the foreign key constraint fails the entire registration transaction. This changes behavior for existing clients and needs explicit validation.

3. PATCH leaves conflicting institution values in study responses

After a PATCH, the study response contains the new piInstitution.id and the old properties[piInstitution] value. SchemaFromStudy handles this for registration payloads, but raw study responses still expose both. Update the legacy property in patchStudy or stop returning it.

4. The backfill test doesn't cover the EXISTS guard

testSkipsValuesThatCannotBeAppliedToTheColumn uses -1 as an orphan ID, but that fails the regex before reaching EXISTS. Use a positive ID with no institution row, such as 2000000000, and add a separate out-of-range case for item 1.

5. The timestamp fix needs a test

The commit fixes the constructor's default createDate by always overwriting it in the reducer, but no test asserts getPiInstitution().getCreateDate().

Since hasOptionalColumn catches every exception and returns empty, a broken alias or mapping could silently produce null dates. Test both populated and null timestamps.

6. Registration updates fetch the study twice

updateStudyFromRegistration calls findStudyById just to read three strings. That opens a REPEATABLE_READ transaction and loads properties, dataset IDs and files. executeUpdateStudy then fetches the study again in the write transaction.

Read the links inside executeUpdateStudy, where the study is already loaded, to avoid the extra fetch and the gap between reading and writing.

7. Smaller issues

• The 18-argument constructor and 13-argument updateStudy are easy to misuse, especially with several adjacent String arguments and all-null call sites. A builder or test fixture would help, as would assertions more specific than any() for every argument.
• piInstitutionId accepts "123" as an integer, unlike the DTO's strict string and boolean deserializers. Add equivalent integer validation for consistency.
• Add a test showing that {"piName": null} remains a no-op while {"piOrcid": null} clears the field.
• Dataset.yaml reuses Study.yaml and documents piInstitution, but DatasetDAO.findDatasetStudyById doesn't select pi_institution_id, so the field is always null on that route.

I also checked that stringPatchProps() excludes the PI columns: a blank piOrcid correctly avoids deleting a study property of that name.