11391 display on create with template#11411
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
📦 Pushed preview images as 🚢 See on GHCR. Use by referencing with full name as printed above, mind the registry name. |
There was a problem hiding this comment.
OK - looks good - thanks for the explanations. As noted, I made a small PR against this one (#11538) that has some suggested cleanup, but this PR works as is, so I'll approve and let you decide whether to merge the other one.
FWIW: I did do some testing on this following the testing instructions to set displayOnCreate for some geo block fields - and verified that I see the geo block when there's no template, when there's a default template, and when you select a non-default template.
| } | ||
| return hasRequiredChildrenDV; | ||
| } | ||
|
|
There was a problem hiding this comment.
You understand the difference better than I do, but I was mostly talking about a mechanical refactor. I made a quick PR against this branch with a refactor to drop the separate variable and reduce duplicate code. It shouldn't change functionality at all.
| } | ||
|
|
||
| public boolean isDisplayOnCreate() { | ||
| // relying on "should" doesn't seem to work in context of a template |
There was a problem hiding this comment.
Got it - I still wonder a bit about the order of those lines, but digging some more I realized why shouldDisplayOnCreate can't work in the MetadataBlock.isDisplayOnCreate call - the fieldTypes being checked there are straight from the db and not the ones from the workingVersion datasetfield.getDatasetFieldType list which has been localized to the local collection. Given that - it's clear why your solution is needed - the metadatablock localDisplayOnCreate has to be set by checking the localized field types from the working version, which is what the code in DatasetVersionUI does.
In the small PR I made against this one, I tried to edit the comments and did a little cleanup to avoid double looping. - I made comments in the PR about it.




What this PR does / why we need it: Makes fields set "display on Create" at the Dataverse level visible on dataset create in the presence of a default template for that dataset (especially if the template does not have default values for that Metadata block)
Which issue(s) this PR closes:
Special notes for your reviewer: Not sure why the "isDisplayOnCreate" in the Metadata block doesn't work. I added a transient boolean that is updated in the DatasetVersionUI when the metadata blocks are added. Again no clear reason why 11421 stopped working - a test was added to make sure that if a parent dataset field type has required children it will be set to required. Necessary for "display on create" and validation.
Suggestions on how to test this:
Set dataset field types outside of the citation block to "displayOnCreate" for a given Dataverse -
Add a template to the dataset, but don't set default values in the metadata block where you set "displayOnCreate". Make that template the default.
Add a dataset - the dataset fields set as display on create should be available for edit.
Also, you can try it with no default template but select one after "add dataset" - as above, the dataset fields set as display on create should be available for edit.
For 11421 set child fields as "required" and make sure that they are included in "display on create". Also that they must be filled in so that a dataset version is valid.
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: