-
Notifications
You must be signed in to change notification settings - Fork 530
11914 set template default #11989
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
base: develop
Are you sure you want to change the base?
11914 set template default #11989
Conversation
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements API endpoints to set and remove the default template for a dataverse collection, addressing issue #11914.
Key Changes:
- Added
POST /api/dataverses/{id}/templates/default/{templateId}endpoint to set a default template - Added
DELETE /api/dataverses/{id}/templates/defaultendpoint to remove the default template - Added test coverage for the set default template endpoint with permission checks
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
src/main/java/edu/harvard/iq/dataverse/api/Dataverses.java |
Implements two new endpoints for setting and removing default templates |
src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java |
Adds helper methods setDefaultTemplate() and removeDefaultTemplate() for testing |
src/test/java/edu/harvard/iq/dataverse/api/DataversesIT.java |
Updates testCreateAndGetTemplates() to test setting default template with authorization checks |
doc/release-notes/11914-set-template-default-api.md |
Documents the new endpoints and their functionality |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…or removing default template
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| ### Functionality | ||
| - Sets the default template of the given dataverse collection. | ||
| - You must have edit dataset permission in the collection in order to use this endpoint. |
Copilot
AI
Nov 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The permission requirement is incorrectly stated as "edit dataset permission" but should be "edit dataverse permission" or "edit collection permission". The command uses @RequiredPermissions(Permission.EditDataverse), not edit dataset permission.
|
|
||
| ### Functionality | ||
| - Removes the default template of the given dataverse collection. | ||
| - You must have edit dataset permission in the collection in order to use this endpoint. |
Copilot
AI
Nov 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The permission requirement is incorrectly stated as "edit dataset permission" but should be "edit dataverse permission" or "edit collection permission". The command uses @RequiredPermissions(Permission.EditDataverse), not edit dataset permission.
|
|
||
| @Override | ||
| public Template execute(CommandContext ctxt) throws CommandException { | ||
| dataverse.setDefaultTemplate(template); |
Copilot
AI
Nov 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The command modifies the dataverse but doesn't persist the changes to the database. After calling dataverse.setDefaultTemplate(template), you should merge the dataverse entity using ctxt.em().merge(dataverse) to ensure the changes are persisted. See similar commands like CreateTemplateCommand (line 55) for reference.
| dataverse.setDefaultTemplate(template); | |
| dataverse.setDefaultTemplate(template); | |
| ctxt.em().merge(dataverse); |
| @Override | ||
| public Dataverse execute(CommandContext ctxt) throws CommandException { | ||
| dataverse.setDefaultTemplate(null); | ||
| return dataverse; |
Copilot
AI
Nov 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The command modifies the dataverse but doesn't persist the changes to the database. After calling dataverse.setDefaultTemplate(null), you should merge the dataverse entity using ctxt.em().merge(dataverse) to ensure the changes are persisted. See similar commands like CreateTemplateCommand (line 55) for reference.
| return dataverse; | |
| Dataverse mergedDataverse = ctxt.em().merge(dataverse); | |
| return mergedDataverse; |
| - Sets the default template of the given dataverse collection. | ||
| - You must have edit dataset permission in the collection in order to use this endpoint. | ||
|
|
||
| ## New Endpoint: DELETE `/dataverses/{id}/template/default` |
Copilot
AI
Nov 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The endpoint path in the documentation uses /template/default (singular "template"), but based on the actual implementation in Dataverses.java the path is /templates/default (plural "templates"). Update the documentation to use the correct plural form: /dataverses/{id}/templates/default.
… for setting and removing default templates
|
📦 Pushed preview images as 🚢 See on GHCR. Use by referencing with full name as printed above, mind the registry name. |
|
|
||
| protected Template findTemplateOrDie(Long templateId, Dataverse dataverse) throws WrappedResponse { | ||
|
|
||
| Template template = dataverse.getTemplates().stream() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to take into account whether the dataverse is inheriting templates from its parents.
sekmiller
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jp-tosca looks good generally - can you take a look at the handling of inherited templates in the find template or die method? thanks!
What this PR does / why we need it:
Which issue(s) this PR closes:
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: