74179: Deleting groups notifications, authorisation check, button on edit group page

src/app/+admin/admin-access-control/group-registry/group-form/group-form.component.html

@@ -9,6 +9,11 @@
       </ng-template>
 
       <ng-template #editheader>
+        <div>
+          <button class="btn btn-light delete-button float-right" [disabled]="!(canDelete$ | async)" (click)="delete()">
+            <i class="fa fa-trash"></i> {{ messagePrefix + '.actions.delete' | translate}}
+          </button>
+        </div>
         <h2 class="border-bottom pb-2">{{messagePrefix + '.head.edit' | translate}}</h2>
       </ng-template>
 

src/app/+admin/admin-access-control/group-registry/group-form/group-form.component.spec.ts

@@ -14,6 +14,7 @@ import { RemoteDataBuildService } from '../../../../core/cache/builders/remote-d
 import { ObjectCacheService } from '../../../../core/cache/object-cache.service';
 import { RestResponse } from '../../../../core/cache/response.models';
 import { DSOChangeAnalyzer } from '../../../../core/data/dso-change-analyzer.service';
+import { AuthorizationDataService } from '../../../../core/data/feature-authorization/authorization-data.service';
 import { PaginatedList } from '../../../../core/data/paginated-list';
 import { RemoteData } from '../../../../core/data/remote-data';
 import { EPersonDataService } from '../../../../core/eperson/eperson-data.service';
