Fix bug in logic for determining whether a workflow group will be lef…

…t empty. Need to check *both* EPerson and subgroup counts.
DSpace · Oct 3, 2023 · fe509ac · fe509ac
1 parent 279318a
commit fe509ac
Showing 1 changed file with 20 additions and 11 deletions.
diff --git a/dspace-api/src/main/java/org/dspace/eperson/GroupServiceImpl.java b/dspace-api/src/main/java/org/dspace/eperson/GroupServiceImpl.java
@@ -179,10 +179,13 @@ public void removeMember(Context context, Group group, EPerson ePerson) throws S
                 for (CollectionRole collectionRole : collectionRoles) {
                     if (StringUtils.equals(collectionRole.getRoleId(), role.getId())
                             && claimedTask.getWorkflowItem().getCollection() == collectionRole.getCollection()) {
-                        //  Get total number of unique EPerson objs who are a member of this group (or subgroup)
-                        int totalMembers = countAllMembers(context, group);
-                        // If only one EPerson is a member, then we cannot delete the last member of this group.
-                        if (totalMembers == 1) {
+                        // Count number of EPersons who are *direct* members of this group
+                        int totalDirectEPersons = ePersonService.countByGroups(context, Set.of(group));
+                        // Count number of Groups which have this groupParent as a direct parent
+                        int totalChildGroups = countByParent(context, group);
+                        // If this group has only one direct EPerson and *zero* child groups, then we cannot delete the
+                        // EPerson or we will leave this group empty.
+                        if (totalDirectEPersons == 1 && totalChildGroups == 0) {
                             throw new IllegalStateException(
                                     "Refused to remove user " + ePerson
                                             .getID() + " from workflow group because the group " + group
@@ -193,10 +196,13 @@ public void removeMember(Context context, Group group, EPerson ePerson) throws S
                 }
             }
             if (!poolTasks.isEmpty()) {
-                //  Get total number of unique EPerson objs who are a member of this group (or subgroup)
-                int totalMembers = countAllMembers(context, group);
-                // If only one EPerson is a member, then we cannot delete the last member of this group.
-                if (totalMembers == 1) {
+                // Count number of EPersons who are *direct* members of this group
+                int totalDirectEPersons = ePersonService.countByGroups(context, Set.of(group));
+                // Count number of Groups which have this groupParent as a direct parent
+                int totalChildGroups = countByParent(context, group);
+                // If this group has only one direct EPerson and *zero* child groups, then we cannot delete the
+                // EPerson or we will leave this group empty.
+                if (totalDirectEPersons == 1 && totalChildGroups == 0) {
                     throw new IllegalStateException(
                             "Refused to remove user " + ePerson
                                     .getID() + " from workflow group because the group " + group
@@ -217,9 +223,12 @@ public void removeMember(Context context, Group groupParent, Group childGroup) t
             List<PoolTask> poolTasks = poolTaskService.findByGroup(context, groupParent);
             if (!poolTasks.isEmpty()) {
                 // Count number of Groups which have this groupParent as a direct parent
-                int totalChildren = countByParent(context, groupParent);
-                // If only one group has this as a parent, we cannot delete the last child group
-                if (totalChildren == 1) {
+                int totalChildGroups = countByParent(context, groupParent);
+                // Count number of EPersons who are *direct* members of this group
+                int totalDirectEPersons = ePersonService.countByGroups(context, Set.of(groupParent));
+                // If this group has only one childGroup and *zero* direct EPersons, then we cannot delete the
+                // childGroup or we will leave this group empty.
+                if (totalChildGroups == 1 && totalDirectEPersons == 0) {
                     throw new IllegalStateException(
                             "Refused to remove sub group " + childGroup
                                     .getID() + " from workflow group because the group " + groupParent