Story: RM-1212 (As a records user I want to perform a records search, select multiple records/record folders from the results and add them all to a hold(s) I have permission to see so that I can easily discover and freeze relevant records)

Sub-tasks:

 * RM-1400 (Change the service so that all holds will be retrieved when adding multiple records/folders)
 * RM-1402 (Change the REST API so that all holds will be retrieved when adding multiple records/folders)

git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/modules/recordsmanagement/HEAD@65918 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

rm-server/config/alfresco/templates/webscripts/org/alfresco/rma/hold.post.desc.xml

@@ -1,11 +1,11 @@
 <webscript>
-   <shortname>Adds an item to the hold(s)</shortname>
+   <shortname>Adds item(s) to the hold(s)</shortname>
    <description><![CDATA[
-   WebScript to add an item (record / record folder) to the hold(s) in the holds container.<br/>
+   WebScript to add item(s) (record(s) / record folder(s)) to the hold(s) in the holds container.<br/>
    The body of the post should be in the form<br/>
    {<br/>
-   &nbsp;&nbsp;&nbsp;"nodeRef" : nodeRef of the item (record / record folder),<br/>
+   &nbsp;&nbsp;&nbsp;"nodeRefs" : array of nodeRefs for item(s) (record(s) / record folder(s)),<br/>
-   &nbsp;&nbsp;&nbsp;"holds" : array of nodeRef for the hold(s)<br/>
+   &nbsp;&nbsp;&nbsp;"holds" : array of nodeRefs for the hold(s)<br/>
    }<br/>
    ]]>
    </description>

rm-server/config/alfresco/templates/webscripts/org/alfresco/rma/hold.put.desc.xml

@@ -1,11 +1,11 @@
 <webscript>
-   <shortname>Removes an item from the hold(s)</shortname>
+   <shortname>Removes item(s) from the hold(s)</shortname>
    <description><![CDATA[
-   WebScript to remove an item (record / record folder) from the hold(s) in the holds container.<br/>
+   WebScript to remove item(s) (record(s) / record folder(s)) from the hold(s) in the holds container.<br/>
    The body of the put should be in the form<br/>
    {<br/>
-   &nbsp;&nbsp;&nbsp;"nodeRef" : nodeRef of the item (record / record folder),<br/>
+   &nbsp;&nbsp;&nbsp;"nodeRefs" : array of nodeRefs for the item(s) (record(s) / record folder(s)),<br/>
-   &nbsp;&nbsp;&nbsp;"holds" : array of nodeRef for the hold(s)<br/>
+   &nbsp;&nbsp;&nbsp;"holds" : array of nodeRefs for the hold(s)<br/>
    }<br/>
    ]]>
    </description>

rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldService.java

@@ -48,9 +48,10 @@ public interface HoldService
     List<NodeRef> getHolds(NodeRef filePlan);
 
     /**
+     * Gets the node reference for the hold with the given name in the given file plan
      * 
-     * @param name
+     * @param name {@link String} The name of the hold
-     * @return
+     * @return {@link NodeRef} of the hold with the given name
      */
     NodeRef getHold(NodeRef filePlan, String name);
 
