90252: Remove old cleanup code

& add tests to confirm that DataService.delete is called

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

@@ -3,9 +3,9 @@ import { HttpClient } from '@angular/common/http';
 import { NO_ERRORS_SCHEMA } from '@angular/core';
 import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing';
 import { FormsModule, ReactiveFormsModule, FormArray, FormControl, FormGroup,Validators, NG_VALIDATORS, NG_ASYNC_VALIDATORS } from '@angular/forms';
-import { BrowserModule } from '@angular/platform-browser';
+import { BrowserModule, By } from '@angular/platform-browser';
 import { ActivatedRoute, Router } from '@angular/router';
-import { NgbModule } from '@ng-bootstrap/ng-bootstrap';
+import { NgbModal, NgbModule } from '@ng-bootstrap/ng-bootstrap';
 import { Store } from '@ngrx/store';
 import { TranslateLoader, TranslateModule, TranslateService } from '@ngx-translate/core';
 import { Observable, of as observableOf } from 'rxjs';
@@ -27,7 +27,7 @@ import { FormBuilderService } from '../../../shared/form/builder/form-builder.se
 import { NotificationsService } from '../../../shared/notifications/notifications.service';
 import { GroupMock, GroupMock2 } from '../../../shared/testing/group-mock';
 import { GroupFormComponent } from './group-form.component';
-import { createSuccessfulRemoteDataObject$ } from '../../../shared/remote-data.utils';
+import { createFailedRemoteDataObject$, createSuccessfulRemoteDataObject$ } from '../../../shared/remote-data.utils';
 import { getMockFormBuilderService } from '../../../shared/mocks/form-builder-service.mock';
 import { getMockTranslateService } from '../../../shared/mocks/translate.service.mock';
 import { TranslateLoaderMock } from '../../../shared/testing/translate-loader.mock';
@@ -35,6 +35,7 @@ import { RouterMock } from '../../../shared/mocks/router.mock';
 import { NotificationsServiceStub } from '../../../shared/testing/notifications-service.stub';
 import { Operation } from 'fast-json-patch';
 import { ValidateGroupExists } from './validators/group-exists.validator';
+import { NoContent } from '../../../core/shared/NoContent.model';
 
 describe('GroupFormComponent', () => {
   let component: GroupFormComponent;
@@ -87,6 +88,9 @@ describe('GroupFormComponent', () => {
       patch(group: Group, operations: Operation[]) {
         return null;
       },
+      delete(objectId: string, copyVirtualMetadata?: string[]): Observable<RemoteData<NoContent>> {
+        return createSuccessfulRemoteDataObject$({});
+      },
       cancelEditGroup(): void {
         this.activeGroup = null;
       },
@@ -348,4 +352,54 @@ describe('GroupFormComponent', () => {
     });
   });
 
+  describe('delete', () => {
+    let deleteButton;
+    let confirmButton;
+    let cancelButton;
+
+    beforeEach(() => {
+      component.initialisePage();
+
+      component.canEdit$ = observableOf(true);
+      component.groupBeingEdited = {
+        permanent: false
+      } as Group;
+
+      fixture.detectChanges();
+      deleteButton = fixture.debugElement.query(By.css('.delete-button')).nativeElement;
+
+      spyOn(groupsDataServiceStub, 'delete').and.callThrough();
+      spyOn(groupsDataServiceStub, 'getActiveGroup').and.returnValue(observableOf({ id: 'active-group' }));
+    });
+
+    describe('if confirmed via modal', () => {
+      beforeEach(() => {
+        deleteButton.click();
+        fixture.detectChanges();
+        confirmButton = (document as any).querySelector('.modal-footer .confirm');
+      });
+
+      it('should call GroupDataService.delete', () => {
+        confirmButton.click();
+        fixture.detectChanges();
+
+        expect(groupsDataServiceStub.delete).toHaveBeenCalledWith('active-group');
+      });
+    });
+
+    describe('if canceled via modal', () => {
+      beforeEach(() => {
+        deleteButton.click();
+        fixture.detectChanges();
+        cancelButton = (document as any).querySelector('.modal-footer .cancel');
+      });
+
+      it('should not call GroupDataService.delete', () => {
+        cancelButton.click();
+        fixture.detectChanges();
+
+        expect(groupsDataServiceStub.delete).not.toHaveBeenCalled();
+      });
+    });
+  });
 });

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

@@ -426,7 +426,7 @@ export class GroupFormComponent implements OnInit, OnDestroy {
               .subscribe((rd: RemoteData<NoContent>) => {
                 if (rd.hasSucceeded) {
                   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 }),
@@ -439,16 +439,6 @@ export class GroupFormComponent implements OnInit, OnDestroy {
     });
   }
 
-  /**
-   * This method will ensure that the page gets reset and that the cache is cleared
-   */
-  reset() {
-    this.groupDataService.getBrowseEndpoint().pipe(take(1)).subscribe((href: string) => {
-      this.requestService.removeByHrefSubstring(href);
-    });
-    this.onCancel();
-  }
-
   /**
    * Cancel the current edit when component is destroyed & unsub all subscriptions
    */

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

