fix: confirm dialog for group deletion

Introduced a confirmation modal before deleting groups in both the Group Registry and Comcol Role components. This ensures users explicitly confirm deletion, reducing accidental data loss. Updated relevant tests and added new translations for modal text.

Author: jlipka

src/app/access-control/group-registry/groups-registry.component.html

@@ -85,7 +85,7 @@ <h2 id="search" class="border-bottom pb-2">{{messagePrefix + 'search.head' | tra
                         }
                         @if (!groupDto.group?.permanent && groupDto.ableToDelete) {
                           <button
-                            (click)="deleteGroup(groupDto)" class="btn btn-outline-danger btn-sm btn-delete"
+                            (click)="confirmDelete(groupDto)" class="btn btn-outline-danger btn-sm btn-delete"
                             title="{{messagePrefix + 'table.edit.buttons.remove' | translate: {name: dsoNameService.getName(groupDto.group) } }}">
                             <i class="fas fa-trash-alt fa-fw"></i>
                           </button>

src/app/access-control/group-registry/groups-registry.component.spec.ts

@@ -381,6 +381,8 @@ describe('GroupsRegistryComponent', () => {
       deleteButton.click();
       fixture.detectChanges();
 
+      (document as any).querySelector('.modal-footer .confirm').click();
+
       expect(groupsDataServiceStub.delete).toHaveBeenCalledWith(mockGroups[0].id);
     });
   });

src/app/access-control/group-registry/groups-registry.component.ts

@@ -9,7 +9,10 @@ import {
   UntypedFormBuilder,
 } from '@angular/forms';
 import { RouterLink } from '@angular/router';
