Skip to content

Respect scope in required field validation for hidden/readonly fields #521

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main-cris
Choose a base branch
from
Open

Respect scope in required field validation for hidden/readonly fields #521

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
51 changes: 34 additions & 17 deletions dspace-api/src/main/java/org/dspace/validation/MetadataValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,14 @@
import org.dspace.content.InProgressSubmission;
import org.dspace.content.Item;
import org.dspace.content.MetadataValue;
import org.dspace.content.WorkspaceItem;
import org.dspace.content.authority.service.MetadataAuthorityService;
import org.dspace.content.service.ItemService;
import org.dspace.core.Constants;
import org.dspace.core.Context;
import org.dspace.core.exception.SQLRuntimeException;
import org.dspace.services.ConfigurationService;
import org.dspace.validation.model.ValidationError;
import org.dspace.workflow.WorkflowItem;

/**
* Execute three validation check on fields validation: - mandatory metadata
Expand All @@ -49,7 +49,7 @@ public class MetadataValidator implements SubmissionStepValidator {

private static final String ERROR_VALIDATION_AUTHORITY_REQUIRED = "error.validation.authority.required";

private static final String ERROR_VALIDATION_REGEX = "error.validation.regex";
private static final String ERROR_VALIDATION_PREFIX = "error.validation.regex";

private static final String ERROR_VALIDATION_NOT_REPEATABLE = "error.validation.notRepeatable";

Expand All @@ -67,6 +67,10 @@ public class MetadataValidator implements SubmissionStepValidator {

@Override
public List<ValidationError> validate(Context context, InProgressSubmission<?> obj, SubmissionStepConfig config) {
// Determine current scope
String currentScope = (obj instanceof WorkspaceItem) ?
DCInput.SUBMISSION_SCOPE : DCInput.WORKFLOW_SCOPE;


List<ValidationError> errors = new ArrayList<>();

Expand Down Expand Up @@ -108,10 +112,18 @@ public List<ValidationError> validate(Context context, InProgressSubmission<?> o
}
}
if (input.isRequired() && !foundResult) {
// for this required qualdrop no value was found, add to the list of error fields
addError(errors, ERROR_VALIDATION_REQUIRED,
"/" + OPERATION_PATH_SECTIONS + "/" + config.getId() + "/" +
input.getFieldName());

// Check if field is visible and not readonly in current scope
boolean isVisibleInCurrentScope = input.isVisible(currentScope);
boolean isReadonlyInCurrentScope = input.isReadOnly(currentScope);

// Only add error if field is visible, not readonly, and allowed for document type
if (isVisibleInCurrentScope && !isReadonlyInCurrentScope && input.isAllowedFor(documentType)) {
// for this required qualdrop no value was found, add to the list of error fields
addError(errors, ERROR_VALIDATION_REQUIRED,
"/" + OPERATION_PATH_SECTIONS + "/" + config.getId() + "/" +
input.getFieldName());
}
}

} else {
Expand Down Expand Up @@ -140,16 +152,21 @@ public List<ValidationError> validate(Context context, InProgressSubmission<?> o
}
}
validateMetadataValues(obj.getCollection(), mdv, input, config,
isAuthorityControlled, fieldKey, errors);
if ((input.isRequired() && mdv.size() == 0)
&& (input.isVisible(DCInput.SUBMISSION_SCOPE)
|| (obj instanceof WorkflowItem && input.isVisible(DCInput.WORKFLOW_SCOPE)))
&& !valuesRemoved) {
// Is the input required for *this* type? In other words, are we looking at a required
// input that is also allowed for this document type
if (input.isAllowedFor(documentType)) {
// since this field is missing add to list of error
// fields
isAuthorityControlled, fieldKey, errors);
if ((input.isRequired() && mdv.size() == 0) && !valuesRemoved) {

// Check if field is visible in current scope
boolean isVisibleInCurrentScope = input.isVisible(currentScope);

// Check if field is readonly or hidden in current scope
boolean isReadonlyInCurrentScope = input.isReadOnly(currentScope);

// Only validate as required if:
// 1. Field is visible in current scope AND
// 2. Field is NOT readonly in current scope AND
// 3. Field is allowed for this document type
if (isVisibleInCurrentScope && !isReadonlyInCurrentScope && input.isAllowedFor(documentType)) {
// since this field is missing add to list of error fields
addError(errors, ERROR_VALIDATION_REQUIRED,
"/" + OPERATION_PATH_SECTIONS + "/" + config.getId() + "/" +
input.getFieldName());
Expand Down Expand Up @@ -181,7 +198,7 @@ private void validateMetadataValues(Collection collection, List<MetadataValue> m

for (MetadataValue md : metadataValues) {
if (! (input.validate(md.getValue()))) {
addError(errors, ERROR_VALIDATION_REGEX,
addError(errors, ERROR_VALIDATION_PREFIX,
"/" + OPERATION_PATH_SECTIONS + "/" + config.getId() + "/" +
input.getFieldName() + "/" + md.getPlace());
}
Expand Down
12 changes: 12 additions & 0 deletions dspace-api/src/test/data/dspaceFolder/config/item-submission.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
<name-map collection-entity-type="CustomEntityType" submission-name="entitytypetest"/>
<name-map collection-handle="123456789/test-duplicate-detection" submission-name="test-duplicate-detection"/>
<name-map collection-handle="123456789/collection-test-patch" submission-name="publicationTestPatch"/>
<name-map collection-handle="123456789/test-metadata-validator" submission-name="test-metadata-validator"/>
</submission-map>


Expand Down Expand Up @@ -319,6 +320,13 @@
<processing-class>org.dspace.app.rest.submit.step.NotifyStep</processing-class>
<type>coarnotify</type>
</step-definition>

<step-definition id="test-metadata-validator-step">
<heading></heading>
<processing-class>org.dspace.app.rest.submit.step.DescribeStep</processing-class>
<type>collection</type>
</step-definition>

<step-definition id="upload-no-required-metadata" mandatory="true">
<heading>submit.progressbar.upload-no-required-metadata</heading>
<processing-class>org.dspace.app.rest.submit.step.UploadStep</processing-class>
Expand Down Expand Up @@ -543,6 +551,10 @@
<step id="traditionalpageone"/>
</submission-process>

<submission-process name="test-metadata-validator">
<step id="test-metadata-validator-step"/>
</submission-process>

<submission-process name="publicationTestPatch">
<step id="collection" />
<step id="traditionalpageone" />
Expand Down
72 changes: 72 additions & 0 deletions dspace-api/src/test/data/dspaceFolder/config/submission-forms.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2450,6 +2450,78 @@ it, please enter the types and the actual numbers or codes.</hint>
</field>
</row>
</form>
<form name="test-metadata-validator-step">
<row>
<field>
<dc-schema>dc</dc-schema>
<dc-element>title</dc-element>
<repeatable>false</repeatable>
<label>Title</label>
<input-type>onebox</input-type>
<required>Title is required</required>
<hint>Enter the title of the item</hint>
</field>
<field>
<dc-schema>dc</dc-schema>
<dc-element>contributor</dc-element>
<dc-qualifier>author</dc-qualifier>
<repeatable>true</repeatable>
<label>Author(s)</label>
<input-type>onebox</input-type>
<required>Author is required</required>
<hint>Enter the author(s) of the item</hint>
<readonly>workflow</readonly>
</field>
<field>
<dc-schema>dc</dc-schema>
<dc-element>date</dc-element>
<dc-qualifier>issued</dc-qualifier>
<repeatable>false</repeatable>
<label>Date Issued</label>
<input-type>date</input-type>
<required>Date issued is required</required>
<hint>Enter the date the item was issued</hint>
<visibility>submission</visibility>
<readonly>submission</readonly>
</field>
</row>
<row>
<field>
<dc-schema>dc</dc-schema>
<dc-element>description</dc-element>
<dc-qualifier>abstract</dc-qualifier>
<repeatable>false</repeatable>
<label>Abstract</label>
<input-type>textarea</input-type>
<required>Abstract is required</required>
<hint>Enter an abstract for the item</hint>
<visibility>workflow</visibility>
<readonly>submission</readonly>
</field>
<field>
<dc-schema>dc</dc-schema>
<dc-element>subject</dc-element>
<repeatable>true</repeatable>
<label>Subject(s)</label>
<input-type>onebox</input-type>
<required>Subject is required</required>
<hint>Enter subject keywords for the item</hint>
<visibility>workflow</visibility>
<readonly>workflow</readonly>
</field>
<field>
<dc-schema>dc</dc-schema>
<dc-element>type</dc-element>
<repeatable>false</repeatable>
<label>Type</label>
<input-type>onebox</input-type>
<required>Type is required</required>
<hint>Enter the type of the item</hint>
<visibility>submission</visibility>
<readonly>all</readonly>
</field>
</row>
</form>
</form-definitions>


Expand Down
176 changes: 176 additions & 0 deletions dspace-api/src/test/java/org/dspace/validation/MetadataValidatorIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
/**
* The contents of this file are subject to the license and copyright
* detailed in the LICENSE and NOTICE files at the root of the source
* tree and available online at
*
* http://www.dspace.org/license/
*/
package org.dspace.validation;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.hasSize;

