Code review comments

This commit is contained in:
cagache
2019-11-11 11:21:28 +02:00
parent eade9c9e0f
commit b36b4e31cb
3 changed files with 14 additions and 37 deletions

View File

@@ -85,7 +85,7 @@ public class AuditAddToHoldTests extends BaseRMRestTest
@Autowired
private RoleService roleService;
private UserModel rmAdmin, rmManagerNoRightsOnHold, rmManagerNoRightsOnNode;
private UserModel rmAdmin, rmManagerNoReadOnHold, rmManagerNoReadOnNode;
private SiteModel privateSite;
private RecordCategory recordCategory;
private RecordCategoryChild recordFolder;
@@ -112,9 +112,9 @@ public class AuditAddToHoldTests extends BaseRMRestTest
privateSite = dataSite.usingUser(rmAdmin).createPrivateRandomSite();
STEP("Create users without rights to add content to a hold.");
rmManagerNoRightsOnHold = roleService.createUserWithSiteRoleRMRoleAndPermission(privateSite,
rmManagerNoReadOnHold = roleService.createUserWithSiteRoleRMRoleAndPermission(privateSite,
UserRole.SiteManager, recordCategory.getId(), UserRoles.ROLE_RM_MANAGER, UserPermissions.PERMISSION_FILING);
rmManagerNoRightsOnNode = roleService.createUserWithRMRoleAndRMNodePermission(UserRoles.ROLE_RM_MANAGER.roleId,
rmManagerNoReadOnNode = roleService.createUserWithRMRoleAndRMNodePermission(UserRoles.ROLE_RM_MANAGER.roleId,
hold1NodeRef, UserPermissions.PERMISSION_FILING);
}
@@ -153,8 +153,8 @@ public class AuditAddToHoldTests extends BaseRMRestTest
{
return new UserModel[][]
{
{ rmManagerNoRightsOnHold },
{ rmManagerNoRightsOnNode }
{ rmManagerNoReadOnHold },
{ rmManagerNoReadOnNode }
};
}
@@ -195,7 +195,7 @@ public class AuditAddToHoldTests extends BaseRMRestTest
rmAuditService.clearAuditLog();
STEP("Try to add the record to a hold by an user with no rights.");
holdsAPI.addItemsToHolds(rmManagerNoRightsOnHold.getUsername(), rmManagerNoRightsOnHold.getPassword(),
holdsAPI.addItemsToHolds(rmManagerNoReadOnHold.getUsername(), rmManagerNoReadOnHold.getPassword(),
SC_INTERNAL_SERVER_ERROR, Collections.singletonList(recordToBeAdded.getId()),
Collections.singletonList(hold1NodeRef));
@@ -284,7 +284,7 @@ public class AuditAddToHoldTests extends BaseRMRestTest
{
holdsList.forEach(hold -> holdsAPI.deleteHold(getAdminUser().getUsername(), getAdminUser().getPassword(), hold));
dataSite.usingAdmin().deleteSite(privateSite);
asList(rmAdmin, rmManagerNoRightsOnHold, rmManagerNoRightsOnNode).forEach(user -> getDataUser().usingAdmin().deleteUser(user));
asList(rmAdmin, rmManagerNoReadOnHold, rmManagerNoReadOnNode).forEach(user -> getDataUser().usingAdmin().deleteUser(user));
getRestAPIFactory().getRecordCategoryAPI().deleteRecordCategory(recordCategory.getId());
}
}

View File

@@ -130,28 +130,6 @@ public class AuditDeleteHoldTests extends BaseRMRestTest
rmAuditService.getAuditEntriesFilteredByEvent(getAdminUser(), DELETE_HOLD).isEmpty());
}
/**
* Given a hold is deleted
* When I view the audit log as an user with no Read permissions over the deleted hold
* Then the delete hold entry isn't visible
*/
@Test
public void deleteHoldAuditEntryNotVisible()
{
STEP("Create a new hold.");
String holdRef = holdsAPI.createHoldAndGetNodeRef(rmAdmin.getUsername(), rmAdmin.getPassword(), HOLD2, HOLD_REASON,
HOLD_DESCRIPTION);
rmAuditService.clearAuditLog();
STEP("Delete the created hold.");
holdsAPI.deleteHold(rmAdmin, holdRef);
STEP("Check that an user with no Read permissions over the hold can't see the entry for the delete hold event.");
assertTrue("The list of events should not contain Delete Hold entry ",
rmAuditService.getAuditEntriesFilteredByEvent(rmManager, DELETE_HOLD).isEmpty());
}
@AfterClass (alwaysRun = true)
public void cleanUpAuditDeleteHoldTests()
{

View File

@@ -87,7 +87,7 @@ public class AuditRemoveFromHoldTests extends BaseRMRestTest
@Autowired
private RoleService roleService;
private UserModel rmAdmin, rmManagerNoRightsOnHold, rmManagerNoRightsOnNode;
private UserModel rmAdmin, rmManagerNoReadOnHold, rmManagerNoReadOnNode;
private SiteModel privateSite;
private RecordCategory recordCategory;
private RecordCategoryChild recordFolder, heldRecordFolder;
@@ -128,9 +128,9 @@ public class AuditRemoveFromHoldTests extends BaseRMRestTest
holdsList);
STEP("Create users without rights to remove content from a hold.");
rmManagerNoRightsOnHold = roleService.createUserWithSiteRoleRMRoleAndPermission(privateSite,
rmManagerNoReadOnHold = roleService.createUserWithSiteRoleRMRoleAndPermission(privateSite,
UserRole.SiteManager, recordCategory.getId(), UserRoles.ROLE_RM_MANAGER, UserPermissions.PERMISSION_FILING);
rmManagerNoRightsOnNode = roleService.createUserWithRMRoleAndRMNodePermission(UserRoles.ROLE_RM_MANAGER.roleId,
rmManagerNoReadOnNode = roleService.createUserWithRMRoleAndRMNodePermission(UserRoles.ROLE_RM_MANAGER.roleId,
hold1NodeRef, UserPermissions.PERMISSION_FILING);
}
@@ -138,7 +138,6 @@ public class AuditRemoveFromHoldTests extends BaseRMRestTest
* Data provider with valid nodes that can be removed from a hold
*
* @return the node id and the node name
* @throws Exception
*/
@DataProvider (name = "validNodesForRemoveFromHold")
public Object[][] getValidNodesForRemoveFromHold()
@@ -164,8 +163,8 @@ public class AuditRemoveFromHoldTests extends BaseRMRestTest
{
return new UserModel[][]
{
{ rmManagerNoRightsOnHold },
{ rmManagerNoRightsOnNode }
{ rmManagerNoReadOnHold },
{ rmManagerNoReadOnNode }
};
}
@@ -226,7 +225,7 @@ public class AuditRemoveFromHoldTests extends BaseRMRestTest
rmAuditService.clearAuditLog();
STEP("Try to remove the record from a hold by an user with no rights.");
holdsAPI.removeItemsFromHolds(rmManagerNoRightsOnHold.getUsername(), rmManagerNoRightsOnHold.getPassword(),
holdsAPI.removeItemsFromHolds(rmManagerNoReadOnHold.getUsername(), rmManagerNoReadOnHold.getPassword(),
SC_INTERNAL_SERVER_ERROR, Collections.singletonList(heldRecord.getId()),
Collections.singletonList(hold1NodeRef));
@@ -315,7 +314,7 @@ public class AuditRemoveFromHoldTests extends BaseRMRestTest
{
holdsList.forEach(hold -> holdsAPI.deleteHold(getAdminUser().getUsername(), getAdminUser().getPassword(), hold));
dataSite.usingAdmin().deleteSite(privateSite);
asList(rmAdmin, rmManagerNoRightsOnHold, rmManagerNoRightsOnNode).forEach(user -> getDataUser().usingAdmin().deleteUser(user));
asList(rmAdmin, rmManagerNoReadOnHold, rmManagerNoReadOnNode).forEach(user -> getDataUser().usingAdmin().deleteUser(user));
getRestAPIFactory().getRecordCategoryAPI().deleteRecordCategory(recordCategory.getId());
}
}