Allow active content to be added to a hold

rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml

@@ -1569,7 +1569,7 @@
             org.alfresco.module.org_alfresco_module_rm.hold.HoldService.getHoldReason=RM.Read.0
             org.alfresco.module.org_alfresco_module_rm.hold.HoldService.setHoldReason=RM_CAP.0.rma:filePlanComponent.EditHold
             org.alfresco.module.org_alfresco_module_rm.hold.HoldService.deleteHold=RM_CAP.0.rma:filePlanComponent.DeleteHold
-            org.alfresco.module.org_alfresco_module_rm.hold.HoldService.addToHold=RM_CAP.0.rma:filePlanComponent.AddToHold
+            org.alfresco.module.org_alfresco_module_rm.hold.HoldService.addToHold=RM_CAP.0.rma:filePlanComponent.AddToHold, ACL_NODE.1.sys:base.Write
             org.alfresco.module.org_alfresco_module_rm.hold.HoldService.addToHolds=RM_ALLOW
             org.alfresco.module.org_alfresco_module_rm.hold.HoldService.removeFromHold=RM_CAP.0.rma:filePlanComponent.RemoveFromHold
             org.alfresco.module.org_alfresco_module_rm.hold.HoldService.removeFromHolds=RM_ALLOW

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

@@ -539,13 +539,15 @@ public class HoldServiceImpl extends ServiceBaseImpl
         ParameterCheck.mandatoryCollection("holds", holds);
         ParameterCheck.mandatory("nodeRef", nodeRef);
 
-        if (!isRecord(nodeRef) && !isRecordFolder(nodeRef))
+        if (!isRecord(nodeRef) && !isRecordFolder(nodeRef) && !instanceOf(nodeRef, ContentModel.TYPE_CONTENT))
         {
             String nodeName = (String) nodeService.getProperty(nodeRef, ContentModel.PROP_NAME);
+            // TODO error message needs to be changed
             throw new AlfrescoRuntimeException("'" + nodeName + "' is neither a record nor a record folder. Only records or record folders can be added to a hold.");
         }
 
-        if (permissionService.hasPermission(nodeRef, RMPermissionModel.FILING) == AccessStatus.DENIED)
+        if ((isRecord(nodeRef) || isRecordFolder(nodeRef)) &&
+                permissionService.hasPermission(nodeRef, RMPermissionModel.FILING) == AccessStatus.DENIED)
         {
             String nodeName = (String) nodeService.getProperty(nodeRef, ContentModel.PROP_NAME);
             throw new AlfrescoRuntimeException("Filing permission on '" + nodeName + "' is needed.");

rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImplUnitTest.java

@@ -27,6 +27,8 @@
 
 package org.alfresco.module.org_alfresco_module_rm.hold;
 
+import static java.util.Arrays.asList;
+
 import static org.alfresco.module.org_alfresco_module_rm.test.util.AlfMock.generateQName;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -51,9 +53,11 @@ import java.util.Map;
 
 import org.alfresco.error.AlfrescoRuntimeException;
 import org.alfresco.model.ContentModel;
+import org.alfresco.module.org_alfresco_module_rm.capability.RMPermissionModel;
 import org.alfresco.module.org_alfresco_module_rm.test.util.BaseUnitTest;
 import org.alfresco.service.cmr.repository.ChildAssociationRef;
 import org.alfresco.service.cmr.repository.NodeRef;
+import org.alfresco.service.cmr.security.AccessStatus;
 import org.alfresco.service.namespace.NamespaceService;
 import org.alfresco.service.namespace.QName;
 import org.alfresco.service.namespace.RegexQNamePattern;
@@ -81,6 +85,7 @@ public class HoldServiceImplUnitTest extends BaseUnitTest
     protected NodeRef holdContainer;
     protected NodeRef hold;
     protected NodeRef hold2;
+    protected NodeRef activeContent;
 
     @Spy @InjectMocks HoldServiceImpl holdService;
 
@@ -95,6 +100,11 @@ public class HoldServiceImplUnitTest extends BaseUnitTest
         hold = generateNodeRef(TYPE_HOLD);
         hold2 = generateNodeRef(TYPE_HOLD);
 
+        activeContent = generateNodeRef();
+        QName contentSubtype = QName.createQName("contentSubtype", "contentSubtype");
+        when(mockedNodeService.getType(activeContent)).thenReturn(contentSubtype);
+        when(mockedDictionaryService.isSubClass(contentSubtype, ContentModel.TYPE_CONTENT)).thenReturn(true);
+
         // setup interactions
         doReturn(holdContainer).when(mockedFilePlanService).getHoldContainer(filePlan);
     }
@@ -165,13 +175,15 @@ public class HoldServiceImplUnitTest extends BaseUnitTest
     public void getHeldWithResults()
     {
         // setup record folder in hold
-        List<ChildAssociationRef> holds = new ArrayList<>(1);
+        List<ChildAssociationRef> holds = new ArrayList<>(2);
         holds.add(new ChildAssociationRef(ASSOC_FROZEN_RECORDS, hold, ASSOC_FROZEN_RECORDS, recordFolder, true, 1));
+        holds.add(new ChildAssociationRef(ASSOC_FROZEN_RECORDS, hold, ASSOC_FROZEN_RECORDS, activeContent, true, 1));
         doReturn(holds).when(mockedNodeService).getChildAssocs(hold, ASSOC_FROZEN_RECORDS, RegexQNamePattern.MATCH_ALL);
 
         List<NodeRef> list = holdService.getHeld(hold);
-        assertEquals(1, list.size());
+        assertEquals(2, list.size());
         assertEquals(recordFolder, list.get(0));
+        assertEquals(activeContent, list.get(1));
     }
 
     @SuppressWarnings({ "unchecked", "rawtypes" })