@@ -65,39 +66,44 @@ public interface HoldService
     List<NodeRef> heldBy(NodeRef nodeRef, boolean includedInHold);
 
     /**
+     * Gets the list of item node references which are in the given hold 
      * 
-     * @param ndoeRef
+     * @param ndoeRef {@link NodeRef} of the hold
-     * @return
+     * @return Lost of item {@link NodeRef}s which are in the given hold
      */
     List<NodeRef> getHeld(NodeRef hold);
 
     /**
+     * Creates a hold with the given name, reason and description for the given file plan
      * 
-     * @param filePlan
+     * @param filePlan The {@link NodeRef} of the file plan
-     * @param name
+     * @param name {@link String} The name of the hold
-     * @param reason
+     * @param reason {@link String} The reason of the hold
-     * @param description
+     * @param description {@link String} The description of the hold
-     * @return
+     * @return The {@link NodeRef} of the created hold
      */
     NodeRef createHold(NodeRef filePlan, String name, String reason, String description);
 
     /**
+     * Gets the hold reason for the given hold node reference
      * 
-     * @param hold
+     * @param hold The {@link NodeRef} of the hold
-     * @return
+     * @return {@link String} The reason of the hold
      */
     String getHoldReason(NodeRef hold);
 
     /**
+     * Sets the hold reason
      * 
-     * @param hold
+     * @param hold The {@link NodeRef} of the hold
-     * @param reason
+     * @param reason {@link String} The reason for the hold
      */
     void setHoldReason(NodeRef hold, String reason);
 
     /**
+     * Deletes the hold
      * 
-     * @param hold
+     * @param hold The {@link NodeRef} of the hold
      */
     void deleteHold(NodeRef hold);
 
@@ -110,9 +116,10 @@ public interface HoldService
     void addToHold(NodeRef hold, NodeRef nodeRef);
 
     /**
+     * Adds the items to the the given hold
      * 
-     * @param hold
+     * @param hold The {@link NodeRef} of the hold to which the items will be added
-     * @param nodeRefs
+     * @param nodeRefs The item {@link NodeRef}s which will be added to the hold
      */
     void addToHold(NodeRef hold, List<NodeRef> nodeRefs);
 
@@ -124,6 +131,14 @@ public interface HoldService
      */
     void addToHolds(List<NodeRef> holds, NodeRef nodeRef);
 
+    /**
+     * Adds the given items to the given list of holds
+     *
+     * @param holds List of holds to which the given items will be added
+     * @param nodeRefs The list of items which will be added to the given holds
+     */
+    void addToHolds(List<NodeRef> holds, List<NodeRef> nodeRefs);
+
     /**
      * Removes the record from the given hold
      *
@@ -133,14 +148,15 @@ public interface HoldService
     void removeFromHold(NodeRef hold, NodeRef nodeRef);
 
     /**
+     * Removes the given items from the given hold
      * 
-     * @param hold
+     * @param hold The hold {@link NodeRef} from which the given items will be removed
-     * @param nodeRefs
+     * @param nodeRefs The list of items which will be removed from the given holds
      */
     void removeFromHold(NodeRef hold, List<NodeRef> nodeRefs);
 
     /**
-     * Removes the record from the given list of hold
+     * Removes the item from the given list of hold
      *
      * @param holds The list {@link NodeRef}s of the holds
      * @param nodeRef The {@link NodeRef} of the record / record folder which will be removed from the given holds
@@ -148,14 +164,24 @@ public interface HoldService
     void removeFromHolds(List<NodeRef> holds, NodeRef nodeRef);
 
     /**
+     * Removes the items from the given holds
      *
-     * @param nodeRef
+     * @param holds List of hold {@link NodeRef}s from which the items will be removed
+     * @param nodeRefs List of item {@link NodeRef}s which will be removed from the given holds
+     */
+    void removeFromHolds(List<NodeRef> holds, List<NodeRef> nodeRefs);
+
+    /**
+     * Removes the given {@link NodeRef} from all the holds
+     *
+     * @param nodeRef The {@link NodeRef} of item which will be removed from all the holds
      */
     void removeFromAllHolds(NodeRef nodeRef);
 
     /**
+     * Removes the given list of {@link NodeRef}s from all the holds
      *
-     * @param nodeRefs
+     * @param nodeRefs The list of item {@link NodeRef}s which will be removed from all the holds
      */
     void removeFromAllHolds(List<NodeRef> nodeRefs);
 }

rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImpl.java

@@ -497,6 +497,21 @@ public class HoldServiceImpl extends ServiceBaseImpl
         }
     }
 
