diff --git a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImpl.java b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImpl.java index ecd7e25a86..105252e84e 100644 --- a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImpl.java +++ b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImpl.java @@ -455,25 +455,35 @@ public class HoldServiceImpl extends ServiceBaseImpl if (!isRecord(nodeRef) && !isRecordFolder(nodeRef)) { - throw new AlfrescoRuntimeException("Can only add records or record folders to a hold."); + String nodeName = (String) nodeService.getProperty(nodeRef, ContentModel.PROP_NAME); + 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) { - throw new AlfrescoRuntimeException("Filing permission on the record or record folder is required."); + String nodeName = (String) nodeService.getProperty(nodeRef, ContentModel.PROP_NAME); + throw new AlfrescoRuntimeException("Filing permission on '" + nodeName + "' is needed."); } for (final NodeRef hold : holds) { if (!isHold(hold)) { - throw new AlfrescoRuntimeException("Can't add to hold, because it isn't a hold. (hold=" + hold.toString() + ")"); + String holdName = (String) nodeService.getProperty(hold, ContentModel.PROP_NAME); + throw new AlfrescoRuntimeException("'" + holdName + "' is not a hold so record folders/records cannot be added."); + } + + if (permissionService.hasPermission(hold, RMPermissionModel.FILING) == AccessStatus.DENIED) + { + String nodeName = (String) nodeService.getProperty(nodeRef, ContentModel.PROP_NAME); + String holdName = (String) nodeService.getProperty(hold, ContentModel.PROP_NAME); + throw new AlfrescoRuntimeException("'" + nodeName + "' can't be added to the hold container as filing permission for '" + holdName + "' is needed."); } // check that the node isn't already in the hold if (!getHeld(hold).contains(nodeRef)) { - // run as system to ensure we have all the appropriate premissions to perform the manipulations we require + // run as system to ensure we have all the appropriate permissions to perform the manipulations we require runAsSystem(new RunAsWork() { @Override diff --git a/rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/service/HoldServiceImplTest.java b/rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/service/HoldServiceImplTest.java index 1dbf63c793..20fb38c2a3 100644 --- a/rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/service/HoldServiceImplTest.java +++ b/rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/service/HoldServiceImplTest.java @@ -147,7 +147,7 @@ public class HoldServiceImplTest extends BaseRMTestCase }); } - public void testAddRecordFolderOrRecordToHoldWithoutFilingPermission() + public void testAddRecordFolderToHoldWithoutFilingPermissionOnRecordFolder() { // Create hold final NodeRef hold = holdService.createHold(filePlan, "hold one", "I have my reasons", "but I'll not describe them here!"); @@ -181,4 +181,38 @@ public class HoldServiceImplTest extends BaseRMTestCase }, userName); } + public void testAddRecordFolderToHoldWithoutFilingPermissionOnHold() + { + // Create hold + final NodeRef hold = holdService.createHold(filePlan, "hold one", "I have my reasons", "but I'll not describe them here!"); + assertNotNull(hold); + + doTestInTransaction(new Test() + { + @Override + public Void run() throws Exception + { + // Add the user to the RM Manager role + filePlanRoleService.assignRoleToAuthority(filePlan, ROLE_NAME_RECORDS_MANAGER, userName); + + // Give the user read permissions on the hold + permissionService.setPermission(hold, userName, RMPermissionModel.READ_RECORDS, true); + + // Give the user filing permissions on the record folder + permissionService.setPermission(rmFolder, userName, RMPermissionModel.FILING, true); + + return null; + } + }, "admin"); + + doTestInTransaction(new FailureTest(AlfrescoRuntimeException.class) + { + @Override + public void run() throws Exception + { + holdService.addToHold(hold, rmFolder); + } + }, userName); + } + }