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 1b0386dee2..591d80bdd5 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 @@ -20,25 +20,33 @@ - - {{eperson.id}} + + {{epersonDTO.eperson.id}} - - {{ dsoNameService.getName(eperson) }} + + {{ dsoNameService.getName(epersonDTO.eperson) }} - {{messagePrefix + '.table.email' | translate}}: {{ eperson.email ? eperson.email : '-' }}
- {{messagePrefix + '.table.netid' | translate}}: {{ eperson.netid ? eperson.netid : '-' }} + {{messagePrefix + '.table.email' | translate}}: {{ epersonDTO.eperson.email ? epersonDTO.eperson.email : '-' }}
+ {{messagePrefix + '.table.netid' | translate}}: {{ epersonDTO.eperson.netid ? epersonDTO.eperson.netid : '-' }}
- +
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 9cc7727557..9a6b2b4f05 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 @@ -222,13 +222,13 @@ describe('MembersListComponent', () => { describe('if first delete button is pressed', () => { beforeEach(() => { + spyOn(component, 'search').and.callThrough(); 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); + it('should trigger the search to add the user back to the search table', () => { + expect(component.search).toHaveBeenCalled(); }); }); }); @@ -264,13 +264,13 @@ describe('MembersListComponent', () => { describe('if first add button is pressed', () => { beforeEach(() => { + spyOn(component, 'search').and.callThrough(); const addButton: DebugElement = fixture.debugElement.query(By.css('#epersonsSearch tbody .fa-plus')); addButton.nativeElement.click(); fixture.detectChanges(); }); - 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(0); + it('should trigger the search to remove the user from the search table', () => { + expect(component.search).toHaveBeenCalled(); }); }); }); 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 4a707ef159..9b123ae447 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 @@ -24,21 +24,29 @@ import { } from '@ngx-translate/core'; import { BehaviorSubject, + combineLatest as observableCombineLatest, Observable, + ObservedValueOf, + of as observableOf, Subscription, } from 'rxjs'; import { + defaultIfEmpty, map, switchMap, take, } from 'rxjs/operators'; import { DSONameService } from '../../../../core/breadcrumbs/dso-name.service'; -import { PaginatedList } from '../../../../core/data/paginated-list.model'; +import { + buildPaginatedList, + 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'; import { EPerson } from '../../../../core/eperson/models/eperson.model'; +import { EpersonDtoModel } from '../../../../core/eperson/models/eperson-dto.model'; import { Group } from '../../../../core/eperson/models/group.model'; import { PaginationService } from '../../../../core/pagination/pagination.service'; import { @@ -137,7 +145,7 @@ export class MembersListComponent implements OnInit, OnDestroy { /** * List of EPeople members of currently active group being edited */ - ePeopleMembersOfGroup: BehaviorSubject> = new BehaviorSubject>(undefined); + ePeopleMembersOfGroup: BehaviorSubject> = new BehaviorSubject(undefined); /** * Pagination config used to display the list of EPeople that are result of EPeople search @@ -226,10 +234,35 @@ export class MembersListComponent implements OnInit, OnDestroy { return rd; } }), - getRemoteDataPayload()) - .subscribe((paginatedListOfEPersons: PaginatedList) => { - this.ePeopleMembersOfGroup.next(paginatedListOfEPersons); - })); + 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.ableToDelete = isMember; + return epersonDtoModel; + }); + return dto$; + })]); + return dtos$.pipe(defaultIfEmpty([]), map((dtos: EpersonDtoModel[]) => { + return buildPaginatedList(epersonListRD.payload.pageInfo, dtos); + })); + }), + ).subscribe((paginatedListOfDTOs: PaginatedList) => { + this.ePeopleMembersOfGroup.next(paginatedListOfDTOs); + }), + ); + } + + /** + * We always return true since this is only used by the top section (which represents all the users part of the group + * in {@link MembersListComponent}) + * + * @param possibleMember EPerson that is a possible member (being tested) of the group currently being edited + */ + isMemberOfGroup(possibleMember: EPerson): Observable { + return observableOf(true); } /** diff --git a/src/app/core/eperson/eperson-data.service.ts b/src/app/core/eperson/eperson-data.service.ts index e0672e3207..0de6de7407 100644 --- a/src/app/core/eperson/eperson-data.service.ts +++ b/src/app/core/eperson/eperson-data.service.ts @@ -107,13 +107,17 @@ export class EPersonDataService extends IdentifiableDataService impleme * @param scope Scope of the EPeople search, default byMetadata * @param query Query of search * @param options Options of search request + * @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 */ - public searchByScope(scope: string, query: string, options: FindListOptions = {}, useCachedVersionIfAvailable?: boolean): Observable>> { + public searchByScope(scope: string, query: string, options: FindListOptions = {}, useCachedVersionIfAvailable?: boolean, reRequestOnStale = true): Observable>> { switch (scope) { case 'metadata': - return this.getEpeopleByMetadata(query.trim(), options, useCachedVersionIfAvailable); + return this.getEpeopleByMetadata(query.trim(), options, useCachedVersionIfAvailable, reRequestOnStale); case 'email': - return this.getEPersonByEmail(query.trim()).pipe( + return this.getEPersonByEmail(query.trim(), useCachedVersionIfAvailable, reRequestOnStale).pipe( map((rd: RemoteData) => { if (rd.hasSucceeded) { // Turn the single EPerson or NoContent in to a PaginatedList @@ -145,7 +149,7 @@ export class EPersonDataService extends IdentifiableDataService impleme }), ); default: - return this.getEpeopleByMetadata(query.trim(), options, useCachedVersionIfAvailable); + return this.getEpeopleByMetadata(query.trim(), options, useCachedVersionIfAvailable, reRequestOnStale); } } diff --git a/src/app/item-page/edit-item-page/modify-item-overview/modify-item-overview.component.spec.ts b/src/app/item-page/edit-item-page/modify-item-overview/modify-item-overview.component.spec.ts index 45759aa90b..a722d07c9d 100644 --- a/src/app/item-page/edit-item-page/modify-item-overview/modify-item-overview.component.spec.ts +++ b/src/app/item-page/edit-item-page/modify-item-overview/modify-item-overview.component.spec.ts @@ -37,6 +37,7 @@ describe('ModifyItemOverviewComponent', () => { fixture = TestBed.createComponent(ModifyItemOverviewComponent); comp = fixture.componentInstance; comp.item = mockItem; + comp.ngOnChanges(); fixture.detectChanges(); }); diff --git a/src/app/item-page/edit-item-page/modify-item-overview/modify-item-overview.component.ts b/src/app/item-page/edit-item-page/modify-item-overview/modify-item-overview.component.ts index 2d4b622ea9..3bd5024837 100644 --- a/src/app/item-page/edit-item-page/modify-item-overview/modify-item-overview.component.ts +++ b/src/app/item-page/edit-item-page/modify-item-overview/modify-item-overview.component.ts @@ -5,7 +5,7 @@ import { import { Component, Input, - OnInit, + OnChanges, } from '@angular/core'; import { TranslateModule } from '@ngx-translate/core'; @@ -21,12 +21,12 @@ import { MetadataMap } from '../../../core/shared/metadata.models'; /** * Component responsible for rendering a table containing the metadatavalues from the to be edited item */ -export class ModifyItemOverviewComponent implements OnInit { +export class ModifyItemOverviewComponent implements OnChanges { @Input() item: Item; metadata: MetadataMap; - ngOnInit(): void { - this.metadata = this.item.metadata; + ngOnChanges(): void { + this.metadata = this.item?.metadata; } } diff --git a/src/app/shared/mydspace-actions/claimed-task/abstract/claimed-task-actions-abstract.component.ts b/src/app/shared/mydspace-actions/claimed-task/abstract/claimed-task-actions-abstract.component.ts index 2af6713885..3d410ca8f6 100644 --- a/src/app/shared/mydspace-actions/claimed-task/abstract/claimed-task-actions-abstract.component.ts +++ b/src/app/shared/mydspace-actions/claimed-task/abstract/claimed-task-actions-abstract.component.ts @@ -1,6 +1,7 @@ import { Component, Injector, + Input, OnDestroy, } from '@angular/core'; import { Router } from '@angular/router'; @@ -44,7 +45,7 @@ export abstract class ClaimedTaskActionsAbstractComponent extends MyDSpaceReload /** * The item object that belonging to the ClaimedTask object */ - item: Item; + @Input() item: Item; /** * Anchor used to reload the pool task. @@ -56,7 +57,7 @@ export abstract class ClaimedTaskActionsAbstractComponent extends MyDSpaceReload /** * The workflowitem object that belonging to the ClaimedTask object */ - workflowitem: WorkflowItem; + @Input() workflowitem: WorkflowItem; protected constructor(protected injector: Injector, protected router: Router, diff --git a/src/app/shared/mydspace-actions/claimed-task/switcher/claimed-task-actions-decorator.ts b/src/app/shared/mydspace-actions/claimed-task/switcher/claimed-task-actions-decorator.ts index 13961db9e1..08948841c8 100644 --- a/src/app/shared/mydspace-actions/claimed-task/switcher/claimed-task-actions-decorator.ts +++ b/src/app/shared/mydspace-actions/claimed-task/switcher/claimed-task-actions-decorator.ts @@ -1,8 +1,10 @@ import { + ADVANCED_WORKFLOW_ACTION_RATING, ADVANCED_WORKFLOW_TASK_OPTION_RATING, AdvancedWorkflowActionRatingComponent, } from '../../../../workflowitems-edit-page/advanced-workflow-action/advanced-workflow-action-rating/advanced-workflow-action-rating.component'; import { + ADVANCED_WORKFLOW_ACTION_SELECT_REVIEWER, ADVANCED_WORKFLOW_TASK_OPTION_SELECT_REVIEWER, AdvancedWorkflowActionSelectReviewerComponent, } from '../../../../workflowitems-edit-page/advanced-workflow-action/advanced-workflow-action-select-reviewer/advanced-workflow-action-select-reviewer.component'; @@ -54,8 +56,8 @@ export const WORKFLOW_TASK_OPTION_DECORATOR_MAP = new Map([ - [ADVANCED_WORKFLOW_TASK_OPTION_RATING, AdvancedWorkflowActionRatingComponent], - [ADVANCED_WORKFLOW_TASK_OPTION_SELECT_REVIEWER, AdvancedWorkflowActionSelectReviewerComponent], + [ADVANCED_WORKFLOW_ACTION_RATING, AdvancedWorkflowActionRatingComponent], + [ADVANCED_WORKFLOW_ACTION_SELECT_REVIEWER, AdvancedWorkflowActionSelectReviewerComponent], ]); /** 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 8dee58795d..e826de1c0e 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 @@ -224,6 +224,38 @@ describe('ReviewersListComponent', () => { })).not.toBeTruthy(); }); }); + + it('should replace the value when a new member is added when multipleReviewers is false', () => { + spyOn(component.selectedReviewersUpdated, 'emit'); + component.multipleReviewers = false; + component.selectedReviewers = [EPersonMock]; + + component.addMemberToGroup(EPersonMock2); + + 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 = [EPersonMock]; + + component.addMemberToGroup(EPersonMock2); + + expect(component.selectedReviewers).toEqual([EPersonMock, EPersonMock2]); + expect(component.selectedReviewersUpdated.emit).toHaveBeenCalledWith([EPersonMock, EPersonMock2]); + }); + + it('should delete the member when present', () => { + spyOn(component.selectedReviewersUpdated, 'emit'); + component.selectedReviewers = [EPersonMock]; + + component.deleteMemberFromGroup(EPersonMock); + + expect(component.selectedReviewers).toEqual([]); + expect(component.selectedReviewersUpdated.emit).toHaveBeenCalledWith([]); + }); }); describe('when a group is selected', () => { @@ -245,37 +277,4 @@ 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 = [EPersonMock]; - - component.addMemberToGroup(EPersonMock2); - - 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 = [EPersonMock]; - - component.addMemberToGroup(EPersonMock2); - - expect(component.selectedReviewers).toEqual([EPersonMock, EPersonMock2]); - expect(component.selectedReviewersUpdated.emit).toHaveBeenCalledWith([EPersonMock, EPersonMock2]); - }); - - it('should delete the member when present', () => { - spyOn(component.selectedReviewersUpdated, 'emit'); - component.selectedReviewers = [EPersonMock]; - - component.deleteMemberFromGroup(EPersonMock); - - expect(component.selectedReviewers).toEqual([]); - 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 830e520655..5ae3a13f31 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 @@ -26,6 +26,10 @@ import { TranslateModule, TranslateService, } from '@ngx-translate/core'; +import { + Observable, + of as observableOf, +} from 'rxjs'; import { EPersonListActionConfig, @@ -36,10 +40,12 @@ import { PaginatedList } from '../../../../core/data/paginated-list.model'; import { EPersonDataService } from '../../../../core/eperson/eperson-data.service'; import { GroupDataService } from '../../../../core/eperson/group-data.service'; import { EPerson } from '../../../../core/eperson/models/eperson.model'; +import { EpersonDtoModel } from '../../../../core/eperson/models/eperson-dto.model'; import { Group } from '../../../../core/eperson/models/group.model'; import { PaginationService } from '../../../../core/pagination/pagination.service'; import { getFirstSucceededRemoteDataPayload } from '../../../../core/shared/operators'; import { ContextHelpDirective } from '../../../../shared/context-help.directive'; +import { hasValue } from '../../../../shared/empty.util'; import { NotificationsService } from '../../../../shared/notifications/notifications.service'; import { PaginationComponent } from '../../../../shared/pagination/pagination.component'; @@ -101,7 +107,7 @@ export class ReviewersListComponent extends MembersListComponent implements OnIn super(groupService, ePersonDataService, translateService, notificationsService, formBuilder, paginationService, router, dsoNameService); } - ngOnInit() { + override ngOnInit(): void { this.searchForm = this.formBuilder.group(({ scope: 'metadata', query: '', @@ -114,6 +120,7 @@ export class ReviewersListComponent extends MembersListComponent implements OnIn if (this.groupId === null) { this.retrieveMembers(this.config.currentPage); } else { + this.unsubFrom(SubKey.ActiveGroup); this.subs.set(SubKey.ActiveGroup, this.groupService.findById(this.groupId).pipe( getFirstSucceededRemoteDataPayload(), ).subscribe((activeGroup: Group) => { @@ -136,25 +143,37 @@ export class ReviewersListComponent extends MembersListComponent implements OnIn retrieveMembers(page: number): void { this.config.currentPage = page; if (this.groupId === null) { - this.unsubFrom(SubKey.Members); - const paginatedListOfEPersons: PaginatedList = new PaginatedList(); - paginatedListOfEPersons.page = this.selectedReviewers; + const paginatedListOfEPersons: PaginatedList = new PaginatedList(); + paginatedListOfEPersons.page = this.selectedReviewers.map((ePerson: EPerson) => Object.assign(new EpersonDtoModel(), { + eperson: ePerson, + ableToDelete: this.isMemberOfGroup(ePerson), + })); this.ePeopleMembersOfGroup.next(paginatedListOfEPersons); } else { super.retrieveMembers(page); } } + /** + * Checks whether the given {@link possibleMember} is part of the {@link selectedReviewers}. + * + * @param possibleMember The {@link EPerson} that needs to be checked + */ + isMemberOfGroup(possibleMember: EPerson): Observable { + return observableOf(hasValue(this.selectedReviewers.find((reviewer: EPerson) => reviewer.id === possibleMember.id))); + } + /** * Removes the {@link eperson} from the {@link selectedReviewers} * * @param eperson The {@link EPerson} to remove */ deleteMemberFromGroup(eperson: EPerson) { - const index = this.selectedReviewers.indexOf(eperson); + const index = this.selectedReviewers.findIndex((reviewer: EPerson) => reviewer.id === eperson.id); if (index !== -1) { this.selectedReviewers.splice(index, 1); } + this.retrieveMembers(this.config.currentPage); this.selectedReviewersUpdated.emit(this.selectedReviewers); } @@ -169,6 +188,7 @@ export class ReviewersListComponent extends MembersListComponent implements OnIn this.selectedReviewers = []; } this.selectedReviewers.push(eperson); + this.retrieveMembers(this.config.currentPage); this.selectedReviewersUpdated.emit(this.selectedReviewers); }