@@ -282,7 +294,7 @@ public class HoldServiceImplUnitTest extends BaseUnitTest
     }
 
     @Test (expected=AlfrescoRuntimeException.class)
-    public void addToHoldNotARecordFolderOrRecord()
+    public void addToHoldNotARecordFolderOrRecordOrActiveContent()
     {
         NodeRef anotherThing = generateNodeRef(TYPE_RECORD_CATEGORY);
         holdService.addToHold(hold, anotherThing);
@@ -297,20 +309,25 @@ public class HoldServiceImplUnitTest extends BaseUnitTest
         verify(mockedNodeService).addChild(hold, recordFolder, ASSOC_FROZEN_RECORDS, ASSOC_FROZEN_RECORDS);
         verify(mockedNodeService).addAspect(eq(recordFolder), eq(ASPECT_FROZEN), any(Map.class));
         verify(mockedNodeService).addAspect(eq(record), eq(ASPECT_FROZEN), any(Map.class));
-        verify(mockedRecordsManagementAuditService, times(1)).auditEvent(eq(recordFolder), anyString());
+        verify(mockedRecordsManagementAuditService).auditEvent(eq(recordFolder), anyString());
 
         holdService.addToHold(hold, record);
         verify(mockedNodeService).addChild(hold, record, ASSOC_FROZEN_RECORDS, ASSOC_FROZEN_RECORDS);
         verify(mockedNodeService).addAspect(eq(recordFolder), eq(ASPECT_FROZEN), any(Map.class));
         verify(mockedNodeService, times(2)).addAspect(eq(record), eq(ASPECT_FROZEN), any(Map.class));
-        verify(mockedRecordsManagementAuditService, times(1)).auditEvent(eq(record), anyString());
+        verify(mockedRecordsManagementAuditService).auditEvent(eq(record), anyString());
+
+        holdService.addToHold(hold, activeContent);
+        verify(mockedNodeService).addChild(hold, activeContent, ASSOC_FROZEN_RECORDS, ASSOC_FROZEN_RECORDS);
+        verify(mockedNodeService).addAspect(eq(activeContent), eq(ASPECT_FROZEN), any(Map.class));
+        verify(mockedRecordsManagementAuditService).auditEvent(eq(activeContent), anyString());
     }
 
     @SuppressWarnings("unchecked")
     @Test
     public void addToHoldAlreadyInHold()
     {
-        doReturn(Collections.singletonList(recordFolder)).when(holdService).getHeld(hold);
+        doReturn(asList(recordFolder, activeContent)).when(holdService).getHeld(hold);
 
         holdService.addToHold(hold, recordFolder);
 
@@ -318,21 +335,40 @@ public class HoldServiceImplUnitTest extends BaseUnitTest
         verify(mockedNodeService, never()).addAspect(eq(recordFolder), eq(ASPECT_FROZEN), any(Map.class));
         verify(mockedNodeService, never()).addAspect(eq(record), eq(ASPECT_FROZEN), any(Map.class));
         verify(mockedRecordsManagementAuditService, never()).auditEvent(eq(recordFolder), anyString());
+
+        holdService.addToHold(hold, activeContent);
+
+        verify(mockedNodeService, never()).addChild(hold, activeContent, ASSOC_FROZEN_RECORDS, ASSOC_FROZEN_RECORDS);
+        verify(mockedNodeService, never()).addAspect(eq(activeContent), eq(ASPECT_FROZEN), any(Map.class));
+        verify(mockedRecordsManagementAuditService, never()).auditEvent(eq(activeContent), anyString());
     }
 
     @SuppressWarnings("unchecked")
     @Test
-    public void addToHoldAldeadyFrozen()
+    public void addToHoldAlreadyFrozen()
     {
         doReturn(true).when(mockedNodeService).hasAspect(recordFolder, ASPECT_FROZEN);
         doReturn(true).when(mockedNodeService).hasAspect(record, ASPECT_FROZEN);
+        doReturn(true).when(mockedNodeService).hasAspect(activeContent, ASPECT_FROZEN);
 
         holdService.addToHold(hold, recordFolder);
 
-        verify(mockedNodeService, times(1)).addChild(hold, recordFolder, ASSOC_FROZEN_RECORDS, ASSOC_FROZEN_RECORDS);
+        verify(mockedNodeService).addChild(hold, recordFolder, ASSOC_FROZEN_RECORDS, ASSOC_FROZEN_RECORDS);
         verify(mockedNodeService, never()).addAspect(eq(recordFolder), eq(ASPECT_FROZEN), any(Map.class));
         verify(mockedNodeService, never()).addAspect(eq(record), eq(ASPECT_FROZEN), any(Map.class));
-        verify(mockedRecordsManagementAuditService, times(1)).auditEvent(eq(recordFolder), anyString());
+        verify(mockedRecordsManagementAuditService).auditEvent(eq(recordFolder), anyString());
+
+        holdService.addToHold(hold, activeContent);
+        verify(mockedNodeService).addChild(hold, activeContent, ASSOC_FROZEN_RECORDS, ASSOC_FROZEN_RECORDS);
+        verify(mockedNodeService, never()).addAspect(eq(activeContent), eq(ASPECT_FROZEN), any(Map.class));
+        verify(mockedRecordsManagementAuditService).auditEvent(eq(activeContent), anyString());
+    }
+
+    @Test (expected = AlfrescoRuntimeException.class)
+    public void addActiveContentToHoldNoPermissionsOnHold()
+    {
+        when(mockedPermissionService.hasPermission(hold, RMPermissionModel.FILING)).thenReturn(AccessStatus.DENIED);
+        holdService.addToHold(hold, activeContent);
     }
 
     @SuppressWarnings("unchecked")