Skip to content

Commit 728c874

Browse files
MatusBekeclaude
andcommitted
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>
1 parent c49ef5e commit 728c874

5 files changed

Lines changed: 47 additions & 4 deletions

File tree

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

Lines changed: 22 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';
@@ -382,6 +382,27 @@ describe('EPeopleRegistryComponent', () => {
382382
expect(modalService.open).not.toHaveBeenCalled();
383383
expect(deleteSpy).not.toHaveBeenCalled();
384384
}));
385+
386+
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(() => {
387+
// Simulates the authenticated-user subscription resolving after the click (so the
388+
// pre-flight self-delete check is bypassed) combined with a backend response whose error
389+
// message can't be pattern-matched (e.g. Spring Boot's default message suppression).
390+
// The self-delete notification must still win over the generic failure one.
391+
modalRef.componentInstance.response = observableOf(true);
392+
component.currentAuthenticatedUserId = EPersonMock.id;
393+
ePersonDataServiceStub.deleteEPerson = jasmine.createSpy('deleteEPerson').and.returnValue(defer(() => {
394+
component.currentAuthenticatedUserId = EPersonMock2.id;
395+
return createFailedRemoteDataObject$(undefined, 400);
396+
}));
397+
398+
component.deleteEPerson(EPersonMock2);
399+
tick();
400+
401+
expect(notificationsService.error).toHaveBeenCalled();
402+
let translatedKey: string;
403+
notificationsService.error.calls.mostRecent().args[0].subscribe((value) => translatedKey = value);
404+
expect(translatedKey).toBe('admin.access-control.epeople.notification.deleted.forbidden.self');
405+
}));
385406
});
386407

387408
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
@@ -224,7 +224,7 @@ export class EPeopleRegistryComponent implements OnInit, OnDestroy {
224224
this.epersonService.deleteEPerson(ePerson).pipe(getFirstCompletedRemoteData()).subscribe((restResponse: RemoteData<NoContent>) => {
225225
if (restResponse.hasSucceeded) {
226226
this.notificationsService.success(this.translateService.get(this.labelPrefix + 'notification.deleted.success', {name: this.dsoNameService.getName(ePerson)}));
227-
} else if (this.deleteGuard.isSelfDeletionError(restResponse)) {
227+
} else if (this.isCurrentUser(ePerson) || this.deleteGuard.isSelfDeletionError(restResponse)) {
228228
this.deleteGuard.showSelfDeleteNotification();
229229
} else {
230230
this.notificationsService.error(this.translateService.get(this.labelPrefix + 'notification.deleted.failure', {

src/app/access-control/epeople-registry/eperson-delete-guard.service.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,9 @@ export class EPersonDeleteGuardService {
8282

8383
/**
8484
* Whether the backend rejected the delete because an admin tried to delete themselves.
85+
* Best-effort only: Spring Boot omits the exception message from the response body by
86+
* default, so callers should treat this as a fallback alongside a client-side identity
87+
* check rather than the sole signal.
8588
*/
8689
isSelfDeletionError(restResponse: RemoteData<NoContent> | null): boolean {
8790
return restResponse?.statusCode === 400 && restResponse?.errorMessage?.toLowerCase().includes('cannot delete yourself');

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

Lines changed: 20 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';
@@ -610,6 +610,25 @@ describe('EPersonFormComponent', () => {
610610
expect(modalService.open).not.toHaveBeenCalled();
611611
expect(deleteSpy).not.toHaveBeenCalled();
612612
});
613+
614+
it('should still show the friendly self-delete notification if the authenticated user id resolves late and the backend rejection carries no usable message', () => {
615+
// Simulates the authenticated-user subscription resolving after the modal was confirmed
616+
// (so the pre-flight self-delete check was bypassed) combined with a backend response
617+
// whose error message can't be pattern-matched (e.g. Spring Boot's default message
618+
// suppression). The self-delete notification must still win over the generic failure one.
619+
spyOn(component.epersonService, 'deleteEPerson').and.returnValue(defer(() => {
620+
component.currentAuthenticatedUserId = eperson.id;
621+
return createFailedRemoteDataObject$(undefined, 400);
622+
}));
623+
624+
const deleteButton = fixture.debugElement.query(By.css('.delete-button'));
625+
deleteButton.triggerEventHandler('click', null);
626+
627+
expect(notificationsService.error).toHaveBeenCalled();
628+
let translatedKey: string;
629+
notificationsService.error.calls.mostRecent().args[0].subscribe((value) => translatedKey = value);
630+
expect(translatedKey).toBe('admin.access-control.epeople.notification.deleted.forbidden.self');
631+
});
613632
});
614633

615634
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
@@ -535,7 +535,7 @@ export class EPersonFormComponent implements OnInit, OnDestroy {
535535
if (restResponse?.hasSucceeded) {
536536
this.notificationsService.success(this.translateService.get(this.labelPrefix + 'notification.deleted.success', { name: this.dsoNameService.getName(eperson) }));
537537
void this.router.navigate([getEPersonsRoute()]);
538-
} else if (this.deleteGuard.isSelfDeletionError(restResponse)) {
538+
} else if (this.isCurrentUser(eperson) || this.deleteGuard.isSelfDeletionError(restResponse)) {
539539
this.deleteGuard.showSelfDeleteNotification();
540540
} else {
541541
this.notificationsService.error(this.translateService.get(this.labelPrefix + 'notification.deleted.failure', {

0 commit comments

Comments
 (0)