From 0da7c15f2eff6229caafccae1be8dd7b10ebc629 Mon Sep 17 00:00:00 2001
From: Tim Donohue <tim.donohue@lyrasis.org>
Date: Wed, 20 Sep 2023 14:28:08 -0500
Subject: [PATCH 01/11] Limit getMembers() and getSubgroups() to only fetching
 one object. These lists are only used to find the size of each

---
 .../group-registry/groups-registry.component.ts    | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/app/access-control/group-registry/groups-registry.component.ts b/src/app/access-control/group-registry/groups-registry.component.ts
index ccfd155e393..06a048ad72d 100644
--- a/src/app/access-control/group-registry/groups-registry.component.ts
+++ b/src/app/access-control/group-registry/groups-registry.component.ts
@@ -216,18 +216,28 @@ export class GroupsRegistryComponent implements OnInit, OnDestroy {
 
   /**
    * Get the members (epersons embedded value of a group)
+   * NOTE: At this time we only grab the *first* member in order to receive the `totalElements` value
+   * needed for our HTML template.
    * @param group
    */
   getMembers(group: Group): Observable<RemoteData<PaginatedList<EPerson>>> {
-    return this.ePersonDataService.findListByHref(group._links.epersons.href).pipe(getFirstSucceededRemoteData());
+    return this.ePersonDataService.findListByHref(group._links.epersons.href, {
+        currentPage: 1,
+        elementsPerPage: 1,
+      }).pipe(getFirstSucceededRemoteData());
   }
 
   /**
    * Get the subgroups (groups embedded value of a group)
+   * NOTE: At this time we only grab the *first* subgroup in order to receive the `totalElements` value
+   * needed for our HTML template.
    * @param group
    */
   getSubgroups(group: Group): Observable<RemoteData<PaginatedList<Group>>> {
-    return this.groupService.findListByHref(group._links.subgroups.href).pipe(getFirstSucceededRemoteData());
+    return this.groupService.findListByHref(group._links.subgroups.href, {
+        currentPage: 1,
+        elementsPerPage: 1,
+      }).pipe(getFirstSucceededRemoteData());
   }
 
   /**

From 97479a29453eaf18031e095b612c1c054f9bb31f Mon Sep 17 00:00:00 2001
From: Tim Donohue <tim.donohue@lyrasis.org>
Date: Wed, 20 Sep 2023 14:30:54 -0500
Subject: [PATCH 02/11] Remove isSubgroupOfGroup() functionality as it loads
 every subgroup at once. Bad peformance for large groups

---
 .../subgroups-list.component.html             |  9 +----
 .../subgroup-list/subgroups-list.component.ts | 33 ++-----------------
 2 files changed, 3 insertions(+), 39 deletions(-)

diff --git a/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.html b/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.html
index d009f0283eb..8268eb5eb45 100644
--- a/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.html
+++ b/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.html
@@ -62,16 +62,9 @@ <h4 id="search" class="border-bottom pb-2">
           <td class="align-middle">{{ dsoNameService.getName((group.object | async)?.payload) }}</td>
           <td class="align-middle">
             <div class="btn-group edit-field">
-              <button *ngIf="(isSubgroupOfGroup(group) | async) && !(isActiveGroup(group) | async)"
-                      (click)="deleteSubgroupFromGroup(group)"
-                      class="btn btn-outline-danger btn-sm deleteButton"
-                      title="{{messagePrefix + '.table.edit.buttons.remove' | translate: { name: dsoNameService.getName(group) } }}">
-                <i class="fas fa-trash-alt fa-fw"></i>
-              </button>
-
               <span *ngIf="(isActiveGroup(group) | async)">{{ messagePrefix + '.table.edit.currentGroup' | translate }}</span>
 
-              <button *ngIf="!(isSubgroupOfGroup(group) | async) && !(isActiveGroup(group) | async)"
+              <button *ngIf="!(isActiveGroup(group) | async)"
                       (click)="addSubgroupToGroup(group)"
                       class="btn btn-outline-primary btn-sm addButton"
                       title="{{messagePrefix + '.table.edit.buttons.add' | translate: { name: dsoNameService.getName(group) } }}">
diff --git a/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.ts b/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.ts
index 0cff730c628..83df0d8cdde 100644
--- a/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.ts
+++ b/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.ts
@@ -3,15 +3,13 @@ import { UntypedFormBuilder } from '@angular/forms';
 import { Router } from '@angular/router';
 import { TranslateService } from '@ngx-translate/core';
 import { BehaviorSubject, Observable, of as observableOf, Subscription } from 'rxjs';
-import { map, mergeMap, switchMap, take } from 'rxjs/operators';
+import { mergeMap, switchMap, take } from 'rxjs/operators';
 import { PaginatedList } from '../../../../core/data/paginated-list.model';
 import { RemoteData } from '../../../../core/data/remote-data';
 import { GroupDataService } from '../../../../core/eperson/group-data.service';
 import { Group } from '../../../../core/eperson/models/group.model';
 import {
-  getFirstCompletedRemoteData,
-  getFirstSucceededRemoteData,
-  getRemoteDataPayload
+  getFirstCompletedRemoteData
 } from '../../../../core/shared/operators';
 import { NotificationsService } from '../../../../shared/notifications/notifications.service';
 import { PaginationComponentOptions } from '../../../../shared/pagination/pagination-component-options.model';
@@ -131,33 +129,6 @@ export class SubgroupsListComponent implements OnInit, OnDestroy {
       }));
   }
 
-  /**
-   * Whether or not the given group is a subgroup of the group currently being edited
-   * @param possibleSubgroup Group that is a possible subgroup (being tested) of the group currently being edited
-   */
-  isSubgroupOfGroup(possibleSubgroup: Group): Observable<boolean> {
-    return this.groupDataService.getActiveGroup().pipe(take(1),
-      mergeMap((activeGroup: Group) => {
-        if (activeGroup != null) {
-          if (activeGroup.uuid === possibleSubgroup.uuid) {
-            return observableOf(false);
-          } else {
-            return this.groupDataService.findListByHref(activeGroup._links.subgroups.href, {
-              currentPage: 1,
-              elementsPerPage: 9999
-            })
-              .pipe(
-                getFirstSucceededRemoteData(),
-                getRemoteDataPayload(),
-                map((listTotalGroups: PaginatedList<Group>) => listTotalGroups.page.filter((groupInList: Group) => groupInList.id === possibleSubgroup.id)),
-                map((groups: Group[]) => groups.length > 0));
-          }
-        } else {
-          return observableOf(false);
-        }
-      }));
-  }
-
   /**
    * Whether or not the given group is the current group being edited
    * @param group Group that is possibly the current group being edited

From 229236634a06f5468dc078eb5d814981ecf3d497 Mon Sep 17 00:00:00 2001
From: Tim Donohue <tim.donohue@lyrasis.org>
Date: Wed, 20 Sep 2023 15:17:34 -0500
Subject: [PATCH 03/11] Fix bug where linked Community/Collection info was
 sometimes listed many times in a row

---
 .../group-form/group-form.component.ts             | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/src/app/access-control/group-registry/group-form/group-form.component.ts b/src/app/access-control/group-registry/group-form/group-form.component.ts
index 0758a455f53..925a8bb8593 100644
--- a/src/app/access-control/group-registry/group-form/group-form.component.ts
+++ b/src/app/access-control/group-registry/group-form/group-form.component.ts
@@ -230,12 +230,14 @@ export class GroupFormComponent implements OnInit, OnDestroy {
             this.groupBeingEdited = activeGroup;
 
             if (linkedObject?.name) {
-              this.formBuilderService.insertFormGroupControl(1, this.formGroup, this.formModel, this.groupCommunity);
-              this.formGroup.patchValue({
-                groupName: activeGroup.name,
-                groupCommunity: linkedObject?.name ?? '',
-                groupDescription: activeGroup.firstMetadataValue('dc.description'),
-              });
+              if (!this.formGroup.controls.groupCommunity) {
+                this.formBuilderService.insertFormGroupControl(1, this.formGroup, this.formModel, this.groupCommunity);
+                this.formGroup.patchValue({
+                  groupName: activeGroup.name,
+                  groupCommunity: linkedObject?.name ?? '',
+                  groupDescription: activeGroup.firstMetadataValue('dc.description'),
+                });
+              }
             } else {
               this.formModel = [
                 this.groupName,

From 43d37196fbaf45648dbd5e604a3ee8d5df2a129e Mon Sep 17 00:00:00 2001
From: Tim Donohue <tim.donohue@lyrasis.org>
Date: Fri, 22 Sep 2023 12:13:39 -0500
Subject: [PATCH 04/11] Remove "isMemberOfGroup()" from members-list component.

---
 .../members-list/members-list.component.html  | 21 +------
 .../members-list/members-list.component.ts    | 56 +++----------------
 2 files changed, 11 insertions(+), 66 deletions(-)

diff --git a/src/app/access-control/group-registry/group-form/members-list/members-list.component.html b/src/app/access-control/group-registry/group-form/members-list/members-list.component.html
index cc9bf34d644..b90a3e405b1 100644
--- a/src/app/access-control/group-registry/group-form/members-list/members-list.component.html
+++ b/src/app/access-control/group-registry/group-form/members-list/members-list.component.html
@@ -69,16 +69,7 @@ <h4 id="search" class="border-bottom pb-2">
           </td>
           <td class="align-middle">
             <div class="btn-group edit-field">
-              <button *ngIf="ePerson.memberOfGroup"
-                      (click)="deleteMemberFromGroup(ePerson)"
-                      [disabled]="actionConfig.remove.disabled"
-                      [ngClass]="['btn btn-sm', actionConfig.remove.css]"
-                      title="{{messagePrefix + '.table.edit.buttons.remove' | translate: { name: dsoNameService.getName(ePerson.eperson) } }}">
-                <i [ngClass]="actionConfig.remove.icon"></i>
-              </button>
-
-              <button *ngIf="!ePerson.memberOfGroup"
-                      (click)="addMemberToGroup(ePerson)"
+              <button (click)="addMemberToGroup(ePerson)"
                       [disabled]="actionConfig.add.disabled"
                       [ngClass]="['btn btn-sm', actionConfig.add.css]"
                       title="{{messagePrefix + '.table.edit.buttons.add' | translate: { name: dsoNameService.getName(ePerson.eperson) } }}">
@@ -133,20 +124,12 @@ <h4>{{messagePrefix + '.headMembers' | translate}}</h4>
           </td>
           <td class="align-middle">
             <div class="btn-group edit-field">
-              <button *ngIf="ePerson.memberOfGroup"
-                      (click)="deleteMemberFromGroup(ePerson)"
+              <button (click)="deleteMemberFromGroup(ePerson)"
                       [disabled]="actionConfig.remove.disabled"
                       [ngClass]="['btn btn-sm', actionConfig.remove.css]"
                       title="{{messagePrefix + '.table.edit.buttons.remove' | translate: { name: dsoNameService.getName(ePerson.eperson) } }}">
                 <i [ngClass]="actionConfig.remove.icon"></i>
               </button>
-              <button *ngIf="!ePerson.memberOfGroup"
-                      (click)="addMemberToGroup(ePerson)"
-                      [disabled]="actionConfig.add.disabled"
-                      [ngClass]="['btn btn-sm', actionConfig.add.css]"
-                      title="{{messagePrefix + '.table.edit.buttons.add' | translate: { name: dsoNameService.getName(ePerson.eperson) } }}">
-                <i [ngClass]="actionConfig.add.icon"></i>
-              </button>
             </div>
           </td>
         </tr>
diff --git a/src/app/access-control/group-registry/group-form/members-list/members-list.component.ts b/src/app/access-control/group-registry/group-form/members-list/members-list.component.ts
index b3e686c0123..704a92d0f9f 100644
--- a/src/app/access-control/group-registry/group-form/members-list/members-list.component.ts
+++ b/src/app/access-control/group-registry/group-form/members-list/members-list.component.ts
@@ -7,10 +7,9 @@ import {
   of as observableOf,
   Subscription,
   BehaviorSubject,
-  combineLatest as observableCombineLatest,
-  ObservedValueOf,
+  combineLatest as observableCombineLatest
 } from 'rxjs';
-import { defaultIfEmpty, map, mergeMap, switchMap, take } from 'rxjs/operators';
+import { defaultIfEmpty, map, switchMap, take } from 'rxjs/operators';
 import { buildPaginatedList, PaginatedList } from '../../../../core/data/paginated-list.model';
 import { RemoteData } from '../../../../core/data/remote-data';
 import { EPersonDataService } from '../../../../core/eperson/eperson-data.service';
@@ -18,10 +17,8 @@ import { GroupDataService } from '../../../../core/eperson/group-data.service';
 import { EPerson } from '../../../../core/eperson/models/eperson.model';
 import { Group } from '../../../../core/eperson/models/group.model';
 import {
-  getFirstSucceededRemoteData,
   getFirstCompletedRemoteData,
-  getAllCompletedRemoteData,
-  getRemoteDataPayload
+  getAllCompletedRemoteData
 } from '../../../../core/shared/operators';
 import { NotificationsService } from '../../../../shared/notifications/notifications.service';
 import { PaginationComponentOptions } from '../../../../shared/pagination/pagination-component-options.model';
@@ -191,14 +188,9 @@ export class MembersListComponent implements OnInit, OnDestroy {
         }),
         switchMap((epersonListRD: RemoteData<PaginatedList<EPerson>>) => {
           const dtos$ = observableCombineLatest([...epersonListRD.payload.page.map((member: EPerson) => {
-            const dto$: Observable<EpersonDtoModel> = observableCombineLatest(
-              this.isMemberOfGroup(member), (isMember: ObservedValueOf<Observable<boolean>>) => {
-                const epersonDtoModel: EpersonDtoModel = new EpersonDtoModel();
-                epersonDtoModel.eperson = member;
-                epersonDtoModel.memberOfGroup = isMember;
-                return epersonDtoModel;
-              });
-            return dto$;
+            const epersonDtoModel: EpersonDtoModel = new EpersonDtoModel();
+            epersonDtoModel.eperson = member;
+            return observableOf(epersonDtoModel);
           })]);
           return dtos$.pipe(defaultIfEmpty([]), map((dtos: EpersonDtoModel[]) => {
             return buildPaginatedList(epersonListRD.payload.pageInfo, dtos);
@@ -209,29 +201,6 @@ export class MembersListComponent implements OnInit, OnDestroy {
         }));
   }
 
-  /**
-   * Whether the given ePerson is a member of the group currently being edited
-   * @param possibleMember  EPerson that is a possible member (being tested) of the group currently being edited
-   */
-  isMemberOfGroup(possibleMember: EPerson): Observable<boolean> {
-    return this.groupDataService.getActiveGroup().pipe(take(1),
-      mergeMap((group: Group) => {
-        if (group != null) {
-          return this.ePersonDataService.findListByHref(group._links.epersons.href, {
-            currentPage: 1,
-            elementsPerPage: 9999
-          })
-            .pipe(
-              getFirstSucceededRemoteData(),
-              getRemoteDataPayload(),
-              map((listEPeopleInGroup: PaginatedList<EPerson>) => listEPeopleInGroup.page.filter((ePersonInList: EPerson) => ePersonInList.id === possibleMember.id)),
-              map((epeople: EPerson[]) => epeople.length > 0));
-        } else {
-          return observableOf(false);
-        }
-      }));
-  }
-
   /**
    * Unsubscribe from a subscription if it's still subscribed, and remove it from the map of
    * active subscriptions
@@ -251,7 +220,6 @@ export class MembersListComponent implements OnInit, OnDestroy {
    * @param ePerson   EPerson we want to delete as member from group that is currently being edited
    */
   deleteMemberFromGroup(ePerson: EpersonDtoModel) {
-    ePerson.memberOfGroup = false;
     this.groupDataService.getActiveGroup().pipe(take(1)).subscribe((activeGroup: Group) => {
       if (activeGroup != null) {
         const response = this.groupDataService.deleteMemberFromGroup(activeGroup, ePerson.eperson);
@@ -267,7 +235,6 @@ export class MembersListComponent implements OnInit, OnDestroy {
    * @param ePerson   EPerson we want to add as member to group that is currently being edited
    */
   addMemberToGroup(ePerson: EpersonDtoModel) {
-    ePerson.memberOfGroup = true;
     this.groupDataService.getActiveGroup().pipe(take(1)).subscribe((activeGroup: Group) => {
       if (activeGroup != null) {
         const response = this.groupDataService.addMemberToGroup(activeGroup, ePerson.eperson);
@@ -321,14 +288,9 @@ export class MembersListComponent implements OnInit, OnDestroy {
         }),
         switchMap((epersonListRD: RemoteData<PaginatedList<EPerson>>) => {
           const dtos$ = observableCombineLatest([...epersonListRD.payload.page.map((member: EPerson) => {
-            const dto$: Observable<EpersonDtoModel> = observableCombineLatest(
-              this.isMemberOfGroup(member), (isMember: ObservedValueOf<Observable<boolean>>) => {
-                const epersonDtoModel: EpersonDtoModel = new EpersonDtoModel();
-                epersonDtoModel.eperson = member;
-                epersonDtoModel.memberOfGroup = isMember;
-                return epersonDtoModel;
-              });
-            return dto$;
+            const epersonDtoModel: EpersonDtoModel = new EpersonDtoModel();
+            epersonDtoModel.eperson = member;
+            return observableOf(epersonDtoModel);
           })]);
           return dtos$.pipe(defaultIfEmpty([]), map((dtos: EpersonDtoModel[]) => {
             return buildPaginatedList(epersonListRD.payload.pageInfo, dtos);

From bffae54b10ea7a4c883cb25512f9c9ac4949f97a Mon Sep 17 00:00:00 2001
From: Tim Donohue <tim.donohue@lyrasis.org>
Date: Mon, 25 Sep 2023 16:34:04 -0500
Subject: [PATCH 05/11] Remove unnecessary EpersonDtoModel. Rework code and
 tests to use EPerson instead.

---
 .../members-list/members-list.component.html  |  48 +++---
 .../members-list.component.spec.ts            | 156 ++++++++----------
 .../members-list/members-list.component.ts    |  62 +++----
 3 files changed, 116 insertions(+), 150 deletions(-)

diff --git a/src/app/access-control/group-registry/group-form/members-list/members-list.component.html b/src/app/access-control/group-registry/group-form/members-list/members-list.component.html
index b90a3e405b1..0f5010e08eb 100644
--- a/src/app/access-control/group-registry/group-form/members-list/members-list.component.html
+++ b/src/app/access-control/group-registry/group-form/members-list/members-list.component.html
@@ -37,10 +37,10 @@ <h4 id="search" class="border-bottom pb-2">
     </div>
   </form>
 
-  <ds-pagination *ngIf="(ePeopleSearchDtos | async)?.totalElements > 0"
+  <ds-pagination *ngIf="(ePeopleSearch | async)?.totalElements > 0"
                  [paginationOptions]="configSearch"
-                 [pageInfoState]="(ePeopleSearchDtos | async)"
-                 [collectionSize]="(ePeopleSearchDtos | async)?.totalElements"
+                 [pageInfoState]="(ePeopleSearch | async)"
+                 [collectionSize]="(ePeopleSearch | async)?.totalElements"
                  [hideGear]="true"
                  [hidePagerWhenSinglePage]="true">
 
@@ -55,24 +55,24 @@ <h4 id="search" class="border-bottom pb-2">
         </tr>
         </thead>
         <tbody>
-        <tr *ngFor="let ePerson of (ePeopleSearchDtos | async)?.page">
-          <td class="align-middle">{{ePerson.eperson.id}}</td>
+        <tr *ngFor="let eperson of (ePeopleSearch | async)?.page">
+          <td class="align-middle">{{eperson.id}}</td>
           <td class="align-middle">
-            <a (click)="ePersonDataService.startEditingNewEPerson(ePerson.eperson)"
+            <a (click)="ePersonDataService.startEditingNewEPerson(eperson)"
                [routerLink]="[ePersonDataService.getEPeoplePageRouterLink()]">
-              {{ dsoNameService.getName(ePerson.eperson) }}
+              {{ dsoNameService.getName(eperson) }}
             </a>
           </td>
           <td class="align-middle">
-            {{messagePrefix + '.table.email' | translate}}: {{ ePerson.eperson.email ? ePerson.eperson.email : '-' }}<br/>
-            {{messagePrefix + '.table.netid' | translate}}: {{ ePerson.eperson.netid ? ePerson.eperson.netid : '-' }}
+            {{messagePrefix + '.table.email' | translate}}: {{ eperson.email ? eperson.email : '-' }}<br/>
+            {{messagePrefix + '.table.netid' | translate}}: {{ eperson.netid ? eperson.netid : '-' }}
           </td>
           <td class="align-middle">
             <div class="btn-group edit-field">
-              <button (click)="addMemberToGroup(ePerson)"
+              <button (click)="addMemberToGroup(eperson)"
                       [disabled]="actionConfig.add.disabled"
                       [ngClass]="['btn btn-sm', actionConfig.add.css]"
-                      title="{{messagePrefix + '.table.edit.buttons.add' | translate: { name: dsoNameService.getName(ePerson.eperson) } }}">
+                      title="{{messagePrefix + '.table.edit.buttons.add' | translate: { name: dsoNameService.getName(eperson) } }}">
                 <i [ngClass]="actionConfig.add.icon"></i>
               </button>
             </div>
@@ -84,7 +84,7 @@ <h4 id="search" class="border-bottom pb-2">
 
   </ds-pagination>
 
-  <div *ngIf="(ePeopleSearchDtos | async)?.totalElements == 0 && searchDone"
+  <div *ngIf="(ePeopleSearch | async)?.totalElements == 0 && searchDone"
        class="alert alert-info w-100 mb-2"
        role="alert">
     {{messagePrefix + '.no-items' | translate}}
@@ -92,10 +92,10 @@ <h4 id="search" class="border-bottom pb-2">
 
   <h4>{{messagePrefix + '.headMembers' | translate}}</h4>
 
-  <ds-pagination *ngIf="(ePeopleMembersOfGroupDtos | async)?.totalElements > 0"
+  <ds-pagination *ngIf="(ePeopleMembersOfGroup | async)?.totalElements > 0"
                  [paginationOptions]="config"
-                 [pageInfoState]="(ePeopleMembersOfGroupDtos | async)"
-                 [collectionSize]="(ePeopleMembersOfGroupDtos | async)?.totalElements"
+                 [pageInfoState]="(ePeopleMembersOfGroup | async)"
+                 [collectionSize]="(ePeopleMembersOfGroup | async)?.totalElements"
                  [hideGear]="true"
                  [hidePagerWhenSinglePage]="true">
 
@@ -110,24 +110,24 @@ <h4>{{messagePrefix + '.headMembers' | translate}}</h4>
         </tr>
         </thead>
         <tbody>
-        <tr *ngFor="let ePerson of (ePeopleMembersOfGroupDtos | async)?.page">
-          <td class="align-middle">{{ePerson.eperson.id}}</td>
+        <tr *ngFor="let eperson of (ePeopleMembersOfGroup | async)?.page">
+          <td class="align-middle">{{eperson.id}}</td>
           <td class="align-middle">
-            <a (click)="ePersonDataService.startEditingNewEPerson(ePerson.eperson)"
+            <a (click)="ePersonDataService.startEditingNewEPerson(eperson)"
                [routerLink]="[ePersonDataService.getEPeoplePageRouterLink()]">
-              {{ dsoNameService.getName(ePerson.eperson) }}
+              {{ dsoNameService.getName(eperson) }}
             </a>
           </td>
           <td class="align-middle">
-            {{messagePrefix + '.table.email' | translate}}: {{ ePerson.eperson.email ? ePerson.eperson.email : '-' }}<br/>
-            {{messagePrefix + '.table.netid' | translate}}: {{ ePerson.eperson.netid ? ePerson.eperson.netid : '-' }}
+            {{messagePrefix + '.table.email' | translate}}: {{ eperson.email ? eperson.email : '-' }}<br/>
+            {{messagePrefix + '.table.netid' | translate}}: {{ eperson.netid ? eperson.netid : '-' }}
           </td>
           <td class="align-middle">
             <div class="btn-group edit-field">
-              <button (click)="deleteMemberFromGroup(ePerson)"
+              <button (click)="deleteMemberFromGroup(eperson)"
                       [disabled]="actionConfig.remove.disabled"
                       [ngClass]="['btn btn-sm', actionConfig.remove.css]"
-                      title="{{messagePrefix + '.table.edit.buttons.remove' | translate: { name: dsoNameService.getName(ePerson.eperson) } }}">
+                      title="{{messagePrefix + '.table.edit.buttons.remove' | translate: { name: dsoNameService.getName(eperson) } }}">
                 <i [ngClass]="actionConfig.remove.icon"></i>
               </button>
             </div>
@@ -139,7 +139,7 @@ <h4>{{messagePrefix + '.headMembers' | translate}}</h4>
 
   </ds-pagination>
 
-  <div *ngIf="(ePeopleMembersOfGroupDtos | async) == undefined || (ePeopleMembersOfGroupDtos | async)?.totalElements == 0" class="alert alert-info w-100 mb-2"
+  <div *ngIf="(ePeopleMembersOfGroup | async) == undefined || (ePeopleMembersOfGroup | async)?.totalElements == 0" class="alert alert-info w-100 mb-2"
        role="alert">
     {{messagePrefix + '.no-members-yet' | translate}}
   </div>
diff --git a/src/app/access-control/group-registry/group-form/members-list/members-list.component.spec.ts b/src/app/access-control/group-registry/group-form/members-list/members-list.component.spec.ts
index 7c8db399bcd..76e38067bc8 100644
--- a/src/app/access-control/group-registry/group-form/members-list/members-list.component.spec.ts
+++ b/src/app/access-control/group-registry/group-form/members-list/members-list.component.spec.ts
@@ -17,7 +17,7 @@ import { Group } from '../../../../core/eperson/models/group.model';
 import { PageInfo } from '../../../../core/shared/page-info.model';
 import { FormBuilderService } from '../../../../shared/form/builder/form-builder.service';
 import { NotificationsService } from '../../../../shared/notifications/notifications.service';
-import { GroupMock, GroupMock2 } from '../../../../shared/testing/group-mock';
+import { GroupMock } from '../../../../shared/testing/group-mock';
 import { MembersListComponent } from './members-list.component';
 import { EPersonMock, EPersonMock2 } from '../../../../shared/testing/eperson.mock';
 import { createSuccessfulRemoteDataObject$ } from '../../../../shared/remote-data.utils';
@@ -39,28 +39,26 @@ describe('MembersListComponent', () => {
   let ePersonDataServiceStub: any;
   let groupsDataServiceStub: any;
   let activeGroup;
-  let allEPersons: EPerson[];
-  let allGroups: Group[];
   let epersonMembers: EPerson[];
-  let subgroupMembers: Group[];
+  let epersonNonMembers: EPerson[];
   let paginationService;
 
   beforeEach(waitForAsync(() => {
     activeGroup = GroupMock;
     epersonMembers = [EPersonMock2];
-    subgroupMembers = [GroupMock2];
-    allEPersons = [EPersonMock, EPersonMock2];
-    allGroups = [GroupMock, GroupMock2];
+    epersonNonMembers = [EPersonMock];
     ePersonDataServiceStub = {
       activeGroup: activeGroup,
       epersonMembers: epersonMembers,
-      subgroupMembers: subgroupMembers,
+      epersonNonMembers: epersonNonMembers,
+      // This method is used to get all the current members
       findListByHref(_href: string): Observable<RemoteData<PaginatedList<EPerson>>> {
         return createSuccessfulRemoteDataObject$(buildPaginatedList<EPerson>(new PageInfo(), groupsDataServiceStub.getEPersonMembers()));
       },
+      // This method is used to search across *non-members*
       searchByScope(scope: string, query: string): Observable<RemoteData<PaginatedList<EPerson>>> {
         if (query === '') {
-          return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), allEPersons));
+          return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), epersonNonMembers));
         }
         return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), []));
       },
@@ -77,22 +75,22 @@ describe('MembersListComponent', () => {
     groupsDataServiceStub = {
       activeGroup: activeGroup,
       epersonMembers: epersonMembers,
-      subgroupMembers: subgroupMembers,
-      allGroups: allGroups,
+      epersonNonMembers: epersonNonMembers,
       getActiveGroup(): Observable<Group> {
         return observableOf(activeGroup);
       },
       getEPersonMembers() {
         return this.epersonMembers;
       },
-      searchGroups(query: string): Observable<RemoteData<PaginatedList<Group>>> {
-        if (query === '') {
-          return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), this.allGroups));
-        }
-        return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), []));
-      },
-      addMemberToGroup(parentGroup, eperson: EPerson): Observable<RestResponse> {
-        this.epersonMembers = [...this.epersonMembers, eperson];
+      addMemberToGroup(parentGroup, epersonToAdd: EPerson): Observable<RestResponse> {
+        // Add eperson to list of members
+        this.epersonMembers = [...this.epersonMembers, epersonToAdd];
+        // Remove eperson from list of non-members
+        this.epersonNonMembers.forEach( (eperson: EPerson, index: number) => {
+          if (eperson.id === epersonToAdd.id) {
+            this.epersonNonMembers.splice(index, 1);
+          }
+        });
         return observableOf(new RestResponse(true, 200, 'Success'));
       },
       clearGroupsRequests() {
@@ -105,14 +103,14 @@ describe('MembersListComponent', () => {
         return '/access-control/groups/' + group.id;
       },
       deleteMemberFromGroup(parentGroup, epersonToDelete: EPerson): Observable<RestResponse> {
-        this.epersonMembers = this.epersonMembers.find((eperson: EPerson) => {
-          if (eperson.id !== epersonToDelete.id) {
-            return eperson;
+        // Remove eperson from list of members
+        this.epersonMembers.forEach( (eperson: EPerson, index: number) => {
+          if (eperson.id === epersonToDelete.id) {
+            this.epersonMembers.splice(index, 1);
           }
         });
-        if (this.epersonMembers === undefined) {
-          this.epersonMembers = [];
-        }
+        // Add eperson to list of non-members
+        this.epersonNonMembers = [...this.epersonNonMembers, epersonToDelete];
         return observableOf(new RestResponse(true, 200, 'Success'));
       }
     };
@@ -160,13 +158,37 @@ describe('MembersListComponent', () => {
     expect(comp).toBeDefined();
   }));
 
-  it('should show list of eperson members of current active group', () => {
-    const epersonIdsFound = fixture.debugElement.queryAll(By.css('#ePeopleMembersOfGroup tr td:first-child'));
-    expect(epersonIdsFound.length).toEqual(1);
-    epersonMembers.map((eperson: EPerson) => {
-      expect(epersonIdsFound.find((foundEl) => {
-        return (foundEl.nativeElement.textContent.trim() === eperson.uuid);
-      })).toBeTruthy();
+  describe('current members list', () => {
+    it('should show list of eperson members of current active group', () => {
+      const epersonIdsFound = fixture.debugElement.queryAll(By.css('#ePeopleMembersOfGroup tr td:first-child'));
+      expect(epersonIdsFound.length).toEqual(1);
+      epersonMembers.map((eperson: EPerson) => {
+        expect(epersonIdsFound.find((foundEl) => {
+          return (foundEl.nativeElement.textContent.trim() === eperson.uuid);
+        })).toBeTruthy();
+      });
+    });
+
+    it('should show a delete button next to each member', () => {
+      const epersonsFound = fixture.debugElement.queryAll(By.css('#ePeopleMembersOfGroup tbody tr'));
+      epersonsFound.map((foundEPersonRowElement: DebugElement) => {
+        const addButton: DebugElement = foundEPersonRowElement.query(By.css('td:last-child .fa-plus'));
+        const deleteButton: DebugElement = foundEPersonRowElement.query(By.css('td:last-child .fa-trash-alt'));
+        expect(addButton).toBeNull();
+        expect(deleteButton).not.toBeNull();
+      });
+    });
+
+    describe('if first delete button is pressed', () => {
+      beforeEach(() => {
+        const deleteButton: DebugElement = fixture.debugElement.query(By.css('#ePeopleMembersOfGroup tbody .fa-trash-alt'));
+        deleteButton.nativeElement.click();
+        fixture.detectChanges();
+      });
+      it('then no ePerson remains as a member of the active group.', () => {
+        const epersonsFound = fixture.debugElement.queryAll(By.css('#ePeopleMembersOfGroup tbody tr'));
+        expect(epersonsFound.length).toEqual(0);
+      });
     });
   });
 
@@ -174,76 +196,40 @@ describe('MembersListComponent', () => {
     describe('when searching without query', () => {
       let epersonsFound: DebugElement[];
       beforeEach(fakeAsync(() => {
-        spyOn(component, 'isMemberOfGroup').and.callFake((ePerson: EPerson) => {
-          return observableOf(activeGroup.epersons.includes(ePerson));
-        });
         component.search({ scope: 'metadata', query: '' });
         tick();
         fixture.detectChanges();
         epersonsFound = fixture.debugElement.queryAll(By.css('#epersonsSearch tbody tr'));
-        // Stop using the fake spy function (because otherwise the clicking on the buttons will not change anything
-        // because they don't change the value of activeGroup.epersons)
-        jasmine.getEnv().allowRespy(true);
-        spyOn(component, 'isMemberOfGroup').and.callThrough();
       }));
 
-      it('should display all epersons', () => {
-        expect(epersonsFound.length).toEqual(2);
+      it('should display only non-members of the group', () => {
+        const epersonIdsFound = fixture.debugElement.queryAll(By.css('#epersonsSearch tbody tr td:first-child'));
+        expect(epersonIdsFound.length).toEqual(1);
+        epersonNonMembers.map((eperson: EPerson) => {
+          expect(epersonIdsFound.find((foundEl) => {
+            return (foundEl.nativeElement.textContent.trim() === eperson.uuid);
+          })).toBeTruthy();
+        });
       });
 
-      describe('if eperson is already a eperson', () => {
-        it('should have delete button, else it should have add button', () => {
-          const memberIds: string[] = activeGroup.epersons.map((ePerson: EPerson) => ePerson.id);
-          epersonsFound.map((foundEPersonRowElement: DebugElement) => {
-            const epersonId: DebugElement = foundEPersonRowElement.query(By.css('td:first-child'));
-            const addButton: DebugElement = foundEPersonRowElement.query(By.css('td:last-child .fa-plus'));
-            const deleteButton: DebugElement = foundEPersonRowElement.query(By.css('td:last-child .fa-trash-alt'));
-            if (memberIds.includes(epersonId.nativeElement.textContent)) {
-              expect(addButton).toBeNull();
-              expect(deleteButton).not.toBeNull();
-            } else {
-              expect(deleteButton).toBeNull();
-              expect(addButton).not.toBeNull();
-            }
-          });
+      it('should display an add button next to non-members, not a delete button', () => {
+        epersonsFound.map((foundEPersonRowElement: DebugElement) => {
+          const addButton: DebugElement = foundEPersonRowElement.query(By.css('td:last-child .fa-plus'));
+          const deleteButton: DebugElement = foundEPersonRowElement.query(By.css('td:last-child .fa-trash-alt'));
+          expect(addButton).not.toBeNull();
+          expect(deleteButton).toBeNull();
         });
       });
 
       describe('if first add button is pressed', () => {
-        beforeEach(fakeAsync(() => {
+        beforeEach(() => {
           const addButton: DebugElement = fixture.debugElement.query(By.css('#epersonsSearch tbody .fa-plus'));
           addButton.nativeElement.click();
-          tick();
           fixture.detectChanges();
-        }));
-        it('then all the ePersons are member of the active group', () => {
-          epersonsFound = fixture.debugElement.queryAll(By.css('#epersonsSearch tbody tr'));
-          expect(epersonsFound.length).toEqual(2);
-          epersonsFound.map((foundEPersonRowElement: DebugElement) => {
-            const addButton: DebugElement = foundEPersonRowElement.query(By.css('td:last-child .fa-plus'));
-            const deleteButton: DebugElement = foundEPersonRowElement.query(By.css('td:last-child .fa-trash-alt'));
-            expect(addButton).toBeNull();
-            expect(deleteButton).not.toBeNull();
-          });
         });
-      });
-
-      describe('if first delete button is pressed', () => {
-        beforeEach(fakeAsync(() => {
-          const deleteButton: DebugElement = fixture.debugElement.query(By.css('#epersonsSearch tbody .fa-trash-alt'));
-          deleteButton.nativeElement.click();
-          tick();
-          fixture.detectChanges();
-        }));
-        it('then no ePerson is member of the active group', () => {
+        it('then all (two) ePersons are member of the active group. No non-members left', () => {
           epersonsFound = fixture.debugElement.queryAll(By.css('#epersonsSearch tbody tr'));
-          expect(epersonsFound.length).toEqual(2);
-          epersonsFound.map((foundEPersonRowElement: DebugElement) => {
-            const addButton: DebugElement = foundEPersonRowElement.query(By.css('td:last-child .fa-plus'));
-            const deleteButton: DebugElement = foundEPersonRowElement.query(By.css('td:last-child .fa-trash-alt'));
-            expect(deleteButton).toBeNull();
-            expect(addButton).not.toBeNull();
-          });
+          expect(epersonsFound.length).toEqual(0);
         });
       });
     });
diff --git a/src/app/access-control/group-registry/group-form/members-list/members-list.component.ts b/src/app/access-control/group-registry/group-form/members-list/members-list.component.ts
index 704a92d0f9f..f49690f6746 100644
--- a/src/app/access-control/group-registry/group-form/members-list/members-list.component.ts
+++ b/src/app/access-control/group-registry/group-form/members-list/members-list.component.ts
@@ -4,13 +4,11 @@ import { Router } from '@angular/router';
 import { TranslateService } from '@ngx-translate/core';
 import {
   Observable,
-  of as observableOf,
   Subscription,
-  BehaviorSubject,
-  combineLatest as observableCombineLatest
+  BehaviorSubject
 } from 'rxjs';
-import { defaultIfEmpty, map, switchMap, take } from 'rxjs/operators';
-import { buildPaginatedList, PaginatedList } from '../../../../core/data/paginated-list.model';
+import { map, switchMap, take } from 'rxjs/operators';
+import { PaginatedList } from '../../../../core/data/paginated-list.model';
 import { RemoteData } from '../../../../core/data/remote-data';
 import { EPersonDataService } from '../../../../core/eperson/eperson-data.service';
 import { GroupDataService } from '../../../../core/eperson/group-data.service';
@@ -18,11 +16,11 @@ import { EPerson } from '../../../../core/eperson/models/eperson.model';
 import { Group } from '../../../../core/eperson/models/group.model';
 import {
   getFirstCompletedRemoteData,
-  getAllCompletedRemoteData
+  getAllCompletedRemoteData,
+  getRemoteDataPayload
 } from '../../../../core/shared/operators';
 import { NotificationsService } from '../../../../shared/notifications/notifications.service';
 import { PaginationComponentOptions } from '../../../../shared/pagination/pagination-component-options.model';
-import { EpersonDtoModel } from '../../../../core/eperson/models/eperson-dto.model';
 import { PaginationService } from '../../../../core/pagination/pagination.service';
 import { DSONameService } from '../../../../core/breadcrumbs/dso-name.service';
 
@@ -93,11 +91,11 @@ export class MembersListComponent implements OnInit, OnDestroy {
   /**
    * EPeople being displayed in search result, initially all members, after search result of search
    */
-  ePeopleSearchDtos: BehaviorSubject<PaginatedList<EpersonDtoModel>> = new BehaviorSubject<PaginatedList<EpersonDtoModel>>(undefined);
+  ePeopleSearch: BehaviorSubject<PaginatedList<EPerson>> = new BehaviorSubject<PaginatedList<EPerson>>(undefined);
   /**
    * List of EPeople members of currently active group being edited
    */
-  ePeopleMembersOfGroupDtos: BehaviorSubject<PaginatedList<EpersonDtoModel>> = new BehaviorSubject<PaginatedList<EpersonDtoModel>>(undefined);
+  ePeopleMembersOfGroup: BehaviorSubject<PaginatedList<EPerson>> = new BehaviorSubject<PaginatedList<EPerson>>(undefined);
 
   /**
    * Pagination config used to display the list of EPeople that are result of EPeople search
@@ -186,18 +184,9 @@ export class MembersListComponent implements OnInit, OnDestroy {
             return rd;
           }
         }),
-        switchMap((epersonListRD: RemoteData<PaginatedList<EPerson>>) => {
-          const dtos$ = observableCombineLatest([...epersonListRD.payload.page.map((member: EPerson) => {
-            const epersonDtoModel: EpersonDtoModel = new EpersonDtoModel();
-            epersonDtoModel.eperson = member;
-            return observableOf(epersonDtoModel);
-          })]);
-          return dtos$.pipe(defaultIfEmpty([]), map((dtos: EpersonDtoModel[]) => {
-            return buildPaginatedList(epersonListRD.payload.pageInfo, dtos);
-          }));
-        }))
-        .subscribe((paginatedListOfDTOs: PaginatedList<EpersonDtoModel>) => {
-          this.ePeopleMembersOfGroupDtos.next(paginatedListOfDTOs);
+        getRemoteDataPayload())
+        .subscribe((paginatedListOfEPersons: PaginatedList<EPerson>) => {
+          this.ePeopleMembersOfGroup.next(paginatedListOfEPersons);
         }));
   }
 
@@ -217,13 +206,13 @@ export class MembersListComponent implements OnInit, OnDestroy {
 
   /**
    * Deletes a given EPerson from the members list of the group currently being edited
-   * @param ePerson   EPerson we want to delete as member from group that is currently being edited
+   * @param eperson   EPerson we want to delete as member from group that is currently being edited
    */
-  deleteMemberFromGroup(ePerson: EpersonDtoModel) {
+  deleteMemberFromGroup(eperson: EPerson) {
     this.groupDataService.getActiveGroup().pipe(take(1)).subscribe((activeGroup: Group) => {
       if (activeGroup != null) {
-        const response = this.groupDataService.deleteMemberFromGroup(activeGroup, ePerson.eperson);
-        this.showNotifications('deleteMember', response, this.dsoNameService.getName(ePerson.eperson), activeGroup);
+        const response = this.groupDataService.deleteMemberFromGroup(activeGroup, eperson);
+        this.showNotifications('deleteMember', response, this.dsoNameService.getName(eperson), activeGroup);
       } else {
         this.notificationsService.error(this.translateService.get(this.messagePrefix + '.notification.failure.noActiveGroup'));
       }
@@ -232,13 +221,13 @@ export class MembersListComponent implements OnInit, OnDestroy {
 
   /**
    * Adds a given EPerson to the members list of the group currently being edited
-   * @param ePerson   EPerson we want to add as member to group that is currently being edited
+   * @param eperson   EPerson we want to add as member to group that is currently being edited
    */
-  addMemberToGroup(ePerson: EpersonDtoModel) {
+  addMemberToGroup(eperson: EPerson) {
     this.groupDataService.getActiveGroup().pipe(take(1)).subscribe((activeGroup: Group) => {
       if (activeGroup != null) {
-        const response = this.groupDataService.addMemberToGroup(activeGroup, ePerson.eperson);
-        this.showNotifications('addMember', response, this.dsoNameService.getName(ePerson.eperson), activeGroup);
+        const response = this.groupDataService.addMemberToGroup(activeGroup, eperson);
+        this.showNotifications('addMember', response, this.dsoNameService.getName(eperson), activeGroup);
       } else {
         this.notificationsService.error(this.translateService.get(this.messagePrefix + '.notification.failure.noActiveGroup'));
       }
@@ -286,18 +275,9 @@ export class MembersListComponent implements OnInit, OnDestroy {
             return rd;
           }
         }),
-        switchMap((epersonListRD: RemoteData<PaginatedList<EPerson>>) => {
-          const dtos$ = observableCombineLatest([...epersonListRD.payload.page.map((member: EPerson) => {
-            const epersonDtoModel: EpersonDtoModel = new EpersonDtoModel();
-            epersonDtoModel.eperson = member;
-            return observableOf(epersonDtoModel);
-          })]);
-          return dtos$.pipe(defaultIfEmpty([]), map((dtos: EpersonDtoModel[]) => {
-            return buildPaginatedList(epersonListRD.payload.pageInfo, dtos);
-          }));
-        }))
-        .subscribe((paginatedListOfDTOs: PaginatedList<EpersonDtoModel>) => {
-          this.ePeopleSearchDtos.next(paginatedListOfDTOs);
+        getRemoteDataPayload())
+        .subscribe((paginatedListOfEPersons: PaginatedList<EPerson>) => {
+          this.ePeopleSearch.next(paginatedListOfEPersons);
         }));
   }
 

From b598f1b5ca9b54c4b5fe23daa3d7b502ee0dc6b2 Mon Sep 17 00:00:00 2001
From: Tim Donohue <tim.donohue@lyrasis.org>
Date: Tue, 26 Sep 2023 12:07:30 -0500
Subject: [PATCH 06/11] Also remove unnecessary EpersonDtoModel from extending
 ReviewersListComponent. Remove "memberOfGroup" from EpersonDtoModel as it is
 no longer used

---
 .../members-list/members-list.component.ts    | 12 +--
 .../core/eperson/models/eperson-dto.model.ts  |  5 -
 .../reviewers-list.component.spec.ts          | 95 ++++++++-----------
 .../reviewers-list.component.ts               | 51 ++++------
 4 files changed, 65 insertions(+), 98 deletions(-)

diff --git a/src/app/access-control/group-registry/group-form/members-list/members-list.component.ts b/src/app/access-control/group-registry/group-form/members-list/members-list.component.ts
index f49690f6746..e828555a80f 100644
--- a/src/app/access-control/group-registry/group-form/members-list/members-list.component.ts
+++ b/src/app/access-control/group-registry/group-form/members-list/members-list.component.ts
@@ -29,8 +29,8 @@ import { DSONameService } from '../../../../core/breadcrumbs/dso-name.service';
  */
 enum SubKey {
   ActiveGroup,
-  MembersDTO,
-  SearchResultsDTO,
+  Members,
+  SearchResults,
 }
 
 /**
@@ -166,8 +166,8 @@ export class MembersListComponent implements OnInit, OnDestroy {
    * @private
    */
   retrieveMembers(page: number): void {
-    this.unsubFrom(SubKey.MembersDTO);
-    this.subs.set(SubKey.MembersDTO,
+    this.unsubFrom(SubKey.Members);
+    this.subs.set(SubKey.Members,
       this.paginationService.getCurrentPagination(this.config.id, this.config).pipe(
         switchMap((currentPagination) => {
           return this.ePersonDataService.findListByHref(this.groupBeingEdited._links.epersons.href, {
@@ -239,8 +239,8 @@ export class MembersListComponent implements OnInit, OnDestroy {
    * @param data  Contains scope and query param
    */
   search(data: any) {
-    this.unsubFrom(SubKey.SearchResultsDTO);
-    this.subs.set(SubKey.SearchResultsDTO,
+    this.unsubFrom(SubKey.SearchResults);
+    this.subs.set(SubKey.SearchResults,
       this.paginationService.getCurrentPagination(this.configSearch.id, this.configSearch).pipe(
         switchMap((paginationOptions) => {
 
diff --git a/src/app/core/eperson/models/eperson-dto.model.ts b/src/app/core/eperson/models/eperson-dto.model.ts
index 0e79902196b..5fa6c7ed68b 100644
--- a/src/app/core/eperson/models/eperson-dto.model.ts
+++ b/src/app/core/eperson/models/eperson-dto.model.ts
@@ -13,9 +13,4 @@ export class EpersonDtoModel {
      * Whether or not the linked EPerson is able to be deleted
      */
     public ableToDelete: boolean;
-    /**
-     * Whether or not this EPerson is member of group on page it is being used on
-     */
-    public memberOfGroup: boolean;
-
 }
diff --git a/src/app/workflowitems-edit-page/advanced-workflow-action/advanced-workflow-action-select-reviewer/reviewers-list/reviewers-list.component.spec.ts b/src/app/workflowitems-edit-page/advanced-workflow-action/advanced-workflow-action-select-reviewer/reviewers-list/reviewers-list.component.spec.ts
index 7c8db782ce6..bcbeef56830 100644
--- a/src/app/workflowitems-edit-page/advanced-workflow-action/advanced-workflow-action-select-reviewer/reviewers-list/reviewers-list.component.spec.ts
+++ b/src/app/workflowitems-edit-page/advanced-workflow-action/advanced-workflow-action-select-reviewer/reviewers-list/reviewers-list.component.spec.ts
@@ -17,7 +17,7 @@ import { Group } from '../../../../core/eperson/models/group.model';
 import { PageInfo } from '../../../../core/shared/page-info.model';
 import { FormBuilderService } from '../../../../shared/form/builder/form-builder.service';
 import { NotificationsService } from '../../../../shared/notifications/notifications.service';
-import { GroupMock, GroupMock2 } from '../../../../shared/testing/group-mock';
+import { GroupMock } from '../../../../shared/testing/group-mock';
 import { ReviewersListComponent } from './reviewers-list.component';
 import { EPersonMock, EPersonMock2 } from '../../../../shared/testing/eperson.mock';
 import {
@@ -31,8 +31,10 @@ import { NotificationsServiceStub } from '../../../../shared/testing/notificatio
 import { RouterMock } from '../../../../shared/mocks/router.mock';
 import { PaginationService } from '../../../../core/pagination/pagination.service';
 import { PaginationServiceStub } from '../../../../shared/testing/pagination-service.stub';
-import { EpersonDtoModel } from '../../../../core/eperson/models/eperson-dto.model';
 
+// NOTE: Because ReviewersListComponent extends MembersListComponent, the below tests ONLY validate
+// features which are *unique* to ReviewersListComponent. All other features are tested in the
+// members-list.component.spec.ts file.
 describe('ReviewersListComponent', () => {
   let component: ReviewersListComponent;
   let fixture: ComponentFixture<ReviewersListComponent>;
@@ -40,31 +42,27 @@ describe('ReviewersListComponent', () => {
   let builderService: FormBuilderService;
   let ePersonDataServiceStub: any;
   let groupsDataServiceStub: any;
-  let activeGroup;
-  let allEPersons;
-  let allGroups;
-  let epersonMembers;
-  let subgroupMembers;
+  let activeGroup: Group;
+  let epersonMembers: EPerson[];
+  let epersonNonMembers: EPerson[];
   let paginationService;
-  let ePersonDtoModel1: EpersonDtoModel;
-  let ePersonDtoModel2: EpersonDtoModel;
 
   beforeEach(waitForAsync(() => {
     activeGroup = GroupMock;
     epersonMembers = [EPersonMock2];
-    subgroupMembers = [GroupMock2];
-    allEPersons = [EPersonMock, EPersonMock2];
-    allGroups = [GroupMock, GroupMock2];
+    epersonNonMembers = [EPersonMock];
     ePersonDataServiceStub = {
       activeGroup: activeGroup,
       epersonMembers: epersonMembers,
-      subgroupMembers: subgroupMembers,
+      epersonNonMembers: epersonNonMembers,
+      // This method is used to get all the current members
       findListByHref(_href: string): Observable<RemoteData<PaginatedList<EPerson>>> {
         return createSuccessfulRemoteDataObject$(buildPaginatedList<EPerson>(new PageInfo(), groupsDataServiceStub.getEPersonMembers()));
       },
+      // This method is used to search across *non-members*
       searchByScope(scope: string, query: string): Observable<RemoteData<PaginatedList<EPerson>>> {
         if (query === '') {
-          return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), allEPersons));
+          return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), epersonNonMembers));
         }
         return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), []));
       },
@@ -81,22 +79,22 @@ describe('ReviewersListComponent', () => {
     groupsDataServiceStub = {
       activeGroup: activeGroup,
       epersonMembers: epersonMembers,
-      subgroupMembers: subgroupMembers,
-      allGroups: allGroups,
+      epersonNonMembers: epersonNonMembers,
       getActiveGroup(): Observable<Group> {
         return observableOf(activeGroup);
       },
       getEPersonMembers() {
         return this.epersonMembers;
       },
-      searchGroups(query: string): Observable<RemoteData<PaginatedList<Group>>> {
-        if (query === '') {
-          return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), this.allGroups));
-        }
-        return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), []));
-      },
-      addMemberToGroup(parentGroup, eperson: EPerson): Observable<RestResponse> {
-        this.epersonMembers = [...this.epersonMembers, eperson];
+      addMemberToGroup(parentGroup, epersonToAdd: EPerson): Observable<RestResponse> {
+        // Add eperson to list of members
+        this.epersonMembers = [...this.epersonMembers, epersonToAdd];
+        // Remove eperson from list of non-members
+        this.epersonNonMembers.forEach( (eperson: EPerson, index: number) => {
+          if (eperson.id === epersonToAdd.id) {
+            this.epersonNonMembers.splice(index, 1);
+          }
+        });
         return observableOf(new RestResponse(true, 200, 'Success'));
       },
       clearGroupsRequests() {
@@ -109,21 +107,20 @@ describe('ReviewersListComponent', () => {
         return '/access-control/groups/' + group.id;
       },
       deleteMemberFromGroup(parentGroup, epersonToDelete: EPerson): Observable<RestResponse> {
-        this.epersonMembers = this.epersonMembers.find((eperson: EPerson) => {
-          if (eperson.id !== epersonToDelete.id) {
-            return eperson;
+        // Remove eperson from list of members
+        this.epersonMembers.forEach( (eperson: EPerson, index: number) => {
+          if (eperson.id === epersonToDelete.id) {
+            this.epersonMembers.splice(index, 1);
           }
         });
-        if (this.epersonMembers === undefined) {
-          this.epersonMembers = [];
-        }
+        // Add eperson to list of non-members
+        this.epersonNonMembers = [...this.epersonNonMembers, epersonToDelete];
         return observableOf(new RestResponse(true, 200, 'Success'));
       },
+      // Used to find the currently active group
       findById(id: string) {
-        for (const group of allGroups) {
-          if (group.id === id) {
-            return createSuccessfulRemoteDataObject$(group);
-          }
+        if (activeGroup.id === id) {
+          return createSuccessfulRemoteDataObject$(activeGroup);
         }
         return createNoContentRemoteDataObject$();
       },
@@ -135,7 +132,7 @@ describe('ReviewersListComponent', () => {
     translateService = getMockTranslateService();
 
     paginationService = new PaginationServiceStub();
-    TestBed.configureTestingModule({
+    return TestBed.configureTestingModule({
       imports: [CommonModule, NgbModule, FormsModule, ReactiveFormsModule, BrowserModule,
         TranslateModule.forRoot({
           loader: {
@@ -169,12 +166,6 @@ describe('ReviewersListComponent', () => {
     fixture.debugElement.nativeElement.remove();
   }));
 
-  beforeEach(() => {
-    ePersonDtoModel1 = new EpersonDtoModel();
-    ePersonDtoModel1.eperson = EPersonMock;
-    ePersonDtoModel2 = new EpersonDtoModel();
-    ePersonDtoModel2.eperson = EPersonMock2;
-  });
 
   describe('when no group is selected', () => {
     beforeEach(() => {
@@ -218,34 +209,32 @@ describe('ReviewersListComponent', () => {
   it('should replace the value when a new member is added when multipleReviewers is false', () => {
     spyOn(component.selectedReviewersUpdated, 'emit');
     component.multipleReviewers = false;
-    component.selectedReviewers = [ePersonDtoModel1];
+    component.selectedReviewers = [EPersonMock];
 
-    component.addMemberToGroup(ePersonDtoModel2);
+    component.addMemberToGroup(EPersonMock2);
 
-    expect(component.selectedReviewers).toEqual([ePersonDtoModel2]);
-    expect(component.selectedReviewersUpdated.emit).toHaveBeenCalledWith([ePersonDtoModel2.eperson]);
+    expect(component.selectedReviewers).toEqual([EPersonMock2]);
+    expect(component.selectedReviewersUpdated.emit).toHaveBeenCalledWith([EPersonMock2]);
   });
 
   it('should add the value when a new member is added when multipleReviewers is true', () => {
     spyOn(component.selectedReviewersUpdated, 'emit');
     component.multipleReviewers = true;
-    component.selectedReviewers = [ePersonDtoModel1];
+    component.selectedReviewers = [EPersonMock];
 
-    component.addMemberToGroup(ePersonDtoModel2);
+    component.addMemberToGroup(EPersonMock2);
 
-    expect(component.selectedReviewers).toEqual([ePersonDtoModel1, ePersonDtoModel2]);
-    expect(component.selectedReviewersUpdated.emit).toHaveBeenCalledWith([ePersonDtoModel1.eperson, ePersonDtoModel2.eperson]);
+    expect(component.selectedReviewers).toEqual([EPersonMock, EPersonMock2]);
+    expect(component.selectedReviewersUpdated.emit).toHaveBeenCalledWith([EPersonMock, EPersonMock2]);
   });
 
   it('should delete the member when present', () => {
     spyOn(component.selectedReviewersUpdated, 'emit');
-    ePersonDtoModel1.memberOfGroup = true;
-    component.selectedReviewers = [ePersonDtoModel1];
+    component.selectedReviewers = [EPersonMock];
 
-    component.deleteMemberFromGroup(ePersonDtoModel1);
+    component.deleteMemberFromGroup(EPersonMock);
 
     expect(component.selectedReviewers).toEqual([]);
-    expect(ePersonDtoModel1.memberOfGroup).toBeFalse();
     expect(component.selectedReviewersUpdated.emit).toHaveBeenCalledWith([]);
   });
 
diff --git a/src/app/workflowitems-edit-page/advanced-workflow-action/advanced-workflow-action-select-reviewer/reviewers-list/reviewers-list.component.ts b/src/app/workflowitems-edit-page/advanced-workflow-action/advanced-workflow-action-select-reviewer/reviewers-list/reviewers-list.component.ts
index 6984a1d86dd..65a106e2e84 100644
--- a/src/app/workflowitems-edit-page/advanced-workflow-action/advanced-workflow-action-select-reviewer/reviewers-list/reviewers-list.component.ts
+++ b/src/app/workflowitems-edit-page/advanced-workflow-action/advanced-workflow-action-select-reviewer/reviewers-list/reviewers-list.component.ts
@@ -8,10 +8,7 @@ import { NotificationsService } from '../../../../shared/notifications/notificat
 import { PaginationService } from '../../../../core/pagination/pagination.service';
 import { Group } from '../../../../core/eperson/models/group.model';
 import { getFirstSucceededRemoteDataPayload } from '../../../../core/shared/operators';
-import { EpersonDtoModel } from '../../../../core/eperson/models/eperson-dto.model';
 import { EPerson } from '../../../../core/eperson/models/eperson.model';
-import { Observable, of as observableOf } from 'rxjs';
-import { hasValue } from '../../../../shared/empty.util';
 import { PaginatedList } from '../../../../core/data/paginated-list.model';
 import {
   MembersListComponent,
@@ -24,8 +21,8 @@ import { DSONameService } from '../../../../core/breadcrumbs/dso-name.service';
  */
 enum SubKey {
   ActiveGroup,
-  MembersDTO,
-  SearchResultsDTO,
+  Members,
+  SearchResults,
 }
 
 /**
@@ -50,7 +47,7 @@ export class ReviewersListComponent extends MembersListComponent implements OnIn
   @Output()
   selectedReviewersUpdated: EventEmitter<EPerson[]> = new EventEmitter();
 
-  selectedReviewers: EpersonDtoModel[] = [];
+  selectedReviewers: EPerson[] = [];
 
   constructor(
     protected groupService: GroupDataService,
@@ -100,54 +97,40 @@ export class ReviewersListComponent extends MembersListComponent implements OnIn
   retrieveMembers(page: number): void {
     this.config.currentPage = page;
     if (this.groupId === null) {
-      this.unsubFrom(SubKey.MembersDTO);
-      const paginatedListOfDTOs: PaginatedList<EpersonDtoModel> = new PaginatedList();
-      paginatedListOfDTOs.page = this.selectedReviewers;
-      this.ePeopleMembersOfGroupDtos.next(paginatedListOfDTOs);
+      this.unsubFrom(SubKey.Members);
+      const paginatedListOfEPersons: PaginatedList<EPerson> = new PaginatedList();
+      paginatedListOfEPersons.page = this.selectedReviewers;
+      this.ePeopleMembersOfGroup.next(paginatedListOfEPersons);
     } else {
       super.retrieveMembers(page);
     }
   }
 
   /**
-   * Checks whether the given {@link possibleMember} is part of the {@link selectedReviewers}.
+   * Removes the {@link eperson} from the {@link selectedReviewers}
    *
-   * @param possibleMember The {@link EPerson} that needs to be checked
+   * @param eperson The {@link EPerson} to remove
    */
-  isMemberOfGroup(possibleMember: EPerson): Observable<boolean> {
-    return observableOf(hasValue(this.selectedReviewers.find((reviewer: EpersonDtoModel) => reviewer.eperson.id === possibleMember.id)));
-  }
-
-  /**
-   * Removes the {@link ePerson} from the {@link selectedReviewers}
-   *
-   * @param ePerson The {@link EpersonDtoModel} containg the {@link EPerson} to remove
-   */
-  deleteMemberFromGroup(ePerson: EpersonDtoModel) {
-    ePerson.memberOfGroup = false;
-    const index = this.selectedReviewers.indexOf(ePerson);
+  deleteMemberFromGroup(eperson: EPerson) {
+    const index = this.selectedReviewers.indexOf(eperson);
     if (index !== -1) {
       this.selectedReviewers.splice(index, 1);
     }
-    this.selectedReviewersUpdated.emit(this.selectedReviewers.map((ePersonDtoModel: EpersonDtoModel) => ePersonDtoModel.eperson));
+    this.selectedReviewersUpdated.emit(this.selectedReviewers);
   }
 
   /**
-   * Adds the {@link ePerson} to the {@link selectedReviewers} (or replaces it when {@link multipleReviewers} is
+   * Adds the {@link eperson} to the {@link selectedReviewers} (or replaces it when {@link multipleReviewers} is
    * `false`). Afterwards it will emit the list.
    *
-   * @param ePerson The {@link EPerson} to add to the list
+   * @param eperson The {@link EPerson} to add to the list
    */
-  addMemberToGroup(ePerson: EpersonDtoModel) {
-    ePerson.memberOfGroup = true;
+  addMemberToGroup(eperson: EPerson) {
     if (!this.multipleReviewers) {
-      for (const selectedReviewer of this.selectedReviewers) {
-        selectedReviewer.memberOfGroup = false;
-      }
       this.selectedReviewers = [];
     }
-    this.selectedReviewers.push(ePerson);
-    this.selectedReviewersUpdated.emit(this.selectedReviewers.map((epersonDtoModel: EpersonDtoModel) => epersonDtoModel.eperson));
+    this.selectedReviewers.push(eperson);
+    this.selectedReviewersUpdated.emit(this.selectedReviewers);
   }
 
 }

From 64f968b246774140b2e3d4f134be7608ab7a6207 Mon Sep 17 00:00:00 2001
From: Tim Donohue <tim.donohue@lyrasis.org>
Date: Mon, 2 Oct 2023 16:35:26 -0500
Subject: [PATCH 07/11] Fix subgroups-list specs so they align with new
 members-list specs

---
 .../subgroups-list.component.spec.ts          | 184 ++++++++++--------
 1 file changed, 102 insertions(+), 82 deletions(-)

diff --git a/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.spec.ts b/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.spec.ts
index ac5750dcaca..7bbbb24f304 100644
--- a/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.spec.ts
+++ b/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.spec.ts
@@ -1,12 +1,12 @@
 import { CommonModule } from '@angular/common';
 import { NO_ERRORS_SCHEMA, DebugElement } from '@angular/core';
-import { ComponentFixture, fakeAsync, flush, inject, TestBed, tick, waitForAsync } from '@angular/core/testing';
+import { ComponentFixture, fakeAsync, flush, inject, TestBed, waitForAsync } from '@angular/core/testing';
 import { FormsModule, ReactiveFormsModule } from '@angular/forms';
 import { BrowserModule, By } from '@angular/platform-browser';
 import { Router } from '@angular/router';
 import { NgbModule } from '@ng-bootstrap/ng-bootstrap';
 import { TranslateLoader, TranslateModule, TranslateService } from '@ngx-translate/core';
-import { Observable, of as observableOf, BehaviorSubject } from 'rxjs';
+import { Observable, of as observableOf } from 'rxjs';
 import { RestResponse } from '../../../../core/cache/response.models';
 import { buildPaginatedList, PaginatedList } from '../../../../core/data/paginated-list.model';
 import { RemoteData } from '../../../../core/data/remote-data';
@@ -18,19 +18,18 @@ import { NotificationsService } from '../../../../shared/notifications/notificat
 import { GroupMock, GroupMock2 } from '../../../../shared/testing/group-mock';
 import { SubgroupsListComponent } from './subgroups-list.component';
 import {
-  createSuccessfulRemoteDataObject$,
-  createSuccessfulRemoteDataObject
+  createSuccessfulRemoteDataObject$
 } from '../../../../shared/remote-data.utils';
 import { RouterMock } from '../../../../shared/mocks/router.mock';
 import { getMockFormBuilderService } from '../../../../shared/mocks/form-builder-service.mock';
 import { getMockTranslateService } from '../../../../shared/mocks/translate.service.mock';
 import { TranslateLoaderMock } from '../../../../shared/testing/translate-loader.mock';
 import { NotificationsServiceStub } from '../../../../shared/testing/notifications-service.stub';
-import { map } from 'rxjs/operators';
 import { PaginationService } from '../../../../core/pagination/pagination.service';
 import { PaginationServiceStub } from '../../../../shared/testing/pagination-service.stub';
 import { DSONameService } from '../../../../core/breadcrumbs/dso-name.service';
 import { DSONameServiceMock } from '../../../../shared/mocks/dso-name.service.mock';
+import { EPersonMock2 } from 'src/app/shared/testing/eperson.mock';
 
 describe('SubgroupsListComponent', () => {
   let component: SubgroupsListComponent;
@@ -39,44 +38,70 @@ describe('SubgroupsListComponent', () => {
   let builderService: FormBuilderService;
   let ePersonDataServiceStub: any;
   let groupsDataServiceStub: any;
-  let activeGroup;
+  let activeGroup: Group;
   let subgroups: Group[];
-  let allGroups: Group[];
+  let groupNonMembers: Group[];
   let routerStub;
   let paginationService;
+  // Define a new mock activegroup for all tests below
+  let mockActiveGroup: Group = Object.assign(new Group(), {
+    handle: null,
+    subgroups: [GroupMock2],
+    epersons: [EPersonMock2],
+    selfRegistered: false,
+    permanent: false,
+    _links: {
+        self: {
+            href: 'https://rest.api/server/api/eperson/groups/activegroupid',
+        },
+        subgroups: { href: 'https://rest.api/server/api/eperson/groups/activegroupid/subgroups' },
+        object: { href: 'https://rest.api/server/api/eperson/groups/activegroupid/object' },
+        epersons: { href: 'https://rest.api/server/api/eperson/groups/activegroupid/epersons' }
+    },
+    _name: 'activegroupname',
+    id: 'activegroupid',
+    uuid: 'activegroupid',
+    type: 'group',
+  });
 
   beforeEach(waitForAsync(() => {
-    activeGroup = GroupMock;
+    activeGroup = mockActiveGroup;
     subgroups = [GroupMock2];
-    allGroups = [GroupMock, GroupMock2];
+    groupNonMembers = [GroupMock];
     ePersonDataServiceStub = {};
     groupsDataServiceStub = {
       activeGroup: activeGroup,
-      subgroups$: new BehaviorSubject(subgroups),
+      subgroups: subgroups,
+      groupNonMembers: groupNonMembers,
       getActiveGroup(): Observable<Group> {
         return observableOf(this.activeGroup);
       },
       getSubgroups(): Group {
-        return this.activeGroup;
+        return this.subgroups;
       },
+      // This method is used to get all the current subgroups
       findListByHref(_href: string): Observable<RemoteData<PaginatedList<Group>>> {
-        return this.subgroups$.pipe(
-          map((currentGroups: Group[]) => {
-            return createSuccessfulRemoteDataObject(buildPaginatedList<Group>(new PageInfo(), currentGroups));
-          })
-        );
+        return createSuccessfulRemoteDataObject$(buildPaginatedList<Group>(new PageInfo(), groupsDataServiceStub.getSubgroups()));
       },
       getGroupEditPageRouterLink(group: Group): string {
         return '/access-control/groups/' + group.id;
       },
+      // This method is used to get all groups which are NOT currently a subgroup member
       searchGroups(query: string): Observable<RemoteData<PaginatedList<Group>>> {
         if (query === '') {
-          return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), allGroups));
+          return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), groupNonMembers));
         }
         return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), []));
       },
-      addSubGroupToGroup(parentGroup, subgroup: Group): Observable<RestResponse> {
-        this.subgroups$.next([...this.subgroups$.getValue(), subgroup]);
+      addSubGroupToGroup(parentGroup, subgroupToAdd: Group): Observable<RestResponse> {
+        // Add group to list of subgroups
+        this.subgroups = [...this.subgroups, subgroupToAdd];
+        // Remove group from list of non-members
+        this.groupNonMembers.forEach( (group: Group, index: number) => {
+          if (group.id === subgroupToAdd.id) {
+            this.groupNonMembers.splice(index, 1);
+          }
+        });
         return observableOf(new RestResponse(true, 200, 'Success'));
       },
       clearGroupsRequests() {
@@ -85,12 +110,15 @@ describe('SubgroupsListComponent', () => {
       clearGroupLinkRequests() {
         // empty
       },
-      deleteSubGroupFromGroup(parentGroup, subgroup: Group): Observable<RestResponse> {
-        this.subgroups$.next(this.subgroups$.getValue().filter((group: Group) => {
-          if (group.id !== subgroup.id) {
-            return group;
+      deleteSubGroupFromGroup(parentGroup, subgroupToDelete: Group): Observable<RestResponse> {
+        // Remove group from list of subgroups
+        this.subgroups.forEach( (group: Group, index: number) => {
+          if (group.id === subgroupToDelete.id) {
+            this.subgroups.splice(index, 1);
           }
-        }));
+        });
+        // Add group to list of non-members
+        this.groupNonMembers = [...this.groupNonMembers, subgroupToDelete];
         return observableOf(new RestResponse(true, 200, 'Success'));
       }
     };
@@ -99,7 +127,7 @@ describe('SubgroupsListComponent', () => {
     translateService = getMockTranslateService();
 
     paginationService = new PaginationServiceStub();
-    TestBed.configureTestingModule({
+    return TestBed.configureTestingModule({
       imports: [CommonModule, NgbModule, FormsModule, ReactiveFormsModule, BrowserModule,
         TranslateModule.forRoot({
           loader: {
@@ -137,30 +165,38 @@ describe('SubgroupsListComponent', () => {
     expect(comp).toBeDefined();
   }));
 
-  it('should show list of subgroups of current active group', () => {
-    const groupIdsFound = fixture.debugElement.queryAll(By.css('#subgroupsOfGroup tr td:first-child'));
-    expect(groupIdsFound.length).toEqual(1);
-    activeGroup.subgroups.map((group: Group) => {
-      expect(groupIdsFound.find((foundEl) => {
-        return (foundEl.nativeElement.textContent.trim() === group.uuid);
-      })).toBeTruthy();
+  describe('current subgroup list', () => {
+    it('should show list of subgroups of current active group', () => {
+      const groupIdsFound = fixture.debugElement.queryAll(By.css('#subgroupsOfGroup tr td:first-child'));
+      expect(groupIdsFound.length).toEqual(1);
+      subgroups.map((group: Group) => {
+        expect(groupIdsFound.find((foundEl) => {
+          return (foundEl.nativeElement.textContent.trim() === group.uuid);
+        })).toBeTruthy();
+      });
     });
-  });
 
-  describe('if first group delete button is pressed', () => {
-    let groupsFound: DebugElement[];
-    beforeEach(fakeAsync(() => {
-      const addButton = fixture.debugElement.query(By.css('#subgroupsOfGroup tbody .deleteButton'));
-      addButton.triggerEventHandler('click', {
-        preventDefault: () => {/**/
-        }
+    it('should show a delete button next to each subgroup', () => {
+      const subgroupsFound = fixture.debugElement.queryAll(By.css('#subgroupsOfGroup tbody tr'));
+      subgroupsFound.map((foundGroupRowElement: DebugElement) => {
+        const addButton: DebugElement = foundGroupRowElement.query(By.css('td:last-child .fa-plus'));
+        const deleteButton: DebugElement = foundGroupRowElement.query(By.css('td:last-child .fa-trash-alt'));
+        expect(addButton).toBeNull();
+        expect(deleteButton).not.toBeNull();
+      });
+    });
+
+    describe('if first group delete button is pressed', () => {
+      let groupsFound: DebugElement[];
+      beforeEach(() => {
+        const deleteButton = fixture.debugElement.query(By.css('#subgroupsOfGroup tbody .deleteButton'));
+        deleteButton.nativeElement.click();
+        fixture.detectChanges();
+      });
+      it('then no subgroup remains as a member of the active group', () => {
+        groupsFound = fixture.debugElement.queryAll(By.css('#subgroupsOfGroup tbody tr'));
+        expect(groupsFound.length).toEqual(0);
       });
-      tick();
-      fixture.detectChanges();
-    }));
-    it('one less subgroup in list from 1 to 0 (of 2 total groups)', () => {
-      groupsFound = fixture.debugElement.queryAll(By.css('#subgroupsOfGroup tbody tr'));
-      expect(groupsFound.length).toEqual(0);
     });
   });
 
@@ -169,54 +205,38 @@ describe('SubgroupsListComponent', () => {
       let groupsFound: DebugElement[];
       beforeEach(fakeAsync(() => {
         component.search({ query: '' });
+        fixture.detectChanges();
         groupsFound = fixture.debugElement.queryAll(By.css('#groupsSearch tbody tr'));
       }));
 
-      it('should display all groups', () => {
-        fixture.detectChanges();
-        groupsFound = fixture.debugElement.queryAll(By.css('#groupsSearch tbody tr'));
-        expect(groupsFound.length).toEqual(2);
-        groupsFound = fixture.debugElement.queryAll(By.css('#groupsSearch tbody tr'));
+      it('should display only non-member groups (i.e. groups that are not a subgroup)', () => {
         const groupIdsFound: DebugElement[] = fixture.debugElement.queryAll(By.css('#groupsSearch tbody tr td:first-child'));
-        allGroups.map((group: Group) => {
+        expect(groupIdsFound.length).toEqual(1);
+        groupNonMembers.map((group: Group) => {
           expect(groupIdsFound.find((foundEl: DebugElement) => {
             return (foundEl.nativeElement.textContent.trim() === group.uuid);
           })).toBeTruthy();
         });
       });
 
-      describe('if group is already a subgroup', () => {
-        it('should have delete button, else it should have add button', () => {
+      it('should display an add button next to non-member groups, not a delete button', () => {
+        groupsFound.map((foundGroupRowElement: DebugElement) => {
+          const addButton: DebugElement = foundGroupRowElement.query(By.css('td:last-child .fa-plus'));
+          const deleteButton: DebugElement = foundGroupRowElement.query(By.css('td:last-child .fa-trash-alt'));
+          expect(addButton).not.toBeNull();
+          expect(deleteButton).toBeNull();
+        });
+      });
+
+      describe('if first add button is pressed', () => {
+        beforeEach(() => {
+          const addButton: DebugElement = fixture.debugElement.query(By.css('#groupsSearch tbody .fa-plus'));
+          addButton.nativeElement.click();
           fixture.detectChanges();
+        });
+        it('then all (two) Groups are subgroups of the active group. No non-members left', () => {
           groupsFound = fixture.debugElement.queryAll(By.css('#groupsSearch tbody tr'));
-          const getSubgroups = groupsDataServiceStub.getSubgroups().subgroups;
-          if (getSubgroups !== undefined && getSubgroups.length > 0) {
-            groupsFound.map((foundGroupRowElement: DebugElement) => {
-              const groupId: DebugElement = foundGroupRowElement.query(By.css('td:first-child'));
-              const addButton: DebugElement = foundGroupRowElement.query(By.css('td:last-child .fa-plus'));
-              const deleteButton: DebugElement = foundGroupRowElement.query(By.css('td:last-child .fa-trash-alt'));
-              expect(addButton).toBeNull();
-              if (activeGroup.id === groupId.nativeElement.textContent) {
-                expect(deleteButton).toBeNull();
-              } else {
-                expect(deleteButton).not.toBeNull();
-              }
-            });
-          } else {
-            const subgroupIds: string[] = activeGroup.subgroups.map((group: Group) => group.id);
-            groupsFound.map((foundGroupRowElement: DebugElement) => {
-              const groupId: DebugElement = foundGroupRowElement.query(By.css('td:first-child'));
-              const addButton: DebugElement = foundGroupRowElement.query(By.css('td:last-child .fa-plus'));
-              const deleteButton: DebugElement = foundGroupRowElement.query(By.css('td:last-child .fa-trash-alt'));
-              if (subgroupIds.includes(groupId.nativeElement.textContent)) {
-                expect(addButton).toBeNull();
-                expect(deleteButton).not.toBeNull();
-              } else {
-                expect(deleteButton).toBeNull();
-                expect(addButton).not.toBeNull();
-              }
-            });
-          }
+          expect(groupsFound.length).toEqual(0);
         });
       });
     });

From 8a10888d2ad7916570472173070370da61320a72 Mon Sep 17 00:00:00 2001
From: Tim Donohue <tim.donohue@lyrasis.org>
Date: Fri, 13 Oct 2023 14:43:28 -0500
Subject: [PATCH 08/11] Refactor members-list and subgroups-list components to
 use new isNotMemberOf endpoints (via services)

---
 .../members-list/members-list.component.html  | 10 +----
 .../members-list.component.spec.ts            |  2 +-
 .../members-list/members-list.component.ts    | 30 +++++++------
 .../subgroups-list.component.html             |  5 +--
 .../subgroups-list.component.spec.ts          |  2 +-
 .../subgroup-list/subgroups-list.component.ts | 35 +++++++---------
 .../core/eperson/eperson-data.service.spec.ts | 25 +++++++++++
 src/app/core/eperson/eperson-data.service.ts  | 28 +++++++++++++
 .../core/eperson/group-data.service.spec.ts   | 28 ++++++++++++-
 src/app/core/eperson/group-data.service.ts    | 42 +++++++++++--------
 src/assets/i18n/en.json5                      | 10 -----
 11 files changed, 139 insertions(+), 78 deletions(-)

diff --git a/src/app/access-control/group-registry/group-form/members-list/members-list.component.html b/src/app/access-control/group-registry/group-form/members-list/members-list.component.html
index 0f5010e08eb..e185d37e286 100644
--- a/src/app/access-control/group-registry/group-form/members-list/members-list.component.html
+++ b/src/app/access-control/group-registry/group-form/members-list/members-list.component.html
@@ -15,14 +15,8 @@ <h4 id="search" class="border-bottom pb-2">
   </h4>
 
   <form [formGroup]="searchForm" (ngSubmit)="search(searchForm.value)" class="d-flex justify-content-between">
-    <div>
-      <select name="scope" id="scope" formControlName="scope" class="form-control" aria-label="Search scope">
-        <option value="metadata">{{messagePrefix + '.search.scope.metadata' | translate}}</option>
-        <option value="email">{{messagePrefix + '.search.scope.email' | translate}}</option>
-      </select>
-    </div>
-    <div class="flex-grow-1 mr-3 ml-3">
-      <div class="form-group input-group">
+    <div class="flex-grow-1 mr-3">
+      <div class="form-group input-group mr-3">
         <input type="text" name="query" id="query" formControlName="query"
                class="form-control" aria-label="Search input">
         <span class="input-group-append">
diff --git a/src/app/access-control/group-registry/group-form/members-list/members-list.component.spec.ts b/src/app/access-control/group-registry/group-form/members-list/members-list.component.spec.ts
index 76e38067bc8..5d97dcade8d 100644
--- a/src/app/access-control/group-registry/group-form/members-list/members-list.component.spec.ts
+++ b/src/app/access-control/group-registry/group-form/members-list/members-list.component.spec.ts
@@ -56,7 +56,7 @@ describe('MembersListComponent', () => {
         return createSuccessfulRemoteDataObject$(buildPaginatedList<EPerson>(new PageInfo(), groupsDataServiceStub.getEPersonMembers()));
       },
       // This method is used to search across *non-members*
-      searchByScope(scope: string, query: string): Observable<RemoteData<PaginatedList<EPerson>>> {
+      searchNonMembers(query: string, group: string): Observable<RemoteData<PaginatedList<EPerson>>> {
         if (query === '') {
           return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), epersonNonMembers));
         }
diff --git a/src/app/access-control/group-registry/group-form/members-list/members-list.component.ts b/src/app/access-control/group-registry/group-form/members-list/members-list.component.ts
index e828555a80f..4924f3168b5 100644
--- a/src/app/access-control/group-registry/group-form/members-list/members-list.component.ts
+++ b/src/app/access-control/group-registry/group-form/members-list/members-list.component.ts
@@ -124,7 +124,6 @@ export class MembersListComponent implements OnInit, OnDestroy {
 
   // Current search in edit group - epeople search form
   currentSearchQuery: string;
-  currentSearchScope: string;
 
   // Whether or not user has done a EPeople search yet
   searchDone: boolean;
@@ -143,12 +142,10 @@ export class MembersListComponent implements OnInit, OnDestroy {
     public dsoNameService: DSONameService,
   ) {
     this.currentSearchQuery = '';
-    this.currentSearchScope = 'metadata';
   }
 
   ngOnInit(): void {
     this.searchForm = this.formBuilder.group(({
-      scope: 'metadata',
       query: '',
     }));
     this.subs.set(SubKey.ActiveGroup, this.groupDataService.getActiveGroup().subscribe((activeGroup: Group) => {
@@ -213,6 +210,11 @@ export class MembersListComponent implements OnInit, OnDestroy {
       if (activeGroup != null) {
         const response = this.groupDataService.deleteMemberFromGroup(activeGroup, eperson);
         this.showNotifications('deleteMember', response, this.dsoNameService.getName(eperson), activeGroup);
+        // Reload search results (if there is an active query).
+        // This will potentially add this deleted subgroup into the list of search results.
+        if (this.currentSearchQuery != null) {
+          this.search({query: this.currentSearchQuery});
+        }
       } else {
         this.notificationsService.error(this.translateService.get(this.messagePrefix + '.notification.failure.noActiveGroup'));
       }
@@ -228,6 +230,11 @@ export class MembersListComponent implements OnInit, OnDestroy {
       if (activeGroup != null) {
         const response = this.groupDataService.addMemberToGroup(activeGroup, eperson);
         this.showNotifications('addMember', response, this.dsoNameService.getName(eperson), activeGroup);
+        // Reload search results (if there is an active query).
+        // This will potentially add this deleted subgroup into the list of search results.
+        if (this.currentSearchQuery != null) {
+          this.search({query: this.currentSearchQuery});
+        }
       } else {
         this.notificationsService.error(this.translateService.get(this.messagePrefix + '.notification.failure.noActiveGroup'));
       }
@@ -235,17 +242,15 @@ export class MembersListComponent implements OnInit, OnDestroy {
   }
 
   /**
-   * Search in the EPeople by name, email or metadata
-   * @param data  Contains scope and query param
+   * Search all EPeople who are NOT a member of the current group by name, email or metadata
+   * @param data  Contains query param
    */
   search(data: any) {
     this.unsubFrom(SubKey.SearchResults);
     this.subs.set(SubKey.SearchResults,
       this.paginationService.getCurrentPagination(this.configSearch.id, this.configSearch).pipe(
         switchMap((paginationOptions) => {
-
           const query: string = data.query;
-          const scope: string = data.scope;
           if (query != null && this.currentSearchQuery !== query && this.groupBeingEdited) {
             this.router.navigate([], {
               queryParamsHandling: 'merge'
@@ -253,19 +258,12 @@ export class MembersListComponent implements OnInit, OnDestroy {
             this.currentSearchQuery = query;
             this.paginationService.resetPage(this.configSearch.id);
           }
-          if (scope != null && this.currentSearchScope !== scope && this.groupBeingEdited) {
-            this.router.navigate([], {
-              queryParamsHandling: 'merge'
-            });
-            this.currentSearchScope = scope;
-            this.paginationService.resetPage(this.configSearch.id);
-          }
           this.searchDone = true;
 
-          return this.ePersonDataService.searchByScope(this.currentSearchScope, this.currentSearchQuery, {
+          return this.ePersonDataService.searchNonMembers(this.currentSearchQuery, this.groupBeingEdited.id, {
             currentPage: paginationOptions.currentPage,
             elementsPerPage: paginationOptions.pageSize
-          });
+          }, false, true);
         }),
         getAllCompletedRemoteData(),
         map((rd: RemoteData<any>) => {
diff --git a/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.html b/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.html
index 8268eb5eb45..d97272ec6fe 100644
--- a/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.html
+++ b/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.html
@@ -62,10 +62,7 @@ <h4 id="search" class="border-bottom pb-2">
           <td class="align-middle">{{ dsoNameService.getName((group.object | async)?.payload) }}</td>
           <td class="align-middle">
             <div class="btn-group edit-field">
-              <span *ngIf="(isActiveGroup(group) | async)">{{ messagePrefix + '.table.edit.currentGroup' | translate }}</span>
-
-              <button *ngIf="!(isActiveGroup(group) | async)"
-                      (click)="addSubgroupToGroup(group)"
+              <button (click)="addSubgroupToGroup(group)"
                       class="btn btn-outline-primary btn-sm addButton"
                       title="{{messagePrefix + '.table.edit.buttons.add' | translate: { name: dsoNameService.getName(group) } }}">
                 <i class="fas fa-plus fa-fw"></i>
diff --git a/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.spec.ts b/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.spec.ts
index 7bbbb24f304..6fe7c2cf676 100644
--- a/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.spec.ts
+++ b/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.spec.ts
@@ -87,7 +87,7 @@ describe('SubgroupsListComponent', () => {
         return '/access-control/groups/' + group.id;
       },
       // This method is used to get all groups which are NOT currently a subgroup member
-      searchGroups(query: string): Observable<RemoteData<PaginatedList<Group>>> {
+      searchNonMemberGroups(query: string, group: string): Observable<RemoteData<PaginatedList<Group>>> {
         if (query === '') {
           return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), groupNonMembers));
         }
diff --git a/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.ts b/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.ts
index 83df0d8cdde..20200e47d22 100644
--- a/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.ts
+++ b/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.ts
@@ -2,8 +2,8 @@ import { Component, Input, OnDestroy, OnInit } from '@angular/core';
 import { UntypedFormBuilder } from '@angular/forms';
 import { Router } from '@angular/router';
 import { TranslateService } from '@ngx-translate/core';
-import { BehaviorSubject, Observable, of as observableOf, Subscription } from 'rxjs';
-import { mergeMap, switchMap, take } from 'rxjs/operators';
+import { BehaviorSubject, Observable, Subscription } from 'rxjs';
+import { switchMap, take } from 'rxjs/operators';
 import { PaginatedList } from '../../../../core/data/paginated-list.model';
 import { RemoteData } from '../../../../core/data/remote-data';
 import { GroupDataService } from '../../../../core/eperson/group-data.service';
@@ -129,20 +129,6 @@ export class SubgroupsListComponent implements OnInit, OnDestroy {
       }));
   }
 
-  /**
-   * Whether or not the given group is the current group being edited
-   * @param group Group that is possibly the current group being edited
-   */
-  isActiveGroup(group: Group): Observable<boolean> {
-    return this.groupDataService.getActiveGroup().pipe(take(1),
-      mergeMap((activeGroup: Group) => {
-        if (activeGroup != null && activeGroup.uuid === group.uuid) {
-          return observableOf(true);
-        }
-        return observableOf(false);
-      }));
-  }
-
   /**
    * Deletes given subgroup from the group currently being edited
    * @param subgroup  Group we want to delete from the subgroups of the group currently being edited
@@ -152,6 +138,11 @@ export class SubgroupsListComponent implements OnInit, OnDestroy {
       if (activeGroup != null) {
         const response = this.groupDataService.deleteSubGroupFromGroup(activeGroup, subgroup);
         this.showNotifications('deleteSubgroup', response, this.dsoNameService.getName(subgroup), activeGroup);
+        // Reload search results (if there is an active query).
+        // This will potentially add this deleted subgroup into the list of search results.
+        if (this.currentSearchQuery != null) {
+          this.search({query: this.currentSearchQuery});
+        }
       } else {
         this.notificationsService.error(this.translateService.get(this.messagePrefix + '.notification.failure.noActiveGroup'));
       }
@@ -168,6 +159,11 @@ export class SubgroupsListComponent implements OnInit, OnDestroy {
         if (activeGroup.uuid !== subgroup.uuid) {
           const response = this.groupDataService.addSubGroupToGroup(activeGroup, subgroup);
           this.showNotifications('addSubgroup', response, this.dsoNameService.getName(subgroup), activeGroup);
+          // Reload search results (if there is an active query).
+          // This will potentially remove this added subgroup from search results.
+          if (this.currentSearchQuery != null) {
+            this.search({query: this.currentSearchQuery});
+          }
         } else {
           this.notificationsService.error(this.translateService.get(this.messagePrefix + '.notification.failure.subgroupToAddIsActiveGroup'));
         }
@@ -178,7 +174,8 @@ export class SubgroupsListComponent implements OnInit, OnDestroy {
   }
 
   /**
-   * Search in the groups (searches by group name and by uuid exact match)
+   * Search all non-member groups (searches by group name and by uuid exact match).  Used to search for
+   * groups that could be added to current group as a subgroup.
    * @param data  Contains query param
    */
   search(data: any) {
@@ -192,10 +189,10 @@ export class SubgroupsListComponent implements OnInit, OnDestroy {
 
     this.unsubFrom(SubKey.SearchResults);
     this.subs.set(SubKey.SearchResults, this.paginationService.getCurrentPagination(this.configSearch.id, this.configSearch).pipe(
-      switchMap((config) => this.groupDataService.searchGroups(this.currentSearchQuery, {
+      switchMap((config) => this.groupDataService.searchNonMemberGroups(this.currentSearchQuery, this.groupBeingEdited.id, {
         currentPage: config.currentPage,
         elementsPerPage: config.pageSize
-      }, true, true, followLink('object')
+      }, false, true, followLink('object')
       ))
     ).subscribe((rd: RemoteData<PaginatedList<Group>>) => {
       this.searchResults$.next(rd);
diff --git a/src/app/core/eperson/eperson-data.service.spec.ts b/src/app/core/eperson/eperson-data.service.spec.ts
index b4b939eebf4..c1bc3563a32 100644
--- a/src/app/core/eperson/eperson-data.service.spec.ts
+++ b/src/app/core/eperson/eperson-data.service.spec.ts
@@ -11,6 +11,7 @@ import {
   EPeopleRegistryCancelEPersonAction,
   EPeopleRegistryEditEPersonAction
 } from '../../access-control/epeople-registry/epeople-registry.actions';
+import { GroupMock } from '../../shared/testing/group-mock';
 import { RequestParam } from '../cache/models/request-param.model';
 import { ChangeAnalyzer } from '../data/change-analyzer';
 import { PatchRequest, PostRequest } from '../data/request.models';
@@ -140,6 +141,30 @@ describe('EPersonDataService', () => {
     });
   });
 
+  describe('searchNonMembers', () => {
+    beforeEach(() => {
+      spyOn(service, 'searchBy');
+    });
+
+    it('search with empty query and a group ID', () => {
+      service.searchNonMembers('', GroupMock.id);
+      const options = Object.assign(new FindListOptions(), {
+        searchParams: [Object.assign(new RequestParam('query', '')),
+                       Object.assign(new RequestParam('group', GroupMock.id))]
+      });
+      expect(service.searchBy).toHaveBeenCalledWith('isNotMemberOf', options, true, true);
+    });
+
+    it('search with query and a group ID', () => {
+      service.searchNonMembers('test', GroupMock.id);
+      const options = Object.assign(new FindListOptions(), {
+        searchParams: [Object.assign(new RequestParam('query', 'test')),
+                       Object.assign(new RequestParam('group', GroupMock.id))]
+      });
+      expect(service.searchBy).toHaveBeenCalledWith('isNotMemberOf', options, true, true);
+    });
+  });
+
   describe('updateEPerson', () => {
     beforeEach(() => {
       spyOn(service, 'findByHref').and.returnValue(createSuccessfulRemoteDataObject$(EPersonMock));
diff --git a/src/app/core/eperson/eperson-data.service.ts b/src/app/core/eperson/eperson-data.service.ts
index 00620655de4..fb8b8bc9b00 100644
--- a/src/app/core/eperson/eperson-data.service.ts
+++ b/src/app/core/eperson/eperson-data.service.ts
@@ -177,6 +177,34 @@ export class EPersonDataService extends IdentifiableDataService<EPerson> impleme
     return this.searchBy(searchMethod, findListOptions, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow);
   }
 
+  /**
+   * Searches for all EPerons which are *not* a member of a given group, via a passed in query
+   * (searches all EPerson metadata and by exact UUID).
+   * Endpoint used: /eperson/epesons/search/isNotMemberOf?query=<:string>&group=<:uuid>
+   * @param query                       search query param
+   * @param group                       UUID of group to exclude results from. Members of this group will never be returned.
+   * @param options
+   * @param useCachedVersionIfAvailable If this is true, the request will only be sent if there's
+   *                                    no valid cached version. Defaults to true
+   * @param reRequestOnStale            Whether or not the request should automatically be re-
+   *                                    requested after the response becomes stale
+   * @param linksToFollow               List of {@link FollowLinkConfig} that indicate which
+   *                                    {@link HALLink}s should be automatically resolved
+   */
+  public searchNonMembers(query: string, group: string, options?: FindListOptions, useCachedVersionIfAvailable = true, reRequestOnStale = true, ...linksToFollow: FollowLinkConfig<EPerson>[]): Observable<RemoteData<PaginatedList<EPerson>>> {
+    const searchParams = [new RequestParam('query', query), new RequestParam('group', group)];
+    let findListOptions = new FindListOptions();
+    if (options) {
+      findListOptions = Object.assign(new FindListOptions(), options);
+    }
+    if (findListOptions.searchParams) {
+      findListOptions.searchParams = [...findListOptions.searchParams, ...searchParams];
+    } else {
+      findListOptions.searchParams = searchParams;
+    }
+    return this.searchBy('isNotMemberOf', findListOptions, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow);
+  }
+
   /**
    * Add a new patch to the object cache
    * The patch is derived from the differences between the given object and its version in the object cache
diff --git a/src/app/core/eperson/group-data.service.spec.ts b/src/app/core/eperson/group-data.service.spec.ts
index b424aed1aa0..6eecfd7fa15 100644
--- a/src/app/core/eperson/group-data.service.spec.ts
+++ b/src/app/core/eperson/group-data.service.spec.ts
@@ -43,11 +43,11 @@ describe('GroupDataService', () => {
   let rdbService;
   let objectCache;
   function init() {
-    restEndpointURL = 'https://dspace.4science.it/dspace-spring-rest/api/eperson';
+    restEndpointURL = 'https://rest.api/server/api/eperson';
     groupsEndpoint = `${restEndpointURL}/groups`;
     groups = [GroupMock, GroupMock2];
     groups$ = createSuccessfulRemoteDataObject$(createPaginatedList(groups));
-    rdbService = getMockRemoteDataBuildServiceHrefMap(undefined, { 'https://dspace.4science.it/dspace-spring-rest/api/eperson/groups': groups$ });
+    rdbService = getMockRemoteDataBuildServiceHrefMap(undefined, { 'https://rest.api/server/api/eperson/groups': groups$ });
     halService = new HALEndpointServiceStub(restEndpointURL);
     objectCache = getMockObjectCacheService();
     TestBed.configureTestingModule({
@@ -111,6 +111,30 @@ describe('GroupDataService', () => {
     });
   });
 
+  describe('searchNonMemberGroups', () => {
+    beforeEach(() => {
+      spyOn(service, 'searchBy');
+    });
+
+    it('search with empty query and a group ID', () => {
+      service.searchNonMemberGroups('', GroupMock.id);
+      const options = Object.assign(new FindListOptions(), {
+        searchParams: [Object.assign(new RequestParam('query', '')),
+                       Object.assign(new RequestParam('group', GroupMock.id))]
+      });
+      expect(service.searchBy).toHaveBeenCalledWith('isNotMemberOf', options, true, true);
+    });
+
+    it('search with query and a group ID', () => {
+      service.searchNonMemberGroups('test', GroupMock.id);
+      const options = Object.assign(new FindListOptions(), {
+        searchParams: [Object.assign(new RequestParam('query', 'test')),
+                       Object.assign(new RequestParam('group', GroupMock.id))]
+      });
+      expect(service.searchBy).toHaveBeenCalledWith('isNotMemberOf', options, true, true);
+    });
+  });
+
   describe('addSubGroupToGroup', () => {
     beforeEach(() => {
       objectCache.getByHref.and.returnValue(observableOf({
diff --git a/src/app/core/eperson/group-data.service.ts b/src/app/core/eperson/group-data.service.ts
index 7b3a14c70b2..683d026bb68 100644
--- a/src/app/core/eperson/group-data.service.ts
+++ b/src/app/core/eperson/group-data.service.ts
@@ -3,7 +3,7 @@ import { Injectable } from '@angular/core';
 
 import { createSelector, select, Store } from '@ngrx/store';
 import { Observable, zip as observableZip } from 'rxjs';
-import { filter, map, take } from 'rxjs/operators';
+import { take } from 'rxjs/operators';
 import {
   GroupRegistryCancelGroupAction,
   GroupRegistryEditGroupAction
@@ -105,23 +105,31 @@ export class GroupDataService extends IdentifiableDataService<Group> implements
   }
 
   /**
-   * Check if the current user is member of to the indicated group
-   *
-   * @param groupName
-   *    the group name
-   * @return boolean
-   *    true if user is member of the indicated group, false otherwise
+   * Searches for all groups which are *not* a member of a given group, via a passed in query
+   * (searches in group name and by exact UUID).
+   * Endpoint used: /eperson/groups/search/isNotMemberOf?query=<:string>&group=<:uuid>
+   * @param query                       search query param
+   * @param group                       UUID of group to exclude results from. Members of this group will never be returned.
+   * @param options
+   * @param useCachedVersionIfAvailable If this is true, the request will only be sent if there's
+   *                                    no valid cached version. Defaults to true
+   * @param reRequestOnStale            Whether or not the request should automatically be re-
+   *                                    requested after the response becomes stale
+   * @param linksToFollow               List of {@link FollowLinkConfig} that indicate which
+   *                                    {@link HALLink}s should be automatically resolved
    */
-  isMemberOf(groupName: string): Observable<boolean> {
-    const searchHref = 'isMemberOf';
-    const options = new FindListOptions();
-    options.searchParams = [new RequestParam('groupName', groupName)];
-
-    return this.searchBy(searchHref, options).pipe(
-      filter((groups: RemoteData<PaginatedList<Group>>) => !groups.isResponsePending),
-      take(1),
-      map((groups: RemoteData<PaginatedList<Group>>) => groups.payload.totalElements > 0)
-    );
+  public searchNonMemberGroups(query: string, group: string, options?: FindListOptions, useCachedVersionIfAvailable = true, reRequestOnStale = true, ...linksToFollow: FollowLinkConfig<Group>[]): Observable<RemoteData<PaginatedList<Group>>> {
+    const searchParams = [new RequestParam('query', query), new RequestParam('group', group)];
+    let findListOptions = new FindListOptions();
+    if (options) {
+      findListOptions = Object.assign(new FindListOptions(), options);
+    }
+    if (findListOptions.searchParams) {
+      findListOptions.searchParams = [...findListOptions.searchParams, ...searchParams];
+    } else {
+      findListOptions.searchParams = searchParams;
+    }
+    return this.searchBy('isNotMemberOf', findListOptions, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow);
   }
 
   /**
diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5
index 3b3578fac5d..2a67a7315dc 100644
--- a/src/assets/i18n/en.json5
+++ b/src/assets/i18n/en.json5
@@ -434,10 +434,6 @@
 
   "admin.access-control.groups.form.members-list.headMembers": "Current Members",
 
-  "admin.access-control.groups.form.members-list.search.scope.metadata": "Metadata",
-
-  "admin.access-control.groups.form.members-list.search.scope.email": "E-mail (exact)",
-
   "admin.access-control.groups.form.members-list.search.button": "Search",
 
   "admin.access-control.groups.form.members-list.table.id": "ID",
@@ -494,8 +490,6 @@
 
   "admin.access-control.groups.form.subgroups-list.table.edit.buttons.add": "Add subgroup with name \"{{name}}\"",
 
-  "admin.access-control.groups.form.subgroups-list.table.edit.currentGroup": "Current group",
-
   "admin.access-control.groups.form.subgroups-list.notification.success.addSubgroup": "Successfully added subgroup: \"{{name}}\"",
 
   "admin.access-control.groups.form.subgroups-list.notification.failure.addSubgroup": "Failed to add subgroup: \"{{name}}\"",
@@ -632,10 +626,6 @@
 
   "advanced-workflow-action-select-reviewer.groups.form.reviewers-list.headMembers": "Current Members",
 
-  "advanced-workflow-action-select-reviewer.groups.form.reviewers-list.search.scope.metadata": "Metadata",
-
-  "advanced-workflow-action-select-reviewer.groups.form.reviewers-list.search.scope.email": "E-mail (exact)",
-
   "advanced-workflow-action-select-reviewer.groups.form.reviewers-list.search.button": "Search",
 
   "advanced-workflow-action-select-reviewer.groups.form.reviewers-list.table.id": "ID",

From 2eb1a17e4e0fd0eb6a13f30c6f8b847f6d9d7a89 Mon Sep 17 00:00:00 2001
From: Tim Donohue <tim.donohue@lyrasis.org>
Date: Fri, 13 Oct 2023 14:56:45 -0500
Subject: [PATCH 09/11] Refactor subgroups-list component's "search()" to act
 same as member-list component's "search()". Avoids reloading the page as
 frequently.

---
 .../subgroup-list/subgroups-list.component.ts | 49 ++++++++++++-------
 1 file changed, 31 insertions(+), 18 deletions(-)

diff --git a/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.ts b/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.ts
index 20200e47d22..119b147a951 100644
--- a/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.ts
+++ b/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.ts
@@ -3,12 +3,13 @@ import { UntypedFormBuilder } from '@angular/forms';
 import { Router } from '@angular/router';
 import { TranslateService } from '@ngx-translate/core';
 import { BehaviorSubject, Observable, Subscription } from 'rxjs';
-import { switchMap, take } from 'rxjs/operators';
+import { map, switchMap, take } from 'rxjs/operators';
 import { PaginatedList } from '../../../../core/data/paginated-list.model';
 import { RemoteData } from '../../../../core/data/remote-data';
 import { GroupDataService } from '../../../../core/eperson/group-data.service';
 import { Group } from '../../../../core/eperson/models/group.model';
 import {
+  getAllCompletedRemoteData,
   getFirstCompletedRemoteData
 } from '../../../../core/shared/operators';
 import { NotificationsService } from '../../../../shared/notifications/notifications.service';
@@ -179,24 +180,36 @@ export class SubgroupsListComponent implements OnInit, OnDestroy {
    * @param data  Contains query param
    */
   search(data: any) {
-    const query: string = data.query;
-    if (query != null && this.currentSearchQuery !== query) {
-      this.router.navigateByUrl(this.groupDataService.getGroupEditPageRouterLink(this.groupBeingEdited));
-      this.currentSearchQuery = query;
-      this.configSearch.currentPage = 1;
-    }
-    this.searchDone = true;
-
     this.unsubFrom(SubKey.SearchResults);
-    this.subs.set(SubKey.SearchResults, this.paginationService.getCurrentPagination(this.configSearch.id, this.configSearch).pipe(
-      switchMap((config) => this.groupDataService.searchNonMemberGroups(this.currentSearchQuery, this.groupBeingEdited.id, {
-        currentPage: config.currentPage,
-        elementsPerPage: config.pageSize
-      }, false, true, followLink('object')
-      ))
-    ).subscribe((rd: RemoteData<PaginatedList<Group>>) => {
-      this.searchResults$.next(rd);
-    }));
+    this.subs.set(SubKey.SearchResults,
+      this.paginationService.getCurrentPagination(this.configSearch.id, this.configSearch).pipe(
+        switchMap((paginationOptions) => {
+          const query: string = data.query;
+          if (query != null && this.currentSearchQuery !== query && this.groupBeingEdited) {
+            this.router.navigate([], {
+              queryParamsHandling: 'merge'
+            });
+            this.currentSearchQuery = query;
+            this.paginationService.resetPage(this.configSearch.id);
+          }
+          this.searchDone = true;
+
+          return this.groupDataService.searchNonMemberGroups(this.currentSearchQuery, this.groupBeingEdited.id, {
+            currentPage: paginationOptions.currentPage,
+            elementsPerPage: paginationOptions.pageSize
+          }, false, true, followLink('object'));
+        }),
+        getAllCompletedRemoteData(),
+        map((rd: RemoteData<any>) => {
+          if (rd.hasFailed) {
+            this.notificationsService.error(this.translateService.get(this.messagePrefix + '.notification.failure', { cause: rd.errorMessage }));
+          } else {
+            return rd;
+          }
+        }))
+        .subscribe((rd: RemoteData<PaginatedList<Group>>) => {
+          this.searchResults$.next(rd);
+        }));
   }
 
   /**

From d163db13f219ab81ca821e1695a2193edb38c79a Mon Sep 17 00:00:00 2001
From: Tim Donohue <tim.donohue@lyrasis.org>
Date: Fri, 13 Oct 2023 15:07:04 -0500
Subject: [PATCH 10/11] Remove seemingly unnecessary page reload after new
 search.

---
 .../group-form/members-list/members-list.component.ts          | 3 ---
 .../group-form/subgroup-list/subgroups-list.component.ts       | 3 ---
 2 files changed, 6 deletions(-)

diff --git a/src/app/access-control/group-registry/group-form/members-list/members-list.component.ts b/src/app/access-control/group-registry/group-form/members-list/members-list.component.ts
index 4924f3168b5..d2ddd700a9b 100644
--- a/src/app/access-control/group-registry/group-form/members-list/members-list.component.ts
+++ b/src/app/access-control/group-registry/group-form/members-list/members-list.component.ts
@@ -252,9 +252,6 @@ export class MembersListComponent implements OnInit, OnDestroy {
         switchMap((paginationOptions) => {
           const query: string = data.query;
           if (query != null && this.currentSearchQuery !== query && this.groupBeingEdited) {
-            this.router.navigate([], {
-              queryParamsHandling: 'merge'
-            });
             this.currentSearchQuery = query;
             this.paginationService.resetPage(this.configSearch.id);
           }
diff --git a/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.ts b/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.ts
index 119b147a951..12cdc27943b 100644
--- a/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.ts
+++ b/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.ts
@@ -186,9 +186,6 @@ export class SubgroupsListComponent implements OnInit, OnDestroy {
         switchMap((paginationOptions) => {
           const query: string = data.query;
           if (query != null && this.currentSearchQuery !== query && this.groupBeingEdited) {
-            this.router.navigate([], {
-              queryParamsHandling: 'merge'
-            });
             this.currentSearchQuery = query;
             this.paginationService.resetPage(this.configSearch.id);
           }

From 9117ac005f575277bf2e025d18afb878d18b0589 Mon Sep 17 00:00:00 2001
From: Tim Donohue <tim.donohue@lyrasis.org>
Date: Thu, 9 Nov 2023 13:57:04 -0600
Subject: [PATCH 11/11] Address feedback. Run empty search on init. Reorder
 sections to list current members before add members (for both eperson and
 groups)

---
 .../members-list/members-list.component.html  | 108 +++++++++---------
 .../members-list/members-list.component.ts    |   1 +
 .../subgroups-list.component.html             |  98 ++++++++--------
 .../subgroup-list/subgroups-list.component.ts |   1 +
 4 files changed, 105 insertions(+), 103 deletions(-)

diff --git a/src/app/access-control/group-registry/group-form/members-list/members-list.component.html b/src/app/access-control/group-registry/group-form/members-list/members-list.component.html
index e185d37e286..c0c77f44ebc 100644
--- a/src/app/access-control/group-registry/group-form/members-list/members-list.component.html
+++ b/src/app/access-control/group-registry/group-form/members-list/members-list.component.html
@@ -1,6 +1,60 @@
 <ng-container>
   <h3 class="border-bottom pb-2">{{messagePrefix + '.head' | translate}}</h3>
 
+  <h4>{{messagePrefix + '.headMembers' | translate}}</h4>
+
+  <ds-pagination *ngIf="(ePeopleMembersOfGroup | async)?.totalElements > 0"
+                 [paginationOptions]="config"
+                 [pageInfoState]="(ePeopleMembersOfGroup | async)"
+                 [collectionSize]="(ePeopleMembersOfGroup | async)?.totalElements"
+                 [hideGear]="true"
+                 [hidePagerWhenSinglePage]="true">
+
+    <div class="table-responsive">
+      <table id="ePeopleMembersOfGroup" class="table table-striped table-hover table-bordered">
+        <thead>
+        <tr>
+          <th scope="col" class="align-middle">{{messagePrefix + '.table.id' | translate}}</th>
+          <th scope="col" class="align-middle">{{messagePrefix + '.table.name' | translate}}</th>
+          <th scope="col" class="align-middle">{{messagePrefix + '.table.identity' | translate}}</th>
+          <th class="align-middle">{{messagePrefix + '.table.edit' | translate}}</th>
+        </tr>
+        </thead>
+        <tbody>
+        <tr *ngFor="let eperson of (ePeopleMembersOfGroup | async)?.page">
+          <td class="align-middle">{{eperson.id}}</td>
+          <td class="align-middle">
+            <a (click)="ePersonDataService.startEditingNewEPerson(eperson)"
+               [routerLink]="[ePersonDataService.getEPeoplePageRouterLink()]">
+              {{ dsoNameService.getName(eperson) }}
+            </a>
+          </td>
+          <td class="align-middle">
+            {{messagePrefix + '.table.email' | translate}}: {{ eperson.email ? eperson.email : '-' }}<br/>
+            {{messagePrefix + '.table.netid' | translate}}: {{ eperson.netid ? eperson.netid : '-' }}
+          </td>
+          <td class="align-middle">
+            <div class="btn-group edit-field">
+              <button (click)="deleteMemberFromGroup(eperson)"
+                      [disabled]="actionConfig.remove.disabled"
+                      [ngClass]="['btn btn-sm', actionConfig.remove.css]"
+                      title="{{messagePrefix + '.table.edit.buttons.remove' | translate: { name: dsoNameService.getName(eperson) } }}">
+                <i [ngClass]="actionConfig.remove.icon"></i>
+              </button>
+            </div>
+          </td>
+        </tr>
+        </tbody>
+      </table>
+    </div>
+
+  </ds-pagination>
+
+  <div *ngIf="(ePeopleMembersOfGroup | async) == undefined || (ePeopleMembersOfGroup | async)?.totalElements == 0" class="alert alert-info w-100 mb-2"
+       role="alert">
+    {{messagePrefix + '.no-members-yet' | translate}}
+  </div>
+
   <h4 id="search" class="border-bottom pb-2">
     <span
       *dsContextHelp="{
@@ -84,58 +138,4 @@ <h4 id="search" class="border-bottom pb-2">
     {{messagePrefix + '.no-items' | translate}}
   </div>
 
-  <h4>{{messagePrefix + '.headMembers' | translate}}</h4>
-
-  <ds-pagination *ngIf="(ePeopleMembersOfGroup | async)?.totalElements > 0"
-                 [paginationOptions]="config"
-                 [pageInfoState]="(ePeopleMembersOfGroup | async)"
-                 [collectionSize]="(ePeopleMembersOfGroup | async)?.totalElements"
-                 [hideGear]="true"
-                 [hidePagerWhenSinglePage]="true">
-
-    <div class="table-responsive">
-      <table id="ePeopleMembersOfGroup" class="table table-striped table-hover table-bordered">
-        <thead>
-        <tr>
-          <th scope="col" class="align-middle">{{messagePrefix + '.table.id' | translate}}</th>
-          <th scope="col" class="align-middle">{{messagePrefix + '.table.name' | translate}}</th>
-          <th scope="col" class="align-middle">{{messagePrefix + '.table.identity' | translate}}</th>
-          <th class="align-middle">{{messagePrefix + '.table.edit' | translate}}</th>
-        </tr>
-        </thead>
-        <tbody>
-        <tr *ngFor="let eperson of (ePeopleMembersOfGroup | async)?.page">
-          <td class="align-middle">{{eperson.id}}</td>
-          <td class="align-middle">
-            <a (click)="ePersonDataService.startEditingNewEPerson(eperson)"
-               [routerLink]="[ePersonDataService.getEPeoplePageRouterLink()]">
-              {{ dsoNameService.getName(eperson) }}
-            </a>
-          </td>
-          <td class="align-middle">
-            {{messagePrefix + '.table.email' | translate}}: {{ eperson.email ? eperson.email : '-' }}<br/>
-            {{messagePrefix + '.table.netid' | translate}}: {{ eperson.netid ? eperson.netid : '-' }}
-          </td>
-          <td class="align-middle">
-            <div class="btn-group edit-field">
-              <button (click)="deleteMemberFromGroup(eperson)"
-                      [disabled]="actionConfig.remove.disabled"
-                      [ngClass]="['btn btn-sm', actionConfig.remove.css]"
-                      title="{{messagePrefix + '.table.edit.buttons.remove' | translate: { name: dsoNameService.getName(eperson) } }}">
-                <i [ngClass]="actionConfig.remove.icon"></i>
-              </button>
-            </div>
-          </td>
-        </tr>
-        </tbody>
-      </table>
-    </div>
-
-  </ds-pagination>
-
-  <div *ngIf="(ePeopleMembersOfGroup | async) == undefined || (ePeopleMembersOfGroup | async)?.totalElements == 0" class="alert alert-info w-100 mb-2"
-       role="alert">
-    {{messagePrefix + '.no-members-yet' | translate}}
-  </div>
-
 </ng-container>
diff --git a/src/app/access-control/group-registry/group-form/members-list/members-list.component.ts b/src/app/access-control/group-registry/group-form/members-list/members-list.component.ts
index d2ddd700a9b..feb90b52b37 100644
--- a/src/app/access-control/group-registry/group-form/members-list/members-list.component.ts
+++ b/src/app/access-control/group-registry/group-form/members-list/members-list.component.ts
@@ -152,6 +152,7 @@ export class MembersListComponent implements OnInit, OnDestroy {
       if (activeGroup != null) {
         this.groupBeingEdited = activeGroup;
         this.retrieveMembers(this.config.currentPage);
+        this.search({query: ''});
       }
     }));
   }
diff --git a/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.html b/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.html
index d97272ec6fe..85fe8974edd 100644
--- a/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.html
+++ b/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.html
@@ -1,6 +1,55 @@
 <ng-container>
   <h3 class="border-bottom pb-2">{{messagePrefix + '.head' | translate}}</h3>
 
+  <h4>{{messagePrefix + '.headSubgroups' | translate}}</h4>
+
+  <ds-pagination *ngIf="(subGroups$ | async)?.payload?.totalElements > 0"
+                 [paginationOptions]="config"
+                 [pageInfoState]="(subGroups$ | async)?.payload"
+                 [collectionSize]="(subGroups$ | async)?.payload?.totalElements"
+                 [hideGear]="true"
+                 [hidePagerWhenSinglePage]="true">
+
+    <div class="table-responsive">
+      <table id="subgroupsOfGroup" class="table table-striped table-hover table-bordered">
+        <thead>
+        <tr>
+          <th scope="col" class="align-middle">{{messagePrefix + '.table.id' | translate}}</th>
+          <th scope="col" class="align-middle">{{messagePrefix + '.table.name' | translate}}</th>
+          <th scope="col" class="align-middle">{{messagePrefix + '.table.collectionOrCommunity' | translate}}</th>
+          <th>{{messagePrefix + '.table.edit' | translate}}</th>
+        </tr>
+        </thead>
+        <tbody>
+        <tr *ngFor="let group of (subGroups$ | async)?.payload?.page">
+          <td class="align-middle">{{group.id}}</td>
+          <td class="align-middle">
+            <a (click)="groupDataService.startEditingNewGroup(group)"
+               [routerLink]="[groupDataService.getGroupEditPageRouterLink(group)]">
+            {{ dsoNameService.getName(group) }}
+          </a>
+          </td>
+          <td class="align-middle">{{ dsoNameService.getName((group.object | async)?.payload)}}</td>
+          <td class="align-middle">
+            <div class="btn-group edit-field">
+              <button (click)="deleteSubgroupFromGroup(group)"
+                      class="btn btn-outline-danger btn-sm deleteButton"
+                      title="{{messagePrefix + '.table.edit.buttons.remove' | translate: { name: dsoNameService.getName(group) } }}">
+                <i class="fas fa-trash-alt fa-fw"></i>
+              </button>
+            </div>
+          </td>
+        </tr>
+        </tbody>
+      </table>
+    </div>
+  </ds-pagination>
+
+  <div *ngIf="(subGroups$ | async)?.payload?.totalElements == 0" class="alert alert-info w-100 mb-2"
+       role="alert">
+    {{messagePrefix + '.no-subgroups-yet' | translate}}
+  </div>
+
   <h4 id="search" class="border-bottom pb-2">
     <span *dsContextHelp="{
             content: 'admin.access-control.groups.form.tooltip.editGroup.addSubgroups',
@@ -80,53 +129,4 @@ <h4 id="search" class="border-bottom pb-2">
     {{messagePrefix + '.no-items' | translate}}
   </div>
 
-  <h4>{{messagePrefix + '.headSubgroups' | translate}}</h4>
-
-  <ds-pagination *ngIf="(subGroups$ | async)?.payload?.totalElements > 0"
-                 [paginationOptions]="config"
-                 [pageInfoState]="(subGroups$ | async)?.payload"
-                 [collectionSize]="(subGroups$ | async)?.payload?.totalElements"
-                 [hideGear]="true"
-                 [hidePagerWhenSinglePage]="true">
-
-    <div class="table-responsive">
-      <table id="subgroupsOfGroup" class="table table-striped table-hover table-bordered">
-        <thead>
-        <tr>
-          <th scope="col" class="align-middle">{{messagePrefix + '.table.id' | translate}}</th>
-          <th scope="col" class="align-middle">{{messagePrefix + '.table.name' | translate}}</th>
-          <th scope="col" class="align-middle">{{messagePrefix + '.table.collectionOrCommunity' | translate}}</th>
-          <th>{{messagePrefix + '.table.edit' | translate}}</th>
-        </tr>
-        </thead>
-        <tbody>
-        <tr *ngFor="let group of (subGroups$ | async)?.payload?.page">
-          <td class="align-middle">{{group.id}}</td>
-          <td class="align-middle">
-            <a (click)="groupDataService.startEditingNewGroup(group)"
-               [routerLink]="[groupDataService.getGroupEditPageRouterLink(group)]">
-            {{ dsoNameService.getName(group) }}
-          </a>
-          </td>
-          <td class="align-middle">{{ dsoNameService.getName((group.object | async)?.payload)}}</td>
-          <td class="align-middle">
-            <div class="btn-group edit-field">
-              <button (click)="deleteSubgroupFromGroup(group)"
-                      class="btn btn-outline-danger btn-sm deleteButton"
-                      title="{{messagePrefix + '.table.edit.buttons.remove' | translate: { name: dsoNameService.getName(group) } }}">
-                <i class="fas fa-trash-alt fa-fw"></i>
-              </button>
-            </div>
-          </td>
-        </tr>
-        </tbody>
-      </table>
-    </div>
-  </ds-pagination>
-
-  <div *ngIf="(subGroups$ | async)?.payload?.totalElements == 0" class="alert alert-info w-100 mb-2"
-       role="alert">
-    {{messagePrefix + '.no-subgroups-yet' | translate}}
-  </div>
-
 </ng-container>
diff --git a/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.ts b/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.ts
index 12cdc27943b..aea545e5543 100644
--- a/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.ts
+++ b/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.ts
@@ -102,6 +102,7 @@ export class SubgroupsListComponent implements OnInit, OnDestroy {
       if (activeGroup != null) {
         this.groupBeingEdited = activeGroup;
         this.retrieveSubGroups();
+        this.search({query: ''});
       }
     }));
   }