From 3de2b50d10f90c566fedcb1542891adf62b3e151 Mon Sep 17 00:00:00 2001 From: Raf Ponsaerts Date: Wed, 23 Sep 2020 15:07:38 +0200 Subject: [PATCH] [Task 72956] applied feedback to the deletion of epersons --- .../epeople-registry.component.spec.ts | 9 ++ .../epeople-registry.component.ts | 58 ++++++++--- .../eperson-form.component.spec.ts | 14 ++- .../eperson-form/eperson-form.component.ts | 99 ++++++++++++------- src/assets/i18n/en.json5 | 11 +++ 5 files changed, 141 insertions(+), 50 deletions(-) diff --git a/src/app/+admin/admin-access-control/epeople-registry/epeople-registry.component.spec.ts b/src/app/+admin/admin-access-control/epeople-registry/epeople-registry.component.spec.ts index 81a66e4782..d9ded99b18 100644 --- a/src/app/+admin/admin-access-control/epeople-registry/epeople-registry.component.spec.ts +++ b/src/app/+admin/admin-access-control/epeople-registry/epeople-registry.component.spec.ts @@ -25,6 +25,9 @@ import { TranslateLoaderMock } from '../../../shared/mocks/translate-loader.mock import { NotificationsServiceStub } from '../../../shared/testing/notifications-service.stub'; import { RouterStub } from '../../../shared/testing/router.stub'; import { AuthorizationDataService } from '../../../core/data/feature-authorization/authorization-data.service'; +import { ObjectCacheService } from '../../../core/cache/object-cache.service'; +import { UUIDService } from '../../../core/shared/uuid.service'; +import { Store } from '@ngrx/store'; describe('EPeopleRegistryComponent', () => { let component: EPeopleRegistryComponent; @@ -35,6 +38,7 @@ describe('EPeopleRegistryComponent', () => { let mockEPeople; let ePersonDataServiceStub: any; let authorizationService: AuthorizationDataService; + let modalService; beforeEach(async(() => { mockEPeople = [EPersonMock, EPersonMock2]; @@ -104,6 +108,9 @@ describe('EPeopleRegistryComponent', () => { { provide: NotificationsService, useValue: new NotificationsServiceStub() }, { provide: FormBuilderService, useValue: builderService }, { provide: AuthorizationDataService, useValue: authorizationService }, + { provide: ObjectCacheService, useValue: {} }, + { provide: UUIDService, useValue: {} }, + { provide: Store, useValue: {} }, { provide: Router, useValue: new RouterStub() }, ], schemas: [NO_ERRORS_SCHEMA] @@ -113,6 +120,8 @@ describe('EPeopleRegistryComponent', () => { beforeEach(() => { fixture = TestBed.createComponent(EPeopleRegistryComponent); component = fixture.componentInstance; + modalService = (component as any).modalService; + spyOn(modalService, 'open').and.returnValue(Object.assign({ componentInstance: Object.assign({ response: observableOf(true) }) })); fixture.detectChanges(); }); diff --git a/src/app/+admin/admin-access-control/epeople-registry/epeople-registry.component.ts b/src/app/+admin/admin-access-control/epeople-registry/epeople-registry.component.ts index e073a39a0f..aa237531b8 100644 --- a/src/app/+admin/admin-access-control/epeople-registry/epeople-registry.component.ts +++ b/src/app/+admin/admin-access-control/epeople-registry/epeople-registry.component.ts @@ -17,6 +17,9 @@ import { FeatureID } from '../../../core/data/feature-authorization/feature-id'; import { AuthorizationDataService } from '../../../core/data/feature-authorization/authorization-data.service'; import { getAllSucceededRemoteDataPayload } from '../../../core/shared/operators'; import { RestResponse } from '../../../core/cache/response.models'; +import { ConfirmationModalComponent } from '../../../shared/confirmation-modal/confirmation-modal.component'; +import { NgbModal } from '@ng-bootstrap/ng-bootstrap'; +import { RequestService } from '../../../core/data/request.service'; @Component({ selector: 'ds-epeople-registry', @@ -71,7 +74,9 @@ export class EPeopleRegistryComponent implements OnInit, OnDestroy { private notificationsService: NotificationsService, private authorizationService: AuthorizationDataService, private formBuilder: FormBuilder, - private router: Router) { + private router: Router, + private modalService: NgbModal, + public requestService: RequestService) { this.currentSearchQuery = ''; this.currentSearchScope = 'metadata'; this.searchForm = this.formBuilder.group(({ @@ -81,6 +86,13 @@ export class EPeopleRegistryComponent implements OnInit, OnDestroy { } ngOnInit() { + this.initialisePage(); + } + + /** + * This method will initialise the page + */ + initialisePage() { this.isEPersonFormShown = false; this.search({ scope: this.currentSearchScope, query: this.currentSearchQuery }); this.subs.push(this.epersonService.getActiveEPerson().subscribe((eperson: EPerson) => { @@ -191,16 +203,28 @@ export class EPeopleRegistryComponent implements OnInit, OnDestroy { */ deleteEPerson(ePerson: EPerson) { if (hasValue(ePerson.id)) { - this.epersonService.deleteEPerson(ePerson).pipe(take(1)).subscribe((restResponse: RestResponse) => { - if (restResponse.isSuccessful) { - this.notificationsService.success(this.translateService.get(this.labelPrefix + 'notification.deleted.success', { name: ePerson.name })); - this.forceUpdateEPeople(); - } else { - this.notificationsService.error('Error occured when trying to delete EPerson with id: ' + ePerson.id + ' with code: ' + restResponse.statusCode + ' and message: ' + restResponse.statusText); - } - this.epersonService.cancelEditEPerson(); - this.isEPersonFormShown = false; - }) + const modalRef = this.modalService.open(ConfirmationModalComponent); + modalRef.componentInstance.dso = ePerson; + modalRef.componentInstance.headerLabel = 'confirmation-modal.delete-eperson.header'; + modalRef.componentInstance.infoLabel = 'confirmation-modal.delete-eperson.info'; + modalRef.componentInstance.cancelLabel = 'confirmation-modal.delete-eperson.cancel'; + modalRef.componentInstance.confirmLabel = 'confirmation-modal.delete-eperson.confirm'; + modalRef.componentInstance.response.pipe(take(1)).subscribe((confirm: boolean) => { + if (confirm) { + if (hasValue(ePerson.id)) { + this.epersonService.deleteEPerson(ePerson).pipe(take(1)).subscribe((restResponse: RestResponse) => { + if (restResponse.isSuccessful) { + this.notificationsService.success(this.translateService.get(this.labelPrefix + 'notification.deleted.success', { name: ePerson.name })); + this.reset(); + this.forceUpdateEPeople(); + } else { + this.notificationsService.error('Error occured when trying to delete EPerson with id: ' + ePerson.id + ' with code: ' + restResponse.statusCode + ' and message: ' + restResponse.statusText); + } + this.epersonService.cancelEditEPerson(); + this.isEPersonFormShown = false; + }) + }} + }); } } @@ -230,4 +254,16 @@ export class EPeopleRegistryComponent implements OnInit, OnDestroy { }); this.search({ query: '' }); } + + /** + * This method will ensure that the page gets reset and that the cache is cleared + */ + reset() { + this.ePeopleDto$.pipe(take(1)).subscribe((epersons: PaginatedList) => { + epersons.page.forEach((eperson: EpersonDtoModel) => { + this.requestService.removeByHrefSubstring(eperson.eperson.self); + }) + }); + this.initialisePage(); + } } diff --git a/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.spec.ts b/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.spec.ts index facdae1524..1b5ac1576b 100644 --- a/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.spec.ts +++ b/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.spec.ts @@ -2,11 +2,11 @@ import { HttpClient } from '@angular/common/http'; import { Store } from '@ngrx/store'; import { of as observableOf } from 'rxjs'; import { CommonModule } from '@angular/common'; -import { NO_ERRORS_SCHEMA } from '@angular/core'; +import { EventEmitter, NgModule, NO_ERRORS_SCHEMA } from '@angular/core'; import { async, ComponentFixture, inject, TestBed, tick } from '@angular/core/testing'; import { FormsModule, ReactiveFormsModule } from '@angular/forms'; import { BrowserModule, By } from '@angular/platform-browser'; -import { NgbModule } from '@ng-bootstrap/ng-bootstrap'; +import { NgbModal, NgbModule, NgbModalModule, NgbActiveModal } from '@ng-bootstrap/ng-bootstrap'; import { TranslateLoader, TranslateModule, TranslateService } from '@ngx-translate/core'; import { Observable } from 'rxjs/internal/Observable'; import { RemoteDataBuildService } from '../../../../core/cache/builders/remote-data-build.service'; @@ -286,16 +286,19 @@ describe('EPersonFormComponent', () => { let ePersonId; let eperson: EPerson; + let modalService; beforeEach(() => { spyOn(authService, 'impersonate').and.callThrough(); ePersonId = 'testEPersonId'; - eperson = Object.assign(new EPerson(), { - id: ePersonId - }); + eperson = EPersonMock; component.epersonInitial = eperson; component.canDelete$ = observableOf(true); spyOn(component.epersonService, 'getActiveEPerson').and.returnValue(observableOf(eperson)); + modalService = (component as any).modalService; + spyOn(modalService, 'open').and.returnValue(Object.assign({ componentInstance: Object.assign({ response: observableOf(true) }) })); + fixture.detectChanges() + }); it ('the delete button should be active if the eperson can be deleted', () => { @@ -312,6 +315,7 @@ describe('EPersonFormComponent', () => { it ('should call the epersonFormComponent delete when clicked on the button' , () => { spyOn(component, 'delete').and.stub(); + spyOn(component.epersonService, 'deleteEPerson').and.returnValue(observableOf(new RestResponse(true, 204, 'No Content'))); const deleteButton = fixture.debugElement.query(By.css('.delete-button')); deleteButton.triggerEventHandler('click', null); expect(component.delete).toHaveBeenCalled(); diff --git a/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.ts b/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.ts index 0dacd93e29..8ed1bd0940 100644 --- a/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.ts +++ b/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.ts @@ -7,7 +7,7 @@ import { DynamicInputModel } from '@ng-dynamic-forms/core'; import { TranslateService } from '@ngx-translate/core'; -import { Subscription, combineLatest, of } from 'rxjs'; +import { Subscription, combineLatest, of, of as observableOf } from 'rxjs'; import { Observable } from 'rxjs/internal/Observable'; import { switchMap, take } from 'rxjs/operators'; import { RestResponse } from '../../../../core/cache/response.models'; @@ -25,6 +25,9 @@ import { PaginationComponentOptions } from '../../../../shared/pagination/pagina import { AuthService } from '../../../../core/auth/auth.service'; import { AuthorizationDataService } from '../../../../core/data/feature-authorization/authorization-data.service'; import { FeatureID } from '../../../../core/data/feature-authorization/feature-id'; +import { ConfirmationModalComponent } from '../../../../shared/confirmation-modal/confirmation-modal.component'; +import { NgbModal } from '@ng-bootstrap/ng-bootstrap'; +import { RequestService } from '../../../../core/data/request.service'; @Component({ selector: 'ds-eperson-form', @@ -159,7 +162,9 @@ export class EPersonFormComponent implements OnInit, OnDestroy { private translateService: TranslateService, private notificationsService: NotificationsService, private authService: AuthService, - private authorizationService: AuthorizationDataService) { + private authorizationService: AuthorizationDataService, + private modalService: NgbModal, + public requestService: RequestService) { this.subs.push(this.epersonService.getActiveEPerson().subscribe((eperson: EPerson) => { this.epersonInitial = eperson; if (hasValue(eperson)) { @@ -169,13 +174,20 @@ export class EPersonFormComponent implements OnInit, OnDestroy { } ngOnInit() { + this.initialisePage(); + } + + /** + * This method will initialise the page + */ + initialisePage() { combineLatest( - this.translateService.get(`${this.messagePrefix}.firstName`), - this.translateService.get(`${this.messagePrefix}.lastName`), - this.translateService.get(`${this.messagePrefix}.email`), - this.translateService.get(`${this.messagePrefix}.canLogIn`), - this.translateService.get(`${this.messagePrefix}.requireCertificate`), - this.translateService.get(`${this.messagePrefix}.emailHint`), + this.translateService.get(`${this.messagePrefix}.firstName`), + this.translateService.get(`${this.messagePrefix}.lastName`), + this.translateService.get(`${this.messagePrefix}.email`), + this.translateService.get(`${this.messagePrefix}.canLogIn`), + this.translateService.get(`${this.messagePrefix}.requireCertificate`), + this.translateService.get(`${this.messagePrefix}.emailHint`), ).subscribe(([firstName, lastName, email, canLogIn, requireCertificate, emailHint]) => { this.firstName = new DynamicInputModel({ id: 'firstName', @@ -207,19 +219,19 @@ export class EPersonFormComponent implements OnInit, OnDestroy { hint: emailHint }); this.canLogIn = new DynamicCheckboxModel( - { - id: 'canLogIn', - label: canLogIn, - name: 'canLogIn', - value: (this.epersonInitial != null ? this.epersonInitial.canLogIn : true) - }); + { + id: 'canLogIn', + label: canLogIn, + name: 'canLogIn', + value: (this.epersonInitial != null ? this.epersonInitial.canLogIn : true) + }); this.requireCertificate = new DynamicCheckboxModel( - { - id: 'requireCertificate', - label: requireCertificate, - name: 'requireCertificate', - value: (this.epersonInitial != null ? this.epersonInitial.requireCertificate : false) - }); + { + id: 'requireCertificate', + label: requireCertificate, + name: 'requireCertificate', + value: (this.epersonInitial != null ? this.epersonInitial.requireCertificate : false) + }); this.formModel = [ this.firstName, this.lastName, @@ -244,7 +256,7 @@ export class EPersonFormComponent implements OnInit, OnDestroy { }); })); this.canImpersonate$ = this.epersonService.getActiveEPerson().pipe( - switchMap((eperson) => this.authorizationService.isAuthorized(FeatureID.LoginOnBehalfOf, hasValue(eperson) ? eperson.self : undefined)) + switchMap((eperson) => this.authorizationService.isAuthorized(FeatureID.LoginOnBehalfOf, hasValue(eperson) ? eperson.self : undefined)) ); this.canDelete$ = this.epersonService.getActiveEPerson().pipe( switchMap((eperson) => this.authorizationService.isAuthorized(FeatureID.CanDelete, hasValue(eperson) ? eperson.self : undefined)) @@ -412,19 +424,28 @@ export class EPersonFormComponent implements OnInit, OnDestroy { * It'll either show a success or error message depending on whether the delete was successful or not. */ delete() { - this.epersonService.getActiveEPerson().pipe(take(1)).subscribe((eperson: EPerson) => { - if (hasValue(eperson.id)) { - this.epersonService.deleteEPerson(eperson).pipe(take(1)).subscribe((restResponse: RestResponse) => { - if (restResponse.isSuccessful) { - this.notificationsService.success(this.translateService.get(this.labelPrefix + 'notification.deleted.success', { name: eperson.name })); - } else { - this.notificationsService.error('Error occured when trying to delete EPerson with id: ' + eperson.id + ' with code: ' + restResponse.statusCode + ' and message: ' + restResponse.statusText); - } - this.cancelForm.emit(); - }) - }} - ) - + this.epersonService.getActiveEPerson().pipe(take(1)).subscribe((eperson: EPerson) => { + const modalRef = this.modalService.open(ConfirmationModalComponent); + modalRef.componentInstance.dso = eperson; + modalRef.componentInstance.headerLabel = 'confirmation-modal.delete-eperson.header'; + modalRef.componentInstance.infoLabel = 'confirmation-modal.delete-eperson.info'; + modalRef.componentInstance.cancelLabel = 'confirmation-modal.delete-eperson.cancel'; + modalRef.componentInstance.confirmLabel = 'confirmation-modal.delete-eperson.confirm'; + modalRef.componentInstance.response.pipe(take(1)).subscribe((confirm: boolean) => { + if (confirm) { + if (hasValue(eperson.id)) { + this.epersonService.deleteEPerson(eperson).pipe(take(1)).subscribe((restResponse: RestResponse) => { + if (restResponse.isSuccessful) { + this.notificationsService.success(this.translateService.get(this.labelPrefix + 'notification.deleted.success', { name: eperson.name })); + this.reset(); + } else { + this.notificationsService.error('Error occured when trying to delete EPerson with id: ' + eperson.id + ' with code: ' + restResponse.statusCode + ' and message: ' + restResponse.statusText); + } + this.cancelForm.emit(); + }) + }} + }); + }) } /** @@ -442,4 +463,14 @@ export class EPersonFormComponent implements OnInit, OnDestroy { this.onCancel(); this.subs.filter((sub) => hasValue(sub)).forEach((sub) => sub.unsubscribe()); } + + /** + * This method will ensure that the page gets reset and that the cache is cleared + */ + reset() { + this.epersonService.getActiveEPerson().pipe(take(1)).subscribe((eperson: EPerson) => { + this.requestService.removeByHrefSubstring(eperson.self); + }); + this.initialisePage(); + } } diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index b6a7c7f7aa..89838a9c0e 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -258,6 +258,10 @@ "admin.access-control.epeople.form.notification.edited.failure": "Failed to edit EPerson \"{{name}}\"", + "admin.access-control.epeople.form.notification.deleted.success": "Successfully deleted EPerson \"{{name}}\"", + + "admin.access-control.epeople.form.notification.deleted.failure": "Failed to delete EPerson \"{{name}}\"", + "admin.access-control.epeople.form.groupsEPersonIsMemberOf": "Member of these groups:", "admin.access-control.epeople.form.table.id": "ID", @@ -1003,6 +1007,13 @@ "confirmation-modal.export-metadata.confirm": "Export", + "confirmation-modal.delete-eperson.header": "Delete EPerson \"{{ dsoName }}\"", + + "confirmation-modal.delete-eperson.info": "Are you sure you want to delete EPerson \"{{ dsoName }}\"", + + "confirmation-modal.delete-eperson.cancel": "Cancel", + + "confirmation-modal.delete-eperson.confirm": "Delete", "error.bitstream": "Error fetching bitstream",