Skip to content

Commit 3fedefa

Browse files
MatusBekeclaudeMatus Kasak
committed
UFAL/Fix misleading notification when admin self-delete is rejected (#1357)
* Fix misleading message when self-delete rejection lacks a matched error text isSelfDeletionError() matches the backend's rejection message as plain text, but Spring Boot omits exception messages from error response bodies by default and DSpaceBadRequestException/IllegalStateException have no dedicated JSON-body exception handler, so the match can silently fail and fall through to the generic, unfriendly failure notification instead of the "you cannot delete your own account" one. Add a deterministic client-side identity check as a fallback alongside the text match so the friendly message shows reliably regardless of what the backend's error body contains. Fixes dataquest-dev/dspace-customers#782 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Removed redundant comments --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: Matus Kasak <matus.kasak@dataquest.sk> (cherry picked from commit 43cf493)
1 parent bd12749 commit 3fedefa

4 files changed

Lines changed: 36 additions & 4 deletions

File tree

src/app/access-control/epeople-registry/epeople-registry.component.spec.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { Router } from '@angular/router';
2-
import { Observable, of as observableOf, throwError as observableThrowError } from 'rxjs';
2+
import { defer, Observable, of as observableOf, throwError as observableThrowError } from 'rxjs';
33
import { CommonModule } from '@angular/common';
44
import { NO_ERRORS_SCHEMA } from '@angular/core';
55
import { ComponentFixture, fakeAsync, TestBed, tick, waitForAsync } from '@angular/core/testing';
@@ -381,6 +381,23 @@ describe('EPeopleRegistryComponent', () => {
381381
expect(modalService.open).not.toHaveBeenCalled();
382382
expect(deleteSpy).not.toHaveBeenCalled();
383383
}));
384+
385+
it('should still show the friendly self-delete notification if the authenticated user id resolves late and the backend rejection carries no usable message', fakeAsync(() => {
386+
modalRef.componentInstance.response = observableOf(true);
387+
component.currentAuthenticatedUserId = EPersonMock.id;
388+
ePersonDataServiceStub.deleteEPerson = jasmine.createSpy('deleteEPerson').and.returnValue(defer(() => {
389+
component.currentAuthenticatedUserId = EPersonMock2.id;
390+
return createFailedRemoteDataObject$(undefined, 400);
391+
}));
392+
393+
component.deleteEPerson(EPersonMock2);
394+
tick();
395+
396+
expect(notificationsService.error).toHaveBeenCalled();
397+
let translatedKey: string;
398+
notificationsService.error.calls.mostRecent().args[0].subscribe((value) => translatedKey = value);
399+
expect(translatedKey).toBe('admin.access-control.epeople.notification.deleted.forbidden.self');
400+
}));
384401
});
385402

386403
describe('delete EPerson button when the isAuthorized returns false', () => {

src/app/access-control/epeople-registry/epeople-registry.component.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ export class EPeopleRegistryComponent implements OnInit, OnDestroy {
233233
this.epersonService.deleteEPerson(ePerson).pipe(getFirstCompletedRemoteData()).subscribe((restResponse: RemoteData<NoContent>) => {
234234
if (restResponse.hasSucceeded) {
235235
this.notificationsService.success(this.translateService.get(this.labelPrefix + 'notification.deleted.success', {name: this.dsoNameService.getName(ePerson)}));
236-
} else if (this.deleteGuard.isSelfDeletionError(restResponse)) {
236+
} else if (this.isCurrentUser(ePerson) || this.deleteGuard.isSelfDeletionError(restResponse)) {
237237
this.deleteGuard.showSelfDeleteNotification();
238238
} else {
239239
this.notificationsService.error(this.translateService.get(this.labelPrefix + 'notification.deleted.failure', {

src/app/access-control/epeople-registry/eperson-form/eperson-form.component.spec.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Observable, of as observableOf, throwError as observableThrowError } from 'rxjs';
1+
import { defer, Observable, of as observableOf, throwError as observableThrowError } from 'rxjs';
22
import { FeatureID } from '../../../core/data/feature-authorization/feature-id';
33
import { EPersonDeleteGuardService } from '../eperson-delete-guard.service';
44
import { CommonModule } from '@angular/common';
@@ -635,6 +635,21 @@ describe('EPersonFormComponent', () => {
635635
expect(modalService.open).not.toHaveBeenCalled();
636636
expect(deleteSpy).not.toHaveBeenCalled();
637637
});
638+
639+
it('should still show the friendly self-delete notification if the authenticated user id resolves late and the backend rejection carries no usable message', () => {
640+
spyOn(component.epersonService, 'deleteEPerson').and.returnValue(defer(() => {
641+
component.currentAuthenticatedUserId = eperson.id;
642+
return createFailedRemoteDataObject$(undefined, 400);
643+
}));
644+
645+
const deleteButton = fixture.debugElement.query(By.css('.delete-button'));
646+
deleteButton.triggerEventHandler('click', null);
647+
648+
expect(notificationsService.error).toHaveBeenCalled();
649+
let translatedKey: string;
650+
notificationsService.error.calls.mostRecent().args[0].subscribe((value) => translatedKey = value);
651+
expect(translatedKey).toBe('admin.access-control.epeople.notification.deleted.forbidden.self');
652+
});
638653
});
639654

640655
describe('self delete button', () => {

src/app/access-control/epeople-registry/eperson-form/eperson-form.component.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -544,7 +544,7 @@ export class EPersonFormComponent implements OnInit, OnDestroy {
544544
if (restResponse?.hasSucceeded) {
545545
this.notificationsService.success(this.translateService.get(this.labelPrefix + 'notification.deleted.success', { name: this.dsoNameService.getName(eperson) }));
546546
void this.router.navigate([getEPersonsRoute()]);
547-
} else if (this.deleteGuard.isSelfDeletionError(restResponse)) {
547+
} else if (this.isCurrentUser(eperson) || this.deleteGuard.isSelfDeletionError(restResponse)) {
548548
this.deleteGuard.showSelfDeleteNotification();
549549
} else {
550550
this.notificationsService.error(this.translateService.get(this.labelPrefix + 'notification.deleted.failure', {

0 commit comments

Comments
 (0)