54472: Tests + caching fix attempts intermediate commit

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({
+    create: (col, uuid?) => Observable.of(new RemoteData(false, false, true, undefined, collection))
-      response: new DSOSuccessResponse(null,'200',null)
-    })
   };
   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(router, 'navigate');
-      spyOn(collectionDataService, 'create').and.returnValue(Observable.of({
+      spyOn(collectionDataService, 'create').and.returnValue(Observable.of(new RemoteData(true, true, false, undefined, collection)));
-        response: Object.assign(new ErrorResponse(new RequestError()), {
-          isSuccessful: false
-        })
-      }));
       comp.onSubmit(data);
       fixture.detectChanges();
-      expect(router.navigateByUrl).not.toHaveBeenCalled();
+      expect(router.navigate).not.toHaveBeenCalled();
     });
   });
 });

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<DSpaceObject>) => {
+      this.collectionDataService.create(collection, uuid).pipe(
-        const url: string = '/collections/' + rd.payload.id;
+        flatMap((rd: RemoteData<Collection>) => this.objectCache.getByUUID(rd.payload.id)),
-        this.router.navigateByUrl(url);
+        isNotEmptyOperator()
+      ).subscribe((col: NormalizedCollection) => {
+        this.router.navigate(['collections', col.id]);
       });
     });
   }

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({
+    create: (com, uuid?) => Observable.of(new RemoteData(false, false, true, undefined, newCommunity))
-      response: new DSOSuccessResponse(null,'200',null)
-    })
   };
   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(router, 'navigate');
-      spyOn(communityDataService, 'create').and.returnValue(Observable.of({
+      spyOn(communityDataService, 'create').and.returnValue(Observable.of(new RemoteData(true, true, false, undefined, newCommunity)));
-        response: Object.assign(new ErrorResponse(new RequestError()), {
-          isSuccessful: false
-        })
-      }));
       comp.onSubmit(data);
       fixture.detectChanges();
-      expect(router.navigateByUrl).not.toHaveBeenCalled();
+      expect(router.navigate).not.toHaveBeenCalled();
     });
   });
 });

src/app/+community-page/create-community-page/create-community-page.component.ts

@@ -20,8 +20,12 @@ export class CreateCommunityPageComponent {
   public parentUUID$: Observable<string>;
   public communityRDObs: Observable<RemoteData<Community>>;
 
-  public constructor(private communityDataService: CommunityDataService, private routeService: RouteService, private router: Router) {
+  public constructor(
-    this.parentUUID$ = this.routeService.getQueryParameterValue('parent').pipe(take(1));
+    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<DSpaceObject>) => {
         if (rd.hasSucceeded) {
-          this.router.navigateByUrl('');
+          if (uuid) {
+            this.router.navigate(['communities', uuid]);
+          } else {
+            this.router.navigate([]);
+          }
         }
       });
     });

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<NormalizedTestObject, any> {
     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<CoreState>;
   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);
-    });
-  });
-
 });

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<NormalizedTestObject, any> {
         protected store: Store<CoreState>,
         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<CoreState>;
     const endpoint = 'https://rest.api/core';
 
@@ -55,7 +58,8 @@ describe('DataService', () => {
             store,
             LINK_NAME,
             halService,
-            notificationsService
+            notificationsService,
+            http
           );
     }
 

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<TNormalized extends NormalizedObject, TDomain> {
   protected abstract responseCache: ResponseCacheService;
@@ -132,7 +133,6 @@ export abstract class DataService<TNormalized extends NormalizedObject, TDomain>
     );
 
     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<TNormalized extends NormalizedObject, TDomain>
     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()
+    );
   }
 
 }

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<DSpaceRESTV2Response> {
     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';

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);
 

src/app/core/shared/operators.ts

@@ -58,7 +58,7 @@ export const getRemoteDataPayload = () =>
 
 export const getSucceededRemoteData = () =>
   <T>(source: Observable<RemoteData<T>>): Observable<RemoteData<T>> =>
-    source.pipe(first((rd: RemoteData<T>) => rd.hasSucceeded));
+    source.pipe(first((rd: RemoteData<T>) => rd.hasSucceeded && !rd.isLoading));
 
 export const toDSpaceObjectListRD = () =>
   <T extends DSpaceObject>(source: Observable<RemoteData<PaginatedList<SearchResult<T>>>>): Observable<RemoteData<PaginatedList<T>>> =>