97414 Advanced workflow actions: refactor for updated rest contract

dspace-api/src/main/java/org/dspace/xmlworkflow/state/actions/Action.java

@@ -10,9 +10,7 @@ package org.dspace.xmlworkflow.state.actions;
 import java.io.IOException;
 import java.sql.SQLException;
 import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
 import javax.servlet.http.HttpServletRequest;
 
 import org.dspace.authorize.AuthorizeException;
@@ -40,7 +38,7 @@ public abstract class Action {
     private WorkflowActionConfig parent;
     private static final String ERROR_FIELDS_ATTRIBUTE = "dspace.workflow.error_fields";
     private List<String> advancedOptions = new ArrayList<>();
-    private Map<String, ActionAdvancedInfo> advancedInfo = new HashMap<>();
+    private List<ActionAdvancedInfo> advancedInfo = new ArrayList<>();
 
     /**
      * Called when a workflow item becomes eligible for this Action.
@@ -202,10 +200,10 @@ public abstract class Action {
     }
 
     protected boolean isAdvanced() {
-        return !advancedOptions.isEmpty();
+        return !getAdvancedOptions().isEmpty();
     }
 
-    protected Map<String, ActionAdvancedInfo> getAdvancedInfo() {
+    protected List<ActionAdvancedInfo> getAdvancedInfo() {
         return advancedInfo;
     }
 }

dspace-api/src/main/java/org/dspace/xmlworkflow/state/actions/ActionAdvancedInfo.java

@@ -8,8 +8,9 @@
 package org.dspace.xmlworkflow.state.actions;
 
 public interface ActionAdvancedInfo {
-    boolean isDescriptionRequired();
+    String getType();
-    void setDescriptionRequired(boolean descriptionRequired);
+    void setType(String type);
-    int getMaxValue();
+    String getId();
-    void setMaxValue(int maxValue);
+    void setId(String id);
+
 }

dspace-api/src/main/java/org/dspace/xmlworkflow/state/actions/WorkflowActionConfig.java

@@ -8,7 +8,6 @@
 package org.dspace.xmlworkflow.state.actions;
 
 import java.util.List;
-import java.util.Map;
 
 import org.dspace.xmlworkflow.state.Step;
 
@@ -72,7 +71,7 @@ public class WorkflowActionConfig {
 
     /**
      * Returns a list of advanced options this user has on this action, resulting in the next step of the workflow
-     * @returnA list of advanced options of this action, resulting in the next step of the workflow
+     * @return A list of advanced options of this action, resulting in the next step of the workflow
      */
     public List<String> getAdvancedOptions() {
         return this.processingAction.getAdvancedOptions();
@@ -90,7 +89,7 @@ public class WorkflowActionConfig {
      * Returns a Map of info for the advanced options this user has on this action
      * @return a Map of info for the advanced options this user has on this action
      */
-    public Map<String, ActionAdvancedInfo> getAdvancedInfo() {
+    public List<ActionAdvancedInfo> getAdvancedInfo() {
         return this.processingAction.getAdvancedInfo();
     }
 

dspace-api/src/main/java/org/dspace/xmlworkflow/state/actions/processingaction/ProcessingAction.java

@@ -8,7 +8,6 @@
 package org.dspace.xmlworkflow.state.actions.processingaction;
 
 import java.sql.SQLException;
-import java.util.List;
 import javax.servlet.http.HttpServletRequest;
 
 import org.dspace.content.service.ItemService;
@@ -49,11 +48,4 @@ public abstract class ProcessingAction extends Action {
             task.getStepID().equals(getParent().getStep().getId()) &&
             task.getActionID().equals(getParent().getId());
     }
-
-    @Override
-    protected List<String> getAdvancedOptions() {
-        List<String> advancedOptions = super.getAdvancedOptions();
-        advancedOptions.add(SUBMIT_EDIT_METADATA);
-        return advancedOptions;
-    }
 }

dspace-api/src/main/java/org/dspace/xmlworkflow/state/actions/processingaction/RatingReviewAction.java

@@ -11,7 +11,6 @@ import java.io.IOException;
 import java.sql.SQLException;
 import java.util.Arrays;
 import java.util.List;
-import java.util.Map;
 import javax.servlet.http.HttpServletRequest;
 
 import org.dspace.app.util.Util;
@@ -24,7 +23,7 @@ import org.dspace.xmlworkflow.state.actions.ActionAdvancedInfo;
 import org.dspace.xmlworkflow.state.actions.ActionResult;
 import org.dspace.xmlworkflow.storedcomponents.XmlWorkflowItem;
 
-public class RatingReviewAction extends ProcessingAction implements ActionAdvancedInfo {
+public class RatingReviewAction extends ProcessingAction {
 
     private static final String RATING = "rating";
 
@@ -66,35 +65,25 @@ public class RatingReviewAction extends ProcessingAction implements ActionAdvanc
 
     @Override
     protected boolean isAdvanced() {
-        return !getOptions().isEmpty();
+        return !getAdvancedOptions().isEmpty();
     }
 
     @Override
-    protected Map<String, ActionAdvancedInfo> getAdvancedInfo() {
+    protected List<ActionAdvancedInfo> getAdvancedInfo() {
-        Map<String, ActionAdvancedInfo> advancedInfo =  super.getAdvancedInfo();
+        List<ActionAdvancedInfo> advancedInfo = super.getAdvancedInfo();
-        ActionAdvancedInfo scoreReviewActionAdvancedInfo = new ScoreReviewActionAdvancedInfo();
+        RatingReviewActionAdvancedInfo ratingReviewActionAdvancedInfo = new RatingReviewActionAdvancedInfo();
-        scoreReviewActionAdvancedInfo.setMaxValue(getMaxValue());
+        ratingReviewActionAdvancedInfo.setDescriptionRequired(descriptionRequired);
-        scoreReviewActionAdvancedInfo.setDescriptionRequired(isDescriptionRequired());
+        ratingReviewActionAdvancedInfo.setMaxValue(maxValue);
-        advancedInfo.put(RATING, scoreReviewActionAdvancedInfo);
+        ratingReviewActionAdvancedInfo.setType(RATING);
+        ratingReviewActionAdvancedInfo.setId(RATING);
+        advancedInfo.add(ratingReviewActionAdvancedInfo);
         return advancedInfo;
     }
 
-    @Override
-    public boolean isDescriptionRequired() {
-        return descriptionRequired;
-    }
-
-    @Override
     public void setDescriptionRequired(boolean descriptionRequired) {
         this.descriptionRequired = descriptionRequired;
     }
 
-    @Override
-    public int getMaxValue() {
-        return maxValue;
-    }
-
-    @Override
     public void setMaxValue(int maxValue) {
         this.maxValue = maxValue;
     }

dspace-api/src/main/java/org/dspace/xmlworkflow/state/actions/processingaction/RatingReviewActionAdvancedInfo.java

@@ -8,10 +8,14 @@
 package org.dspace.xmlworkflow.state.actions.processingaction;
 
 import org.dspace.xmlworkflow.state.actions.ActionAdvancedInfo;
+import org.springframework.util.DigestUtils;
 
-public class ScoreReviewActionAdvancedInfo implements ActionAdvancedInfo {
+public class RatingReviewActionAdvancedInfo implements ActionAdvancedInfo {
     private boolean descriptionRequired;
     private int maxValue;
+    private String type;
+    private String id;
+
 
     public boolean isDescriptionRequired() {
         return descriptionRequired;
@@ -28,4 +32,27 @@ public class ScoreReviewActionAdvancedInfo implements ActionAdvancedInfo {
     public void setMaxValue(int maxValue) {
         this.maxValue = maxValue;
     }
+
+    @Override
+    public String getType() {
+        return type;
+    }
+
+    @Override
+    public void setType(String type) {
+        this.type = "action_info_" + type;
+    }
+
+    @Override
+    public String getId() {
+        return id;
+    }
+
+    @Override
+    public void setId(String type) {
+        String idString = type
+            + ";descriptionRequired," + descriptionRequired
+            + ";maxValue," + maxValue;
+        this.id = DigestUtils.md5DigestAsHex(idString.getBytes());
+    }
 }

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

@@ -0,0 +1,33 @@
+/**
+ * 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.app.rest.converter;
+
+import org.dspace.app.rest.model.RatingReviewActionAdvancedInfoRest;
+import org.dspace.app.rest.projection.Projection;
+import org.dspace.xmlworkflow.state.actions.processingaction.RatingReviewActionAdvancedInfo;
+
+public class RatingReviewActionAdvancedInfoConverter
+    implements DSpaceConverter<RatingReviewActionAdvancedInfo, RatingReviewActionAdvancedInfoRest> {
+
+    @Override
+    public RatingReviewActionAdvancedInfoRest convert(RatingReviewActionAdvancedInfo modelObject,
+                                                      Projection projection) {
+        RatingReviewActionAdvancedInfoRest restModel = new RatingReviewActionAdvancedInfoRest();
+        restModel.setProjection(projection);
+        restModel.setDescriptionRequired(modelObject.isDescriptionRequired());
+        restModel.setMaxValue(modelObject.getMaxValue());
+        restModel.setType(modelObject.getType());
+        restModel.setId(modelObject.getId());
+        return restModel;
+    }
+
+    @Override
+    public Class<RatingReviewActionAdvancedInfo> getModelClass() {
+        return RatingReviewActionAdvancedInfo.class;
+    }
+}

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

@@ -1,30 +0,0 @@
-/**
- * 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.app.rest.converter;
-
-import org.dspace.app.rest.model.ScoreReviewActionAdvancedInfoRest;
-import org.dspace.app.rest.projection.Projection;
-import org.dspace.xmlworkflow.state.actions.processingaction.ScoreReviewActionAdvancedInfo;
-
-public class ScoreReviewActionAdvancedInfoConverter
-    implements DSpaceConverter<ScoreReviewActionAdvancedInfo, ScoreReviewActionAdvancedInfoRest> {
-
-    @Override
-    public ScoreReviewActionAdvancedInfoRest convert(ScoreReviewActionAdvancedInfo modelObject, Projection projection) {
-        ScoreReviewActionAdvancedInfoRest restModel = new ScoreReviewActionAdvancedInfoRest();
-        restModel.setProjection(projection);
-        restModel.setDescriptionRequired(modelObject.isDescriptionRequired());
-        restModel.setMaxValue(modelObject.getMaxValue());
-        return restModel;
-    }
-
-    @Override
-    public Class<ScoreReviewActionAdvancedInfo> getModelClass() {
-        return ScoreReviewActionAdvancedInfo.class;
-    }
-}

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

@@ -26,8 +26,10 @@ public class WorkflowActionConverter implements DSpaceConverter<WorkflowActionCo
         restModel.setProjection(projection);
         restModel.setId(modelObject.getId());
         restModel.setOptions(modelObject.getOptions());
+        if (modelObject.isAdvanced()) {
             restModel.setAdvancedOptions(modelObject.getAdvancedOptions());
             restModel.setAdvancedInfo(modelObject.getAdvancedInfo());
+        }
         return restModel;
     }
 

dspace-server-webapp/src/main/java/org/dspace/app/rest/model/RatingReviewActionAdvancedInfoRest.java

@@ -7,10 +7,12 @@
  */
 package org.dspace.app.rest.model;
 
-public class ScoreReviewActionAdvancedInfoRest extends WorkflowActionRest {
+public class RatingReviewActionAdvancedInfoRest extends WorkflowActionRest {
 
     private boolean descriptionRequired;
     private int maxValue;
+    private String type;
+    private String id;
 
     /**
      * Generic getter for the description required boolean
@@ -48,6 +50,42 @@ public class ScoreReviewActionAdvancedInfoRest extends WorkflowActionRest {
         this.maxValue = maxValue;
     }
 
+    /**
+     * Generic getter for the type
+     *
+     * @return the type of this ScoreReviewActionAdvancedInfoRest
+     */
+    @Override
+    public String getType() {
+        return type;
+    }
 
+    /**
+     * Generic setter for the type
+     *
+     * @param type The type to be set on this ScoreReviewActionAdvancedInfoRest
+     */
+    public void setType(String type) {
+        this.type = type;
+    }
 
+    /**
+     * Generic getter for the id
+     *
+     * @return the id of this ScoreReviewActionAdvancedInfoRest
+     */
+    @Override
+    public String getId() {
+        return id;
+    }
+
+    /**
+     * Generic setter for the id
+     *
+     * @param id The id to be set on this ScoreReviewActionAdvancedInfoRest
+     */
+    @Override
+    public void setId(String id) {
+        this.id = id;
+    }
 }

dspace-server-webapp/src/main/java/org/dspace/app/rest/model/WorkflowActionRest.java

@@ -8,8 +8,9 @@
 package org.dspace.app.rest.model;
 
 import java.util.List;
-import java.util.Map;
 
+import com.fasterxml.jackson.annotation.JsonInclude;
+import org.apache.commons.collections4.CollectionUtils;
 import org.dspace.app.rest.RestResourceController;
 import org.dspace.xmlworkflow.state.actions.ActionAdvancedInfo;
 
@@ -26,7 +27,7 @@ public class WorkflowActionRest extends BaseObjectRest<String> {
 
     private List<String> options;
     private List<String> advancedOptions;
-    private Map<String, ActionAdvancedInfo> advancedInfo;
+    private List<ActionAdvancedInfo> advancedInfo;
 
     @Override
     public String getCategory() {
@@ -66,6 +67,7 @@ public class WorkflowActionRest extends BaseObjectRest<String> {
      *
      * @return the advanced options value of this WorkflowActionRest
      */
+    @JsonInclude(JsonInclude.Include.NON_NULL)
     public List<String> getAdvancedOptions() {
         return advancedOptions;
     }
@@ -85,7 +87,7 @@ public class WorkflowActionRest extends BaseObjectRest<String> {
      * @return the advanced boolean value of this WorkflowActionRest
      */
     public boolean getAdvanced() {
-        return !advancedOptions.isEmpty();
+        return CollectionUtils.isNotEmpty(getAdvancedOptions());
     }
 
     /**
@@ -93,7 +95,8 @@ public class WorkflowActionRest extends BaseObjectRest<String> {
      *
      * @return the advanced info value of this WorkflowActionRest
      */
-    public Map<String, ActionAdvancedInfo> getAdvancedInfo() {
+    @JsonInclude(JsonInclude.Include.NON_NULL)
+    public List<ActionAdvancedInfo> getAdvancedInfo() {
         return advancedInfo;
     }
 
@@ -102,7 +105,7 @@ public class WorkflowActionRest extends BaseObjectRest<String> {
      *
      * @param advancedInfo The advanced info to be set on this WorkflowActionRest
      */
-    public void setAdvancedInfo(Map<String, ActionAdvancedInfo> advancedInfo) {
+    public void setAdvancedInfo(List<ActionAdvancedInfo> advancedInfo) {
         this.advancedInfo = advancedInfo;
     }
 }

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

@@ -14,8 +14,6 @@ import static org.springframework.test.web.servlet.request.MockMvcRequestBuilder
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
 
-import java.util.Map;
-
 import org.dspace.app.rest.matcher.WorkflowActionMatcher;
 import org.dspace.app.rest.model.WorkflowActionRest;
 import org.dspace.app.rest.repository.WorkflowActionRestRepository;
@@ -85,8 +83,7 @@ public class WorkflowActionRestRepositoryIT extends AbstractControllerIntegratio
             .andExpect(status().isOk())
             // has options
             .andExpect(jsonPath("$.options", not(empty())))
-            .andExpect(jsonPath("$.advancedOptions", not(empty())))
+            .andExpect(jsonPath("$.advanced", is(false)))
-            .andExpect(jsonPath("$.advanced", is(true)))
             //Matches expected corresponding rest action values
             .andExpect(jsonPath("$", Matchers.is(
                 WorkflowActionMatcher.matchWorkflowActionEntry(existentWorkflow)
@@ -104,7 +101,6 @@ public class WorkflowActionRestRepositoryIT extends AbstractControllerIntegratio
             .andExpect(status().isOk())
             // has no options
             .andExpect(jsonPath("$.options", empty()))
-            .andExpect(jsonPath("$.advancedOptions", empty()))
             .andExpect(jsonPath("$.advanced", is(false)))
             //Matches expected corresponding rest action values
             .andExpect(jsonPath("$", Matchers.is(
@@ -153,24 +149,4 @@ public class WorkflowActionRestRepositoryIT extends AbstractControllerIntegratio
             )));
     }
 
-    @Test
-    public void getWorkflowActionByName_ExistentWithOptions_scorereviewaction() throws Exception {
-        String token = getAuthToken(eperson.getEmail(), password);
-        String nameActionWithOptions = "scorereviewaction";
-        WorkflowActionConfig existentWorkflow = xmlWorkflowFactory.getActionByName(nameActionWithOptions);
-        //When we call this facets endpoint
-        getClient(token).perform(get(WORKFLOW_ACTIONS_ENDPOINT + "/" + nameActionWithOptions))
-            //We expect a 200 is ok status
-            .andExpect(status().isOk())
-            // has options
-            .andExpect(jsonPath("$.options", not(empty())))
-            .andExpect(jsonPath("$.advancedOptions", not(empty())))
-            .andExpect(jsonPath("$.advanced", is(true)))
-            .andExpect(jsonPath("$.advancedInfo", is(Map.of())))
-            //Matches expected corresponding rest action values
-            .andExpect(jsonPath("$", Matchers.is(
-                WorkflowActionMatcher.matchWorkflowActionEntry(existentWorkflow)
-            )));
-    }
-
 }

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

@@ -32,7 +32,6 @@ public class WorkflowActionMatcher {
         return allOf(
                 hasJsonPath("$.id", is(workflowAction.getId())),
                 hasJsonPath("$.options", is(workflowAction.getOptions())),
-                hasJsonPath("$.advancedOptions", is(workflowAction.getAdvancedOptions())),
                 hasJsonPath("$.advanced", is(workflowAction.isAdvanced())),
                 hasJsonPath("$._links.self.href", containsString(WORKFLOW_ACTIONS_ENDPOINT + workflowAction.getId()))
         );