72750: Finalize Delete eperson: workflow

dspace-api/src/main/java/org/dspace/eperson/EPersonServiceImpl.java

@@ -249,7 +249,7 @@ public class EPersonServiceImpl extends DSpaceObjectServiceImpl<EPerson> impleme
             log.error("This IOException: " + ex + " occured while deleting Eperson with the ID: " + ePerson.getID());
             throw new AuthorizeException(new EPersonDeletionException());
         } catch (EPersonDeletionException e) {
-            throw new IllegalStateException(e.getMessage());
+            throw new IllegalStateException(e);
         }
     }
 
@@ -284,7 +284,7 @@ public class EPersonServiceImpl extends DSpaceObjectServiceImpl<EPerson> impleme
             List<EPerson> ePeople = groupService.allMembers(context, group);
             if (ePeople.size() == 1 && ePeople.contains(ePerson)) {
                 throw new IllegalStateException(
-                        "Refused to delete user " + ePerson.getID() + " because it’s part of the group " + group
+                        "Refused to delete user " + ePerson.getID() + " because it is part of the group " + group
                                 .getID());
             }
         }

dspace-api/src/main/java/org/dspace/eperson/GroupServiceImpl.java

@@ -42,6 +42,10 @@ import org.dspace.eperson.service.EPersonService;
 import org.dspace.eperson.service.GroupService;
 import org.dspace.event.Event;
 import org.dspace.util.UUIDUtils;
+import org.dspace.xmlworkflow.Role;
+import org.dspace.xmlworkflow.factory.XmlWorkflowFactory;
+import org.dspace.xmlworkflow.state.Step;
+import org.dspace.xmlworkflow.storedcomponents.ClaimedTask;
 import org.dspace.xmlworkflow.storedcomponents.CollectionRole;
 import org.dspace.xmlworkflow.storedcomponents.PoolTask;
 import org.dspace.xmlworkflow.storedcomponents.service.ClaimedTaskService;
@@ -88,6 +92,8 @@ public class GroupServiceImpl extends DSpaceObjectServiceImpl<Group> implements
     protected PoolTaskService poolTaskService;
     @Autowired(required = true)
     protected ClaimedTaskService claimedTaskService;
+    @Autowired(required = true)
+    protected XmlWorkflowFactory workflowFactory;
 
     protected GroupServiceImpl() {
         super();
@@ -151,18 +157,45 @@ public class GroupServiceImpl extends DSpaceObjectServiceImpl<Group> implements
                                    groupChild.getName(), getIdentifiers(context, groupParent)));
     }
 
+    /**
+     * Removes a member of a group.
+     * The removal will be refused if the group is linked to a workflow step which has claimed tasks or pool tasks
+     * and no other member is present in the group to handle these.
+     * @param context DSpace context object
+     * @param group   DSpace group
+     * @param ePerson eperson
+     * @throws SQLException
+     */
     @Override
     public void removeMember(Context context, Group group, EPerson ePerson) throws SQLException {
         List<CollectionRole> collectionRoles = collectionRoleService.findByGroup(context, group);
         if (!collectionRoles.isEmpty()) {
             List<PoolTask> poolTasks = poolTaskService.findByGroup(context, group);
+            List<ClaimedTask> claimedTasks = claimedTaskService.findByEperson(context, ePerson);
+            for (ClaimedTask claimedTask : claimedTasks) {
+                Step stepByName = workflowFactory.getStepByName(claimedTask.getStepID());
+                Role role = stepByName.getRole();
+                for (CollectionRole collectionRole : collectionRoles) {
+                    if (StringUtils.equals(collectionRole.getRoleId(), role.getId())
+                            && claimedTask.getWorkflowItem().getCollection() == collectionRole.getCollection()) {
+                        List<EPerson> ePeople = allMembers(context, group);
+                        if (ePeople.size() == 1 && ePeople.contains(ePerson)) {
+                            throw new IllegalStateException(
+                                    "Refused to remove user " + ePerson
+                                            .getID() + " from workflow group because the group " + group
+                                            .getID() + " has tasks assigned and no other members");
+                        }
+
+                    }
+                }
+            }
             if (!poolTasks.isEmpty()) {
                 List<EPerson> ePeople = allMembers(context, group);
                 if (ePeople.size() == 1 && ePeople.contains(ePerson)) {
                     throw new IllegalStateException(
                             "Refused to remove user " + ePerson
                                     .getID() + " from workflow group because the group " + group
-                                    .getID() + " has no other members");
+                                    .getID() + " has tasks assigned and no other members");
                 }
             }
         }