import java.util.List;
import java.util.stream.Collectors;

import org.dspace.AbstractIntegrationTestWithDatabase;
import org.dspace.app.util.DCInputsReader;
import org.dspace.app.util.SubmissionConfig;
import org.dspace.app.util.SubmissionConfigReader;
import org.dspace.app.util.SubmissionStepConfig;
import org.dspace.builder.CollectionBuilder;
import org.dspace.builder.CommunityBuilder;
import org.dspace.builder.WorkflowItemBuilder;
import org.dspace.builder.WorkspaceItemBuilder;
import org.dspace.content.Collection;
import org.dspace.content.Community;
import org.dspace.content.WorkspaceItem;
import org.dspace.content.authority.factory.ContentAuthorityServiceFactory;
import org.dspace.content.factory.ContentServiceFactory;
import org.dspace.content.service.ItemService;
import org.dspace.services.ConfigurationService;
import org.dspace.services.factory.DSpaceServicesFactory;
import org.dspace.validation.model.ValidationError;
import org.dspace.workflow.WorkflowItem;
import org.junit.Before;
import org.junit.Test;

/**
* Integration test for {@link MetadataValidator} to ensure it respects
* readonly and hidden scopes for required fields.
*
* This test class relies on a custom submission definition ("test-metadata-validator")
* which must be configured in dspace-api/src/test/data/dspaceFolder/config/submission-forms.xml and
* dspace-api/src/test/data/dspaceFolder/config/item-submission.xml.
*
*/
public class MetadataValidatorIT extends AbstractIntegrationTestWithDatabase {

private Collection collection;
private MetadataValidator validator;
private SubmissionStepConfig submissionStepConfig;
private ItemService itemService = ContentServiceFactory.getInstance().getItemService();
private ConfigurationService configurationService = DSpaceServicesFactory.getInstance().getConfigurationService();

@Before
@Override
public void setUp() throws Exception {
super.setUp();

context.turnOffAuthorisationSystem();

Community community = CommunityBuilder.createCommunity(context).build();
collection = CollectionBuilder.createCollection(context, community)
.withSubmissionDefinition("test-metadata-validator")
.build();

context.restoreAuthSystemState();

SubmissionConfigReader submissionConfigReader = new SubmissionConfigReader();
SubmissionConfig submissionConfig = submissionConfigReader.getSubmissionConfigByCollection(collection);
// Assumes the test submission process has one step defined
submissionStepConfig = submissionConfig.getStep(0);

validator = new MetadataValidator();
validator.setName("test-validator");
validator.setItemService(itemService);
validator.setMetadataAuthorityService(ContentAuthorityServiceFactory.getInstance()
.getMetadataAuthorityService());
validator.setConfigurationService(configurationService);
validator.setInputReader(new DCInputsReader());
}

/**
* In SUBMISSION scope, ensures validation reports only required fields
* not marked readonly/hidden for submission. Missing dc.title and
* dc.contributor.author should be flagged; dc.date.issued is ignored.
*/
@Test
public void testValidationInSubmissionScopeWithErrors() throws Exception {
context.turnOffAuthorisationSystem();
WorkspaceItem wsi = WorkspaceItemBuilder.createWorkspaceItem(context, collection).build();
context.restoreAuthSystemState();

List<ValidationError> errors = validator.validate(context, wsi, submissionStepConfig);

assertThat(errors, hasSize(1));

List<String> errorPaths = getErrorPaths(errors);
assertThat(errorPaths, containsInAnyOrder(
"/sections/test-metadata-validator-step/dc.title",
"/sections/test-metadata-validator-step/dc.contributor.author"
));
}

/**
* In SUBMISSION scope, ensures no errors when all required fields
* for this scope are present. Fields readonly/hidden in submission
* can remain empty without errors.
*/
@Test
public void testValidationInSubmissionScopeWithoutErrors() throws Exception {
context.turnOffAuthorisationSystem();
WorkspaceItem wsi = WorkspaceItemBuilder.createWorkspaceItem(context, collection)
.withTitle("Test Title")
.withAuthor("Test, Author")
.withIssueDate("2025-08-14")
.build();
context.restoreAuthSystemState();

List<ValidationError> errors = validator.validate(context, wsi, submissionStepConfig);

assertThat(errors, empty());
}

/**
* In WORKFLOW scope, ensures validation reports only required fields
* not marked readonly/hidden for workflow. Missing dc.title and
* dc.description.abstract should be flagged; others are ignored.
*/
@Test
public void testValidationInWorkflowScopeWithErrors() throws Exception {
context.turnOffAuthorisationSystem();
WorkflowItem wfi = WorkflowItemBuilder.createWorkflowItem(context, collection).build();
context.restoreAuthSystemState();

List<ValidationError> errors = validator.validate(context, wfi, submissionStepConfig);

assertThat(errors, hasSize(1));

List<String> errorPaths = getErrorPaths(errors);
assertThat(errorPaths, containsInAnyOrder(
"/sections/test-metadata-validator-step/dc.title",
"/sections/test-metadata-validator-step/dc.description.abstract"
));
}

/**
* In WORKFLOW scope, ensures no errors when all required fields
* for this scope are present. Fields readonly/hidden in workflow
* can remain empty without errors.
*/
@Test
public void testValidationInWorkflowScopeWithoutErrors() throws Exception {
context.turnOffAuthorisationSystem();
WorkflowItem wfi = WorkflowItemBuilder.createWorkflowItem(context, collection).build();

// Add metadata for fields required in the workflow scope
itemService.addMetadata(context, wfi.getItem(), "dc", "title", null, null, "Test Title");
itemService.addMetadata(context, wfi.getItem(), "dc", "description", "abstract", null, "Test Abstract");
itemService.addMetadata(context, wfi.getItem(), "dc", "subject", null, null, "Test Subject");

context.restoreAuthSystemState();

List<ValidationError> errors = validator.validate(context, wfi, submissionStepConfig);

assertThat(errors, empty());
}

private List<String> getErrorPaths(List<ValidationError> errors) {
return errors.stream()
.flatMap(error -> error.getPaths().stream())
.collect(Collectors.toList());
}
}