-import { NgbTooltipModule } from '@ng-bootstrap/ng-bootstrap';
+import {
+  NgbModal,
+  NgbTooltipModule,
+} from '@ng-bootstrap/ng-bootstrap';
 import {
   TranslateModule,
   TranslateService,
@@ -27,6 +30,7 @@ import {
   defaultIfEmpty,
   map,
   switchMap,
+  takeUntil,
   tap,
 } from 'rxjs/operators';
 
@@ -57,6 +61,7 @@ import {
 } from '../../core/shared/operators';
 import { PageInfo } from '../../core/shared/page-info.model';
 import { BtnDisabledDirective } from '../../shared/btn-disabled.directive';
+import { ConfirmationModalComponent } from '../../shared/confirmation-modal/confirmation-modal.component';
 import { hasValue } from '../../shared/empty.util';
 import { ThemedLoadingComponent } from '../../shared/loading/themed-loading.component';
 import { NotificationsService } from '../../shared/notifications/notifications.service';
@@ -142,6 +147,7 @@ export class GroupsRegistryComponent implements OnInit, OnDestroy {
               private paginationService: PaginationService,
               public requestService: RequestService,
               public dsoNameService: DSONameService,
+              private modalService: NgbModal,
   ) {
     this.currentSearchQuery = '';
     this.searchForm = this.formBuilder.group(({
@@ -314,4 +320,30 @@ export class GroupsRegistryComponent implements OnInit, OnDestroy {
     this.paginationService.clearPagination(this.config.id);
   }
 
+  confirmDelete(group: GroupDtoModel): void {
+    const modalRef = this.modalService.open(ConfirmationModalComponent);
+    modalRef.componentInstance.name = this.dsoNameService.getName(group.group);
+    modalRef.componentInstance.headerLabel = 'admin.access-control.epeople.table.edit.buttons.remove.modal.header';
+    modalRef.componentInstance.infoLabel = 'admin.access-control.epeople.table.edit.buttons.remove.modal.info';
+    modalRef.componentInstance.cancelLabel = 'admin.access-control.epeople.table.edit.buttons.remove.modal.cancel';
+    modalRef.componentInstance.confirmLabel = 'admin.access-control.epeople.table.edit.buttons.remove.modal.confirm';
+    modalRef.componentInstance.brandColor = 'danger';
+    modalRef.componentInstance.confirmIcon = 'fas fa-trash';
+
+    const modalSub: Subscription = modalRef.componentInstance.response.pipe(
+      takeUntil(modalRef.closed),
+    ).subscribe((result: boolean) => {
+      if (result === true) {
+        this.deleteGroup(group);
+      }
+    });
+
+    void modalRef.result.then().finally(() => {
+      modalRef.close();
+      if (modalSub && !modalSub.closed) {
+        modalSub.unsubscribe();
+      }
+    });
+  }
+
 }

src/app/shared/comcol/comcol-forms/edit-comcol-page/comcol-role/comcol-role.component.html

@@ -51,7 +51,7 @@ <h2 class="h4 w-100">
         @if (hasCustomGroup$ | async) {
           <button
             class="btn btn-danger delete"
-            (click)="delete()">
+            (click)="confirmDelete(dsoNameService.getName(group))">
             <i class="fas fa-trash" aria-hidden="true"></i> {{'comcol-role.edit.delete' | translate}}
           </button>
         }

src/app/shared/comcol/comcol-forms/edit-comcol-page/comcol-role/comcol-role.component.spec.ts

@@ -10,6 +10,7 @@ import {
 import { By } from '@angular/platform-browser';
 import { NoopAnimationsModule } from '@angular/platform-browser/animations';
 import { RouterTestingModule } from '@angular/router/testing';
+import { NgbModule } from '@ng-bootstrap/ng-bootstrap';
 import { TranslateModule } from '@ngx-translate/core';
 import { of } from 'rxjs';
 
@@ -36,17 +37,22 @@ describe('ComcolRoleComponent', () => {
   let comcolRole;
   let notificationsService;
 
-  const requestService = { hasByHref$: () => of(true) };
+  const requestService = {
+    hasByHref$: () => of(true),
+    setStaleByHrefSubstring: () => of(true),
+  };
 
   const groupService = {
     findByHref: jasmine.createSpy('findByHref'),
     createComcolGroup: jasmine.createSpy('createComcolGroup').and.returnValue(of({})),
     deleteComcolGroup: jasmine.createSpy('deleteComcolGroup').and.returnValue(of({})),
+    clearGroupsRequests: jasmine.createSpy('clearGroupsRequests'),
   };
 
   beforeEach(waitForAsync(() => {
     TestBed.configureTestingModule({
       imports: [
+        NgbModule,
         RouterTestingModule.withRoutes([]),
         TranslateModule.forRoot(),
         NoopAnimationsModule,
@@ -174,17 +180,18 @@ describe('ComcolRoleComponent', () => {
         name: 'custom group name',
       };
       statusCode = 200;
-      comp.comcolRole = comcolRole;
+      comp.comcolRole = {
+        name: 'test role name' + Math.random(),
+        href: 'test role link',
+      };
+      comp.roleName$ = of(comcolRole.name);
       fixture.detectChanges();
     });
 
     it('should have a delete button but no create or restrict button', (done) => {
-      expect(de.query(By.css('.btn.create')))
-        .toBeNull();
-      expect(de.query(By.css('.btn.restrict')))
-        .toBeNull();
-      expect(de.query(By.css('.btn.delete')))
-        .toBeTruthy();
+      expect(de.query(By.css('.btn.create'))).toBeNull();
+      expect(de.query(By.css('.btn.restrict'))).toBeNull();
+      expect(de.query(By.css('.btn.delete'))).toBeTruthy();
       done();
     });
 
@@ -194,7 +201,16 @@ describe('ComcolRoleComponent', () => {
         de.query(By.css('.btn.delete')).nativeElement.click();
       });
 
+      afterEach(() => {
+        const modal = document.querySelector('ds-confirmation-modal');
+        if (modal) {
+          modal.remove();
+        }
+      });
+
       it('should call the groupService delete method', (done) => {
+        (document as any).querySelector('.modal-footer .confirm').click();
+        fixture.detectChanges();
         expect(groupService.deleteComcolGroup).toHaveBeenCalled();
         done();
       });
@@ -204,12 +220,24 @@ describe('ComcolRoleComponent', () => {
       beforeEach(() => {
         groupService.deleteComcolGroup.and.returnValue(createFailedRemoteDataObject$());
         de.query(By.css('.btn.delete')).nativeElement.click();
+        fixture.detectChanges();
+      });
+
+      afterEach(() => {
+        const modal = document.querySelector('ds-confirmation-modal');
+        if (modal) {
+          modal.remove();
+        }
       });
 
       it('should show an error notification', (done) => {
+        (document as any).querySelector('.modal-footer .confirm').click();
+        fixture.detectChanges();
+
         expect(notificationsService.error).toHaveBeenCalled();
         done();
       });
     });
+
   });
 });

src/app/shared/comcol/comcol-forms/edit-comcol-page/comcol-role/comcol-role.component.ts

@@ -5,18 +5,21 @@ import {
   OnInit,
 } from '@angular/core';
 import { RouterLink } from '@angular/router';
+import { NgbModal } from '@ng-bootstrap/ng-bootstrap';
 import {
   TranslateModule,
   TranslateService,
 } from '@ngx-translate/core';
 import {
   BehaviorSubject,
   Observable,
+  Subscription,
 } from 'rxjs';
 import {
   filter,
   map,
   switchMap,
+  takeUntil,
 } from 'rxjs/operators';
 
 import { getGroupEditRoute } from '../../../../../access-control/access-control-routing-paths';
@@ -34,6 +37,7 @@ import {
   getFirstCompletedRemoteData,
 } from '../../../../../core/shared/operators';
 import { AlertComponent } from '../../../../alert/alert.component';
+import { ConfirmationModalComponent } from '../../../../confirmation-modal/confirmation-modal.component';
 import {
   hasNoValue,
   hasValue,
@@ -115,6 +119,7 @@ export class ComcolRoleComponent implements OnInit {
     protected notificationsService: NotificationsService,
     protected translateService: TranslateService,
     public dsoNameService: DSONameService,
+    private modalService: NgbModal,
   ) {
   }
 
@@ -215,4 +220,31 @@ export class ComcolRoleComponent implements OnInit {
 
     this.roleName$ = this.translateService.get(`comcol-role.edit.${this.comcolRole.name}.name`);
   }
+
+  confirmDelete(groupName: string): void {
+    const modalRef = this.modalService.open(ConfirmationModalComponent);
+
+    modalRef.componentInstance.name = groupName;
+    modalRef.componentInstance.headerLabel = 'comcol-role.edit.delete.modal.header';
+    modalRef.componentInstance.infoLabel = 'comcol-role.edit.delete.modal.info';
+    modalRef.componentInstance.cancelLabel = 'comcol-role.edit.delete.modal.cancel';
+    modalRef.componentInstance.confirmLabel = 'comcol-role.edit.delete.modal.confirm';
+    modalRef.componentInstance.brandColor = 'danger';
+    modalRef.componentInstance.confirmIcon = 'fas fa-trash';
+
+    const modalSub: Subscription = modalRef.componentInstance.response.pipe(
+      takeUntil(modalRef.closed),
+    ).subscribe((result: boolean) => {
+      if (result === true) {
+        this.delete();
+      }
+    });
+
+    void modalRef.result.then().finally(() => {
+      modalRef.close();
+      if (modalSub && !modalSub.closed) {
+        modalSub.unsubscribe();
+      }
+    });
+  }
 }

src/assets/i18n/en.json5

@@ -305,6 +305,14 @@
 
   "admin.access-control.epeople.table.edit.buttons.remove": "Delete \"{{name}}\"",
 
+  "admin.access-control.epeople.table.edit.buttons.remove.modal.header": "Delete Group \"{{ dsoName }}\"",
+
+  "admin.access-control.epeople.table.edit.buttons.remove.modal.info": "Are you sure you want to delete Group \"{{ dsoName }}\" and all its associated policies?",
+
+  "admin.access-control.epeople.table.edit.buttons.remove.modal.cancel": "Cancel",
+
+  "admin.access-control.epeople.table.edit.buttons.remove.modal.confirm": "Delete",
+
   "admin.access-control.epeople.no-items": "No EPeople to show.",
 
   "admin.access-control.epeople.form.create": "Create EPerson",
@@ -1523,6 +1531,14 @@
 
   "comcol-role.edit.delete.error.title": "Failed to delete the '{{ role }}' role's group",
 
+  "comcol-role.edit.delete.modal.header": "Delete Group \"{{ dsoName }}\"",
+
+  "comcol-role.edit.delete.modal.info": "Are you sure you want to delete Group \"{{ dsoName }}\" and all its associated policies?",
+
+  "comcol-role.edit.delete.modal.cancel": "Cancel",
+
+  "comcol-role.edit.delete.modal.confirm": "Delete",
+
   "comcol-role.edit.community-admin.name": "Administrators",
 
   "comcol-role.edit.collection-admin.name": "Administrators",