From dde149aceacdd2b658c4632f72098a5b0b7120cd Mon Sep 17 00:00:00 2001 From: Marie Verdonck Date: Thu, 4 Feb 2021 12:11:18 +0100 Subject: [PATCH] 75832: [Issue 962]: Fix unnecessary calls creating/deleting groups - feedback --- .../members-list/members-list.component.ts | 163 ++++++++++-------- src/assets/i18n/en.json5 | 2 + 2 files changed, 96 insertions(+), 69 deletions(-) diff --git a/src/app/+admin/admin-access-control/group-registry/group-form/members-list/members-list.component.ts b/src/app/+admin/admin-access-control/group-registry/group-form/members-list/members-list.component.ts index 7a09f6ac8b..d01d117a86 100644 --- a/src/app/+admin/admin-access-control/group-registry/group-form/members-list/members-list.component.ts +++ b/src/app/+admin/admin-access-control/group-registry/group-form/members-list/members-list.component.ts @@ -2,8 +2,14 @@ import { Component, Input, OnDestroy, OnInit } from '@angular/core'; import { FormBuilder } from '@angular/forms'; import { Router } from '@angular/router'; import { TranslateService } from '@ngx-translate/core'; -import { Observable, of as observableOf, Subscription, BehaviorSubject, ObservedValueOf, combineLatest as observableCombineLatest} from 'rxjs'; -import { map, mergeMap, take } from 'rxjs/operators'; +import { + Observable, + of as observableOf, + Subscription, + BehaviorSubject, + combineLatest as observableCombineLatest, ObservedValueOf, +} from 'rxjs'; +import { map, mergeMap, 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'; @@ -13,7 +19,7 @@ import { Group } from '../../../../../core/eperson/models/group.model'; import { getRemoteDataPayload, getFirstSucceededRemoteData, - getFirstCompletedRemoteData + getFirstCompletedRemoteData, getAllCompletedRemoteData } from '../../../../../core/shared/operators'; import { NotificationsService } from '../../../../../shared/notifications/notifications.service'; import { PaginationComponentOptions } from '../../../../../shared/pagination/pagination-component-options.model'; @@ -23,9 +29,7 @@ import {EpersonDtoModel} from '../../../../../core/eperson/models/eperson-dto.mo * Keys to keep track of specific subscriptions */ enum SubKey { - Members, ActiveGroup, - SearchResults, MembersDTO, SearchResultsDTO, } @@ -45,12 +49,10 @@ export class MembersListComponent implements OnInit, OnDestroy { /** * EPeople being displayed in search result, initially all members, after search result of search */ - searchResults$: BehaviorSubject>> = new BehaviorSubject(undefined); ePeopleSearchDtos: BehaviorSubject> = new BehaviorSubject>(undefined); /** * List of EPeople members of currently active group being edited */ - members$: BehaviorSubject>> = new BehaviorSubject(undefined); ePeopleMembersOfGroupDtos: BehaviorSubject> = new BehaviorSubject>(undefined); /** @@ -135,17 +137,58 @@ export class MembersListComponent implements OnInit, OnDestroy { * @private */ private retrieveMembers(page: number) { - this.unsubFrom(SubKey.Members); - this.subs.set( - SubKey.Members, - this.ePersonDataService.findAllByHref(this.groupBeingEdited._links.epersons.href, { - currentPage: page, - elementsPerPage: this.config.pageSize - } - ).subscribe((rd: RemoteData>) => { - this.members$.next(rd); - if (rd.payload !== undefined) { - this.setEpersonDTOsFromResult(rd, this.ePeopleMembersOfGroupDtos, SubKey.MembersDTO); + this.unsubFrom(SubKey.MembersDTO); + this.subs.set(SubKey.MembersDTO, this.ePersonDataService.findAllByHref(this.groupBeingEdited._links.epersons.href, { + currentPage: page, + elementsPerPage: this.config.pageSize + }).pipe( + getAllCompletedRemoteData(), + map((rd: RemoteData) => { + if (rd.hasFailed) { + this.notificationsService.error(this.translateService.get(this.messagePrefix + '.notification.failure', {cause: rd.errorMessage})); + } else { + return rd; + } + }), + switchMap((epersonListRD: RemoteData>) => { + const dtos$ = observableCombineLatest(...epersonListRD.payload.page.map((member: EPerson) => { + const dto$: Observable = observableCombineLatest( + this.isMemberOfGroup(member), (isMember: ObservedValueOf>) => { + const epersonDtoModel: EpersonDtoModel = new EpersonDtoModel(); + epersonDtoModel.eperson = member; + epersonDtoModel.memberOfGroup = isMember; + return epersonDtoModel; + }); + return dto$; + })); + return dtos$.pipe(map((dtos: EpersonDtoModel[]) => { + return buildPaginatedList(epersonListRD.payload.pageInfo, dtos); + })) + })) + .subscribe((paginatedListOfDTOs: PaginatedList) => { + this.ePeopleMembersOfGroupDtos.next(paginatedListOfDTOs); + })); + } + + /** + * Whether or not 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 { + return this.groupDataService.getActiveGroup().pipe(take(1), + mergeMap((group: Group) => { + if (group != null) { + return this.ePersonDataService.findAllByHref(group._links.epersons.href, { + currentPage: 1, + elementsPerPage: 9999 + }, false) + .pipe( + getFirstSucceededRemoteData(), + getRemoteDataPayload(), + map((listEPeopleInGroup: PaginatedList) => listEPeopleInGroup.page.filter((ePersonInList: EPerson) => ePersonInList.id === possibleMember.id)), + map((epeople: EPerson[]) => epeople.length > 0)); + } else { + return observableOf(false); } })); } @@ -173,6 +216,7 @@ export class MembersListComponent implements OnInit, OnDestroy { if (activeGroup != null) { const response = this.groupDataService.deleteMemberFromGroup(activeGroup, ePerson.eperson); this.showNotifications('deleteMember', response, ePerson.eperson.name, activeGroup); + this.search({ scope: this.currentSearchScope, query: this.currentSearchQuery }); } else { this.notificationsService.error(this.translateService.get(this.messagePrefix + '.notification.failure.noActiveGroup')); } @@ -195,29 +239,6 @@ export class MembersListComponent implements OnInit, OnDestroy { }); } - /** - * Whether or not 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 { - return this.groupDataService.getActiveGroup().pipe(take(1), - mergeMap((group: Group) => { - if (group != null) { - return this.ePersonDataService.findAllByHref(group._links.epersons.href, { - currentPage: 1, - elementsPerPage: 9999 - }) - .pipe( - getFirstSucceededRemoteData(), - getRemoteDataPayload(), - map((listEPeopleInGroup: PaginatedList) => listEPeopleInGroup.page.filter((ePersonInList: EPerson) => ePersonInList.id === possibleMember.id)), - map((epeople: EPerson[]) => epeople.length > 0)); - } else { - return observableOf(false); - } - })); - } - /** * Search in the EPeople by name, email or metadata * @param data Contains scope and query param @@ -237,34 +258,38 @@ export class MembersListComponent implements OnInit, OnDestroy { } this.searchDone = true; - this.unsubFrom(SubKey.SearchResults); - this.subs.set(SubKey.SearchResults, this.ePersonDataService.searchByScope(this.currentSearchScope, this.currentSearchQuery, { - currentPage: this.configSearch.currentPage, - elementsPerPage: this.configSearch.pageSize - }).subscribe((rd: RemoteData>) => { - this.searchResults$.next(rd); - if (rd.payload !== undefined) { - this.setEpersonDTOsFromResult(rd, this.ePeopleSearchDtos, SubKey.SearchResultsDTO); - } - })); - } - - private setEpersonDTOsFromResult(rd: RemoteData>, addTo: BehaviorSubject>, subkey) { - this.unsubFrom(subkey); - const dtos$ = observableCombineLatest(...rd.payload.page.map((member: EPerson) => { - const dto$: Observable = observableCombineLatest( - this.isMemberOfGroup(member), (isMember: ObservedValueOf>) => { - const epersonDtoModel: EpersonDtoModel = new EpersonDtoModel(); - epersonDtoModel.eperson = member; - epersonDtoModel.memberOfGroup = isMember; - return epersonDtoModel; - }); - return dto$; - })); - this.subs.set(subkey,dtos$.pipe(map((dtos: EpersonDtoModel[]) => { - const paginatedListOfDTOs = buildPaginatedList(rd.payload.pageInfo, dtos); - addTo.next(paginatedListOfDTOs); - })).subscribe()); + this.unsubFrom(SubKey.SearchResultsDTO); + this.subs.set(SubKey.SearchResultsDTO, + this.ePersonDataService.searchByScope(this.currentSearchScope, this.currentSearchQuery, { + currentPage: this.configSearch.currentPage, + elementsPerPage: this.configSearch.pageSize + }, false).pipe( + getAllCompletedRemoteData(), + map((rd: RemoteData) => { + if (rd.hasFailed) { + this.notificationsService.error(this.translateService.get(this.messagePrefix + '.notification.failure', {cause: rd.errorMessage})); + } else { + return rd; + } + }), + switchMap((epersonListRD: RemoteData>) => { + const dtos$ = observableCombineLatest(...epersonListRD.payload.page.map((member: EPerson) => { + const dto$: Observable = observableCombineLatest( + this.isMemberOfGroup(member), (isMember: ObservedValueOf>) => { + const epersonDtoModel: EpersonDtoModel = new EpersonDtoModel(); + epersonDtoModel.eperson = member; + epersonDtoModel.memberOfGroup = isMember; + return epersonDtoModel; + }); + return dto$; + })); + return dtos$.pipe(map((dtos: EpersonDtoModel[]) => { + return buildPaginatedList(epersonListRD.payload.pageInfo, dtos); + })) + })) + .subscribe((paginatedListOfDTOs: PaginatedList) => { + this.ePeopleSearchDtos.next(paginatedListOfDTOs); + })); } /** diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 5ddde2a307..cb1453beed 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -402,6 +402,8 @@ "admin.access-control.groups.form.members-list.no-items": "No EPeople found in that search", + "admin.access-control.groups.form.subgroups-list.notification.failure": "Something went wrong: \"{{cause}}\"", + "admin.access-control.groups.form.subgroups-list.head": "Groups", "admin.access-control.groups.form.subgroups-list.search.head": "Add Subgroup",