+    /**
+     * @see org.alfresco.module.org_alfresco_module_rm.hold.HoldService#addToHolds(java.util.List, java.util.List)
+     */
+    @Override
+    public void addToHolds(List<NodeRef> holds, List<NodeRef> nodeRefs)
+    {
+        ParameterCheck.mandatoryCollection("holds", holds);
+        ParameterCheck.mandatoryCollection("nodeRefs", nodeRefs);
+
+        for (NodeRef nodeRef : nodeRefs)
+        {
+            addToHolds(holds, nodeRef);
+        }
+    }
+
     /**
      * @see org.alfresco.module.org_alfresco_module_rm.hold.HoldService#removeFromHold(org.alfresco.service.cmr.repository.NodeRef, org.alfresco.service.cmr.repository.NodeRef)
      */
@@ -564,6 +579,21 @@ public class HoldServiceImpl extends ServiceBaseImpl
         }
     }
 
+    /**
+     * @see org.alfresco.module.org_alfresco_module_rm.hold.HoldService#removeFromHolds(java.util.List, java.util.List)
+     */
+    @Override
+    public void removeFromHolds(List<NodeRef> holds, List<NodeRef> nodeRefs)
+    {
+        ParameterCheck.mandatoryCollection("holds", holds);
+        ParameterCheck.mandatoryCollection("nodeRefs", nodeRefs);
+
+        for (NodeRef nodeRef : nodeRefs)
+        {
+            removeFromHolds(holds, nodeRef);
+        }
+    }
+
     /**
      * @see org.alfresco.module.org_alfresco_module_rm.hold.HoldService#removeFromAllHolds(org.alfresco.service.cmr.repository.NodeRef)
      */

rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/script/hold/BaseHold.java

@@ -111,19 +111,19 @@ public abstract class BaseHold extends DeclarativeWebScript
     {
         JSONObject json = getJSONFromContent(req);
         List<NodeRef> holds = getHolds(json);
-        NodeRef nodeRef = getItemNodeRef(json);
+        List<NodeRef> nodeRefs = getItemNodeRefs(json);
-        doAction(holds, nodeRef);
+        doAction(holds, nodeRefs);
         return new HashMap<String, Object>();
     }
 
     /**
      * Abstract method which will be implemented in the subclasses.
-     * It will either add the item to the hold(s) or remove it from the hold(s)
+     * It will either add the item(s) to the hold(s) or remove it/them from the hold(s)
      *
      * @param holds List of hold {@link NodeRef}(s)
-     * @param nodeRef {@link NodeRef} of an item (record / record folder) which will be either added to the hold(s) or removed from the hol(s)
+     * @param nodeRefs List of item {@link NodeRef}(s) (record(s) / record folder(s)) which will be either added to the hold(s) or removed from the hold(s)
      */
-    abstract void doAction(List<NodeRef> holds, NodeRef nodeRef);
+    abstract void doAction(List<NodeRef> holds, List<NodeRef> nodeRefs);
 
     /**
      * Helper method the get the json object from the request
@@ -154,40 +154,51 @@ public abstract class BaseHold extends DeclarativeWebScript
     }
 
     /**
-     * Helper method to get the {@link NodeRef} for the item (record / record folder) which will be added to the hold(s)
+     * Helper method to get the {@link NodeRef}s for the items(s) (record(s) / record folder(s)) which will be added to the hold(s)
      *
      * @param json The request content as JSON object
-     * @return The {@link NodeRef} of the item which will be added to the hold(s)
+     * @return List of item {@link NodeRef}s which will be added to the hold(s)
      */
