Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
138a62a
#11391 fix mdb display on create
sekmiller Apr 8, 2025
8353cda
Merge branch 'develop' into 11391-displayOnCreate-with-template
sekmiller Apr 8, 2025
88d72cf
typo
sekmiller Apr 8, 2025
67e5c5c
Merge branch 'develop' into 11391-displayOnCreate-with-template
sekmiller Apr 10, 2025
ce1f019
Merge branch 'develop' into 11391-displayOnCreate-with-template
sekmiller Apr 15, 2025
0c7db76
#11391 add logic for conditionally required subfields
sekmiller Apr 16, 2025
9b4516c
#11391 change requirement to "any" for parent required
sekmiller Apr 16, 2025
29c8cd9
#11391 add comment
sekmiller Apr 16, 2025
a84d4d7
Merge branch 'develop' into 11391-displayOnCreate-with-template
sekmiller Apr 17, 2025
0b17352
#11391 code cleanup
sekmiller Apr 17, 2025
6956a85
#11391 missed one
sekmiller Apr 17, 2025
f3b68ca
Merge branch 'develop' into 11391-displayOnCreate-with-template
sekmiller Apr 18, 2025
b3c914c
Merge branch 'develop' into 11391-displayOnCreate-with-template
sekmiller Apr 22, 2025
2f13a7e
Merge branch 'develop' into 11391-displayOnCreate-with-template
sekmiller Apr 22, 2025
6353eb3
Merge branch 'develop' into 11391-displayOnCreate-with-template
sekmiller Apr 23, 2025
7ad65fb
#11391 fix req'd parent/conditional logic
sekmiller Apr 23, 2025
fac6017
Merge branch 'develop' into 11391-displayOnCreate-with-template
sekmiller Apr 28, 2025
9bb2d69
Merge branch 'develop' into 11391-displayOnCreate-with-template
sekmiller Apr 29, 2025
af90095
Merge branch 'develop' into 11391-displayOnCreate-with-template
sekmiller May 1, 2025
6d5533f
Merge branch 'develop' into 11391-displayOnCreate-with-template
sekmiller May 6, 2025
86df1c3
Merge branch 'develop' into 11391-displayOnCreate-with-template
sekmiller May 7, 2025
c2ac931
Merge branch 'develop' into 11391-displayOnCreate-with-template
sekmiller May 8, 2025
80fded1
Merge branch 'develop' into 11391-displayOnCreate-with-template
sekmiller May 15, 2025
1df00af
Merge branch 'develop' into 11391-displayOnCreate-with-template
sekmiller May 20, 2025
533500f
#11391 code review suggestions/fixes
sekmiller May 20, 2025
e474e1c
Merge branch 'develop' into 11391-displayOnCreate-with-template
sekmiller May 20, 2025
4aad960
#11391 re-order calls
sekmiller May 21, 2025
988a7bb
Merge branch 'develop' into 11391-displayOnCreate-with-template
sekmiller May 27, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 45 additions & 3 deletions src/main/java/edu/harvard/iq/dataverse/DatasetField.java
Original file line number Diff line number Diff line change
Expand Up @@ -433,11 +433,13 @@ public void setValidationMessage(String validationMessage) {
private Boolean required;
@Transient
private Boolean hasRequiredChildren;


@Transient
private Boolean hasRequiredChildrenDV;

public boolean isRequired() {
if (required == null) {
required = false;

required = false;
if (this.datasetFieldType.isRequired()) {
required = true;
} else {
Expand All @@ -464,11 +466,51 @@ public boolean isRequired() {
{
required = false;
}

Comment thread
qqmyers marked this conversation as resolved.
//this is needed to enforce required children validation and display on create #11421
//set the parent to required only if a child is set to required at the DV level
if (this.datasetFieldType.isCompound() && isHasRequiredChildrenDV()) {
required = true;
}

}

return required;
}

public boolean isHasRequiredChildrenDV() {

if (hasRequiredChildrenDV == null) {
hasRequiredChildrenDV = false;
}

Dataverse dv = getDataverse();
while (!dv.isMetadataBlockRoot()) {
if (dv.getOwner() == null) {
break; // we are at the root; which by defintion is metadata blcok root, regarldess of the value
}
dv = dv.getOwner();
}

List<DataverseFieldTypeInputLevel> dftilListFirst = dv.getDataverseFieldTypeInputLevels();

if (getDatasetFieldType().isHasChildren() && (!dftilListFirst.isEmpty())) {
for (DatasetFieldType child : getDatasetFieldType().getChildDatasetFieldTypes()) {
for (DataverseFieldTypeInputLevel dftilTest : dftilListFirst) {
if (child.equals(dftilTest.getDatasetFieldType())) {
//We only want to get it here if it is required by
//the DV and not by default at the installation/DB level
if (dftilTest.isRequired() && !child.isRequired()) {
hasRequiredChildrenDV = true;
Comment thread
qqmyers marked this conversation as resolved.
break;
}
}
}
}
}
return hasRequiredChildrenDV;
}

Copy link
Copy Markdown
Member

@qqmyers qqmyers May 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside from lines 518/519, it looks like this method and the next one are the same? Is there a way to consolidate? E.g. is isHasRequiredChildren == isHasRequiredChildrenDV || this.datasetFieldType.isHasRequiredChildren() (not requiring two transients)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem here is maintaining the conditionally required child dataset set field type. If the child is required at the installation level (in the db), but it's parent is not required in the db then it is conditionally required. There's no way to make a child dataset field type conditionally required at the dv level, because there's no way to set the parent as required. We are assuming that if a child field is made required at the dv level then the parent must also be required.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 isHasRequiredChildren() {
if (hasRequiredChildren == null) {
hasRequiredChildren = false;
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/edu/harvard/iq/dataverse/DatasetPage.java
Original file line number Diff line number Diff line change
Expand Up @@ -2132,8 +2132,9 @@ private String init(boolean initFull) {
ownerId = dataset.getOwner().getId();
datasetNextMajorVersion = this.dataset.getNextMajorVersionString();
datasetNextMinorVersion = this.dataset.getNextMinorVersionString();
datasetVersionUI = datasetVersionUI.initDatasetVersionUI(workingVersion, false);
updateDatasetFieldInputLevels();
datasetVersionUI = datasetVersionUI.initDatasetVersionUI(workingVersion, false);

setExistReleasedVersion(resetExistRealeaseVersion());
//moving setVersionTabList to tab change event
//setVersionTabList(resetVersionTabList());
Expand Down
4 changes: 4 additions & 0 deletions src/main/java/edu/harvard/iq/dataverse/DatasetVersionUI.java
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,10 @@ public void setMetadataValueBlocks(DatasetVersion datasetVersion) {
mdb.setEmpty(false);
datasetFieldsForView.add(dsf);
}
//Setting Local Display on Create on mdb when there are any set at dataverse level
if (dsf.getDatasetFieldType().getLocalDisplayOnCreate() != null && dsf.getDatasetFieldType().getLocalDisplayOnCreate()){
mdb.setLocalDisplayOnCreate(true);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ public String cloneTemplate(Template templateIn) {
Template newOne = templateIn.cloneNewTemplate(templateIn);
String name = BundleUtil.getStringFromBundle("page.copy") +" " + templateIn.getName();
newOne.setName(name);
newOne.setUsageCount(new Long(0));
newOne.setUsageCount(Long.valueOf(0));
newOne.setCreateTime(new Timestamp(new Date().getTime()));
newOne.setDataverse(dataverse);

Expand Down
16 changes: 16 additions & 0 deletions src/main/java/edu/harvard/iq/dataverse/MetadataBlock.java
Original file line number Diff line number Diff line change
Expand Up @@ -101,14 +101,30 @@ public void setDatasetFieldTypes(List<DatasetFieldType> datasetFieldTypes) {
}

public boolean isDisplayOnCreate() {
// relying on "should" doesn't seem to work in context of a template
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should relies on a transient datsetfieldtype localdisplayoncreate that must not be getting getting set in the DatasetVersionUI init method. If it were, it think it should work. Is it just because of the order of these 2 lines:

datasetVersionUI = datasetVersionUI.initDatasetVersionUI(workingVersion, false);
updateDatasetFieldInputLevels();
- the updateDatasetFieldInputLevels sets localdisplayOncreate, right after the dvUI is inited. (Same ordering of these two calls in the DatasetPage.edit method.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did try switching the order here and commenting out the section in init-UI but it still doesn't display the metadata block that has only display on create set at the DV level.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

// adding a transient that is updated in the DatasetVersionUI to fix
for (DatasetFieldType dsfType : datasetFieldTypes) {
boolean shouldDisplayOnCreate = dsfType.shouldDisplayOnCreate();
if (shouldDisplayOnCreate) {
return true;
}
}
if (getLocalDisplayOnCreate() != null){
return getLocalDisplayOnCreate();
}
return false;
}

@Transient
private Boolean localDisplayOnCreate;

public Boolean getLocalDisplayOnCreate() {
return localDisplayOnCreate;
}

public void setLocalDisplayOnCreate(Boolean localDisplayOnCreate) {
this.localDisplayOnCreate = localDisplayOnCreate;
}

public String getDisplayName() {
return displayName;
Expand Down
2 changes: 1 addition & 1 deletion src/main/webapp/metadataFragment.xhtml
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@
<div class="panel-body">
<ui:repeat value="#{metadataBlockVal.value}" var="dsf" varStatus="curField">
<div class="form-group" role="group"
jsf:rendered="#{((editMode == 'METADATA' or dsf.datasetFieldType.shouldDisplayOnCreate() or !dsf.isEmpty() or dsf.required) and dsf.include) or (!datasetPage and dsf.include)}">
jsf:rendered="#{((editMode == 'METADATA' or dsf.datasetFieldType.shouldDisplayOnCreate() or !dsf.isEmpty() or dsf.required or dsf.hasRequiredChildren)) or (!datasetPage and dsf.include)}">
<!-- Primitive fields -->
<p:fragment id="editPrimitiveValueFragment" rendered="#{dsf.datasetFieldType.primitive}">
<p:autoUpdate/>
Expand Down