@@ -79,7 +79,7 @@
                     </button>
                   </ng-container>
                   <button *ngIf="!groupDto.group?.permanent && groupDto.ableToDelete"
-                          (click)="deleteGroup(groupDto)" class="btn btn-outline-danger btn-sm"
+                          (click)="deleteGroup(groupDto)" class="btn btn-outline-danger btn-sm btn-delete"
                           title="{{messagePrefix + 'table.edit.buttons.remove' | translate: {name: groupDto.group.name} }}">
                     <i class="fas fa-trash-alt fa-fw"></i>
                   </button>

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

@@ -31,6 +31,7 @@ import { RouterMock } from '../../shared/mocks/router.mock';
 import { PaginationService } from '../../core/pagination/pagination.service';
 import { PaginationServiceStub } from '../../shared/testing/pagination-service.stub';
 import { FeatureID } from '../../core/data/feature-authorization/feature-id';
+import { NoContent } from '../../core/shared/NoContent.model';
 
 describe('GroupRegistryComponent', () => {
   let component: GroupsRegistryComponent;
@@ -145,7 +146,10 @@ describe('GroupRegistryComponent', () => {
           totalPages: 1,
           currentPage: 1
         }), [result]));
-      }
+      },
+      delete(objectId: string, copyVirtualMetadata?: string[]): Observable<RemoteData<NoContent>> {
+        return createSuccessfulRemoteDataObject$({});
+      },
     };
     dsoDataServiceStub = {
       findByHref(href: string): Observable<RemoteData<DSpaceObject>> {
@@ -301,4 +305,29 @@ describe('GroupRegistryComponent', () => {
       });
     });
   });
+
+  describe('delete', () => {
+    let deleteButton;
+
+    beforeEach(fakeAsync(() => {
+      spyOn(groupsDataServiceStub, 'delete').and.callThrough();
+
+      setIsAuthorized(true, true);
+
+      // force rerender after setup changes
+      component.search({ query: '' });
+      tick();
+      fixture.detectChanges();
+
+      // only mockGroup[0] is deletable, so we should only get one button
+      deleteButton = fixture.debugElement.query(By.css('.btn-delete')).nativeElement;
+    }));
+
+    it('should call GroupDataService.delete', () => {
+      deleteButton.click();
+      fixture.detectChanges();
+
+      expect(groupsDataServiceStub.delete).toHaveBeenCalledWith(mockGroups[0].id);
+    });
+  });
 });

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

@@ -199,7 +199,6 @@ export class GroupsRegistryComponent implements OnInit, OnDestroy {
           if (rd.hasSucceeded) {
             this.deletedGroupsIds = [...this.deletedGroupsIds, group.group.id];
             this.notificationsService.success(this.translateService.get(this.messagePrefix + 'notification.deleted.success', { name: group.group.name }));
-            this.reset();
           } else {
             this.notificationsService.error(
               this.translateService.get(this.messagePrefix + 'notification.deleted.failure.title', { name: group.group.name }),
@@ -209,17 +208,6 @@ export class GroupsRegistryComponent implements OnInit, OnDestroy {
     }
   }
 
-  /**
-   * This method will set everything to stale, which will cause the lists on this page to update.
-   */
-  reset() {
-    this.groupService.getBrowseEndpoint().pipe(
-      take(1)
-    ).subscribe((href: string) => {
-      this.requestService.setStaleByHrefSubstring(href);
-    });
-  }
-
   /**
    * Get the members (epersons embedded value of a group)
    * @param group

src/app/admin/admin-registries/metadata-registry/metadata-registry.component.ts

@@ -128,7 +128,6 @@ export class MetadataRegistryComponent {
    * Delete all the selected metadata schemas
    */
   deleteSchemas() {
-    this.registryService.clearMetadataSchemaRequests().subscribe();
     this.registryService.getSelectedMetadataSchemas().pipe(take(1)).subscribe(
       (schemas) => {
         const tasks$ = [];
@@ -148,7 +147,6 @@ export class MetadataRegistryComponent {
           }
           this.registryService.deselectAllMetadataSchema();
           this.registryService.cancelEditMetadataSchema();
-          this.forceUpdateSchemas();
         });
       }
     );

src/app/admin/admin-registries/metadata-schema/metadata-schema.component.ts

@@ -174,15 +174,12 @@ export class MetadataSchemaComponent implements OnInit {
           const failedResponses = responses.filter((response: RemoteData<NoContent>) => response.hasFailed);
           if (successResponses.length > 0) {
             this.showNotification(true, successResponses.length);
-            this.registryService.clearMetadataFieldRequests();
-
           }
           if (failedResponses.length > 0) {
             this.showNotification(false, failedResponses.length);
           }
           this.registryService.deselectAllMetadataField();
           this.registryService.cancelEditMetadataField();
-          this.forceUpdateFields();
         });
       }
     );

src/app/collection-page/delete-collection-page/delete-collection-page.component.ts

@@ -24,8 +24,7 @@ export class DeleteCollectionPageComponent extends DeleteComColPageComponent<Col
     protected route: ActivatedRoute,
     protected notifications: NotificationsService,
     protected translate: TranslateService,
-    protected requestService: RequestService
   ) {
-    super(dsoDataService, router, route, notifications, translate, requestService);
+    super(dsoDataService, router, route, notifications, translate);
   }
 }

