[DT-3869] Define the Study Template CSV v1 contract and fixtures

[kevinmarete]

Addresses

https://broadworkbench.atlassian.net/browse/DT-3869

Summary

• Defines the canonical Study Template CSV v1 contract.
• Adds synthetic valid and invalid fixture files for future parser tests.
• Adds fixture integrity tests covering CSV structure, UTF-8, BOM handling, versioning, duplicate fields, and structured errors.
• Adds Apache Commons CSV as a test-only dependency.
• No parser, endpoint, or API behavior is introduced.

---

Have you read CONTRIBUTING.md lately? If not, do that first.

• Label PR with a Jira ticket number and include a link to the ticket
• Label PR with a security risk modifier [no, low, medium, high]
• PR describes scope of changes
• Get a minimum of one thumbs worth of review, preferably two if enough team members are available
• Get PO sign-off for all non-trivial UI or workflow changes
• Verify all tests go green
• Test this change deployed correctly and works on dev environment after deployment

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Pull request overview

Copilot reviewed 16 out of 17 changed files in this pull request and generated no new comments.

Suppressed comments (1)

docs/study-template-v1.md:65

• The contract text says JSON values must be "compact", but the canonical fixtures include whitespace inside JSON arrays/objects (e.g., "cloud": ["GCP"]). If whitespace is allowed (as implied by the fixtures and ObjectMapper parsing), the contract should say "JSON" rather than requiring a compact representation to avoid ambiguity for producers/consumers.

| Array | Compact JSON array in one CSV cell, such as `["Genomic","Phenotypic"]`. CSV escaping still applies. |
| Object | Compact JSON object in one CSV cell, used only for supported asset payloads and `fileTypes` entries. |

[Copilot]

Pull request overview

Copilot reviewed 16 out of 17 changed files in this pull request and generated 3 comments.

[rushtong]

Nice work 👍🏽

[fboulnois]

Have we run these templates by Jonathan? I see some JSON in the multi-consent-group. I think these templates were meant to be user facing, so that might become complicated with embedded JSON.

[otchet-broad]

The alternativeSharingPlan should be run to ground.

[Copilot]

Pull request overview

Copilot reviewed 16 out of 17 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/test/java/org/broadinstitute/consent/http/models/dto/registration/StudyTemplateV1FixturesTest.java:115

• This duplicate-detection key uses a NUL character (\0), which makes assertion output hard to interpret. Use a visible delimiter (e.g., ::) to keep debugging straightforward.

            .map(csvRecord -> csvRecord.get(1) + '\0' + csvRecord.get(2) + '\0' + csvRecord.get(4))

src/test/java/org/broadinstitute/consent/http/models/dto/registration/StudyTemplateV1FixturesTest.java:61

• Building fieldKey using a NUL character (\0) makes assertion failure messages hard to read/debug and can behave oddly in logs. Prefer a visible delimiter (e.g., ::) via String.join to keep keys unambiguous and readable.

This issue also appears on line 115 of the same file.

        String fieldKey = csvRecord.get(1) + '\0' + csvRecord.get(2) + '\0' + csvRecord.get(4);

[kevinmarete]

@jlaw-codes — DT-3869 defines the v1 CSV contract for importing one study and its datasets into a registration draft (doc: docs/study-template-v1.md). Nothing is parsed or wired up yet; the contract gates that work, so I need a product call on two things before it merges.

1. Dropping the alternative data sharing plan from the template

The template can't carry file uploads. Rather than exclude only alternativeDataSharingPlanFileName, I've removed the whole alternativeDataSharingPlan* group — the boolean, reasons, explanation, dataSubmitted, dataReleased, both target dates, and accessManagement. The reasoning: importing the flags without the document produces a draft the user still has to open the form to finish, so the CSV saves them nothing.

Is that right — a user who needs an alternative sharing plan fills that section in on the populated draft, where the upload lives? Or would you rather the non-file fields import anyway so they only have to attach the document?

