From b8252a1a8eac709778fc3c21f558e39783460c47 Mon Sep 17 00:00:00 2001 From: lotte Date: Thu, 24 Jan 2019 14:13:22 +0100 Subject: [PATCH] Fixed feedback --- .../item-delete/item-delete.component.spec.ts | 34 +++++++------------ .../item-delete/item-delete.component.ts | 11 +++--- src/app/core/data/data.service.ts | 10 ++++++ src/app/core/data/item-data.service.spec.ts | 21 ------------ src/app/core/data/item-data.service.ts | 34 ++----------------- src/app/core/data/request.models.ts | 3 ++ 6 files changed, 34 insertions(+), 79 deletions(-) diff --git a/src/app/+item-page/edit-item-page/item-delete/item-delete.component.spec.ts b/src/app/+item-page/edit-item-page/item-delete/item-delete.component.spec.ts index c03ae7c3fe..6d435c8de8 100644 --- a/src/app/+item-page/edit-item-page/item-delete/item-delete.component.spec.ts +++ b/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, { - id: 'fake-id' - }) + item: new RemoteData(false, false, true, null, mockItem) }) }; @@ -63,10 +59,10 @@ describe('ItemDeleteComponent', () => { imports: [CommonModule, FormsModule, RouterTestingModule.withRoutes([]), TranslateModule.forRoot(), NgbModule.forRoot()], declarations: [ItemDeleteComponent], providers: [ - {provide: ActivatedRoute, useValue: routeStub}, - {provide: Router, useValue: routerStub}, - {provide: ItemDataService, useValue: mockItemDataService}, - {provide: NotificationsService, useValue: notificationsServiceStub}, + { provide: ActivatedRoute, useValue: routeStub }, + { provide: Router, useValue: routerStub }, + { provide: ItemDataService, useValue: mockItemDataService }, + { provide: NotificationsService, useValue: notificationsServiceStub }, ], schemas: [ CUSTOM_ELEMENTS_SCHEMA ] @@ -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.id); - expect(comp.processRestResponse).toHaveBeenCalled(); + expect(mockItemDataService.delete).toHaveBeenCalledWith(mockItem); + expect(comp.notify).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')]); }); }); diff --git a/src/app/+item-page/edit-item-page/item-delete/item-delete.component.ts b/src/app/+item-page/edit-item-page/item-delete/item-delete.component.ts index 95f25c67bc..2700b45475 100644 --- a/src/app/+item-page/edit-item-page/item-delete/item-delete.component.ts +++ b/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( - (response: RestResponse) => { - this.processRestResponse(response); + this.itemDataService.delete(this.item).pipe(first()).subscribe( + (succeeded: boolean) => { + 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) { - if (response.isSuccessful) { + notify(succeeded: boolean) { + if (succeeded) { this.notificationsService.success(this.translateService.get('item.edit.' + this.messageKey + '.success')); this.router.navigate(['']); } else { diff --git a/src/app/core/data/data.service.ts b/src/app/core/data/data.service.ts index f4d7bcdf4f..440013c31c 100644 --- a/src/app/core/data/data.service.ts +++ b/src/app/core/data/data.service.ts @@ -95,6 +95,11 @@ export abstract class DataService(hrefObs) as Observable>>; } + /** + * 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 { const requestId = this.requestService.generateRequestId(); diff --git a/src/app/core/data/item-data.service.spec.ts b/src/app/core/data/item-data.service.spec.ts index 6cf7e503d3..02c70791b5 100644 --- a/src/app/core/data/item-data.service.spec.ts +++ b/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)); - - }); - }); - }); diff --git a/src/app/core/data/item-data.service.ts b/src/app/core/data/item-data.service.ts index c67b49f70d..bd3c42a67c 100644 --- a/src/app/core/data/item-data.service.ts +++ b/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 { */ public getItemWithdrawEndpoint(itemId: string): Observable { 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 { */ public getItemDiscoverableEndpoint(itemId: string): Observable { return this.halService.getEndpoint(this.linkPath).pipe( - map((endpoint: string) => this.getFindByIDHref(endpoint, itemId)) - ); - } - - /** - * Get the endpoint to delete the item - * @param itemId - */ - public getItemDeleteEndpoint(itemId: string): Observable { - return this.halService.getEndpoint(this.linkPath).pipe( - map((endpoint: string) => this.getFindByIDHref(endpoint, itemId)) + map((endpoint: string) => this.getIDHref(endpoint, itemId)) ); } @@ -129,22 +119,4 @@ export class ItemDataService extends DataService { 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) - ); - } - } diff --git a/src/app/core/data/request.models.ts b/src/app/core/data/request.models.ts index 6f0ff5b605..e4920058ac 100644 --- a/src/app/core/data/request.models.ts +++ b/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,