@@ -40,6 +41,7 @@ describe('GroupFormComponent', () => {
   let builderService: FormBuilderService;
   let ePersonDataServiceStub: any;
   let groupsDataServiceStub: any;
+  let authorizationService: AuthorizationDataService;
   let router;
 
   let groups;
@@ -88,6 +90,9 @@ describe('GroupFormComponent', () => {
         return createSuccessfulRemoteDataObject$(new PaginatedList(new PageInfo(), []))
       }
     };
+    authorizationService = jasmine.createSpyObj('authorizationService', {
+      isAuthorized: observableOf(true)
+    });
     builderService = getMockFormBuilderService();
     translateService = getMockTranslateService();
     router = new RouterMock();
@@ -115,6 +120,7 @@ describe('GroupFormComponent', () => {
         { provide: HALEndpointService, useValue: {} },
         { provide: ActivatedRoute, useValue: { data: observableOf({ dso: { payload: {} } }), params: observableOf({}) } },
         { provide: Router, useValue: router },
+        { provide: AuthorizationDataService, useValue: authorizationService },
       ],
       schemas: [NO_ERRORS_SCHEMA]
     }).compileComponents();

src/app/+admin/admin-access-control/group-registry/group-form/group-form.component.ts

@@ -1,6 +1,7 @@
 import { Component, EventEmitter, HostListener, OnDestroy, OnInit, Output } from '@angular/core';
 import { FormGroup } from '@angular/forms';
 import { ActivatedRoute, Router } from '@angular/router';
+import { NgbModal } from '@ng-bootstrap/ng-bootstrap';
 import {
   DynamicFormControlModel,
   DynamicFormLayout,
@@ -8,15 +9,20 @@ import {
   DynamicTextAreaModel
 } from '@ng-dynamic-forms/core';
 import { TranslateService } from '@ngx-translate/core';
+import { Observable } from 'rxjs/internal/Observable';
 import { combineLatest } from 'rxjs/internal/observable/combineLatest';
 import { Subscription } from 'rxjs/internal/Subscription';
-import { take } from 'rxjs/operators';
+import { switchMap, take } from 'rxjs/operators';
 import { RestResponse } from '../../../../core/cache/response.models';
+import { AuthorizationDataService } from '../../../../core/data/feature-authorization/authorization-data.service';
+import { FeatureID } from '../../../../core/data/feature-authorization/feature-id';
 import { PaginatedList } from '../../../../core/data/paginated-list';
+import { RequestService } from '../../../../core/data/request.service';
 import { EPersonDataService } from '../../../../core/eperson/eperson-data.service';
 import { GroupDataService } from '../../../../core/eperson/group-data.service';
 import { Group } from '../../../../core/eperson/models/group.model';
 import { getRemoteDataPayload, getSucceededRemoteData } from '../../../../core/shared/operators';
+import { ConfirmationModalComponent } from '../../../../shared/confirmation-modal/confirmation-modal.component';
 import { hasValue, isNotEmpty } from '../../../../shared/empty.util';
 import { FormBuilderService } from '../../../../shared/form/builder/form-builder.service';
 import { NotificationsService } from '../../../../shared/notifications/notifications.service';
@@ -89,19 +95,34 @@ export class GroupFormComponent implements OnInit, OnDestroy {
    */
   groupBeingEdited: Group;
 
+  /**
+   * Observable whether or not the logged in user is allowed to delete the Group
+   */
+  canDelete$: Observable<boolean>;
+
   constructor(public groupDataService: GroupDataService,
               private ePersonDataService: EPersonDataService,
               private formBuilderService: FormBuilderService,
               private translateService: TranslateService,
               private notificationsService: NotificationsService,
               private route: ActivatedRoute,
-              protected router: Router) {
+              protected router: Router,
+              private authorizationService: AuthorizationDataService,
+              private modalService: NgbModal,
+              public requestService: RequestService) {
   }
 
   ngOnInit() {
+    this.initialisePage();
+  }
+
+  initialisePage() {
     this.subs.push(this.route.params.subscribe((params) => {
       this.setActiveGroup(params.groupId)
     }));
+    this.canDelete$ = this.groupDataService.getActiveGroup().pipe(
+      switchMap((group: Group) => this.authorizationService.isAuthorized(FeatureID.CanDelete, hasValue(group) ? group.self : undefined))
+    );
     combineLatest(
       this.translateService.get(`${this.messagePrefix}.groupName`),
       this.translateService.get(`${this.messagePrefix}.groupDescription`),
@@ -269,6 +290,48 @@ export class GroupFormComponent implements OnInit, OnDestroy {
     });
   }
 
+  /**
+   * Deletes the Group from the Repository. The Group will be the only that this form is showing.
+   * It'll either show a success or error message depending on whether the delete was successful or not.
+   */
+  delete() {
+    this.groupDataService.getActiveGroup().pipe(take(1)).subscribe((group: Group) => {
+      const modalRef = this.modalService.open(ConfirmationModalComponent);
+      modalRef.componentInstance.dso = group;
+      modalRef.componentInstance.headerLabel = this.messagePrefix + '.delete-group.modal.header';
+      modalRef.componentInstance.infoLabel = this.messagePrefix + '.delete-group.modal.info';
+      modalRef.componentInstance.cancelLabel = this.messagePrefix + '.delete-group.modal.cancel';
+      modalRef.componentInstance.confirmLabel = this.messagePrefix + '.delete-group.modal.confirm';
+      modalRef.componentInstance.response.pipe(take(1)).subscribe((confirm: boolean) => {
+        if (confirm) {
+          if (hasValue(group.id)) {
+            this.groupDataService.deleteGroup(group).pipe(take(1))
+              .subscribe(([success, optionalErrorMessage]: [boolean, string]) => {
+                if (success) {
+                  this.notificationsService.success(this.translateService.get(this.messagePrefix + '.notification.deleted.success', { name: group.name }));
+                  this.reset();
+                  this.onCancel();
+                } else {
+                  this.notificationsService.error(
+                    this.translateService.get(this.messagePrefix + '.notification.deleted.failure.title', { name: group.name }),
+                    this.translateService.get(this.messagePrefix + '.notification.deleted.failure.content', { cause: optionalErrorMessage }));
+                }
+              })
+          }}
+      });
+    })
+  }
+
+  /**
+   * This method will ensure that the page gets reset and that the cache is cleared
+   */
+  reset() {
+    this.groupDataService.getActiveGroup().pipe(take(1)).subscribe((group: Group) => {
+      this.requestService.removeByHrefSubstring(group.self);
+    });
+    this.initialisePage();
+  }
+
   /**
    * Cancel the current edit when component is destroyed & unsub all subscriptions
    */

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

@@ -30,10 +30,10 @@
       </form>
 
       <ds-pagination
-        *ngIf="(groups | async)?.payload?.totalElements > 0"
+        *ngIf="(groupsDto$ | async)?.totalElements > 0"
         [paginationOptions]="config"
-        [pageInfoState]="(groups | async)?.payload"
+        [pageInfoState]="pageInfoState$"
-        [collectionSize]="(groups | async)?.payload?.totalElements"
+        [collectionSize]="(pageInfoState$ | async)?.totalElements"
         [hideGear]="true"
         [hidePagerWhenSinglePage]="true"
         (pageChange)="onPageChange($event)">
@@ -50,21 +50,21 @@
             </tr>
             </thead>
             <tbody>
-            <tr *ngFor="let group of (groups | async)?.payload?.page">
+            <tr *ngFor="let groupDto of (groupsDto$ | async)?.page">
-              <td>{{group.id}}</td>
+              <td>{{groupDto.group.id}}</td>
-              <td>{{group.name}}</td>
+              <td>{{groupDto.group.name}}</td>
-              <td>{{(getMembers(group) | async)?.payload?.totalElements + (getSubgroups(group) | async)?.payload?.totalElements}}</td>
+              <td>{{(getMembers(groupDto.group) | async)?.payload?.totalElements + (getSubgroups(groupDto.group) | async)?.payload?.totalElements}}</td>
               <!--              <td>{{getOptionalComColFromName(group.name)}}</td>-->
               <td>
                 <div class="btn-group edit-field">
-                  <button [routerLink]="groupService.getGroupEditPageRouterLink(group)"
+                  <button [routerLink]="groupService.getGroupEditPageRouterLink(groupDto.group)"
                           class="btn btn-outline-primary btn-sm"
-                          title="{{messagePrefix + 'table.edit.buttons.edit' | translate: {name: group.name} }}">
+                          title="{{messagePrefix + 'table.edit.buttons.edit' | translate: {name: groupDto.group.name} }}">
                     <i class="fas fa-edit fa-fw"></i>
                   </button>
-                  <button *ngIf="!group?.permanent" (click)="deleteGroup(group)"
+                  <button *ngIf="!groupDto.group?.permanent && groupDto.ableToDelete"
-                          class="btn btn-outline-danger btn-sm"
+                          (click)="deleteGroup(groupDto.group)" class="btn btn-outline-danger btn-sm"
-                          title="{{messagePrefix + 'table.edit.buttons.remove' | translate: {name: group.name} }}">
+                          title="{{messagePrefix + 'table.edit.buttons.remove' | translate: {name: groupDto.group.name} }}">
                     <i class="fas fa-trash-alt fa-fw"></i>
                   </button>
                 </div>
@@ -75,7 +75,7 @@
         </div>
       </ds-pagination>
 
-      <div *ngIf="(groups | async)?.payload?.totalElements == 0" class="alert alert-info w-100 mb-2" role="alert">
+      <div *ngIf="(pageInfoState$ | async)?.totalElements == 0" class="alert alert-info w-100 mb-2" role="alert">
         {{messagePrefix + 'no-items' | translate}}
       </div>
 

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

@@ -6,7 +6,8 @@ import { BrowserModule, By } from '@angular/platform-browser';
 import { Router } from '@angular/router';
 import { NgbModule } from '@ng-bootstrap/ng-bootstrap';
 import { TranslateLoader, TranslateModule } from '@ngx-translate/core';
-import { Observable } from 'rxjs/internal/Observable';
+import { Observable, of as observableOf } from 'rxjs';
+import { AuthorizationDataService } from '../../../core/data/feature-authorization/authorization-data.service';
 import { PaginatedList } from '../../../core/data/paginated-list';
 import { RemoteData } from '../../../core/data/remote-data';
 import { EPersonDataService } from '../../../core/eperson/eperson-data.service';
@@ -30,6 +31,7 @@ describe('GroupRegistryComponent', () => {
   let fixture: ComponentFixture<GroupsRegistryComponent>;
   let ePersonDataServiceStub: any;
   let groupsDataServiceStub: any;
+  let authorizationService: AuthorizationDataService;
 
   let mockGroups;
   let mockEPeople;
@@ -77,6 +79,9 @@ describe('GroupRegistryComponent', () => {
         return createSuccessfulRemoteDataObject$(new PaginatedList(new PageInfo(), [result]));
       }
     };
+    authorizationService = jasmine.createSpyObj('authorizationService', {
+      isAuthorized: observableOf(true)
+    });
     TestBed.configureTestingModule({
       imports: [CommonModule, NgbModule, FormsModule, ReactiveFormsModule, BrowserModule,
         TranslateModule.forRoot({
@@ -93,6 +98,7 @@ describe('GroupRegistryComponent', () => {
         { provide: NotificationsService, useValue: new NotificationsServiceStub() },
         { provide: RouteService, useValue: routeServiceStub },
         { provide: Router, useValue: new RouterMock() },
+        { provide: AuthorizationDataService, useValue: authorizationService },
       ],
       schemas: [NO_ERRORS_SCHEMA]
     }).compileComponents();

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

@@ -1,16 +1,22 @@
-import { Component, OnInit } from '@angular/core';
+import { Component, OnDestroy, OnInit } from '@angular/core';
 import { FormBuilder } from '@angular/forms';
 import { Router } from '@angular/router';
 import { TranslateService } from '@ngx-translate/core';
-import { Observable } from 'rxjs';
+import { BehaviorSubject, combineLatest, Observable } from 'rxjs';
-import { take } from 'rxjs/operators';
+import { Subscription } from 'rxjs/internal/Subscription';
+import { map, switchMap, take } from 'rxjs/operators';
+import { AuthorizationDataService } from '../../../core/data/feature-authorization/authorization-data.service';
+import { FeatureID } from '../../../core/data/feature-authorization/feature-id';
 import { PaginatedList } from '../../../core/data/paginated-list';
 import { RemoteData } from '../../../core/data/remote-data';
 import { EPersonDataService } from '../../../core/eperson/eperson-data.service';
 import { GroupDataService } from '../../../core/eperson/group-data.service';
 import { EPerson } from '../../../core/eperson/models/eperson.model';
+import { GroupDtoModel } from '../../../core/eperson/models/group-dto.model';
 import { Group } from '../../../core/eperson/models/group.model';
 import { RouteService } from '../../../core/services/route.service';
+import { getAllSucceededRemoteDataPayload } from '../../../core/shared/operators';
+import { PageInfo } from '../../../core/shared/page-info.model';
 import { hasValue } from '../../../shared/empty.util';
 import { NotificationsService } from '../../../shared/notifications/notifications.service';
 import { PaginationComponentOptions } from '../../../shared/pagination/pagination-component-options.model';
@@ -23,7 +29,7 @@ import { PaginationComponentOptions } from '../../../shared/pagination/paginatio
  * A component used for managing all existing groups within the repository.
  * The admin can create, edit or delete groups here.
  */
-export class GroupsRegistryComponent implements OnInit {
+export class GroupsRegistryComponent implements OnInit, OnDestroy {
 
   messagePrefix = 'admin.access-control.groups.';
 
@@ -37,9 +43,19 @@ export class GroupsRegistryComponent implements OnInit {
   });
 
   /**
-   * A list of all the current groups within the repository or the result of the search
+   * A list of all the current Groups within the repository or the result of the search
    */
-  groups: Observable<RemoteData<PaginatedList<Group>>>;
+  groups$: BehaviorSubject<RemoteData<PaginatedList<Group>>> = new BehaviorSubject<RemoteData<PaginatedList<Group>>>({} as any);
+  /**
+   * A BehaviorSubject with the list of GroupDtoModel objects made from the Groups in the repository or
+   * as the result of the search
+   */
+  groupsDto$: BehaviorSubject<PaginatedList<GroupDtoModel>> = new BehaviorSubject<PaginatedList<GroupDtoModel>>({} as any);
+
+  /**
+   * An observable for the pageInfo, needed to pass to the pagination component
+   */
+  pageInfoState$: BehaviorSubject<PageInfo> = new BehaviorSubject<PageInfo>(undefined);
 
   // The search form
   searchForm;
@@ -47,13 +63,19 @@ export class GroupsRegistryComponent implements OnInit {
   // Current search in groups registry
   currentSearchQuery: string;
 
+  /**
+   * List of subscriptions
+   */
+  subs: Subscription[] = [];
+
   constructor(public groupService: GroupDataService,
               private ePersonDataService: EPersonDataService,
               private translateService: TranslateService,
               private notificationsService: NotificationsService,
               private formBuilder: FormBuilder,
               protected routeService: RouteService,
-              private router: Router) {
+              private router: Router,
+              private authorizationService: AuthorizationDataService) {
     this.currentSearchQuery = '';
     this.searchForm = this.formBuilder.group(({
       query: this.currentSearchQuery,
@@ -84,26 +106,49 @@ export class GroupsRegistryComponent implements OnInit {
       this.currentSearchQuery = query;
       this.config.currentPage = 1;
     }
-    this.groups = this.groupService.searchGroups(this.currentSearchQuery.trim(), {
+    this.subs.push(this.groupService.searchGroups(this.currentSearchQuery.trim(), {
       currentPage: this.config.currentPage,
       elementsPerPage: this.config.pageSize
-    });
+    }).subscribe((groupsRD: RemoteData<PaginatedList<Group>>) => {
+        this.groups$.next(groupsRD)
+      }
+    ));
+
+    this.subs.push(this.groups$.pipe(
+      getAllSucceededRemoteDataPayload(),
+      switchMap((groups) => {
+        return combineLatest(...groups.page.map((group) => {
+          return this.authorizationService.isAuthorized(FeatureID.CanDelete, hasValue(group) ? group.self : undefined).pipe(
+            map((authorized: boolean) => {
+              const groupDtoModel: GroupDtoModel = new GroupDtoModel();
+              groupDtoModel.ableToDelete = authorized;
+              groupDtoModel.group = group;
+              return groupDtoModel;
+            })
+          );
+        })).pipe(map((dtos: GroupDtoModel[]) => {
+          return new PaginatedList(groups.pageInfo, dtos);
+        }))
+      })).subscribe((value: PaginatedList<GroupDtoModel>) => {
+      this.groupsDto$.next(value);
+      this.pageInfoState$.next(value.pageInfo);
+    }));
   }
 
   /**
    * Delete Group
    */
   deleteGroup(group: Group) {
-    // TODO (backend)
-    console.log('TODO implement editGroup', group);
-    this.notificationsService.error('TODO implement deleteGroup (not yet implemented in backend)');
     if (hasValue(group.id)) {
-      this.groupService.deleteGroup(group).pipe(take(1)).subscribe((success: boolean) => {
+      this.groupService.deleteGroup(group).pipe(take(1))
+        .subscribe(([success, optionalErrorMessage]: [boolean, string]) => {
           if (success) {
             this.notificationsService.success(this.translateService.get(this.messagePrefix + 'notification.deleted.success', { name: group.name }));
             this.forceUpdateGroup();
           } else {
-          this.notificationsService.error(this.translateService.get(this.messagePrefix + 'notification.deleted.failure', { name: group.name }));
+            this.notificationsService.error(
+              this.translateService.get(this.messagePrefix + 'notification.deleted.failure.title', { name: group.name }),
+              this.translateService.get(this.messagePrefix + 'notification.deleted.failure.content', { cause: optionalErrorMessage }));
           }
         })
     }
@@ -151,4 +196,15 @@ export class GroupsRegistryComponent implements OnInit {
   getOptionalComColFromName(groupName: string): string {
     return this.groupService.getUUIDFromString(groupName);
   }
+
+  /**
+   * Unsub all subscriptions
+   */
+  ngOnDestroy(): void {
+    this.cleanupSubscribes();
+  }
+
+  cleanupSubscribes() {
+    this.subs.filter((sub) => hasValue(sub)).forEach((sub) => sub.unsubscribe());
+  }
 }

src/app/core/eperson/group-data.service.ts

@@ -16,7 +16,7 @@ import { FollowLinkConfig } from '../../shared/utils/follow-link-config.model';
 import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service';
 import { RequestParam } from '../cache/models/request-param.model';
 import { ObjectCacheService } from '../cache/object-cache.service';
-import { RestResponse } from '../cache/response.models';
+import { ErrorResponse, RestResponse } from '../cache/response.models';
 import { DataService } from '../data/data.service';
 import { DSOChangeAnalyzer } from '../data/dso-change-analyzer.service';
 import { PaginatedList } from '../data/paginated-list';
@@ -125,10 +125,13 @@ export class GroupDataService extends DataService<Group> {
 
   /**
    * Method to delete a group
-   * @param id The group id to delete
+   * @param group The group to delete
    */
-  public deleteGroup(group: Group): Observable<boolean> {
+  public deleteGroup(group: Group): Observable<[boolean, string]> {
-    return this.delete(group.id).pipe(map((response: RestResponse) => response.isSuccessful));
+    return this.delete(group.id).pipe(map((response: RestResponse) => {
+      const errorMessage = response.isSuccessful === false ? (response as ErrorResponse).errorMessage : undefined;
+      return [response.isSuccessful, errorMessage];
+    }));
   }
 
   /**

src/app/core/eperson/models/group-dto.model.ts

@@ -0,0 +1,17 @@
+import { Group } from './group.model';
+
+/**
+ * This class serves as a Data Transfer Model that contains the Group and whether or not it's able to be deleted
+ */
+export class GroupDtoModel {
+
+  /**
+   * The Group linked to this object
+   */
+  public group: Group;
+  /**
+   * Whether or not the linked Group is able to be deleted
+   */
+  public ableToDelete: boolean;
+
+}

src/assets/i18n/en.json5

@@ -308,7 +308,9 @@
 
   "admin.access-control.groups.notification.deleted.success": "Successfully deleted group \"{{name}}\"",
 
-  "admin.access-control.groups.notification.deleted.failure": "Failed to delete group \"{{name}}\"",
+  "admin.access-control.groups.notification.deleted.failure.title": "Failed to delete group \"{{name}}\"",
+
+  "admin.access-control.groups.notification.deleted.failure.content": "Cause: \"{{cause}}\"",
 
 
   "admin.access-control.groups.form.head.create": "Create group",
@@ -325,6 +327,22 @@
 
   "admin.access-control.groups.form.notification.created.failure.groupNameInUse": "Failed to create Group with name: \"{{name}}\", make sure the name is not already in use.",
 
+  "admin.access-control.groups.form.actions.delete": "Delete Group",
+
+  "admin.access-control.groups.form.delete-group.modal.header": "Delete Group \"{{ dsoName }}\"",
+
+  "admin.access-control.groups.form.delete-group.modal.info": "Are you sure you want to delete Group \"{{ dsoName }}\"",
+
+  "admin.access-control.groups.form.delete-group.modal.cancel": "Cancel",
+
+  "admin.access-control.groups.form.delete-group.modal.confirm": "Delete",
+
+  "admin.access-control.groups.form.notification.deleted.success": "Successfully deleted group \"{{ name }}\"",
+
+  "admin.access-control.groups.form.notification.deleted.failure.title": "Failed to delete group \"{{ name }}\"",
+
+  "admin.access-control.groups.form.notification.deleted.failure.content": "Cause: \"{{ cause }}\"",
+
   "admin.access-control.groups.form.members-list.head": "EPeople",
 
   "admin.access-control.groups.form.members-list.search.head": "Add EPeople",