src/app/collection-page/edit-collection-page/collection-metadata/collection-metadata.component.spec.ts

@@ -49,9 +49,6 @@ describe('CollectionMetadataComponent', () => {
     success: {},
     error: {}
   });
-  const objectCache = jasmine.createSpyObj('objectCache', {
-    remove: {}
-  });
   const requestService = jasmine.createSpyObj('requestService', {
     setStaleByHrefSubstring: {}
   });
@@ -65,8 +62,7 @@ describe('CollectionMetadataComponent', () => {
         { provide: ItemTemplateDataService, useValue: itemTemplateServiceStub },
         { provide: ActivatedRoute, useValue: { parent: { data: observableOf({ dso: createSuccessfulRemoteDataObject(collection) }) } } },
         { provide: NotificationsService, useValue: notificationsService },
-        { provide: ObjectCacheService, useValue: objectCache },
+        { provide: RequestService, useValue: requestService },
-        { provide: RequestService, useValue: requestService }
       ],
       schemas: [NO_ERRORS_SCHEMA]
     }).compileComponents();
@@ -95,21 +91,19 @@ describe('CollectionMetadataComponent', () => {
   });
 
   describe('deleteItemTemplate', () => {
-    describe('when delete returns a success', () => {
+    beforeEach(() => {
-      beforeEach(() => {
+      (itemTemplateService.deleteByCollectionID as jasmine.Spy).and.returnValue(observableOf(true));
-        (itemTemplateService.deleteByCollectionID as jasmine.Spy).and.returnValue(observableOf(true));
+      comp.deleteItemTemplate();
-        comp.deleteItemTemplate();
+    });
-      });
 
+    it('should call ItemTemplateService.deleteByCollectionID', () => {
+      expect(itemTemplateService.deleteByCollectionID).toHaveBeenCalledWith(template, 'collection-id');
+    });
+
+    describe('when delete returns a success', () => {
       it('should display a success notification', () => {
         expect(notificationsService.success).toHaveBeenCalled();
       });
-
-      it('should reset related object and request cache', () => {
-        expect(requestService.setStaleByHrefSubstring).toHaveBeenCalledWith(collectionTemplateHref);
-        expect(requestService.setStaleByHrefSubstring).toHaveBeenCalledWith(template.self);
-        expect(requestService.setStaleByHrefSubstring).toHaveBeenCalledWith(collection.self);
-      });
     });
 
     describe('when delete returns a failure', () => {

src/app/collection-page/edit-collection-page/collection-metadata/collection-metadata.component.ts

@@ -38,8 +38,7 @@ export class CollectionMetadataComponent extends ComcolMetadataComponent<Collect
     protected route: ActivatedRoute,
     protected notificationsService: NotificationsService,
     protected translate: TranslateService,
-    protected objectCache: ObjectCacheService,
+    protected requestService: RequestService,
-    protected requestService: RequestService
   ) {
     super(collectionDataService, router, route, notificationsService, translate);
   }
@@ -93,23 +92,9 @@ export class CollectionMetadataComponent extends ComcolMetadataComponent<Collect
         getFirstSucceededRemoteDataPayload(),
       )),
     );
-    const templateHref$ = collection$.pipe(
+    combineLatestObservable(collection$, template$).pipe(
-      switchMap((collection) => this.itemTemplateService.getCollectionEndpoint(collection.id)),
+      switchMap(([collection, template]) => {
-    );
+        return this.itemTemplateService.deleteByCollectionID(template, collection.uuid);
-
-    combineLatestObservable(collection$, template$, templateHref$).pipe(
-      switchMap(([collection, template, templateHref]) => {
-        return this.itemTemplateService.deleteByCollectionID(template, collection.uuid).pipe(
-          tap((success: boolean) => {
-            if (success) {
-              this.objectCache.remove(templateHref);
-              this.objectCache.remove(template.self);
-              this.requestService.setStaleByHrefSubstring(template.self);
-              this.requestService.setStaleByHrefSubstring(templateHref);
-              this.requestService.setStaleByHrefSubstring(collection.self);
-            }
-          })
-        );
       })
     ).subscribe((success: boolean) => {
       if (success) {

src/app/community-page/delete-community-page/delete-community-page.component.ts

@@ -24,9 +24,8 @@ export class DeleteCommunityPageComponent extends DeleteComColPageComponent<Comm
     protected route: ActivatedRoute,
     protected notifications: NotificationsService,
     protected translate: TranslateService,
-    protected requestService: RequestService
   ) {
-    super(dsoDataService, router, route, notifications, translate, requestService);
+    super(dsoDataService, router, route, notifications, translate);
   }
 
 }

src/app/core/data/comcol-data.service.spec.ts

@@ -22,6 +22,8 @@ import {
 import { BitstreamDataService } from './bitstream-data.service';
 import { CoreState } from '../core-state.model';
 import { FindListOptions } from './find-list-options.model';
+import { DSpaceObject } from '../shared/dspace-object.model';
+import { Bitstream } from '../shared/bitstream.model';
 
 const LINK_NAME = 'test';
 
@@ -244,4 +246,75 @@ describe('ComColDataService', () => {
       });
     });
   });
+
+  describe('deleteLogo', () => {
+    let dso;
+
+    beforeEach(() => {
+      dso = {
+        _links: {
+          logo: {
+            href: 'logo-href'
+          }
+        }
+      };
+    });
+
+    describe('when DSO has no logo', () => {
+      beforeEach(() => {
+        dso.logo = undefined;
+      });
+
+      it('should return a failed RD', (done) => {
+        service.deleteLogo(dso).subscribe(rd => {
+          expect(rd.hasFailed).toBeTrue();
+          expect(bitstreamDataService.deleteByHref).not.toHaveBeenCalled();
+          done();
+        });
+      });
+    });
+
+    describe('when DSO has a logo', () => {
+      let logo;
+
+      beforeEach(() => {
+        logo = Object.assign(new Bitstream, {
+          id: 'logo-id',
+          _links: {
+            self: {
+              href: 'logo-href',
+            }
+          }
+        });
+      });
+
+      describe('that can be retrieved', () => {
+        beforeEach(() => {
+          dso.logo = createSuccessfulRemoteDataObject$(logo);
+        });
+
+        it('should call BitstreamDataService.deleteByHref', (done) => {
+          service.deleteLogo(dso).subscribe(rd => {
+            expect(rd.hasSucceeded).toBeTrue();
+            expect(bitstreamDataService.deleteByHref).toHaveBeenCalledWith('logo-href');
+            done();
+          });
+        });
+      });
+
+      describe('that cannot be retrieved', () => {
+        beforeEach(() => {
+          dso.logo = createFailedRemoteDataObject$(logo);
+        });
+
+        it('should not call BitstreamDataService.deleteByHref', (done) => {
+          service.deleteLogo(dso).subscribe(rd => {
+            expect(rd.hasFailed).toBeTrue();
+            expect(bitstreamDataService.deleteByHref).not.toHaveBeenCalled();
+            done();
+          });
+        });
+      });
+    });
+  });
 });

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

@@ -21,7 +21,7 @@ import { EPersonDataService } from './eperson-data.service';
 import { EPerson } from './models/eperson.model';
 import { EPersonMock, EPersonMock2 } from '../../shared/testing/eperson.mock';
 import { HALEndpointServiceStub } from '../../shared/testing/hal-endpoint-service.stub';
-import { createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils';
+import { createNoContentRemoteDataObject$, createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils';
 import { getMockRemoteDataBuildServiceHrefMap } from '../../shared/mocks/remote-data-build.service.mock';
 import { TranslateLoaderMock } from '../../shared/mocks/translate-loader.mock';
 import { getMockRequestService } from '../../shared/mocks/request.service.mock';
@@ -287,13 +287,12 @@ describe('EPersonDataService', () => {
 
   describe('deleteEPerson', () => {
     beforeEach(() => {
-      spyOn(service, 'findById').and.returnValue(createSuccessfulRemoteDataObject$(EPersonMock));
+      spyOn(service, 'delete').and.returnValue(createNoContentRemoteDataObject$());
       service.deleteEPerson(EPersonMock).subscribe();
     });
 
-    it('should send DeleteRequest', () => {
+    it('should call DataService.delete with the EPerson\'s UUID', () => {
-      const expected = new DeleteRequest(requestService.generateRequestId(), epersonsEndpoint + '/' + EPersonMock.uuid);
+      expect(service.delete).toHaveBeenCalledWith(EPersonMock.id);
-      expect(requestService.send).toHaveBeenCalledWith(expected);
     });
   });
 

src/app/core/registry/registry.service.spec.ts

@@ -386,6 +386,10 @@ describe('RegistryService', () => {
       result = registryService.deleteMetadataSchema(mockSchemasList[0].id);
     });
 
+    it('should defer to MetadataSchemaDataService.delete', () => {
+      expect(metadataSchemaService.delete).toHaveBeenCalledWith(`${mockSchemasList[0].id}`);
+    });
+
     it('should return a successful response', () => {
       result.subscribe((response: RemoteData<NoContent>) => {
         expect(response.hasSucceeded).toBe(true);
@@ -400,6 +404,10 @@ describe('RegistryService', () => {
       result = registryService.deleteMetadataField(mockFieldsList[0].id);
     });
 
+    it('should defer to MetadataFieldDataService.delete', () => {
+      expect(metadataFieldService.delete).toHaveBeenCalledWith(`${mockFieldsList[0].id}`);
+    });
+
     it('should return a successful response', () => {
       result.subscribe((response: RemoteData<NoContent>) => {
         expect(response.hasSucceeded).toBe(true);

src/app/item-page/edit-item-page/item-bitstreams/item-bitstreams.component.ts

@@ -147,7 +147,6 @@ export class ItemBitstreamsComponent extends AbstractItemUpdateComponent impleme
     // Perform the setup actions from above in order and display notifications
     removedResponses$.pipe(take(1)).subscribe((responses: RemoteData<NoContent>[]) => {
       this.displayNotifications('item.edit.bitstreams.notifications.remove', responses);
-      this.reset();
       this.submitting = false;
     });
   }
@@ -242,27 +241,6 @@ export class ItemBitstreamsComponent extends AbstractItemUpdateComponent impleme
     );
   }
 
-  /**
-   * De-cache the current item (it should automatically reload due to itemUpdateSubscription)
-   */
-  reset() {
-    this.refreshItemCache();
-  }
-
-  /**
-   * Remove the current item's cache from object- and request-cache
-   */
-  refreshItemCache() {
-    this.bundles$.pipe(take(1)).subscribe((bundles: Bundle[]) => {
-      bundles.forEach((bundle: Bundle) => {
-        this.objectCache.remove(bundle.self);
-        this.requestService.removeByHrefSubstring(bundle.self);
-      });
-      this.objectCache.remove(this.item.self);
-      this.requestService.removeByHrefSubstring(this.item.self);
-    });
-  }
-
   /**
    * Unsubscribe from open subscriptions whenever the component gets destroyed
    */

src/app/shared/comcol/comcol-forms/comcol-form/comcol-form.component.spec.ts

@@ -55,6 +55,9 @@ describe('ComColFormComponent', () => {
     })
   ];
 
+  const logo = {
+    id: 'logo'
+  };
   const logoEndpoint = 'rest/api/logo/endpoint';
   const dsoService = Object.assign({
     getLogoEndpoint: () => observableOf(logoEndpoint),
@@ -207,7 +210,7 @@ describe('ComColFormComponent', () => {
       beforeEach(() => {
         initComponent(Object.assign(new Community(), {
           id: 'community-id',
-          logo: createSuccessfulRemoteDataObject$({}),
+          logo: createSuccessfulRemoteDataObject$(logo),
           _links: {
             self: { href: 'community-self' },
             logo: { href: 'community-logo' },
@@ -225,28 +228,31 @@ describe('ComColFormComponent', () => {
 
       describe('submit with logo marked for deletion', () => {
         beforeEach(() => {
+          spyOn(dsoService, 'deleteLogo').and.callThrough();
           comp.markLogoForDeletion = true;
         });
 
+        it('should call dsoService.deleteLogo on the DSO', () => {
+          comp.onSubmit();
+          fixture.detectChanges();
+
+          expect(dsoService.deleteLogo).toHaveBeenCalledWith(comp.dso);
+        });
+
         describe('when dsoService.deleteLogo returns a successful response', () => {
           beforeEach(() => {
-            spyOn(dsoService, 'deleteLogo').and.returnValue(createSuccessfulRemoteDataObject$({}));
+            dsoService.deleteLogo.and.returnValue(createSuccessfulRemoteDataObject$({}));
             comp.onSubmit();
           });
 
           it('should display a success notification', () => {
             expect(notificationsService.success).toHaveBeenCalled();
           });
-
-          it('should remove the object\'s cache', () => {
-            expect(requestServiceStub.removeByHrefSubstring).toHaveBeenCalled();
-            expect(objectCacheStub.remove).toHaveBeenCalled();
-          });
         });
 
         describe('when dsoService.deleteLogo returns an error response', () => {
           beforeEach(() => {
-            spyOn(dsoService, 'deleteLogo').and.returnValue(createFailedRemoteDataObject$('Error', 500));
+            dsoService.deleteLogo.and.returnValue(createFailedRemoteDataObject$('Error', 500));
             comp.onSubmit();
           });
 

src/app/shared/comcol/comcol-forms/comcol-form/comcol-form.component.ts

@@ -184,7 +184,6 @@ export class ComColFormComponent<T extends Collection | Community> implements On
         }
         this.dso.logo = undefined;
         this.uploadFilesOptions.method = RestRequestMethod.POST;
-        this.refreshCache();
         this.finish.emit();
       });
     }

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

@@ -68,7 +68,6 @@ describe('DeleteComColPageComponent', () => {
       {
         delete: createNoContentRemoteDataObject$(),
         findByHref: jasmine.createSpy('findByHref'),
-        refreshCache: jasmine.createSpy('refreshCache')
       });
 
     routerStub = {
@@ -79,10 +78,6 @@ describe('DeleteComColPageComponent', () => {
       data: observableOf(community)
     };
 
-    requestServiceStub = jasmine.createSpyObj('RequestService', {
-      removeByHrefSubstring: jasmine.createSpy('removeByHrefSubstring')
-    });
-
     translateServiceStub = jasmine.createSpyObj('TranslateService', {
       instant: jasmine.createSpy('instant')
     });
@@ -99,7 +94,6 @@ describe('DeleteComColPageComponent', () => {
         { provide: ActivatedRoute, useValue: routeStub },
         { provide: NotificationsService, useValue: new NotificationsServiceStub() },
         { provide: TranslateService, useValue: translateServiceStub },
-        { provide: RequestService, useValue: requestServiceStub }
       ],
       schemas: [NO_ERRORS_SCHEMA]
     }).compileComponents();
@@ -159,7 +153,6 @@ describe('DeleteComColPageComponent', () => {
       scheduler.flush();
       fixture.detectChanges();
       expect(notificationsService.error).toHaveBeenCalled();
-      expect(dsoDataService.refreshCache).not.toHaveBeenCalled();
       expect(router.navigate).toHaveBeenCalled();
     });
 
@@ -169,7 +162,6 @@ describe('DeleteComColPageComponent', () => {
       scheduler.flush();
       fixture.detectChanges();
       expect(notificationsService.success).toHaveBeenCalled();
-      expect(dsoDataService.refreshCache).toHaveBeenCalled();
       expect(router.navigate).toHaveBeenCalled();
     });
 

src/app/shared/comcol/comcol-forms/delete-comcol-page/delete-comcol-page.component.ts

@@ -41,7 +41,6 @@ export class DeleteComColPageComponent<TDomain extends Community | Collection> i
     protected route: ActivatedRoute,
     protected notifications: NotificationsService,
     protected translate: TranslateService,
-    protected requestService: RequestService
   ) {
   }
 
@@ -61,7 +60,6 @@ export class DeleteComColPageComponent<TDomain extends Community | Collection> i
         if (response.hasSucceeded) {
           const successMessage = this.translate.instant((dso as any).type + '.delete.notification.success');
           this.notifications.success(successMessage);
-          this.dsoDataService.refreshCache(dso);
         } else {
           const errorMessage = this.translate.instant((dso as any).type + '.delete.notification.fail');
           this.notifications.error(errorMessage);

src/app/shared/item/item-versions/item-versions-delete-modal/item-versions-delete-modal.component.html

@@ -8,12 +8,12 @@
     <p class="pb-2">{{ "item.version.delete.modal.text" | translate : {version: versionNumber} }}</p>
   </div>
   <div class="modal-footer">
-    <button class="btn btn-outline-secondary btn-sm"
+    <button class="btn btn-outline-secondary btn-sm cancel"
             (click)="onModalClose()"
             title="{{'item.version.delete.modal.button.cancel.tooltip' | translate}}">
       <i class="fas fa-times fa-fw"></i> {{'item.version.delete.modal.button.cancel' | translate}}
     </button>
-    <button class="btn btn-danger btn-sm"
+    <button class="btn btn-danger btn-sm confirm"
             (click)="onModalSubmit()"
             title="{{'item.version.delete.modal.button.confirm.tooltip' | translate}}">
       <i class="fas fa-check fa-fw"></i> {{'item.version.delete.modal.button.confirm' | translate}}

src/app/shared/item/item-versions/item-versions-delete-modal/item-versions-delete-modal.component.ts

@@ -1,5 +1,6 @@
-import { Component } from '@angular/core';
+import { Component, Output } from '@angular/core';
 import { NgbActiveModal } from '@ng-bootstrap/ng-bootstrap';
+import { Subject } from 'rxjs';
 
 @Component({
   selector: 'ds-item-versions-delete-modal',
@@ -7,6 +8,11 @@ import { NgbActiveModal } from '@ng-bootstrap/ng-bootstrap';
   styleUrls: ['./item-versions-delete-modal.component.scss']
 })
 export class ItemVersionsDeleteModalComponent {
+  /**
+   * An event fired when the cancel or confirm button is clicked, with respectively false or true
+   */
+  @Output()
+  response: Subject<boolean> = new Subject();
 
   versionNumber: number;
 
@@ -15,10 +21,12 @@ export class ItemVersionsDeleteModalComponent {
   }
 
   onModalClose() {
+    this.response.next(false);
     this.activeModal.dismiss();
   }
 
   onModalSubmit() {
+    this.response.next(true);
     this.activeModal.close();
   }
 

src/app/shared/item/item-versions/item-versions.component.spec.ts

@@ -1,5 +1,7 @@
 import { ItemVersionsComponent } from './item-versions.component';
-import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing';
+import {
+  ComponentFixture, discardPeriodicTasks, fakeAsync, flush, flushMicrotasks, TestBed, tick, waitForAsync
+} from '@angular/core/testing';
 import { VarDirective } from '../../utils/var.directive';
 import { TranslateModule } from '@ngx-translate/core';
 import { RouterTestingModule } from '@angular/router/testing';
@@ -8,7 +10,7 @@ import { Item } from '../../../core/shared/item.model';
 import { Version } from '../../../core/shared/version.model';
 import { VersionHistory } from '../../../core/shared/version-history.model';
 import { VersionHistoryDataService } from '../../../core/data/version-history-data.service';
-import { By } from '@angular/platform-browser';
+import { BrowserModule, By } from '@angular/platform-browser';
 import { createSuccessfulRemoteDataObject$ } from '../../remote-data.utils';
 import { createPaginatedList } from '../../testing/utils.test';
 import { EMPTY, of, of as observableOf } from 'rxjs';
@@ -17,7 +19,7 @@ import { PaginationServiceStub } from '../../testing/pagination-service.stub';
 import { AuthService } from '../../../core/auth/auth.service';
 import { VersionDataService } from '../../../core/data/version-data.service';
 import { ItemDataService } from '../../../core/data/item-data.service';
-import { FormBuilder } from '@angular/forms';
+import { FormBuilder, FormsModule, ReactiveFormsModule } from '@angular/forms';
 import { NotificationsService } from '../../notifications/notifications.service';
 import { NotificationsServiceStub } from '../../testing/notifications-service.stub';
 import { AuthorizationDataService } from '../../../core/data/feature-authorization/authorization-data.service';
@@ -25,6 +27,11 @@ import { FeatureID } from '../../../core/data/feature-authorization/feature-id';
 import { WorkspaceitemDataService } from '../../../core/submission/workspaceitem-data.service';
 import { WorkflowItemDataService } from '../../../core/submission/workflowitem-data.service';
 import { ConfigurationDataService } from '../../../core/data/configuration-data.service';
+import { Group } from '../../../core/eperson/models/group.model';
+import { Router } from '@angular/router';
+import { RouterStub } from '../../testing/router.stub';
+import { NgbModal, NgbModule } from '@ng-bootstrap/ng-bootstrap';
+import { CommonModule } from '@angular/common';
 
 describe('ItemVersionsComponent', () => {
   let component: ItemVersionsComponent;
@@ -70,6 +77,7 @@ describe('ItemVersionsComponent', () => {
   versionHistory.versions = createSuccessfulRemoteDataObject$(createPaginatedList(versions));
 
   const item1 = Object.assign(new Item(), { // is a workspace item
+    id: 'item-identifier-1',
     uuid: 'item-identifier-1',
     handle: '123456789/1',
     version: createSuccessfulRemoteDataObject$(version1),
@@ -80,6 +88,7 @@ describe('ItemVersionsComponent', () => {
     }
   });
   const item2 = Object.assign(new Item(), {
+    id: 'item-identifier-2',
     uuid: 'item-identifier-2',
     handle: '123456789/2',
     version: createSuccessfulRemoteDataObject$(version2),
@@ -95,6 +104,8 @@ describe('ItemVersionsComponent', () => {
 
   const versionHistoryServiceSpy = jasmine.createSpyObj('versionHistoryService', {
     getVersions: createSuccessfulRemoteDataObject$(createPaginatedList(versions)),
+    getVersionHistoryFromVersion$: of(versionHistory),
+    getLatestVersionItemFromHistory$: of(item1),  // called when version2 is deleted
   });
   const authenticationServiceSpy = jasmine.createSpyObj('authenticationService', {
     isAuthenticated: observableOf(true),
@@ -115,11 +126,19 @@ describe('ItemVersionsComponent', () => {
     findByPropertyName: of(true),
   });
 
+  const itemDataServiceSpy = jasmine.createSpyObj('itemDataService', {
+    delete: createSuccessfulRemoteDataObject$({}),
+  });
+
+  const routerSpy = jasmine.createSpyObj('router', {
+    navigateByUrl: null,
+  });
+
   beforeEach(waitForAsync(() => {
 
     TestBed.configureTestingModule({
       declarations: [ItemVersionsComponent, VarDirective],
-      imports: [TranslateModule.forRoot(), RouterTestingModule.withRoutes([])],
+      imports: [TranslateModule.forRoot(), CommonModule, NgbModule, FormsModule, ReactiveFormsModule, BrowserModule],
       providers: [
         {provide: PaginationService, useValue: new PaginationServiceStub()},
         {provide: FormBuilder, useValue: new FormBuilder()},
@@ -127,11 +146,12 @@ describe('ItemVersionsComponent', () => {
         {provide: AuthService, useValue: authenticationServiceSpy},
         {provide: AuthorizationDataService, useValue: authorizationServiceSpy},
         {provide: VersionHistoryDataService, useValue: versionHistoryServiceSpy},
-        {provide: ItemDataService, useValue: {}},
+        {provide: ItemDataService, useValue: itemDataServiceSpy},
         {provide: VersionDataService, useValue: versionServiceSpy},
         {provide: WorkspaceitemDataService, useValue: workspaceItemDataServiceSpy},
         {provide: WorkflowItemDataService, useValue: workflowItemDataServiceSpy},
         {provide: ConfigurationDataService, useValue: configurationServiceSpy},
+        { provide: Router, useValue: routerSpy },
       ],
       schemas: [NO_ERRORS_SCHEMA]
     }).compileComponents();
@@ -275,4 +295,51 @@ describe('ItemVersionsComponent', () => {
     });
   });
 
+  describe('when deleting a version', () => {
+    let deleteButton;
+    let confirmButton;
+    let cancelButton;
+
+    beforeEach(() => {
+      const canDelete = (featureID: FeatureID, url: string ) => of(featureID === FeatureID.CanDeleteVersion);
+      authorizationServiceSpy.isAuthorized.and.callFake(canDelete);
+
+      fixture.detectChanges();
+
+      // delete the last version in the table (version2 → item2)
+      deleteButton = fixture.debugElement.queryAll(By.css('.version-row-element-delete'))[1].nativeElement;
+
+      itemDataServiceSpy.delete.calls.reset();
+    });
+
+    describe('if confirmed via modal', () => {
+      beforeEach(() => {
+        deleteButton.click();
+        fixture.detectChanges();
+        confirmButton = (document as any).querySelector('.modal-footer .confirm');
+      });
+
+      it('should call ItemService.delete', () => {
+        confirmButton.click();
+        fixture.detectChanges();
+
+        expect(itemDataServiceSpy.delete).toHaveBeenCalledWith(item2.id);
+      });
+    });
+
+    describe('if canceled via modal', () => {
+      beforeEach(() => {
+        deleteButton.click();
+        fixture.detectChanges();
+        cancelButton = (document as any).querySelector('.modal-footer .cancel');
+      });
+
+      it('should not call ItemService.delete', () => {
+        cancelButton.click();
+        fixture.detectChanges();
+
+        expect(itemDataServiceSpy.delete).not.toHaveBeenCalled();
+      });
+    });
+  });
 });

src/app/shared/item/item-versions/item-versions.component.ts

@@ -283,44 +283,42 @@ export class ItemVersionsComponent implements OnInit {
     activeModal.componentInstance.firstVersion = false;
 
     // On modal submit/dismiss
-    activeModal.result.then(() => {
+    activeModal.componentInstance.response.pipe(take(1)).subscribe((ok) => {
-      versionItem$.pipe(
+      if (ok) {
-        getFirstSucceededRemoteDataPayload<Item>(),
+        versionItem$.pipe(
-        // Retrieve version history and invalidate cache
+          getFirstSucceededRemoteDataPayload<Item>(),
-        mergeMap((item: Item) => combineLatest([
+          // Retrieve version history
-          of(item),
+          mergeMap((item: Item) => combineLatest([
-          this.versionHistoryService.getVersionHistoryFromVersion$(version).pipe(
+            of(item),
-            tap((versionHistory: VersionHistory) => {
+            this.versionHistoryService.getVersionHistoryFromVersion$(version)
-              this.versionHistoryService.invalidateVersionHistoryCache(versionHistory.id);
+          ])),
-            })
+          // Delete item
-          )
+          mergeMap(([item, versionHistory]: [Item, VersionHistory]) => combineLatest([
-        ])),
+            this.deleteItemAndGetResult$(item),
-        // Delete item
+            of(versionHistory)
-        mergeMap(([item, versionHistory]: [Item, VersionHistory]) => combineLatest([
+          ])),
-          this.deleteItemAndGetResult$(item),
+          // Retrieve new latest version
-          of(versionHistory)
+          mergeMap(([deleteItemResult, versionHistory]: [boolean, VersionHistory]) => combineLatest([
-        ])),
+            of(deleteItemResult),
-        // Retrieve new latest version
+            this.versionHistoryService.getLatestVersionItemFromHistory$(versionHistory).pipe(
-        mergeMap(([deleteItemResult, versionHistory]: [boolean, VersionHistory]) => combineLatest([
+              tap(() => {
-          of(deleteItemResult),
+                this.getAllVersions(of(versionHistory));
-          this.versionHistoryService.getLatestVersionItemFromHistory$(versionHistory).pipe(
+              }),
-            tap(() => {
+            )
-              this.getAllVersions(of(versionHistory));
+          ])),
-            }),
+        ).subscribe(([deleteHasSucceeded, newLatestVersionItem]: [boolean, Item]) => {
-          )
+          // Notify operation result and redirect to latest item
-        ])),
+          if (deleteHasSucceeded) {
-      ).subscribe(([deleteHasSucceeded, newLatestVersionItem]: [boolean, Item]) => {
+            this.notificationsService.success(null, this.translateService.get(successMessageKey, {'version': versionNumber}));
-        // Notify operation result and redirect to latest item
+          } else {
-        if (deleteHasSucceeded) {
+            this.notificationsService.error(null, this.translateService.get(failureMessageKey, {'version': versionNumber}));
-          this.notificationsService.success(null, this.translateService.get(successMessageKey, {'version': versionNumber}));
+          }
-        } else {
+          if (redirectToLatest) {
-          this.notificationsService.error(null, this.translateService.get(failureMessageKey, {'version': versionNumber}));
+            const path = getItemEditVersionhistoryRoute(newLatestVersionItem);
-        }
+            this.router.navigateByUrl(path);
-        if (redirectToLatest) {
+          }
-          const path = getItemEditVersionhistoryRoute(newLatestVersionItem);
+        });
-          this.router.navigateByUrl(path);
+      }
-        }
-      });
     });
   }
 

src/app/shared/resource-policies/resource-policies.component.spec.ts

@@ -343,6 +343,16 @@ describe('ResourcePoliciesComponent test suite', () => {
         fixture.detectChanges();
       });
 
+      it('should call ResourcePolicyService.delete for the checked policies', () => {
+        resourcePolicyService.delete.and.returnValue(observableOf(true));
+        scheduler = getTestScheduler();
+        scheduler.schedule(() => comp.deleteSelectedResourcePolicies());
+        scheduler.flush();
+
+        // only the first one is checked
+        expect(resourcePolicyService.delete).toHaveBeenCalledWith(resourcePolicy.id);
+      });
+
       it('should notify success when delete is successful', () => {
 
         resourcePolicyService.delete.and.returnValue(observableOf(true));

src/app/shared/resource-policies/resource-policies.component.ts

@@ -157,7 +157,6 @@ export class ResourcePoliciesComponent implements OnInit, OnDestroy {
         } else {
           this.notificationsService.error(null, this.translate.get('resource-policies.delete.failure.content'));
         }
-        this.requestService.setStaleByHrefSubstring(this.resourceUUID);
         this.processingDelete$.next(false);
       })
     );