Allow Dataset Creation API to take Template ID

[stevenwinship]

What this PR does / why we need it: Users creating Datasets need to be able to set the Template via the Create API

Which issue(s) this PR closes:#12132

• Closes Support applying template id in API of dataset creation and persist dataset–template associations #12132

Special notes for your reviewer:

Suggestions on how to test this:

Does this PR introduce a user interface change? If mockups are available, please link/include them here:

Is there a release notes update needed for this change?:

Additional documentation:

[coveralls]

coverage: 25.003% (-0.001%) from 25.004% — 12132-dataset-creation-with-template into develop

[github-actions]

Test Results

404 tests   389 ✅  33m 52s ⏱️
 55 suites   15 💤
 55 files      0 ❌

Results for commit d5908b6.

♻️ This comment has been updated with latest results.

[ekraffmiller]

@stevenwinship to support the SPA, we also need the Get Dataset API to return the template_id, can you add that to this PR? Or would it be better to have a separate issue?

[ekraffmiller]

@stevenwinship to support the SPA, we also need the Get Dataset API to return the template_id, can you add that to this PR? Or would it be better to have a separate issue?

Update from Steve, this is already in the current PR. thanks!

[rtreacy]

Claude
PR #12405 Review: Allow Dataset Creation API to take Template ID

What it does

Adds support for "templateId": $templateId in the POST /dataverses/{identifier}/datasets JSON payload. JsonParser.parseDataset() now looks up the template and calls dataset.setTemplate(template); JsonPrinter echoes templateId back in dataset JSON; and CreateNewDatasetCommand's existing (Dataset, DataverseRequest, Template, boolean) constructor is tweaked to fall back to theDataset.getTemplate() when no Template object is passed explicitly — which is how the value set by the JSON parser reaches the command that bumps the template's usage count. JsonParser's constructor gained a TemplateServiceBean dependency, which required updating every call site (production and test) — hence the size of the diff.

Correctness issues

1. Missing dataverse scoping on the template lookup (authorization gap). templateService.find(templateId) (JsonParser.java:292) looks up any template in the system by ID, with no check that it belongs to the target dataverse (owner) or one of its ancestors. Contrast with the UI flow (DatasetPage.java:2247), which only offers templates from dataverseService.find(ownerId).getTemplates(). Via this API, a user with AddDataset permission on dataverse A can supply the numeric ID of a template that lives in an unrelated, possibly private/restricted dataverse B, and:

• dataset.setTemplate(template) persists that cross-dataverse association and is echoed back as templateId in the dataset JSON.

• CreateNewDatasetCommand.postPersist calls ctxt.templates().incrementUsageCount(template.getId()) unconditionally on that foreign template.

  The practical impact is limited — in this API flow getVersionToPersist() still calls the no-arg theDataset.getOrCreateEditVersion(), so the template's field defaults are never applied to the new dataset (unlike the UI path); the effect is essentially an ID-based association plus a usage-count bump on a template the caller may have no visibility into. Still, it's a real scoping gap versus the UI's behavior and worth validating (template.getDataverse() is an ancestor of owner, or reject with 400) before merging.

2. Non-existent templateId fails silently. templateService.find() returns null for an unknown ID (thin wrapper over em.find), and the code does nothing to handle that — dataset.setTemplate(null) is a no-op. Every other invalid-value case in this same method throws a JsonParseException (see the Invalid dataset type: ... branch immediately above this new code, JsonParser.java:288). A typo'd or stale templateId should produce a clear 400 error ("Invalid templateId: " + templateId), not silently create the dataset without the requested template.

3. obj.getInt("templateId", 0) can throw an uncaught ClassCastException. If a caller sends "templateId": "5" (string instead of number), JsonObject.getInt throws ClassCastException, not JsonParseException. The only wrapper at the call site (Dataverses.parseDataset, line ~779) catches JsonParsingException | JsonParseException — a ClassCastException would propagate as an unhandled 500 instead of a clean 400. Worth wrapping the read (or catching/rethrowing as JsonParseException) for consistency with how the rest of this method reports bad input.

Test coverage

• The new DataversesIT test covers the happy path well (create with a valid templateId, verify it round-trips via GET, clean up).
• No test for an invalid/non-existent templateId (would currently reveal issue #2 — silently succeeds instead of erroring).
• No test asserting that a templateId from an unrelated dataverse is rejected (would currently reveal issue #1 — silently succeeds).
• Since neither is currently rejected, existing tests can't catch these gaps; adding negative-path tests here would both document intended behavior and force the scoping/error-handling to be decided explicitly.

Style / minor

• Several files unrelated in substance to the feature (ImportGenericServiceBean.java, ImportServiceBean.java, SchemaDotOrgExporterTest.java, FeedbackUtilTest.java, DataversesIT.java) show large import-block reshuffles/wildcards (import edu.harvard.iq.dataverse.;, jakarta.ejb.;, java.util.*;) alongside the one-line change they actually needed (adding a TemplateServiceBean mock/field). This looks like an IDE "organize imports" pass swept up in the commit — harmless (the project's style guide explicitly says wildcard imports are "neither encouraged nor discouraged," and checkstyle's AvoidStarImport is disabled), but it's pure churn that bloats the diff and increases merge-conflict risk for unrelated in-flight PRs touching these same files. Worth trimming to just the needed changes if easy to do, though not blocking.
• Long.valueOf(templateId) where templateId is declared int templateId = obj.getInt(...) is a bit roundabout (auto-boxing an int into a Long via valueOf rather than just widening) — cosmetic only.

Verdict

The core wiring (constructor threading, JSON round-trip) is correct and the happy-path test passes. Before merging, I'd want: (1) a decision on whether templateId should be scoped to the target dataverse's own/inherited templates, since that's the existing UI behavior and its absence is the main risk here, and (2) explicit error handling for an unknown templateId instead of silent success, matching the pattern used for every other invalid field in this method.

[rtreacy]

@stevenwinship can you take a look at the Correctness issues, Test coverage and Style / motor sections of the Claude review above?

[stevenwinship]

Made fixes and added tests based on Claude

[sonarqubecloud]

Quality Gate failed

Failed conditions
10.9% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

[github-actions]

📦 Pushed preview images as

ghcr.io/gdcc/dataverse:12132-dataset-creation-with-template

ghcr.io/gdcc/configbaker:12132-dataset-creation-with-template

🚢 See on GHCR. Use by referencing with full name as printed above, mind the registry name.