-    protected NodeRef getItemNodeRef(JSONObject json)
+    protected List<NodeRef> getItemNodeRefs(JSONObject json)
     {
-        String nodeRefString = null;
+        List<NodeRef> nodeRefs = new ArrayList<NodeRef>();
         try
         {
-            nodeRefString = json.getString("nodeRef");
+            JSONArray nodeRefsArray = json.getJSONArray("nodeRefs");
+            for (int i = 0; i < nodeRefsArray.length(); i++)
+            {
+                NodeRef nodeReference = new NodeRef(nodeRefsArray.getString(i));
+                checkItemNodeRef(nodeReference);
+                nodeRefs.add(nodeReference);
+            }
         }
         catch (JSONException je)
         {
             throw new WebScriptException(Status.STATUS_BAD_REQUEST,
-                    "Could not get the nodeRef from the json object.", je);
+                    "Could not get information from the json array.", je);
         }
 
-        NodeRef nodeRef = new NodeRef(nodeRefString);
+        return nodeRefs;
+    }
 
+    /**
+     * Helper method for checking the node reference for an item
+     *
+     * @param nodeRef The {@link NodeRef} of an item (record / record folder)
+     */
+    private void checkItemNodeRef(NodeRef nodeRef)
+    {
         // ensure that the node exists
         if (!nodeService.exists(nodeRef))
         {
             throw new WebScriptException(Status.STATUS_BAD_REQUEST, "Item being added to hold does not exist.");
         }
 
-        // ensure that the node we are adding to the hold is a record or
+        // ensure that the node we are adding to the hold is a record or record folder
-        // record folder
         if (!recordService.isRecord(nodeRef) && !recordFolderService.isRecordFolder(nodeRef))
         {
             throw new WebScriptException(Status.STATUS_BAD_REQUEST, "Items added to a hold must be either a record or record folder.");
         }
-        
-        return nodeRef;
     }
 
     /**
@@ -205,7 +216,26 @@ public abstract class BaseHold extends DeclarativeWebScript
             for (int i = 0; i < holdsArray.length(); i++)
             {
                 NodeRef nodeRef = new NodeRef(holdsArray.getString(i));
+                checkHoldNodeRef(nodeRef);
+                holds.add(nodeRef);
+            }
+        }
+        catch (JSONException je)
+        {
+            throw new WebScriptException(Status.STATUS_BAD_REQUEST,
+                    "Could not get information from the json array.", je);
+        }
 
+        return holds;
+    }
+
+    /**
+     * Helper method for checking the node reference for a hold
+     *
+     * @param nodeRef The {@link NodeRef} of a hold
+     */
+    private void checkHoldNodeRef(NodeRef nodeRef)
+    {
         // check the hold exists
         if (!nodeService.exists(nodeRef))
         {
@@ -217,15 +247,5 @@ public abstract class BaseHold extends DeclarativeWebScript
         {
             throw new WebScriptException(Status.STATUS_BAD_REQUEST, "Items are being added to a node that isn't a hold.");
         }
-                
-                holds.add(nodeRef);
-            }
-        }
-        catch (JSONException je)
-        {
-            throw new WebScriptException(Status.STATUS_BAD_REQUEST,
-                    "Could not get information from json array.", je);
-        }
-        return holds;
     }
 }

rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/script/hold/HoldPost.java

@@ -31,11 +31,11 @@ import org.alfresco.service.cmr.repository.NodeRef;
 public class HoldPost extends BaseHold
 {
     /**
-     * @see org.alfresco.module.org_alfresco_module_rm.script.hold.BaseHold#doAction(java.util.List, org.alfresco.service.cmr.repository.NodeRef)
+     * @see org.alfresco.module.org_alfresco_module_rm.script.hold.BaseHold#doAction(java.util.List, java.util.List)
      */
     @Override
-    void doAction(List<NodeRef> holds, NodeRef nodeRef)
+    void doAction(List<NodeRef> holds, List<NodeRef> nodeRefs)
     {
-        getHoldService().addToHolds(holds, nodeRef);
+        getHoldService().addToHolds(holds, nodeRefs);
     }
 }

rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/script/hold/HoldPut.java

