Refactor members-list and subgroups-list components to use new isNotMemberOf endpoints (via services)

src/app/access-control/group-registry/group-form/members-list/members-list.component.html

@@ -15,14 +15,8 @@
   </h4>
 
   <form [formGroup]="searchForm" (ngSubmit)="search(searchForm.value)" class="d-flex justify-content-between">
-    <div>
+    <div class="flex-grow-1 mr-3">
-      <select name="scope" id="scope" formControlName="scope" class="form-control" aria-label="Search scope">
+      <div class="form-group input-group mr-3">
-        <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">
         <input type="text" name="query" id="query" formControlName="query"
                class="form-control" aria-label="Search input">
         <span class="input-group-append">

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));
         }

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
+   * Search all EPeople who are NOT a member of the current group by name, email or metadata
-   * @param data  Contains scope and query param
+   * @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>) => {

src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.html

@@ -62,10 +62,7 @@
           <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 (click)="addSubgroupToGroup(group)"
-
-              <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) } }}">
                 <i class="fas fa-plus fa-fw"></i>

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));
         }

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 { BehaviorSubject, Observable, Subscription } from 'rxjs';
-import { mergeMap, switchMap, take } from 'rxjs/operators';
+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);

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));

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

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({

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
+   * 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).
-   * @param groupName
+   * Endpoint used: /eperson/groups/search/isNotMemberOf?query=<:string>&group=<:uuid>
-   *    the group name
+   * @param query                       search query param
-   * @return boolean
+   * @param group                       UUID of group to exclude results from. Members of this group will never be returned.
-   *    true if user is member of the indicated group, false otherwise
+   * @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> {
+  public searchNonMemberGroups(query: string, group: string, options?: FindListOptions, useCachedVersionIfAvailable = true, reRequestOnStale = true, ...linksToFollow: FollowLinkConfig<Group>[]): Observable<RemoteData<PaginatedList<Group>>> {
-    const searchHref = 'isMemberOf';
+    const searchParams = [new RequestParam('query', query), new RequestParam('group', group)];
-    const options = new FindListOptions();
+    let findListOptions = new FindListOptions();
-    options.searchParams = [new RequestParam('groupName', groupName)];
+    if (options) {
-
+      findListOptions = Object.assign(new FindListOptions(), options);
-    return this.searchBy(searchHref, options).pipe(
+    }
-      filter((groups: RemoteData<PaginatedList<Group>>) => !groups.isResponsePending),
+    if (findListOptions.searchParams) {
-      take(1),
+      findListOptions.searchParams = [...findListOptions.searchParams, ...searchParams];
-      map((groups: RemoteData<PaginatedList<Group>>) => groups.payload.totalElements > 0)
+    } else {
-    );
+      findListOptions.searchParams = searchParams;
+    }
+    return this.searchBy('isNotMemberOf', findListOptions, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow);
   }
 
   /**

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",