dspace-api/src/test/java/org/dspace/eperson/EPersonInWorkflowTest.java

@@ -49,7 +49,6 @@ import org.dspace.xmlworkflow.storedcomponents.service.CollectionRoleService;
 import org.dspace.xmlworkflow.storedcomponents.service.PoolTaskService;
 import org.dspace.xmlworkflow.storedcomponents.service.XmlWorkflowItemService;
 import org.junit.Before;
-import org.junit.Ignore;
 import org.junit.Test;
 import org.springframework.mock.web.MockHttpServletRequest;
 
@@ -127,23 +126,21 @@ public class EPersonInWorkflowTest extends AbstractIntegrationTestWithDatabase {
      *
      * This test will perform the following checks:
      * - create a workspace item, and let it move to step 1
-     * - approve it by user B
+     * - claim it by user B
-     * - verify that the item moved to step 2
-     * - Claim it by user C
-     * - delete user B
-     * - verify the delete is refused
-     * - remove user B from step 3
-     * - approve it by user C
-     * - verify that the item is archived without any actions apart from removing user B
      * - delete user B
      * - verify the delete is refused
      * - remove user B from step 1
+     * - verify that the removal is refused due to B being the last member in the workflow group and the group having
+     * a claimed item
+     * - approve it by user B and let it move to step 2
+     * - remove user B from step 3
+     * - approve it by user C
+     * - verify that the item is archived without any actions apart from removing user B
      * - delete user B
      * - verify the delete succeeds
      *
      * @throws Exception
      */
-    @Ignore
     @Test
     public void testDeleteUserWhenOnlyUserInGroup1() throws Exception {
         context.turnOffAuthorisationSystem();
@@ -169,6 +166,8 @@ public class EPersonInWorkflowTest extends AbstractIntegrationTestWithDatabase {
         httpServletRequest.setParameter("submit_approve", "submit_approve");
 
         executeWorkflowAction(httpServletRequest, workflowUserB, workflow, workflowItem, REVIEW_STEP, CLAIM_ACTION);
+        assertRemovalOfEpersonFromWorkflowGroup(workflowUserB, collection, REVIEW_ROLE, false);
+
         executeWorkflowAction(httpServletRequest, workflowUserB, workflow, workflowItem, REVIEW_STEP, REVIEW_ACTION);
 
         executeWorkflowAction(httpServletRequest, workflowUserC, workflow, workflowItem, EDIT_STEP, CLAIM_ACTION);
@@ -198,7 +197,8 @@ public class EPersonInWorkflowTest extends AbstractIntegrationTestWithDatabase {
      * - delete user B
      * - verify the delete is refused
      * - remove user B from step 1
-     * - verify that the removal is denied
+     * - verify that the removal is refused due to B being the last member in the workflow group and the group having a
+     * pool task
      * - approve it by user B and let it move to step 2
      * - remove user B from step 3
      * - delete user B
@@ -321,20 +321,21 @@ public class EPersonInWorkflowTest extends AbstractIntegrationTestWithDatabase {
      *
      * This test will perform the following checks:
      * - create a workspace item, and let it move to step 1
-     * - claim it by user B, but don’t approve it
+     * - approve it by user B, and let it move to step 2
+     * - approve it by user C, and let it move to step 3
+     * - claim it by user B
+     * - remove user B from step 1
      * - delete user B
      * - verify the delete is refused
-     * - remove user B from step 1
+     * - remove user B from step 3, verify that the removal is refused due to user B having a claimed task and there
-     * - remove user B from step 3
+     * being no other members in step 3
+     * - approve it by user B
      * - delete user B
-     * - verify the delete succeeds
+     * - verify the delete suceeds
-     * - verify that the item moved to step 2 without any actions apart from deleting user B
+     * - verify that the item is archived
-     * - Approve it by user C
-     * - verify that the item is archived without any actions apart from the approving in step 2
      *
      * @throws Exception
      */
-    @Ignore
     @Test
     public void testDeleteUserWhenOnlyUserInGroup4() throws Exception {
         context.turnOffAuthorisationSystem();
@@ -360,15 +361,20 @@ public class EPersonInWorkflowTest extends AbstractIntegrationTestWithDatabase {
         httpServletRequest.setParameter("submit_approve", "submit_approve");
 
         executeWorkflowAction(httpServletRequest, workflowUserB, workflow, workflowItem, REVIEW_STEP, CLAIM_ACTION);
-
+        executeWorkflowAction(httpServletRequest, workflowUserB, workflow, workflowItem, REVIEW_STEP, REVIEW_ACTION);
-        assertDeletionOfEperson(workflowUserB, false);
-        assertRemovalOfEpersonFromWorkflowGroup(workflowUserB, collection, REVIEW_ROLE, false);
-        assertRemovalOfEpersonFromWorkflowGroup(workflowUserB, collection, FINAL_EDIT_ROLE, false);
-        assertDeletionOfEperson(workflowUserB, true);
-
-
         executeWorkflowAction(httpServletRequest, workflowUserC, workflow, workflowItem, EDIT_STEP, CLAIM_ACTION);
         executeWorkflowAction(httpServletRequest, workflowUserC, workflow, workflowItem, EDIT_STEP, EDIT_ACTION);
+        executeWorkflowAction(httpServletRequest, workflowUserB, workflow, workflowItem, FINAL_EDIT_STEP, CLAIM_ACTION);
+
+        assertDeletionOfEperson(workflowUserB, false);
+        assertRemovalOfEpersonFromWorkflowGroup(workflowUserB, collection, REVIEW_ROLE, true);
+        assertRemovalOfEpersonFromWorkflowGroup(workflowUserB, collection, FINAL_EDIT_ROLE, false);
+
+        executeWorkflowAction(httpServletRequest, workflowUserB, workflow, workflowItem, FINAL_EDIT_STEP,
+                              FINAL_EDIT_ACTION);
+
+        assertRemovalOfEpersonFromWorkflowGroup(workflowUserB, collection, FINAL_EDIT_ROLE, true);
+        assertDeletionOfEperson(workflowUserB, true);
 
         assertTrue(workflowItem.getItem().isArchived());
 
@@ -376,124 +382,75 @@ public class EPersonInWorkflowTest extends AbstractIntegrationTestWithDatabase {
 
     /**
      * This test has the following setup:
-     * - Step 1: user B
+     * - Collection A - Step 1: user B
-     * - Step 2: user C
+     * - Collection A - Step 2: user C
-     * - Step 3: user B
+     * - Collection A - Step 3: user B
      *
-     * - create a workspace item, and let it move to step 1
+     * - Collection B - Step 1: user B
-     * - Approve it by user B
+     *
-     * - verify that the item moved to step 2
+     * This test will perform the following checks:
-     * - claim it by user C, but don’t approve it
+     * - create a workspace item in Collection A, and let it move to step 1
-     * - delete user C
+     * - claim it by user B
+     * - delete user B
      * - verify the delete is refused
-     * - remove user C from step 2
+     * - remove user B from Col A - step 3
-     * - delete user C
+     * - remove user B from Col B - step 1
-     * - verify the delete succeeds
+     * - remove user B from Col A - step 1
-     * - verify that the item moved to step 3 without any actions apart from deleting user C
+     * - Verify that the removal from Col A - step 1 is refused because user B has a claimed task in that collection and
-     * - Approve it by user B
+     * no other user is present
+     * - approve it by user B, and let it move to step 2
+     * - remove user B from Col A - step 1
+     * - verify it succeeds
+     * - delete user B
+     * - verify it succeeds
+     * - approve it by user C
      * - verify that the item is archived
      *
      * @throws Exception
      */
-    @Ignore
     @Test
     public void testDeleteUserWhenOnlyUserInGroup5() throws Exception {
         context.turnOffAuthorisationSystem();
 
         Community parent = CommunityBuilder.createCommunity(context).build();
-        Collection collection = CollectionBuilder.createCollection(context, parent)
+        Collection collectionA = CollectionBuilder.createCollection(context, parent)
-                                                 .withWorkflowGroup(1, workflowUserB)
+                                                  .withWorkflowGroup(1, workflowUserB)
-                                                 .withWorkflowGroup(2, workflowUserC)
+                                                  .withWorkflowGroup(2, workflowUserC)
-                                                 .withWorkflowGroup(3, workflowUserB)
+                                                  .withWorkflowGroup(3, workflowUserB)
-                                                 .build();
+                                                  .build();
 
-        WorkspaceItem wsi = WorkspaceItemBuilder.createWorkspaceItem(context, collection)
+        Collection collectionB = CollectionBuilder.createCollection(context, parent)
+                                                  .withWorkflowGroup(1, workflowUserB)
+                                                  .build();
+
+        WorkspaceItem wsi = WorkspaceItemBuilder.createWorkspaceItem(context, collectionA)
                                                 .withSubmitter(workflowUserA)
                                                 .withTitle("Test item full workflow")
                                                 .withIssueDate("2019-03-06")
                                                 .withSubject("ExtraEntry")
                                                 .build();
 
-        Workflow workflow = XmlWorkflowServiceFactory.getInstance().getWorkflowFactory().getWorkflow(collection);
+        Workflow workflow = XmlWorkflowServiceFactory.getInstance().getWorkflowFactory().getWorkflow(collectionA);
 
         XmlWorkflowItem workflowItem = xmlWorkflowService.startWithoutNotify(context, wsi);
         MockHttpServletRequest httpServletRequest = new MockHttpServletRequest();
         httpServletRequest.setParameter("submit_approve", "submit_approve");
 
         executeWorkflowAction(httpServletRequest, workflowUserB, workflow, workflowItem, REVIEW_STEP, CLAIM_ACTION);
+
+        assertRemovalOfEpersonFromWorkflowGroup(workflowUserB, collectionA, FINAL_EDIT_ROLE, true);
+        assertRemovalOfEpersonFromWorkflowGroup(workflowUserB, collectionB, REVIEW_ROLE, true);
+        assertRemovalOfEpersonFromWorkflowGroup(workflowUserB, collectionA, REVIEW_ROLE, false);
+
         executeWorkflowAction(httpServletRequest, workflowUserB, workflow, workflowItem, REVIEW_STEP, REVIEW_ACTION);
 
-        executeWorkflowAction(httpServletRequest, workflowUserC, workflow, workflowItem, EDIT_STEP, CLAIM_ACTION);
+        assertRemovalOfEpersonFromWorkflowGroup(workflowUserB, collectionA, REVIEW_ROLE, true);
-        assertDeletionOfEperson(workflowUserC, false);
+        assertDeletionOfEperson(workflowUserB, true);
-        assertRemovalOfEpersonFromWorkflowGroup(workflowUserC, collection, EDIT_ROLE, false);
-        assertDeletionOfEperson(workflowUserC, true);
 
-        executeWorkflowAction(httpServletRequest, workflowUserB, workflow, workflowItem, FINAL_EDIT_STEP, CLAIM_ACTION);
-        executeWorkflowAction(httpServletRequest, workflowUserB, workflow, workflowItem, FINAL_EDIT_STEP,
-                              FINAL_EDIT_ACTION);
-
-        assertTrue(workflowItem.getItem().isArchived());
-
-    }
-
-    /**
-     * This test has the following setup:
-     * - Step 1: user B
-     * - Step 2: user C
-     * - Step 3: user B
-     *
-     * - create a workspace item, and let it move to step 1
-     * - Approve it by user B
-     * - verify that the item moved to step 2
-     * - claim it by user C, but don’t approve it
-     * - delete user C
-     * - verify the delete is refused
-     * - remove user C from step 2
-     * - delete user C
-     * - verify the delete succeeds
-     * - verify that the item moved to step 3 without any actions apart from deleting user C
-     * - Approve it by user B
-     * - verify that the item is archived
-     *
-     * @throws Exception
-     */
-    @Ignore
-    @Test
-    public void testDeleteUserWhenOnlyUserInGroup6() throws Exception {
-        context.turnOffAuthorisationSystem();
-
-        Community parent = CommunityBuilder.createCommunity(context).build();
-        Collection collection = CollectionBuilder.createCollection(context, parent)
-                                                 .withWorkflowGroup(1, workflowUserB)
-                                                 .withWorkflowGroup(2, workflowUserC)
-                                                 .withWorkflowGroup(3, workflowUserB)
-                                                 .build();
-
-        WorkspaceItem wsi = WorkspaceItemBuilder.createWorkspaceItem(context, collection)
-                                                .withSubmitter(workflowUserA)
-                                                .withTitle("Test item full workflow")
-                                                .withIssueDate("2019-03-06")
-                                                .withSubject("ExtraEntry")
-                                                .build();
-
-        Workflow workflow = XmlWorkflowServiceFactory.getInstance().getWorkflowFactory().getWorkflow(collection);
-
-        XmlWorkflowItem workflowItem = xmlWorkflowService.startWithoutNotify(context, wsi);
-        MockHttpServletRequest httpServletRequest = new MockHttpServletRequest();
-        httpServletRequest.setParameter("submit_approve", "submit_approve");
-
-        executeWorkflowAction(httpServletRequest, workflowUserB, workflow, workflowItem, REVIEW_STEP, CLAIM_ACTION);
-        executeWorkflowAction(httpServletRequest, workflowUserB, workflow, workflowItem, REVIEW_STEP, REVIEW_ACTION);
 
         executeWorkflowAction(httpServletRequest, workflowUserC, workflow, workflowItem, EDIT_STEP, CLAIM_ACTION);
         executeWorkflowAction(httpServletRequest, workflowUserC, workflow, workflowItem, EDIT_STEP, EDIT_ACTION);
 
-        executeWorkflowAction(httpServletRequest, workflowUserB, workflow, workflowItem, FINAL_EDIT_STEP, CLAIM_ACTION);
-        assertDeletionOfEperson(workflowUserB, false);
-        assertRemovalOfEpersonFromWorkflowGroup(workflowUserB, collection, REVIEW_ROLE, false);
-        assertRemovalOfEpersonFromWorkflowGroup(workflowUserB, collection, FINAL_EDIT_ROLE, false);
-        assertDeletionOfEperson(workflowUserB, true);
-
         assertTrue(workflowItem.getItem().isArchived());
 
     }
@@ -520,7 +477,7 @@ public class EPersonInWorkflowTest extends AbstractIntegrationTestWithDatabase {
      * @throws Exception
      */
     @Test
-    public void testDeleteUserWhenOnlyUserInGroup7() throws Exception {
+    public void testDeleteUserWhenOnlyUserInGroup6() throws Exception {
         context.turnOffAuthorisationSystem();
 
         Community parent = CommunityBuilder.createCommunity(context).build();
@@ -584,7 +541,7 @@ public class EPersonInWorkflowTest extends AbstractIntegrationTestWithDatabase {
      * @throws Exception
      */
     @Test
-    public void testDeleteUserWhenOnlyUserInGroup8() throws Exception {
+    public void testDeleteUserWhenOnlyUserInGroup7() throws Exception {
         context.turnOffAuthorisationSystem();
 
         Community parent = CommunityBuilder.createCommunity(context).build();