@@ -31,11 +31,11 @@ import org.alfresco.service.cmr.repository.NodeRef;
 public class HoldPut extends BaseHold
 {
     /**
-     * @see org.alfresco.module.org_alfresco_module_rm.script.hold.BaseHold#doAction(java.util.List, org.alfresco.service.cmr.repository.NodeRef)
+     * @see org.alfresco.module.org_alfresco_module_rm.script.hold.BaseHold#doAction(java.util.List, java.util.List)
      */
     @Override
-    void doAction(List<NodeRef> holds, NodeRef nodeRef)
+    void doAction(List<NodeRef> holds, List<NodeRef> nodeRefs)
     {
-        getHoldService().removeFromHolds(holds, nodeRef);
+        getHoldService().removeFromHolds(holds, nodeRefs);
     }
 }

rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/script/hold/BaseHoldWebScriptUnitTest.java

@@ -19,6 +19,9 @@ public abstract class BaseHoldWebScriptUnitTest extends BaseWebScriptUnitTest
     protected NodeRef hold1NodeRef;
     protected NodeRef hold2NodeRef;
     protected List<NodeRef> holds;
+    protected List<NodeRef> records;
+    protected List<NodeRef> recordFolders;
+    protected List<NodeRef> filePlanComponents;
 
     /**
      * @see org.alfresco.module.org_alfresco_module_rm.test.util.BaseUnitTest#before()
@@ -35,5 +38,14 @@ public abstract class BaseHoldWebScriptUnitTest extends BaseWebScriptUnitTest
         // list of holds
         holds = new ArrayList<NodeRef>(2);
         Collections.addAll(holds, hold1NodeRef, hold2NodeRef);
+
+        // list of records
+        records = Collections.singletonList(record);
+
+        // list of record folders
+        recordFolders = Collections.singletonList(recordFolder);
+
+        // list of file plan components
+        filePlanComponents = Collections.singletonList(filePlanComponent);
     }
 }

rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/script/hold/BaseHoldWebScriptWithContentUnitTest.java

@@ -22,17 +22,34 @@ public abstract class BaseHoldWebScriptWithContentUnitTest extends BaseHoldWebSc
     /**
      * Helper method to build JSON content to send to hold webscripts.
      */
