diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldService.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldService.java index db3550a13a..9940e17007 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldService.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldService.java @@ -69,7 +69,7 @@ public interface HoldService /** * Gets the list of all the holds within the holds container for the given node reference * - * @param nodeRef The {@link NodeRef} of the record / record folder + * @param nodeRef The {@link NodeRef} of the record / record folder /active content * @param includedInHold true to retrieve the list of hold node references which will include the node reference * false to get a list of node references which will not have the given node reference * @return List of hold node references @@ -119,10 +119,10 @@ public interface HoldService void deleteHold(NodeRef hold); /** - * Adds the record to the given hold + * Adds the item to the given hold * - * @param hold The {@link NodeRef} of the hold - * @param nodeRef The {@link NodeRef} of the record / record folder which will be added to the given hold + * @param hold The {@link NodeRef} of the hold + * @param nodeRef The {@link NodeRef} of the record / record folder / active content which will be added to the given hold */ void addToHold(NodeRef hold, NodeRef nodeRef); @@ -135,10 +135,10 @@ public interface HoldService void addToHold(NodeRef hold, List nodeRefs); /** - * Adds the record to the given list of holds + * Adds the item to the given list of holds * * @param holds The list of {@link NodeRef}s of the holds - * @param nodeRef The {@link NodeRef} of the record / record folder which will be added to the given holds + * @param nodeRef The {@link NodeRef} of the record / record folder / active content which will be added to the given holds */ void addToHolds(List holds, NodeRef nodeRef); diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImpl.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImpl.java index 7e769ec9b2..51020bf8a5 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImpl.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImpl.java @@ -303,7 +303,7 @@ public class HoldServiceImpl extends ServiceBaseImpl if (!includedInHold) { // invert list to get list of holds that do not contain this node - NodeRef filePlan = filePlanService.getFilePlan(nodeRef); + NodeRef filePlan = isFilePlanComponent(nodeRef) ? filePlanService.getFilePlan(nodeRef) : filePlanService.getFilePlanBySiteId(FilePlanService.DEFAULT_RM_SITE_ID); List allHolds = getHolds(filePlan); result = ListUtils.subtract(allHolds, new ArrayList<>(holdsNotIncludingNodeRef)); } diff --git a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/AddActiveContentToHoldTest.java b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/AddActiveContentToHoldTest.java new file mode 100644 index 0000000000..75d8dc2541 --- /dev/null +++ b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/AddActiveContentToHoldTest.java @@ -0,0 +1,242 @@ +/* + * #%L + * Alfresco Records Management Module + * %% + * Copyright (C) 2005 - 2019 Alfresco Software Limited + * %% + * This file is part of the Alfresco software. + * - + * If the software was purchased under a paid Alfresco license, the terms of + * the paid license agreement will prevail. Otherwise, the software is + * provided under the following open source license terms: + * - + * Alfresco is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * - + * Alfresco is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * - + * You should have received a copy of the GNU Lesser General Public License + * along with Alfresco. If not, see . + * #L% + */ +package org.alfresco.module.org_alfresco_module_rm.test.integration.hold; + +import org.alfresco.error.AlfrescoRuntimeException; +import org.alfresco.module.org_alfresco_module_rm.test.util.BaseRMTestCase; +import org.alfresco.repo.security.authentication.AuthenticationUtil; +import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork; +import org.alfresco.repo.security.permissions.AccessDeniedException; +import org.alfresco.repo.site.SiteModel; +import org.alfresco.service.cmr.repository.NodeRef; +import org.springframework.extensions.webscripts.GUID; + +/** + * Add Active Content To Hold Integration Tests + * + * @author Claudia Agache + * @since 3.2 + */ +public class AddActiveContentToHoldTest extends BaseRMTestCase +{ + @Override + protected boolean isCollaborationSiteTest() + { + return true; + } + + @Override + protected boolean isUserTest() + { + return true; + } + + public void testAddDocumentToHold() + { + doBehaviourDrivenTest(new BehaviourDrivenTest() + { + private NodeRef hold; + + public void given() + { + // Check that the document is not a record + assertFalse("The document should not be a record", recordService.isRecord(dmDocument)); + + // create a hold + hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate()); + + // assert current states + assertFalse(freezeService.isFrozen(dmDocument)); + assertFalse(freezeService.hasFrozenChildren(dmFolder)); + + // additional check for child held caching + assertFalse(nodeService.hasAspect(dmFolder, ASPECT_HELD_CHILDREN)); + } + + public void when() + { + // add the active content to hold + holdService.addToHold(hold, dmDocument); + } + + public void then() + { + // active content is held + assertTrue(freezeService.isFrozen(dmDocument)); + + // collaboration folder has frozen children + assertFalse(freezeService.isFrozen(dmFolder)); + assertTrue(freezeService.hasFrozenChildren(dmFolder)); + + // collaboration folder is not held + assertFalse(holdService.getHeld(hold).contains(dmFolder)); + assertFalse(holdService.heldBy(dmFolder, true).contains(hold)); + + // hold contains active content + assertTrue(holdService.getHeld(hold).contains(dmDocument)); + assertTrue(holdService.heldBy(dmDocument, true).contains(hold)); + + // additional check for child held caching + assertTrue(nodeService.hasAspect(dmFolder, ASPECT_HELD_CHILDREN)); + assertEquals(1, nodeService.getProperty(dmFolder, PROP_HELD_CHILDREN_COUNT)); + } + }); + } + + public void testAddDocumentToHoldAsNonRMUser() + { + doBehaviourDrivenTest(new BehaviourDrivenTest(AccessDeniedException.class) + { + private NodeRef hold; + + public void given() + { + // Check that the document is not a record + assertFalse("The document should not be a record", recordService.isRecord(dmDocument)); + + // create a hold + hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate()); + + // assert current states + assertFalse(freezeService.isFrozen(dmDocument)); + } + + public void when() + { + // add the active content to hold as a non RM user + AuthenticationUtil.runAs( + (RunAsWork) () -> { + holdService.addToHold(hold, dmDocument); + return null; + }, dmCollaborator); + } + }); + } + + public void testAddDocumentToHoldNoWritePermissionOnDoc() + { + doBehaviourDrivenTest(new BehaviourDrivenTest(AlfrescoRuntimeException.class) + { + private NodeRef hold; + + public void given() + { + // Check that the document is not a record + assertFalse("The document should not be a record", recordService.isRecord(dmDocument)); + + // create a hold + hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate()); + + // assert current states + assertFalse(freezeService.isFrozen(dmDocument)); + } + + public void when() + { + // add the active content to hold as a RM admin who has Add to Hold Capability and filing permission on + // hold, but no Write permissions on doc + AuthenticationUtil.runAs( + (RunAsWork) () -> { + holdService.addToHold(hold, dmDocument); + return null; + }, rmAdminName); + } + }); + } + + public void testAddDocumentToHoldNoFilingPermissionOnHold() + { + doBehaviourDrivenTest(new BehaviourDrivenTest(AlfrescoRuntimeException.class) + { + private NodeRef hold; + + public void given() + { + // Check that the document is not a record + assertFalse("The document should not be a record", recordService.isRecord(dmDocument)); + + // create a hold + hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate()); + + // assert current states + assertFalse(freezeService.isFrozen(dmDocument)); + + //add recordsManagerPerson as manager in collaboration site to have write permissions on dmDocument + siteService.setMembership(collabSiteId, recordsManagerName, SiteModel.SITE_MANAGER); + } + + public void when() + { + // add the active content to hold as a RM manager who has Add to Hold Capability and write permission on + // doc, but no filing permission on hold + AuthenticationUtil.runAs( + (RunAsWork) () -> { + holdService.addToHold(hold, dmDocument); + return null; + }, recordsManagerName); + } + }); + } + + + public void testAddDocumentToHoldNoCapability() + { + doBehaviourDrivenTest(new BehaviourDrivenTest(AlfrescoRuntimeException.class) + { + private NodeRef hold; + + public void given() + { + // Check that the document is not a record + assertFalse("The document should not be a record", recordService.isRecord(dmDocument)); + + // create a hold + hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate()); + + // assert current states + assertFalse(freezeService.isFrozen(dmDocument)); + + //add powerUserPerson as manager in collaboration site to have write permissions on dmDocument + siteService.setMembership(collabSiteId, powerUserName, SiteModel.SITE_MANAGER); + + //assign powerUserPerson filing permission on hold + filePlanPermissionService.setPermission(hold, powerUserName, FILING); + } + + public void when() + { + // add the active content to hold as a RM power user who has write permission on doc and filing + // permission on hold, but no Add To Hold capability + AuthenticationUtil.runAs( + (RunAsWork) () -> { + holdService.addToHold(hold, dmDocument); + return null; + }, powerUserName); + } + }); + } +} diff --git a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/AddRemoveFromHoldTest.java b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/AddRemoveFromHoldTest.java index 581a6c922c..0a31e20fa5 100644 --- a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/AddRemoveFromHoldTest.java +++ b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/AddRemoveFromHoldTest.java @@ -45,64 +45,6 @@ public class AddRemoveFromHoldTest extends BaseRMTestCase { private static final int RECORD_COUNT = 10; - @Override - protected boolean isCollaborationSiteTest() - { - return true; - } - - public void testAddActiveContentToHold() - { - doBehaviourDrivenTest(new BehaviourDrivenTest() - { - private NodeRef hold; - - public void given() - { - // Check that the document is not a record - assertFalse("The document should not be a record", recordService.isRecord(dmDocument)); - - // create a hold - hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate()); - - // assert current states - assertFalse(freezeService.isFrozen(dmDocument)); - assertFalse(freezeService.hasFrozenChildren(dmFolder)); - - // additional check for child held caching - assertFalse(nodeService.hasAspect(dmFolder, ASPECT_HELD_CHILDREN)); - } - - public void when() throws Exception - { - // add the active content to hold - holdService.addToHold(hold, dmDocument); - } - - public void then() - { - // active content is held - assertTrue(freezeService.isFrozen(dmDocument)); - - // collaboration folder has frozen children - assertFalse(freezeService.isFrozen(dmFolder)); - assertTrue(freezeService.hasFrozenChildren(dmFolder)); - - // collaboration folder is not held - assertFalse(holdService.getHeld(hold).contains(dmFolder)); - assertFalse(holdService.heldBy(dmFolder, true).contains(hold)); - - // hold contains active content - assertTrue(holdService.getHeld(hold).contains(dmDocument)); - assertTrue(holdService.heldBy(dmDocument, true).contains(hold)); - - // additional check for child held caching - assertTrue(nodeService.hasAspect(dmFolder, ASPECT_HELD_CHILDREN)); - assertEquals(1, nodeService.getProperty(dmFolder, PROP_HELD_CHILDREN_COUNT)); - } - }); - } - public void testAddRecordToHold() { doBehaviourDrivenTest(new BehaviourDrivenTest() diff --git a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/util/BaseRMTestCase.java b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/util/BaseRMTestCase.java index 57f2151c23..204ca4c8e8 100644 --- a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/util/BaseRMTestCase.java +++ b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/util/BaseRMTestCase.java @@ -253,6 +253,7 @@ public abstract class BaseRMTestCase extends RetryingTransactionHelperTestCase protected String powerUserName; protected String securityOfficerName; protected String recordsManagerName; + protected String rmAdminName; /** test people */ protected NodeRef userPerson; @@ -260,6 +261,7 @@ public abstract class BaseRMTestCase extends RetryingTransactionHelperTestCase protected NodeRef powerUserPerson; protected NodeRef securityOfficerPerson; protected NodeRef recordsManagerPerson; + protected NodeRef rmAdminPerson; /** test records */ protected NodeRef recordOne; @@ -656,13 +658,18 @@ public abstract class BaseRMTestCase extends RetryingTransactionHelperTestCase recordsManagerPerson = createPerson(recordsManagerName); filePlanRoleService.assignRoleToAuthority(filePlan, FilePlanRoleService.ROLE_RECORDS_MANAGER, recordsManagerName); + rmAdminName = GUID.generate(); + rmAdminPerson = createPerson(rmAdminName); + filePlanRoleService.assignRoleToAuthority(filePlan, FilePlanRoleService.ROLE_ADMIN, rmAdminName); + testUsers = new String[] { userName, rmUserName, powerUserName, securityOfficerName, - recordsManagerName + recordsManagerName, + rmAdminName }; if (isFillingForAllUsers()) diff --git a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImplUnitTest.java b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImplUnitTest.java index 72faf25a86..19fa820027 100644 --- a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImplUnitTest.java +++ b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImplUnitTest.java @@ -54,10 +54,12 @@ 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.fileplan.FilePlanService; 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.cmr.security.PermissionService; import org.alfresco.service.namespace.NamespaceService; import org.alfresco.service.namespace.QName; import org.alfresco.service.namespace.RegexQNamePattern; @@ -107,6 +109,7 @@ public class HoldServiceImplUnitTest extends BaseUnitTest // setup interactions doReturn(holdContainer).when(mockedFilePlanService).getHoldContainer(filePlan); + doReturn(filePlan).when(mockedFilePlanService).getFilePlanBySiteId(FilePlanService.DEFAULT_RM_SITE_ID); } @Test @@ -125,6 +128,9 @@ public class HoldServiceImplUnitTest extends BaseUnitTest holds.add(new ChildAssociationRef(ASSOC_FROZEN_RECORDS, hold2, ASSOC_FROZEN_RECORDS, recordFolder, true, 2)); doReturn(holds).when(mockedNodeService).getParentAssocs(recordFolder, ASSOC_FROZEN_RECORDS, ASSOC_FROZEN_RECORDS); + //setup active content in multiple holds + doReturn(holds).when(mockedNodeService).getParentAssocs(activeContent, ASSOC_FROZEN_RECORDS, ASSOC_FROZEN_RECORDS); + // check that both holds are found for record folder List heldByHolds = holdService.heldBy(recordFolder, true); assertNotNull(heldByHolds); @@ -136,6 +142,12 @@ public class HoldServiceImplUnitTest extends BaseUnitTest assertNotNull(heldByHolds); assertEquals(2, heldByHolds.size()); assertTrue(holdService.heldBy(record, false).isEmpty()); + + // check that both holds are found for active content + heldByHolds = holdService.heldBy(activeContent, true); + assertNotNull(heldByHolds); + assertEquals(2, heldByHolds.size()); + assertTrue(holdService.heldBy(activeContent, false).isEmpty()); } @Test (expected=AlfrescoRuntimeException.class) @@ -371,6 +383,13 @@ public class HoldServiceImplUnitTest extends BaseUnitTest holdService.addToHold(hold, activeContent); } + @Test (expected = AlfrescoRuntimeException.class) + public void addActiveContentToHoldNoPermissionsOnContent() + { + when(mockedPermissionService.hasPermission(activeContent, PermissionService.WRITE)).thenReturn(AccessStatus.DENIED); + holdService.addToHold(hold, activeContent); + } + @SuppressWarnings("unchecked") @Test public void addToHolds()