From f9203a5cb693ee49c246c061cc1273cd401dd497 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Fri, 14 Sep 2018 12:02:20 +0200 Subject: [PATCH] 54472: Tests + caching fix attempts intermediate commit --- .../create-collection-page.component.spec.ts | 25 ++++--- .../create-collection-page.component.ts | 18 +++-- .../create-community-page.component.spec.ts | 25 ++++--- .../create-community-page.component.ts | 14 +++- src/app/core/data/comcol-data.service.spec.ts | 69 ++----------------- src/app/core/data/data.service.spec.ts | 8 ++- src/app/core/data/data.service.ts | 10 +-- .../dspace-rest-v2/dspace-rest-v2.service.ts | 2 +- .../core/metadata/metadata.service.spec.ts | 9 +++ src/app/core/shared/operators.ts | 2 +- 10 files changed, 76 insertions(+), 106 deletions(-) diff --git a/src/app/+collection-page/create-collection-page/create-collection-page.component.spec.ts b/src/app/+collection-page/create-collection-page/create-collection-page.component.spec.ts index efe0e3da97..6196945d80 100644 --- a/src/app/+collection-page/create-collection-page/create-collection-page.component.spec.ts +++ b/src/app/+collection-page/create-collection-page/create-collection-page.component.spec.ts @@ -31,10 +31,13 @@ describe('CreateCollectionPageComponent', () => { name: 'test community' }); + const collection = Object.assign(new Collection(), { + uuid: 'ce41d451-97ed-4a9c-94a1-7de34f16a9f4', + name: 'new collection' + }); + const collectionDataServiceStub = { - create: (com, uuid?) => Observable.of({ - response: new DSOSuccessResponse(null,'200',null) - }) + create: (col, uuid?) => Observable.of(new RemoteData(false, false, true, undefined, collection)) }; const communityDataServiceStub = { findById: (uuid) => Observable.of(new RemoteData(false, false, true, null, Object.assign(new Community(), { @@ -46,7 +49,7 @@ describe('CreateCollectionPageComponent', () => { getQueryParameterValue: (param) => Observable.of(community.uuid) }; const routerStub = { - navigateByUrl: (url) => url + navigate: (commands) => commands }; beforeEach(async(() => { @@ -78,22 +81,18 @@ describe('CreateCollectionPageComponent', () => { }; it('should navigate when successful', () => { - spyOn(router, 'navigateByUrl'); + spyOn(router, 'navigate'); comp.onSubmit(data); fixture.detectChanges(); - expect(router.navigateByUrl).toHaveBeenCalled(); + expect(router.navigate).toHaveBeenCalled(); }); it('should not navigate on failure', () => { - spyOn(router, 'navigateByUrl'); - spyOn(collectionDataService, 'create').and.returnValue(Observable.of({ - response: Object.assign(new ErrorResponse(new RequestError()), { - isSuccessful: false - }) - })); + spyOn(router, 'navigate'); + spyOn(collectionDataService, 'create').and.returnValue(Observable.of(new RemoteData(true, true, false, undefined, collection))); comp.onSubmit(data); fixture.detectChanges(); - expect(router.navigateByUrl).not.toHaveBeenCalled(); + expect(router.navigate).not.toHaveBeenCalled(); }); }); }); diff --git a/src/app/+collection-page/create-collection-page/create-collection-page.component.ts b/src/app/+collection-page/create-collection-page/create-collection-page.component.ts index aea405ea09..4d94a53035 100644 --- a/src/app/+collection-page/create-collection-page/create-collection-page.component.ts +++ b/src/app/+collection-page/create-collection-page/create-collection-page.component.ts @@ -10,11 +10,14 @@ import { Router } from '@angular/router'; import { DSOSuccessResponse, ErrorResponse } from '../../core/cache/response-cache.models'; import { Observable } from 'rxjs/Observable'; import { ResponseCacheEntry } from '../../core/cache/response-cache.reducer'; -import { first, map, take } from 'rxjs/operators'; +import { first, flatMap, map, take } from 'rxjs/operators'; import { RemoteData } from '../../core/data/remote-data'; -import { isNotEmpty } from '../../shared/empty.util'; +import { hasValueOperator, isNotEmpty, isNotEmptyOperator } from '../../shared/empty.util'; import { DSpaceObject } from '../../core/shared/dspace-object.model'; import { HttpEvent } from '@angular/common/http'; +import { getSucceededRemoteData } from '../../core/shared/operators'; +import { ObjectCacheService } from '../../core/cache/object-cache.service'; +import { NormalizedCollection } from '../../core/cache/models/normalized-collection.model'; @Component({ selector: 'ds-create-collection', @@ -30,7 +33,8 @@ export class CreateCollectionPageComponent { private collectionDataService: CollectionDataService, private communityDataService: CommunityDataService, private routeService: RouteService, - private router: Router + private router: Router, + private objectCache: ObjectCacheService ) { this.parentUUID$ = this.routeService.getQueryParameterValue('parent'); this.parentUUID$.subscribe((uuid: string) => { @@ -52,9 +56,11 @@ export class CreateCollectionPageComponent { // TODO: metadata for news and provenance ] }); - this.collectionDataService.create(collection, uuid).subscribe((rd: RemoteData) => { - const url: string = '/collections/' + rd.payload.id; - this.router.navigateByUrl(url); + this.collectionDataService.create(collection, uuid).pipe( + flatMap((rd: RemoteData) => this.objectCache.getByUUID(rd.payload.id)), + isNotEmptyOperator() + ).subscribe((col: NormalizedCollection) => { + this.router.navigate(['collections', col.id]); }); }); } diff --git a/src/app/+community-page/create-community-page/create-community-page.component.spec.ts b/src/app/+community-page/create-community-page/create-community-page.component.spec.ts index 3c4c7648da..6c5eb5f591 100644 --- a/src/app/+community-page/create-community-page/create-community-page.component.spec.ts +++ b/src/app/+community-page/create-community-page/create-community-page.component.spec.ts @@ -27,20 +27,23 @@ describe('CreateCommunityPageComponent', () => { name: 'test community' }); + const newCommunity = Object.assign(new Community(), { + uuid: '1ff59938-a69a-4e62-b9a4-718569c55d48', + name: 'new community' + }); + const communityDataServiceStub = { findById: (uuid) => Observable.of(new RemoteData(false, false, true, null, Object.assign(new Community(), { uuid: uuid, name: community.name }))), - create: (com, uuid?) => Observable.of({ - response: new DSOSuccessResponse(null,'200',null) - }) + create: (com, uuid?) => Observable.of(new RemoteData(false, false, true, undefined, newCommunity)) }; const routeServiceStub = { getQueryParameterValue: (param) => Observable.of(community.uuid) }; const routerStub = { - navigateByUrl: (url) => url + navigate: (commands) => commands }; beforeEach(async(() => { @@ -70,22 +73,18 @@ describe('CreateCommunityPageComponent', () => { }; it('should navigate when successful', () => { - spyOn(router, 'navigateByUrl'); + spyOn(router, 'navigate'); comp.onSubmit(data); fixture.detectChanges(); - expect(router.navigateByUrl).toHaveBeenCalled(); + expect(router.navigate).toHaveBeenCalled(); }); it('should not navigate on failure', () => { - spyOn(router, 'navigateByUrl'); - spyOn(communityDataService, 'create').and.returnValue(Observable.of({ - response: Object.assign(new ErrorResponse(new RequestError()), { - isSuccessful: false - }) - })); + spyOn(router, 'navigate'); + spyOn(communityDataService, 'create').and.returnValue(Observable.of(new RemoteData(true, true, false, undefined, newCommunity))); comp.onSubmit(data); fixture.detectChanges(); - expect(router.navigateByUrl).not.toHaveBeenCalled(); + expect(router.navigate).not.toHaveBeenCalled(); }); }); }); diff --git a/src/app/+community-page/create-community-page/create-community-page.component.ts b/src/app/+community-page/create-community-page/create-community-page.component.ts index afb90141c9..a991bf7127 100644 --- a/src/app/+community-page/create-community-page/create-community-page.component.ts +++ b/src/app/+community-page/create-community-page/create-community-page.component.ts @@ -20,8 +20,12 @@ export class CreateCommunityPageComponent { public parentUUID$: Observable; public communityRDObs: Observable>; - public constructor(private communityDataService: CommunityDataService, private routeService: RouteService, private router: Router) { - this.parentUUID$ = this.routeService.getQueryParameterValue('parent').pipe(take(1)); + public constructor( + private communityDataService: CommunityDataService, + private routeService: RouteService, + private router: Router + ) { + this.parentUUID$ = this.routeService.getQueryParameterValue('parent'); this.parentUUID$.subscribe((uuid: string) => { if (isNotEmpty(uuid)) { this.communityRDObs = this.communityDataService.findById(uuid); @@ -42,7 +46,11 @@ export class CreateCommunityPageComponent { }); this.communityDataService.create(community, uuid).pipe(take(1)).subscribe((rd: RemoteData) => { if (rd.hasSucceeded) { - this.router.navigateByUrl(''); + if (uuid) { + this.router.navigate(['communities', uuid]); + } else { + this.router.navigate([]); + } } }); }); diff --git a/src/app/core/data/comcol-data.service.spec.ts b/src/app/core/data/comcol-data.service.spec.ts index ce3c581b31..0c03b64a1d 100644 --- a/src/app/core/data/comcol-data.service.spec.ts +++ b/src/app/core/data/comcol-data.service.spec.ts @@ -17,6 +17,7 @@ import { HALEndpointService } from '../shared/hal-endpoint.service'; import { Community } from '../shared/community.model'; import { AuthService } from '../auth/auth.service'; import { NotificationsService } from '../../shared/notifications/notifications.service'; +import { HttpClient } from '@angular/common/http'; const LINK_NAME = 'test'; @@ -37,6 +38,7 @@ class TestService extends ComColDataService { protected halService: HALEndpointService, protected authService: AuthService, protected notificationsService: NotificationsService, + protected http: HttpClient, protected linkPath: string ) { super(); @@ -57,6 +59,8 @@ describe('ComColDataService', () => { const rdbService = {} as RemoteDataBuildService; const store = {} as Store; const EnvConfig = {} as GlobalConfig; + const notificationsService = {} as NotificationsService; + const http = {} as HttpClient; const scopeID = 'd9d30c0c-69b7-4369-8397-ca67c888974d'; const communitiesEndpoint = 'https://rest.api/core/communities'; @@ -115,6 +119,8 @@ describe('ComColDataService', () => { objectCache, halService, authService, + notificationsService, + http, LINK_NAME ); } @@ -177,67 +183,4 @@ describe('ComColDataService', () => { }); - describe('create', () => { - let community: Community; - const name = 'test community'; - - beforeEach(() => { - community = Object.assign(new Community(), { - name: name - }); - spyOn(service, 'buildCreateParams'); - }); - - describe('when creating a top-level community', () => { - it('should build params without parent UUID', () => { - scheduler.schedule(() => service.create(community).subscribe()); - scheduler.flush(); - expect(service.buildCreateParams).toHaveBeenCalledWith(community); - }); - }); - - describe('when creating a community part of another community', () => { - let parentCommunity: Community; - const parentName = 'test parent community'; - const parentUUID = 'a20da287-e174-466a-9926-f66b9300d347'; - - beforeEach(() => { - parentCommunity = Object.assign(new Community(), { - id: parentUUID, - uuid: parentUUID, - name: parentName - }); - }); - - it('should build params with parent UUID', () => { - scheduler.schedule(() => service.create(community, parentUUID).subscribe()); - scheduler.flush(); - expect(service.buildCreateParams).toHaveBeenCalledWith(community, parentUUID); - }); - }); - }); - - describe('buildCreateParams', () => { - let community: Community; - const name = 'test community'; - let parentCommunity: Community; - const parentName = 'test parent community'; - const parentUUID = 'a20da287-e174-466a-9926-f66b9300d347'; - - beforeEach(() => { - community = Object.assign(new Community(), { - name: name - }); - parentCommunity = Object.assign(new Community(), { - id: parentUUID, - uuid: parentUUID, - name: parentName - }); - }); - - it('should return the correct url parameters', () => { - expect(service.buildCreateParams(community, parentUUID)).toEqual('?name=' + name + '&parent=' + parentUUID); - }); - }); - }); diff --git a/src/app/core/data/data.service.spec.ts b/src/app/core/data/data.service.spec.ts index 8abf4e51ca..4faea85755 100644 --- a/src/app/core/data/data.service.spec.ts +++ b/src/app/core/data/data.service.spec.ts @@ -10,6 +10,7 @@ import { Observable } from 'rxjs/Observable'; import { FindAllOptions } from './request.models'; import { SortOptions, SortDirection } from '../cache/models/sort-options.model'; import { NotificationsService } from '../../shared/notifications/notifications.service'; +import { HttpClient } from '@angular/common/http'; const LINK_NAME = 'test' @@ -25,7 +26,8 @@ class TestService extends DataService { protected store: Store, protected linkPath: string, protected halService: HALEndpointService, - protected notificationsService: NotificationsService + protected notificationsService: NotificationsService, + protected http: HttpClient ) { super(); } @@ -44,6 +46,7 @@ describe('DataService', () => { const halService = {} as HALEndpointService; const rdbService = {} as RemoteDataBuildService; const notificationsService = {} as NotificationsService; + const http = {} as HttpClient; const store = {} as Store; const endpoint = 'https://rest.api/core'; @@ -55,7 +58,8 @@ describe('DataService', () => { store, LINK_NAME, halService, - notificationsService + notificationsService, + http ); } diff --git a/src/app/core/data/data.service.ts b/src/app/core/data/data.service.ts index 13550b2358..0d746816c7 100644 --- a/src/app/core/data/data.service.ts +++ b/src/app/core/data/data.service.ts @@ -18,11 +18,11 @@ import { } from './request.models'; import { RequestService } from './request.service'; import { NormalizedObject } from '../cache/models/normalized-object.model'; -import { distinctUntilChanged, map, switchMap, take, withLatestFrom } from 'rxjs/operators'; +import { distinctUntilChanged, flatMap, map, switchMap, take, withLatestFrom } from 'rxjs/operators'; import { configureRequest, filterSuccessfulResponses, - getResponseFromSelflink + getResponseFromSelflink, getSucceededRemoteData } from '../shared/operators'; import { ResponseCacheEntry } from '../cache/response-cache.reducer'; import { HttpOptions } from '../dspace-rest-v2/dspace-rest-v2.service'; @@ -31,6 +31,7 @@ import { ErrorResponse, GenericSuccessResponse } from '../cache/response-cache.m import { AuthService } from '../auth/auth.service'; import { NotificationsService } from '../../shared/notifications/notifications.service'; import { NotificationOptions } from '../../shared/notifications/models/notification-options.model'; +import { combineLatest } from 'rxjs/observable/combineLatest'; export abstract class DataService { protected abstract responseCache: ResponseCacheService; @@ -132,7 +133,6 @@ export abstract class DataService ); const payload$ = request$.pipe( - take(1), map((request: RestRequest) => request.href), getResponseFromSelflink(this.responseCache), map((response: ResponseCacheEntry) => { @@ -151,7 +151,9 @@ export abstract class DataService const requestEntry$ = this.requestService.getByUUID(requestId); const responseCache$ = endpoint$.pipe(getResponseFromSelflink(this.responseCache)); - return this.rdbService.toRemoteDataObservable(requestEntry$, responseCache$, payload$); + return this.rdbService.toRemoteDataObservable(requestEntry$, responseCache$, payload$).pipe( + getSucceededRemoteData() + ); } } diff --git a/src/app/core/dspace-rest-v2/dspace-rest-v2.service.ts b/src/app/core/dspace-rest-v2/dspace-rest-v2.service.ts index eafa9c46a0..8c5b5f1ac6 100644 --- a/src/app/core/dspace-rest-v2/dspace-rest-v2.service.ts +++ b/src/app/core/dspace-rest-v2/dspace-rest-v2.service.ts @@ -61,7 +61,7 @@ export class DSpaceRESTv2Service { request(method: RestRequestMethod, url: string, body?: any, options?: HttpOptions): Observable { const requestOptions: HttpOptions = {}; requestOptions.body = body; - if (method === RestRequestMethod.Post && isNotEmpty(body.name)) { + if (method === RestRequestMethod.Post && isNotEmpty(body) && isNotEmpty(body.name)) { requestOptions.body = this.buildFormData(body); } requestOptions.observe = 'response'; diff --git a/src/app/core/metadata/metadata.service.spec.ts b/src/app/core/metadata/metadata.service.spec.ts index f8f36a358e..ca3d3b0e95 100644 --- a/src/app/core/metadata/metadata.service.spec.ts +++ b/src/app/core/metadata/metadata.service.spec.ts @@ -34,6 +34,10 @@ import { MockItem } from '../../shared/mocks/mock-item'; import { MockTranslateLoader } from '../../shared/mocks/mock-translate-loader'; import { BrowseService } from '../browse/browse.service'; import { HALEndpointService } from '../shared/hal-endpoint.service'; +import { AuthService } from '../auth/auth.service'; +import { REQUEST } from '@nguniversal/express-engine/tokens'; +import { NotificationsService } from '../../shared/notifications/notifications.service'; +import { HttpClient } from '@angular/common/http'; /* tslint:disable:max-classes-per-file */ @Component({ @@ -69,6 +73,7 @@ describe('MetadataService', () => { let uuidService: UUIDService; let remoteDataBuildService: RemoteDataBuildService; let itemDataService: ItemDataService; + let authService: AuthService; let location: Location; let router: Router; @@ -115,6 +120,9 @@ describe('MetadataService', () => { { provide: RemoteDataBuildService, useValue: remoteDataBuildService }, { provide: GLOBAL_CONFIG, useValue: ENV_CONFIG }, { provide: HALEndpointService, useValue: {}}, + { provide: AuthService, useValue: {} }, + { provide: NotificationsService, useValue: {} }, + { provide: HttpClient, useValue: {} }, Meta, Title, ItemDataService, @@ -127,6 +135,7 @@ describe('MetadataService', () => { title = TestBed.get(Title); itemDataService = TestBed.get(ItemDataService); metadataService = TestBed.get(MetadataService); + authService = TestBed.get(AuthService); envConfig = TestBed.get(GLOBAL_CONFIG); diff --git a/src/app/core/shared/operators.ts b/src/app/core/shared/operators.ts index 350a664e9d..b44be403b8 100644 --- a/src/app/core/shared/operators.ts +++ b/src/app/core/shared/operators.ts @@ -58,7 +58,7 @@ export const getRemoteDataPayload = () => export const getSucceededRemoteData = () => (source: Observable>): Observable> => - source.pipe(first((rd: RemoteData) => rd.hasSucceeded)); + source.pipe(first((rd: RemoteData) => rd.hasSucceeded && !rd.isLoading)); export const toDSpaceObjectListRD = () => (source: Observable>>>): Observable>> =>