2. How much hand-authored JSON is acceptable

Each row is one field. Multi-value fields like dataTypes are one row per item:

1,study,study,,dataTypes,Genomic
1,study,study,,dataTypes,Phenotypic

That keeps JSON out of every field a typical study needs. It survives in two optional places — fileTypes on a dataset, and non-file assets (workspaces, publications, presentations, clinical trials, funding, IP), where the whole payload is one JSON object in a cell:

1,asset,workspace-1,study,workspaces,"{""name"":""...."",""platform"":""Terra"",...}"

Neither appears in a minimal template. Is that acceptable for the people who'll actually fill these out? If not, the realistic alternative is dropping non-file assets from v1 and adding them once there's a UI to author them — there's no
lossless flat CSV form for those payloads.

One thing that would help me judge #2: how do you expect producers to get the template — download a pre-filled one from DUOS, or build it from this spec? If DUOS generates it, the JSON is something people edit rather than write, and it matters much less.

[jlaw-codes]

Fine to exclude Alt DS plans
No JSON is preferred

[Copilot]

Pull request overview

Copilot reviewed 23 out of 24 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/test/java/org/broadinstitute/consent/http/models/dto/registration/StudyTemplateV1FixturesTest.java:50

• The Javadoc says fileTypes "stays a single JSON cell", but this contradicts the v1 contract (and the rest of this test) which asserts that v1 has no JSON encoding and that fileTypes is modeled via fileType records. This comment is misleading for future readers maintaining the fixture rules.

  /**
   * Array-typed fields that the contract encodes as one row per item, so the same (recordType,
   * recordId, field) tuple may legitimately repeat. Every other field is single-valued. {@code
   * fileTypes} is an object array and stays a single JSON cell.
   */

[kevinmarete]

Pushed two rounds of changes since your reviews. Summary of what moved:

• Alternative data sharing plan — the whole alternativeDataSharingPlan* group is gone, not just the filename field. @jlaw-codes confirmed.
• No JSON in the template. Also from @jlaw-codes. Scalar arrays now repeat their row (dataTypes on two rows instead of "[""Genomic""]"), fileTypes became a fileType record type, and non-file assets are deferred to v2 — their payloads nest, so a JSON-free encoding would need dotted paths or second-level child records.
• 5 MiB limit — now has a rationale section; see the thread below.
• Excel / Google Sheets — new Spreadsheet compatibility section and two new fixtures; see the thread below.

The plan in duos-ui is updated to match, including the ticket changes these decisions force: DataBiosphere/duos-ui#3828.

@otchet-broad this should cover your change request when you get a chance.

[kevinmarete]

@fboulnois yes — took it to Jonathan, and he preferred no JSON, so it's all gone rather than justified. Scalar arrays repeat their row, fileTypes became its own record type, and non-file assets are deferred to v2 since their payloads nest. Even minimal-valid.csv no longer has "[""Genomic""]" in it.

He also confirmed the template gets downloaded from the DUOS UI rather than hand-built, which is now a v1 requirement on the UI side — DataBiosphere/duos-ui#3828 adds it to Ticket 4.

[Copilot]

Pull request overview

Copilot reviewed 23 out of 24 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/test/java/org/broadinstitute/consent/http/models/dto/registration/StudyTemplateV1FixturesTest.java:50

• The Javadoc for SCALAR_ARRAY_FIELDS says fileTypes “stays a single JSON cell”, but this contradicts the v1 contract and the rest of this test (v1 has no JSON-in-cell encoding; fileTypes is represented via fileType records). This comment should be corrected to avoid misleading future parser/test work.

  /**
   * Array-typed fields that the contract encodes as one row per item, so the same (recordType,
   * recordId, field) tuple may legitimately repeat. Every other field is single-valued. {@code
   * fileTypes} is an object array and stays a single JSON cell.
   */

[Copilot]

Pull request overview

Copilot reviewed 23 out of 24 changed files in this pull request and generated no new comments.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud