Merged in DSC-573 (pull request DSpace#697)

DSC-573, DSC-977: Improved handling of hidden submission steps

Approved-by: Giuseppe Digilio

dspace-api/src/main/java/org/dspace/app/util/SubmissionStepConfig.java

@@ -11,6 +11,9 @@
 import java.util.Map;
 
 import org.apache.commons.lang3.BooleanUtils;
+import org.dspace.content.InProgressSubmission;
+import org.dspace.content.WorkspaceItem;
+import org.hibernate.proxy.HibernateProxyHelper;
 
 /**
  * Class representing configuration for a single step within an Item Submission
@@ -179,6 +182,45 @@ public String getVisibilityOutside() {
         return visibilityOutside;
     }
 
+    public boolean isHiddenForInProgressSubmission(InProgressSubmission<?> obj) {
+
+        String scopeToCheck = getScope(obj);
+
+        if (scope == null || scopeToCheck == null) {
+            return false;
+        }
+
+        String visibility = getVisibility();
+        String visibilityOutside = getVisibilityOutside();
+
+        if (scope.equalsIgnoreCase(scopeToCheck)) {
+            return "hidden".equalsIgnoreCase(visibility);
+        } else {
+            return visibilityOutside == null || "hidden".equalsIgnoreCase(visibilityOutside);
+        }
+
+    }
+
+    public boolean isReadOnlyForInProgressSubmission(InProgressSubmission<?> obj) {
+
+        String scopeToCheck = getScope(obj);
+
+        if (scope == null || scopeToCheck == null) {
+            return false;
+        }
+
+        String visibility = scope.equalsIgnoreCase(scopeToCheck) ? getVisibility() : getVisibilityOutside();
+        return "read-only".equalsIgnoreCase(visibility);
+
+    }
+
+    private String getScope(InProgressSubmission<?> obj) {
+        if (HibernateProxyHelper.getClassWithoutInitializingProxy(obj).equals(WorkspaceItem.class)) {
+            return "submission";
+        }
+        return "workflow";
+    }
+
     /**
      * Get the number of this step in the current Submission process config.
      * Step numbers start with #0 (although step #0 is ALWAYS the special

dspace-api/src/main/java/org/dspace/validation/service/impl/ValidationServiceImpl.java

@@ -9,6 +9,7 @@
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.stream.Collectors;
 import javax.annotation.PostConstruct;
 
 import org.dspace.app.util.SubmissionConfig;
@@ -54,23 +55,51 @@ private void setup() throws SubmissionConfigReaderException {
     @Override
     public List<ValidationError> validate(Context context, InProgressSubmission<?> obj) {
 
+        SubmissionConfig submissionConfig = submissionConfigReader.getSubmissionConfigByInProgressSubmission(obj);
+
         List<ValidationError> errors = new ArrayList<ValidationError>();
 
-        SubmissionConfig submissionConfig = submissionConfigReader.getSubmissionConfigByInProgressSubmission(obj);
+        errors.addAll(notHiddenStepsValidations(context, obj, submissionConfig));
+        errors.addAll(globalValidations(context, obj, submissionConfig));
+
+        return errors;
+
+    }
+
+    private List<ValidationError> notHiddenStepsValidations(Context context, InProgressSubmission<?> obj,
+        SubmissionConfig submissionConfig) {
+
+        List<ValidationError> errors = new ArrayList<ValidationError>();
 
         for (SubmissionStepConfig stepConfig : submissionConfig) {
+
+            if (isStepHiddenOrReadOnly(stepConfig, obj)) {
+                continue;
+            }
+
             stepValidators.stream()
                 .filter(validation -> validation.getName().equals(stepConfig.getType()))
                 .flatMap(validation -> validation.validate(context, obj, stepConfig).stream())
                 .forEach(errors::add);
+
         }
 
-        globalValidators.stream()
+        return errors;
+
+    }
+
+    private List<ValidationError> globalValidations(Context context, InProgressSubmission<?> obj,
+        SubmissionConfig submissionConfig) {
+
+        return globalValidators.stream()
             .flatMap(validator -> validator.validate(context, obj, submissionConfig).stream())
-            .forEach(errors::add);
+            .collect(Collectors.toList());
 
-        return errors;
+    }
 
+    private boolean isStepHiddenOrReadOnly(SubmissionStepConfig stepConfig, InProgressSubmission<?> obj) {
+        return stepConfig.isHiddenForInProgressSubmission(obj) || stepConfig.isReadOnlyForInProgressSubmission(obj);
     }
 
+
 }

dspace-api/src/test/data/dspaceFolder/config/item-submission.xml

@@ -57,7 +57,6 @@
             <heading></heading>
             <processing-class>org.dspace.app.rest.submit.step.CollectionStep</processing-class>
             <type>collection</type>
-            <scope visibility="hidden" visibilityOutside="hidden">submission</scope>
         </step-definition>
         <step-definition id="traditionalpageone" mandatory="true">
             <heading>submit.progressbar.describe.stepone</heading>
@@ -245,18 +244,46 @@
 			<type>correction</type>
 			<scope visibilityOutside="hidden">workflow</scope>
 	    </step-definition>
-	    
+
 	    <step-definition id="controlled-vocabulary-test" mandatory="true">
             <heading>submit.progressbar.describe.stepone</heading>
             <processing-class>org.dspace.app.rest.submit.step.DescribeStep</processing-class>
             <type>submission-form</type>
         </step-definition>
-            
+
         <step-definition id="identifiers">
             <heading>submit.progressbar.identifiers</heading>
             <processing-class>org.dspace.app.rest.submit.step.ShowIdentifiersStep</processing-class>
             <type>identifiers</type>
         </step-definition>
+
+        <step-definition id="test-outside-workflow-hidden" mandatory="true">
+            <heading>submit.progressbar.describe.stepone</heading>
+            <processing-class>org.dspace.app.rest.submit.step.DescribeStep</processing-class>
+            <type>submission-form</type>
+            <scope visibilityOutside="hidden">workflow</scope>
+        </step-definition>
+
+        <step-definition id="test-outside-submission-hidden" mandatory="true">
+            <heading>submit.progressbar.describe.stepone</heading>
+            <processing-class>org.dspace.app.rest.submit.step.DescribeStep</processing-class>
+            <type>submission-form</type>
+            <scope visibilityOutside="hidden">submission</scope>
+        </step-definition>
+
+        <step-definition id="test-never-hidden" mandatory="true">
+            <heading></heading>
+            <processing-class>org.dspace.app.rest.submit.step.CollectionStep</processing-class>
+            <type>collection</type>
+        </step-definition>
+
+        <step-definition id="test-always-hidden" mandatory="true">
+            <heading></heading>
+            <processing-class>org.dspace.app.rest.submit.step.CollectionStep</processing-class>
+            <type>collection</type>
+            <scope visibility="hidden" visibilityOutside="hidden">submission</scope>
+        </step-definition>
+
     </step-definitions>
 
     <!-- The submission-definitions map lays out the detailed definition of -->
@@ -320,6 +347,13 @@
             <step id="license"/>
         </submission-process>
 
+        <submission-process name="test-hidden">
+            <step id="test-outside-workflow-hidden"/>
+            <step id="test-outside-submission-hidden"/>
+            <step id="test-never-hidden"/>
+            <step id="test-always-hidden"/>
+        </submission-process>
+
         <!--This "traditional" process defines the DEFAULT item submission process -->
         <submission-process name="traditional-cris">
 

dspace-api/src/test/data/dspaceFolder/config/spring/api/edititem-service.xml

@@ -90,6 +90,15 @@
                             </property>
                             <property name="submissionDefinition" value="traditional-with-custom-url" />
                         </bean>
+                        <bean class="org.dspace.content.edit.EditItemMode">
+                            <property name="name" value="MODE-TEST-HIDDEN" />
+                            <property name="security">
+                              <value type="org.dspace.content.security.CrisSecurity">
+                                ADMIN
+                              </value>
+                            </property>
+                            <property name="submissionDefinition" value="test-hidden" />
+                        </bean>
                         <bean class="org.dspace.content.edit.EditItemMode">
                             <property name="name" value="MODE-WITH-MANY-SECURITIES" />
                             <property name="securities">

dspace-api/src/test/data/dspaceFolder/config/submission-forms.xml

@@ -1955,7 +1955,7 @@ You can leave out the day and/or month if they aren't applicable.</hint>
 			</field>
      	</row>
 	</form>
-	
+
 	<form name="controlled-vocabulary-test">
 	     <row>
 	         <field>
@@ -1996,7 +1996,35 @@ You can leave out the day and/or month if they aren't applicable.</hint>
 		     </field>
 	       </row>
         </form>
-
+
+    <form name="test-outside-workflow-hidden">
+        <row>
+            <field>
+                <dc-schema>dc</dc-schema>
+                <dc-element>title</dc-element>
+                <dc-qualifier></dc-qualifier>
+                <repeatable>false</repeatable>
+                <label>Title</label>
+                <input-type>onebox</input-type>
+                <required>Field required</required>
+            </field>
+        </row>
+    </form>
+
+    <form name="test-outside-submission-hidden">
+        <row>
+            <field>
+                <dc-schema>dc</dc-schema>
+                <dc-element>type</dc-element>
+                <dc-qualifier></dc-qualifier>
+                <repeatable>false</repeatable>
+                <label>Type</label>
+                <input-type>onebox</input-type>
+                <required>Field required</required>
+            </field>
+        </row>
+    </form>
+
  </form-definitions>
 
 

dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/AInprogressItemConverter.java

@@ -101,6 +101,10 @@ protected void fillFromModel(T obj, R witem, Projection projection) {
             for (SubmissionSectionRest sections : def.getPanels()) {
                 SubmissionStepConfig stepConfig = submissionSectionConverter.toModel(sections);
 
+                if (stepConfig.isHiddenForInProgressSubmission(obj)) {
+                    continue;
+                }
+
                 /*
                  * First, load the step processing class (using the current
                  * class loader)

dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/EditItemConverter.java

@@ -101,6 +101,10 @@ protected void fillFromModel(EditItem obj, EditItemRest rest, Projection project
             for (SubmissionSectionRest sections : def.getPanels()) {
                 SubmissionStepConfig stepConfig = submissionSectionConverter.toModel(sections);
 
+                if (stepConfig.isHiddenForInProgressSubmission(obj)) {
+                    continue;
+                }
+
                 /*
                  * First, load the step processing class (using the current
                  * class loader)

dspace-server-webapp/src/test/java/org/dspace/app/rest/AuthorizationRestRepositoryIT.java

@@ -25,7 +25,9 @@
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
 
+import java.io.IOException;
 import java.io.Serializable;
+import java.sql.SQLException;
 import java.util.ArrayList;
 import java.util.Date;
 import java.util.HashMap;
@@ -64,6 +66,7 @@
 import org.dspace.app.rest.projection.DefaultProjection;
 import org.dspace.app.rest.test.AbstractControllerIntegrationTest;
 import org.dspace.app.rest.utils.Utils;
+import org.dspace.authorize.AuthorizeException;
 import org.dspace.builder.CollectionBuilder;
 import org.dspace.builder.CommunityBuilder;
 import org.dspace.builder.EPersonBuilder;
@@ -81,8 +84,13 @@
 import org.dspace.eperson.EPerson;
 import org.dspace.eperson.Group;
 import org.dspace.services.ConfigurationService;
+import org.dspace.xmlworkflow.storedcomponents.PoolTask;
+import org.dspace.xmlworkflow.storedcomponents.XmlWorkflowItem;
+import org.dspace.xmlworkflow.storedcomponents.service.PoolTaskService;
+import org.dspace.xmlworkflow.storedcomponents.service.XmlWorkflowItemService;
 import org.hamcrest.Matcher;
 import org.hamcrest.Matchers;
+import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -117,6 +125,12 @@ public class AuthorizationRestRepositoryIT extends AbstractControllerIntegration
     @Autowired
     private ItemConverter itemConverter;
 
+    @Autowired
+    private PoolTaskService poolTaskService;
+
+    @Autowired
+    private XmlWorkflowItemService xmlWorkflowItemService;
+
     @Autowired
     private Utils utils;
     private SiteService siteService;
@@ -175,6 +189,14 @@ public void setUp() throws Exception {
         configurationService.setProperty("webui.user.assumelogin", true);
     }
 
+    @After
+    public void cleanUp() throws Exception {
+        context.turnOffAuthorisationSystem();
+        poolTaskService.findAll(context).forEach(this::deletePoolTask);
+        xmlWorkflowItemService.findAll(context).forEach(this::deleteWorkflowItem);
+        context.restoreAuthSystemState();
+    }
+
     @Test
     /**
      * This method is not implemented
@@ -2824,7 +2846,17 @@ public void verifySpecialGroupForNonAdministrativeUsersTest() throws Exception {
                                .andExpect(status().isOk())
                                .andExpect(jsonPath("$.errors").doesNotExist());
 
-        getClient(epersonToken).perform(get("/api/submission/workspaceitems/" + workspaceItemIdRef.get()))
+        AtomicReference<Integer> workflowItemIdRef = new AtomicReference<Integer>();
+
+        getClient(epersonToken).perform(post("/api/workflow/workflowitems")
+            .content("/api/submission/workspaceitems/" + workspaceItemIdRef.get())
+            .contentType(textUriContentType))
+            .andExpect(status().isCreated())
+            .andDo(result -> workflowItemIdRef.set(read(result.getResponse().getContentAsString(), "$.id")));
+
+        String tokenAdmin = getAuthToken(admin.getEmail(), password);
+
+        getClient(tokenAdmin).perform(get("/api/workflow/workflowitems/" + workflowItemIdRef.get()))
                                .andExpect(status().isOk())
                                .andExpect(jsonPath("$.sections.correction.metadata", hasSize(equalTo(3))))
                                .andExpect(jsonPath("$.sections.correction.empty", is(false)))
@@ -2931,5 +2963,20 @@ private String getAuthorizationID(String epersonUuid, String featureName, String
                 + id.toString();
     }
 
+    private void deletePoolTask(PoolTask poolTask) {
+        try {
+            poolTaskService.delete(context, poolTask);
+        } catch (SQLException | AuthorizeException e) {
+            throw new RuntimeException(e);
+        }
+    }
+
+    private void deleteWorkflowItem(XmlWorkflowItem workflowItem) {
+        try {
+            xmlWorkflowItemService.delete(context, workflowItem);
+        } catch (SQLException | AuthorizeException | IOException e) {
+            throw new RuntimeException(e);
+        }
+    }
 
 }