Fixed feedback

src/app/+item-page/edit-item-page/item-delete/item-delete.component.spec.ts

@@ -27,8 +27,6 @@ let routerStub;
 let mockItemDataService: ItemDataService;
 let routeStub;
 let notificationsServiceStub;
-let successfulRestResponse;
-let failRestResponse;
 
 describe('ItemDeleteComponent', () => {
   beforeEach(async(() => {
@@ -46,14 +44,12 @@ describe('ItemDeleteComponent', () => {
     });
 
     mockItemDataService = jasmine.createSpyObj('mockItemDataService', {
-      delete: observableOf(new RestResponse(true, '200'))
+      delete: observableOf(true)
     });
 
     routeStub = {
       data: observableOf({
-        item: new RemoteData(false, false, true, null, {
+        item: new RemoteData(false, false, true, null, mockItem)
-          id: 'fake-id'
-        })
       })
     };
 
@@ -74,9 +70,6 @@ describe('ItemDeleteComponent', () => {
   }));
 
   beforeEach(() => {
-    successfulRestResponse = new RestResponse(true, '200');
-    failRestResponse = new RestResponse(false, '500');
-
     fixture = TestBed.createComponent(ItemDeleteComponent);
     comp = fixture.componentInstance;
     fixture.detectChanges();
@@ -95,22 +88,21 @@ describe('ItemDeleteComponent', () => {
 
   describe('performAction', () => {
     it('should call delete function from the ItemDataService', () => {
-      spyOn(comp, 'processRestResponse');
+      spyOn(comp, 'notify');
       comp.performAction();
-
+      expect(mockItemDataService.delete).toHaveBeenCalledWith(mockItem);
-      expect(mockItemDataService.delete).toHaveBeenCalledWith(mockItem.id);
+      expect(comp.notify).toHaveBeenCalled();
-      expect(comp.processRestResponse).toHaveBeenCalled();
     });
   });
-  describe('processRestResponse', () => {
+  describe('notify', () => {
     it('should navigate to the homepage on successful deletion of the item', () => {
-      comp.processRestResponse(successfulRestResponse);
+      comp.notify(true);
       expect(routerStub.navigate).toHaveBeenCalledWith(['']);
     });
   });
-  describe('processRestResponse', () => {
+  describe('notify', () => {
     it('should navigate to the item edit page on failed deletion of the item', () => {
-      comp.processRestResponse(failRestResponse);
+      comp.notify(false);
       expect(routerStub.navigate).toHaveBeenCalledWith([getItemEditPath('fake-id')]);
     });
   });

src/app/+item-page/edit-item-page/item-delete/item-delete.component.ts

@@ -19,20 +19,19 @@ export class ItemDeleteComponent extends AbstractSimpleItemActionComponent {
    * Perform the delete action to the item
    */
   performAction() {
-    this.itemDataService.delete(this.item.id).pipe(first()).subscribe(
+    this.itemDataService.delete(this.item).pipe(first()).subscribe(
-      (response: RestResponse) => {
+      (succeeded: boolean) => {
-        this.processRestResponse(response);
+        this.notify(succeeded);
       }
     );
   }
 
   /**
-   * Process the RestResponse retrieved from the server.
    * When the item is successfully delete, navigate to the homepage, otherwise navigate back to the item edit page
    * @param response
    */
-  processRestResponse(response: RestResponse) {
+  notify(succeeded: boolean) {
-    if (response.isSuccessful) {
+    if (succeeded) {
       this.notificationsService.success(this.translateService.get('item.edit.' + this.messageKey + '.success'));
       this.router.navigate(['']);
     } else {

src/app/core/data/data.service.ts

@@ -95,6 +95,11 @@ export abstract class DataService<TNormalized extends NormalizedObject, TDomain
     return this.rdbService.buildList<TNormalized, TDomain>(hrefObs) as Observable<RemoteData<PaginatedList<TDomain>>>;
   }
 
+  /**
+   * Create the HREF for a specific object based on its identifier
+   * @param endpoint The base endpoint for the type of object
+   * @param resourceID The identifier for the object
+   */
   getIDHref(endpoint, resourceID): string {
     return `${endpoint}/${resourceID}`;
   }
@@ -199,6 +204,11 @@ export abstract class DataService<TNormalized extends NormalizedObject, TDomain
     )
   }
 
+  /**
+   * Delete an existing DSpace Object on the server
+   * @param dso The DSpace Object to be removed
+   * Return an observable that emits true when the deletion was successful, false when it failed
+   */
   delete(dso: TDomain): Observable<boolean> {
     const requestId = this.requestService.generateRequestId();
 

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

@@ -162,25 +162,4 @@ describe('ItemDataService', () => {
     });
   });
 
-  describe('getItemDeleteEndpoint', () => {
-    beforeEach(() => {
-      scheduler = getTestScheduler();
-      service = initTestService();
-    });
-
-    it('should return the endpoint to make an item private or public', () => {
-      const result = service.getItemDeleteEndpoint(scopeID);
-      const expected = cold('a', {a: ScopedItemEndpoint});
-
-      expect(result).toBeObservable(expected);
-    });
-
-    it('should delete the item', () => {
-      const expected = new RestResponse(true, '200');
-      const result = service.delete(scopeID);
-      result.subscribe((v) => expect(v).toEqual(expected));
-
-    });
-  });
-
 });

src/app/core/data/item-data.service.ts

@@ -14,7 +14,7 @@ import { URLCombiner } from '../url-combiner/url-combiner';
 import { DataService } from './data.service';
 import { RequestService } from './request.service';
 import { HALEndpointService } from '../shared/hal-endpoint.service';
-import { DeleteRequest, FindAllOptions, PatchRequest, RestRequest } from './request.models';
+import { FindAllOptions, PatchRequest, RestRequest } from './request.models';
 import { ObjectCacheService } from '../cache/object-cache.service';
 import { NotificationsService } from '../../shared/notifications/notifications.service';
 import { HttpClient } from '@angular/common/http';
@@ -64,7 +64,7 @@ export class ItemDataService extends DataService<NormalizedItem, Item> {
    */
   public getItemWithdrawEndpoint(itemId: string): Observable<string> {
     return this.halService.getEndpoint(this.linkPath).pipe(
-      map((endpoint: string) => this.getFindByIDHref(endpoint, itemId))
+      map((endpoint: string) => this.getIDHref(endpoint, itemId))
     );
   }
 
@@ -74,17 +74,7 @@ export class ItemDataService extends DataService<NormalizedItem, Item> {
    */
   public getItemDiscoverableEndpoint(itemId: string): Observable<string> {
     return this.halService.getEndpoint(this.linkPath).pipe(
-      map((endpoint: string) => this.getFindByIDHref(endpoint, itemId))
+      map((endpoint: string) => this.getIDHref(endpoint, itemId))
-    );
-  }
-
-  /**
-   * Get the endpoint to delete the item
-   * @param itemId
-   */
-  public getItemDeleteEndpoint(itemId: string): Observable<string> {
-    return this.halService.getEndpoint(this.linkPath).pipe(
-      map((endpoint: string) => this.getFindByIDHref(endpoint, itemId))
     );
   }
 
@@ -129,22 +119,4 @@ export class ItemDataService extends DataService<NormalizedItem, Item> {
       map((requestEntry: RequestEntry) => requestEntry.response)
     );
   }
-
-  /**
-   * Delete the item
-   * @param itemId
-   */
-  public delete(itemId: string) {
-    return this.getItemDeleteEndpoint(itemId).pipe(
-      distinctUntilChanged(),
-      map((endpointURL: string) =>
-        new DeleteRequest(this.requestService.generateRequestId(), endpointURL)
-      ),
-      configureRequest(this.requestService),
-      map((request: RestRequest) => request.href),
-      getRequestFromRequestHref(this.requestService),
-      map((requestEntry: RequestEntry) => requestEntry.response)
-    );
-  }
-
 }

src/app/core/data/request.models.ts

@@ -227,6 +227,9 @@ export class CreateRequest extends PostRequest {
   }
 }
 
+/**
+ * Request to delete an object based on its identifier
+ */
 export class DeleteByIDRequest extends DeleteRequest {
   constructor(
     uuid: string,