-    protected String buildContent(NodeRef nodeRef, List<NodeRef> holds)
+    protected String buildContent(List<NodeRef> nodeRefs, List<NodeRef> holds)
     {
         StringBuilder builder = new StringBuilder(255);
         builder.append("{");
 
-        if (nodeRef != null)
+        if (nodeRefs != null)
         {
-            builder.append("'nodeRef':'").append(nodeRef.toString()).append("'");
+            builder.append("'nodeRefs':[");
+
+            boolean bFirst = true;
+            for (NodeRef nodeRef : nodeRefs)
+            {
+                if (bFirst == false)
+                {
+                    builder.append(",");
+                }
+                else
+                {
+                    bFirst = false;
                 }
 
-        if (nodeRef != null && holds != null)
+                builder.append("'" + nodeRef.toString() + "'");
+            }
+
+            builder.append("]");
+        }
+
+        if (nodeRefs != null && holds != null)
         {
             builder.append(",");
         }
@@ -93,7 +110,7 @@ public abstract class BaseHoldWebScriptWithContentUnitTest extends BaseHoldWebSc
         when(mockedNodeService.exists(eq(hold1NodeRef))).thenReturn(false);
 
         // build content
-        String content = buildContent(record, holds);
+        String content = buildContent(records, holds);
 
         // expected exception
         exception.expect(WebScriptException.class);
@@ -115,7 +132,7 @@ public abstract class BaseHoldWebScriptWithContentUnitTest extends BaseHoldWebSc
         when(mockedNodeService.exists(eq(record))).thenReturn(false);
 
         // build content
-        String content = buildContent(record, holds);
+        String content = buildContent(records, holds);
 
         // expected exception
         exception.expect(WebScriptException.class);
@@ -134,7 +151,7 @@ public abstract class BaseHoldWebScriptWithContentUnitTest extends BaseHoldWebSc
     public void holdMissingFromContent() throws Exception
     {
         // build content
-        String content = buildContent(record, null);
+        String content = buildContent(records, null);
 
         // expected exception
         exception.expect(WebScriptException.class);
@@ -173,7 +190,7 @@ public abstract class BaseHoldWebScriptWithContentUnitTest extends BaseHoldWebSc
     {
         // build json content to send to server
         List<NodeRef> notAHold = Collections.singletonList(recordFolder);
-        String content = buildContent(record, notAHold);    
+        String content = buildContent(records, notAHold);
 
         // expected exception
         exception.expect(WebScriptException.class);
@@ -193,7 +210,7 @@ public abstract class BaseHoldWebScriptWithContentUnitTest extends BaseHoldWebSc
     {
         // build json content to send to server
         List<NodeRef> notAHold = Collections.singletonList(recordFolder);
-        String content = buildContent(filePlanComponent, notAHold);    
+        String content = buildContent(filePlanComponents, notAHold);
 
         // expected exception
         exception.expect(WebScriptException.class);

rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/script/hold/HoldPostUnitTest.java

@@ -70,14 +70,14 @@ public class HoldPostUnitTest extends BaseHoldWebScriptWithContentUnitTest
     public void addRecordToHolds() throws Exception
     {
         // build json to send to server
-        String content = buildContent(record, holds);        
+        String content = buildContent(records, holds);
 
         // execute web script
         JSONObject json = executeJSONWebScript(Collections.EMPTY_MAP, content);
         assertNotNull(json);
 
         // verify that the record was added to the holds
-        verify(mockedHoldService, times(1)).addToHolds(holds, record);
+        verify(mockedHoldService, times(1)).addToHolds(holds, records);
     }
 
     /**
@@ -88,13 +88,13 @@ public class HoldPostUnitTest extends BaseHoldWebScriptWithContentUnitTest
     public void addRecordFolderToHolds() throws Exception
     {
         // build json to send to server
-        String content = buildContent(recordFolder, holds);        
+        String content = buildContent(recordFolders, holds);
 
         // execute web script
         JSONObject json = executeJSONWebScript(Collections.EMPTY_MAP, content);
         assertNotNull(json);
 
         // verify that the record was added to the holds
-        verify(mockedHoldService, times(1)).addToHolds(holds, recordFolder);        
+        verify(mockedHoldService, times(1)).addToHolds(holds, recordFolders);
     }
 }

rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/script/hold/HoldPutUnitTest.java

@@ -70,14 +70,14 @@ public class HoldPutUnitTest extends BaseHoldWebScriptWithContentUnitTest
     public void removeRecordFromHolds() throws Exception
     {
         // build json to send to server
-        String content = buildContent(record, holds);        
+        String content = buildContent(records, holds);
 
         // execute web script
         JSONObject json = executeJSONWebScript(Collections.EMPTY_MAP, content);
         assertNotNull(json);
 
         // verify that the record was removed from holds
-        verify(mockedHoldService, times(1)).removeFromHolds(holds, record);
+        verify(mockedHoldService, times(1)).removeFromHolds(holds, records);
     }
 
     /**
@@ -88,13 +88,13 @@ public class HoldPutUnitTest extends BaseHoldWebScriptWithContentUnitTest
     public void removeRecordFolderFromHolds() throws Exception
     {
         // build json to send to server
-        String content = buildContent(recordFolder, holds);        
+        String content = buildContent(recordFolders, holds);
 
         // execute web script
         JSONObject json = executeJSONWebScript(Collections.EMPTY_MAP, content);
         assertNotNull(json);
 
         // verify that the record was removed from holds
-        verify(mockedHoldService, times(1)).removeFromHolds(holds, recordFolder);       
+        verify(mockedHoldService, times(1)).removeFromHolds(holds, recordFolders);
     }
 }