From b14b52dd923c7ad2778df7266b3ba5ea4adcb048 Mon Sep 17 00:00:00 2001 From: Ross Gale Date: Tue, 20 Aug 2019 13:56:14 +0100 Subject: [PATCH 1/3] RM-6873 adding code review changes --- .../hold/RemoveActiveContentFromHoldTest.java | 14 ++++++++------ .../model/rma/aspect/FrozenAspectUnitTest.java | 18 ++++++------------ 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/RemoveActiveContentFromHoldTest.java b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/RemoveActiveContentFromHoldTest.java index 8a4181116e..2a08fb6877 100644 --- a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/RemoveActiveContentFromHoldTest.java +++ b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/RemoveActiveContentFromHoldTest.java @@ -28,11 +28,13 @@ package org.alfresco.module.org_alfresco_module_rm.test.integration.hold; import static org.alfresco.repo.security.authentication.AuthenticationUtil.getAdminUserName; +import java.util.ArrayList; +import java.util.List; + 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; @@ -114,8 +116,10 @@ public class RemoveActiveContentFromHoldTest extends BaseRMTestCase { hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate()); hold2 = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate()); - holdService.addToHold(hold, dmDocument); - holdService.addToHold(hold2, dmDocument); + final List holds = new ArrayList<>(2); + holds.add(hold); + holds.add(hold2); + holdService.addToHolds(holds, dmDocument); } public void when() @@ -162,7 +166,7 @@ public class RemoveActiveContentFromHoldTest extends BaseRMTestCase /** * Given a piece of active content on hold - * When I try to remove the active content to the hold without the remove hold capability + * When I try to remove the active content from the hold without the remove hold capability * Then an access denied exception is thrown */ public void testRemoveDocumentFromHoldFailsWithoutRemoveHoldPermission() @@ -173,10 +177,8 @@ public class RemoveActiveContentFromHoldTest extends BaseRMTestCase public void given() { - //add powerUserPerson as manager in collaboration site to have write permissions on dmDocument AuthenticationUtil.runAs( (RunAsWork) () -> { - siteService.setMembership(collabSiteId, powerUserName, SiteModel.SITE_MANAGER); hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate()); holdService.addToHold(hold, dmDocument); return null; diff --git a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/FrozenAspectUnitTest.java b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/FrozenAspectUnitTest.java index 8a323d794b..8cddad40a2 100644 --- a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/FrozenAspectUnitTest.java +++ b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/FrozenAspectUnitTest.java @@ -42,7 +42,6 @@ import java.util.Set; import org.alfresco.model.ContentModel; import org.alfresco.module.org_alfresco_module_rm.freeze.FreezeService; import org.alfresco.module.org_alfresco_module_rm.util.TransactionalResourceHelper; -import org.alfresco.repo.security.permissions.AccessDeniedException; import org.alfresco.rest.framework.core.exceptions.PermissionDeniedException; import org.alfresco.service.cmr.dictionary.DictionaryService; import org.alfresco.service.cmr.repository.ChildAssociationRef; @@ -90,9 +89,6 @@ public class FrozenAspectUnitTest @Mock private ChildAssociationRef mockOldRef; - @Mock - private ChildAssociationRef mockNewRef; - @Mock private Set mockSet; @@ -116,11 +112,11 @@ public class FrozenAspectUnitTest when(mockNodeService.hasAspect(folder, ASPECT_HELD_CHILDREN)).thenReturn(true); when(mockNodeService.getProperty(folder, PROP_HELD_CHILDREN_COUNT)).thenReturn(1); when(mockApplicationContext.getBean("dbNodeService")).thenReturn(mockNodeService); - when(mockNodeService.exists(content)).thenReturn(true); when(mockFreezeService.isFrozen(content)).thenReturn(false); children.add(mockChildRef); when(mockNodeService.getChildAssocs(content)).thenReturn(children); when(mockChildRef.isPrimary()).thenReturn(true); + when(mockNodeService.hasAspect(record, ASPECT_RECORD)).thenReturn(true); } /** @@ -129,7 +125,6 @@ public class FrozenAspectUnitTest @Test public void testRemoveAspectForRecords() { - when(mockNodeService.hasAspect(record, ASPECT_RECORD)).thenReturn(true); when(mockNodeService.getPrimaryParent(record)).thenReturn(mockChildAssociationRef); when(mockChildAssociationRef.getParentRef()).thenReturn(folder); frozenAspect.onRemoveAspect(record, null); @@ -151,7 +146,7 @@ public class FrozenAspectUnitTest } /** - * Test that the remove code is only ran for records or active content + * Test that the remove code is only run for records or active content */ @Test public void testRemoveAspectForContentDoesntUpdateForOtherTypes() @@ -201,7 +196,6 @@ public class FrozenAspectUnitTest @Test public void testOnAddAspectForRecord() { - when(mockNodeService.getType(record)).thenReturn(ContentModel.TYPE_CONTENT); when(mockNodeService.getPrimaryParent(record)).thenReturn(mockParentRef); when(mockParentRef.getParentRef()).thenReturn(parent); when(mockNodeService.hasAspect(parent, ASPECT_HELD_CHILDREN)).thenReturn(true); @@ -211,17 +205,17 @@ public class FrozenAspectUnitTest } /** - * Test on add for a content node + * Test on add aspect for a content node */ @Test public void testOnAddAspectForContent() { - when(mockNodeService.getType(record)).thenReturn(ContentModel.TYPE_CONTENT); - when(mockNodeService.getPrimaryParent(record)).thenReturn(mockParentRef); + when(mockNodeService.getType(content)).thenReturn(ContentModel.TYPE_CONTENT); + when(mockNodeService.getPrimaryParent(content)).thenReturn(mockParentRef); when(mockParentRef.getParentRef()).thenReturn(parent); when(mockNodeService.hasAspect(parent, ASPECT_HELD_CHILDREN)).thenReturn(false); when(mockNodeService.getType(parent)).thenReturn(ContentModel.TYPE_FOLDER); - frozenAspect.onAddAspect(record, null); + frozenAspect.onAddAspect(content, null); verify(mockNodeService, times(1)).addAspect(any(NodeRef.class), any(QName.class), anyMap()); } From cd10d7eb7ecadea6540a2a4032a7329fbd69529a Mon Sep 17 00:00:00 2001 From: Ross Gale Date: Tue, 20 Aug 2019 13:57:42 +0100 Subject: [PATCH 2/3] RM-6873 adding code review changes --- .../test/integration/hold/RemoveActiveContentFromHoldTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/RemoveActiveContentFromHoldTest.java b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/RemoveActiveContentFromHoldTest.java index 2a08fb6877..7f940ef310 100644 --- a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/RemoveActiveContentFromHoldTest.java +++ b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/RemoveActiveContentFromHoldTest.java @@ -39,7 +39,7 @@ import org.alfresco.service.cmr.repository.NodeRef; import org.springframework.extensions.webscripts.GUID; /** - * Remove Active Content To Hold Integration Tests + * Remove active content from hold integration tests * * @author Ross Gale * @since 3.2 From e9319efcf0209ec7c2de10ace74990980b783b6e Mon Sep 17 00:00:00 2001 From: Ross Gale Date: Tue, 20 Aug 2019 13:58:26 +0100 Subject: [PATCH 3/3] RM-6873 adding code review changes --- .../test/integration/hold/RemoveActiveContentFromHoldTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/RemoveActiveContentFromHoldTest.java b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/RemoveActiveContentFromHoldTest.java index 7f940ef310..170bec08ab 100644 --- a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/RemoveActiveContentFromHoldTest.java +++ b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/RemoveActiveContentFromHoldTest.java @@ -138,7 +138,7 @@ public class RemoveActiveContentFromHoldTest extends BaseRMTestCase /** * Given a piece of active content on hold - * When I try to remove the active content to the hold without permission + * When I try to remove the active content from the hold without permission * Then an access denied exception is thrown */ public void testRemoveDocumentFromHoldFailsWithoutFilingPermission()