From 8132270a099534dac2ac3b6eef7238ae171a7efe Mon Sep 17 00:00:00 2001 From: Matus Beke Date: Tue, 7 Jul 2026 11:08:25 +0200 Subject: [PATCH 1/2] 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 --- .../epeople-registry.component.spec.ts | 23 ++++++++++++++++++- .../epeople-registry.component.ts | 2 +- .../eperson-delete-guard.service.ts | 3 +++ .../eperson-form.component.spec.ts | 21 ++++++++++++++++- .../eperson-form/eperson-form.component.ts | 2 +- 5 files changed, 47 insertions(+), 4 deletions(-) diff --git a/src/app/access-control/epeople-registry/epeople-registry.component.spec.ts b/src/app/access-control/epeople-registry/epeople-registry.component.spec.ts index 5b3097dd6bd..dd89aeede8c 100644 --- a/src/app/access-control/epeople-registry/epeople-registry.component.spec.ts +++ b/src/app/access-control/epeople-registry/epeople-registry.component.spec.ts @@ -1,5 +1,5 @@ import { Router } from '@angular/router'; -import { Observable, of as observableOf, throwError as observableThrowError } from 'rxjs'; +import { defer, Observable, of as observableOf, throwError as observableThrowError } from 'rxjs'; import { CommonModule } from '@angular/common'; import { NO_ERRORS_SCHEMA } from '@angular/core'; import { ComponentFixture, fakeAsync, TestBed, tick, waitForAsync } from '@angular/core/testing'; @@ -382,6 +382,27 @@ describe('EPeopleRegistryComponent', () => { expect(modalService.open).not.toHaveBeenCalled(); expect(deleteSpy).not.toHaveBeenCalled(); })); + + 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(() => { + // Simulates the authenticated-user subscription resolving after the click (so the + // pre-flight self-delete check is bypassed) combined with a backend response whose error + // message can't be pattern-matched (e.g. Spring Boot's default message suppression). + // The self-delete notification must still win over the generic failure one. + modalRef.componentInstance.response = observableOf(true); + component.currentAuthenticatedUserId = EPersonMock.id; + ePersonDataServiceStub.deleteEPerson = jasmine.createSpy('deleteEPerson').and.returnValue(defer(() => { + component.currentAuthenticatedUserId = EPersonMock2.id; + return createFailedRemoteDataObject$(undefined, 400); + })); + + component.deleteEPerson(EPersonMock2); + tick(); + + expect(notificationsService.error).toHaveBeenCalled(); + let translatedKey: string; + notificationsService.error.calls.mostRecent().args[0].subscribe((value) => translatedKey = value); + expect(translatedKey).toBe('admin.access-control.epeople.notification.deleted.forbidden.self'); + })); }); describe('delete EPerson button when the isAuthorized returns false', () => { diff --git a/src/app/access-control/epeople-registry/epeople-registry.component.ts b/src/app/access-control/epeople-registry/epeople-registry.component.ts index 3e04efe00b5..14d9b245e29 100644 --- a/src/app/access-control/epeople-registry/epeople-registry.component.ts +++ b/src/app/access-control/epeople-registry/epeople-registry.component.ts @@ -224,7 +224,7 @@ export class EPeopleRegistryComponent implements OnInit, OnDestroy { this.epersonService.deleteEPerson(ePerson).pipe(getFirstCompletedRemoteData()).subscribe((restResponse: RemoteData) => { if (restResponse.hasSucceeded) { this.notificationsService.success(this.translateService.get(this.labelPrefix + 'notification.deleted.success', {name: this.dsoNameService.getName(ePerson)})); - } else if (this.deleteGuard.isSelfDeletionError(restResponse)) { + } else if (this.isCurrentUser(ePerson) || this.deleteGuard.isSelfDeletionError(restResponse)) { this.deleteGuard.showSelfDeleteNotification(); } else { this.notificationsService.error(this.translateService.get(this.labelPrefix + 'notification.deleted.failure', { diff --git a/src/app/access-control/epeople-registry/eperson-delete-guard.service.ts b/src/app/access-control/epeople-registry/eperson-delete-guard.service.ts index e5449da3758..22131bc9372 100644 --- a/src/app/access-control/epeople-registry/eperson-delete-guard.service.ts +++ b/src/app/access-control/epeople-registry/eperson-delete-guard.service.ts @@ -82,6 +82,9 @@ export class EPersonDeleteGuardService { /** * Whether the backend rejected the delete because an admin tried to delete themselves. + * Best-effort only: Spring Boot omits the exception message from the response body by + * default, so callers should treat this as a fallback alongside a client-side identity + * check rather than the sole signal. */ isSelfDeletionError(restResponse: RemoteData | null): boolean { return restResponse?.statusCode === 400 && restResponse?.errorMessage?.toLowerCase().includes('cannot delete yourself'); diff --git a/src/app/access-control/epeople-registry/eperson-form/eperson-form.component.spec.ts b/src/app/access-control/epeople-registry/eperson-form/eperson-form.component.spec.ts index c1edf22f15e..da4779158a5 100644 --- a/src/app/access-control/epeople-registry/eperson-form/eperson-form.component.spec.ts +++ b/src/app/access-control/epeople-registry/eperson-form/eperson-form.component.spec.ts @@ -1,4 +1,4 @@ -import { Observable, of as observableOf, throwError as observableThrowError } from 'rxjs'; +import { defer, Observable, of as observableOf, throwError as observableThrowError } from 'rxjs'; import { FeatureID } from '../../../core/data/feature-authorization/feature-id'; import { EPersonDeleteGuardService } from '../eperson-delete-guard.service'; import { CommonModule } from '@angular/common'; @@ -610,6 +610,25 @@ describe('EPersonFormComponent', () => { expect(modalService.open).not.toHaveBeenCalled(); expect(deleteSpy).not.toHaveBeenCalled(); }); + + it('should still show the friendly self-delete notification if the authenticated user id resolves late and the backend rejection carries no usable message', () => { + // Simulates the authenticated-user subscription resolving after the modal was confirmed + // (so the pre-flight self-delete check was bypassed) combined with a backend response + // whose error message can't be pattern-matched (e.g. Spring Boot's default message + // suppression). The self-delete notification must still win over the generic failure one. + spyOn(component.epersonService, 'deleteEPerson').and.returnValue(defer(() => { + component.currentAuthenticatedUserId = eperson.id; + return createFailedRemoteDataObject$(undefined, 400); + })); + + const deleteButton = fixture.debugElement.query(By.css('.delete-button')); + deleteButton.triggerEventHandler('click', null); + + expect(notificationsService.error).toHaveBeenCalled(); + let translatedKey: string; + notificationsService.error.calls.mostRecent().args[0].subscribe((value) => translatedKey = value); + expect(translatedKey).toBe('admin.access-control.epeople.notification.deleted.forbidden.self'); + }); }); describe('self delete button', () => { diff --git a/src/app/access-control/epeople-registry/eperson-form/eperson-form.component.ts b/src/app/access-control/epeople-registry/eperson-form/eperson-form.component.ts index 36304a67b60..85dc774a095 100644 --- a/src/app/access-control/epeople-registry/eperson-form/eperson-form.component.ts +++ b/src/app/access-control/epeople-registry/eperson-form/eperson-form.component.ts @@ -535,7 +535,7 @@ export class EPersonFormComponent implements OnInit, OnDestroy { if (restResponse?.hasSucceeded) { this.notificationsService.success(this.translateService.get(this.labelPrefix + 'notification.deleted.success', { name: this.dsoNameService.getName(eperson) })); void this.router.navigate([getEPersonsRoute()]); - } else if (this.deleteGuard.isSelfDeletionError(restResponse)) { + } else if (this.isCurrentUser(eperson) || this.deleteGuard.isSelfDeletionError(restResponse)) { this.deleteGuard.showSelfDeleteNotification(); } else { this.notificationsService.error(this.translateService.get(this.labelPrefix + 'notification.deleted.failure', { From 33c524cf2614be0418dcced010e4d6dcaa4b2132 Mon Sep 17 00:00:00 2001 From: Matus Kasak Date: Thu, 9 Jul 2026 09:29:34 +0200 Subject: [PATCH 2/2] Removed redundant comments --- .../epeople-registry/epeople-registry.component.spec.ts | 4 ---- .../epeople-registry/eperson-delete-guard.service.ts | 3 --- .../eperson-form/eperson-form.component.spec.ts | 4 ---- 3 files changed, 11 deletions(-) diff --git a/src/app/access-control/epeople-registry/epeople-registry.component.spec.ts b/src/app/access-control/epeople-registry/epeople-registry.component.spec.ts index dd89aeede8c..a42b760dcfd 100644 --- a/src/app/access-control/epeople-registry/epeople-registry.component.spec.ts +++ b/src/app/access-control/epeople-registry/epeople-registry.component.spec.ts @@ -384,10 +384,6 @@ describe('EPeopleRegistryComponent', () => { })); 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(() => { - // Simulates the authenticated-user subscription resolving after the click (so the - // pre-flight self-delete check is bypassed) combined with a backend response whose error - // message can't be pattern-matched (e.g. Spring Boot's default message suppression). - // The self-delete notification must still win over the generic failure one. modalRef.componentInstance.response = observableOf(true); component.currentAuthenticatedUserId = EPersonMock.id; ePersonDataServiceStub.deleteEPerson = jasmine.createSpy('deleteEPerson').and.returnValue(defer(() => { diff --git a/src/app/access-control/epeople-registry/eperson-delete-guard.service.ts b/src/app/access-control/epeople-registry/eperson-delete-guard.service.ts index 22131bc9372..e5449da3758 100644 --- a/src/app/access-control/epeople-registry/eperson-delete-guard.service.ts +++ b/src/app/access-control/epeople-registry/eperson-delete-guard.service.ts @@ -82,9 +82,6 @@ export class EPersonDeleteGuardService { /** * Whether the backend rejected the delete because an admin tried to delete themselves. - * Best-effort only: Spring Boot omits the exception message from the response body by - * default, so callers should treat this as a fallback alongside a client-side identity - * check rather than the sole signal. */ isSelfDeletionError(restResponse: RemoteData | null): boolean { return restResponse?.statusCode === 400 && restResponse?.errorMessage?.toLowerCase().includes('cannot delete yourself'); diff --git a/src/app/access-control/epeople-registry/eperson-form/eperson-form.component.spec.ts b/src/app/access-control/epeople-registry/eperson-form/eperson-form.component.spec.ts index da4779158a5..cbef5cf6033 100644 --- a/src/app/access-control/epeople-registry/eperson-form/eperson-form.component.spec.ts +++ b/src/app/access-control/epeople-registry/eperson-form/eperson-form.component.spec.ts @@ -612,10 +612,6 @@ describe('EPersonFormComponent', () => { }); it('should still show the friendly self-delete notification if the authenticated user id resolves late and the backend rejection carries no usable message', () => { - // Simulates the authenticated-user subscription resolving after the modal was confirmed - // (so the pre-flight self-delete check was bypassed) combined with a backend response - // whose error message can't be pattern-matched (e.g. Spring Boot's default message - // suppression). The self-delete notification must still win over the generic failure one. spyOn(component.epersonService, 'deleteEPerson').and.returnValue(defer(() => { component.currentAuthenticatedUserId = eperson.id; return createFailedRemoteDataObject$(undefined, 400);