fix https://github.com/HumanBrainProject/interactive-viewer/issues/453

chore: added tests for history.service
chore: added tests to urlUtil

package.json

@@ -38,7 +38,7 @@
     "@angular/platform-browser-dynamic": "^7.2.15",
     "@angular/router": "^7.2.15",
     "@ngrx/effects": "^7.4.0",
-    "@ngrx/store": "^6.0.1",
+    "@ngrx/store": "^7.4.0",
     "@ngtools/webpack": "^6.0.5",
     "@types/chart.js": "^2.7.20",
     "@types/jasmine": "^3.3.12",
@@ -59,6 +59,7 @@
     "html2canvas": "^1.0.0-rc.1",
     "jasmine": "^3.1.0",
     "jasmine-core": "^3.5.0",
+    "jasmine-marbles": "^0.6.0",
     "jasmine-spec-reporter": "^4.2.1",
     "json-loader": "^0.5.7",
     "karma": "^4.1.0",

src/atlasViewer/atlasViewer.history.service.spec.ts

+import { AtlasViewerHistoryUseEffect } from './atlasViewer.history.service'
+import { TestBed, tick, fakeAsync, flush } from '@angular/core/testing'
+import { provideMockActions } from '@ngrx/effects/testing'
+import { provideMockStore } from '@ngrx/store/testing'
+import { Observable, of, Subscription } from 'rxjs'
+import { Action, Store } from '@ngrx/store'
+import { defaultRootState } from '../services/stateStore.service'
+import { HttpClientModule } from '@angular/common/http'
+import { cold } from 'jasmine-marbles'
+
+const bigbrainJson = require('!json-loader!src/res/ext/bigbrain.json')
+
+const actions$: Observable<Action> = of({type: 'TEST'})
+
+describe('atlasviewer.history.service.ts', () => {
+  beforeEach(() => {
+    TestBed.configureTestingModule({
+      imports: [
+        HttpClientModule
+      ],
+      providers: [
+        AtlasViewerHistoryUseEffect,
+        provideMockActions(() => actions$),
+        provideMockStore({ initialState: defaultRootState })
+      ]
+    })
+  })
+
+  afterEach(() => {
+  })
+
+  describe('currentStateSearchParam$', () => {
+
+    it('should fire when template is set', () => {
+
+      const effect = TestBed.get(AtlasViewerHistoryUseEffect)
+      const store = TestBed.get(Store)
+      const { viewerState } = defaultRootState
+      store.setState({
+        ...defaultRootState,
+        viewerState: {
+          ...viewerState,
+          templateSelected: bigbrainJson
+        }
+      })
+      
+      const expected = cold('(a)', {
+        a: 'templateSelected=Big+Brain+%28Histology%29'
+      })
+      expect(effect.currentStateSearchParam$).toBeObservable(expected)
+    })
+  
+    it('should fire when template and parcellation is set', () => {
+  
+      const effect = TestBed.get(AtlasViewerHistoryUseEffect)
+      const store = TestBed.get(Store)
+      const { viewerState } = defaultRootState
+      store.setState({
+        ...defaultRootState,
+        viewerState: {
+          ...viewerState,
+          templateSelected: bigbrainJson,
+          parcellationSelected: bigbrainJson.parcellations[0]
+        }
+      })
+      
+      const expected = cold('(a)', {
+        a: 'templateSelected=Big+Brain+%28Histology%29&parcellationSelected=Cytoarchitectonic+Maps'
+      })
+      
+      expect(effect.currentStateSearchParam$).toBeObservable(expected)
+    })
+  })
+
+
+  describe('setNewSearchString$', () => {
+
+    const obj = {
+      spiedFn: () => {}
+    }
+    const subscriptions: Subscription[] = []
+
+    let spy
+
+    beforeAll(() => {
+      spy = spyOn(obj, 'spiedFn')
+    })
+
+    beforeEach(() => {
+      spy.calls.reset()
+    })
+
+    afterEach(() => {
+      while (subscriptions.length > 0) subscriptions.pop().unsubscribe()
+    })
+
+    it('should fire when set', fakeAsync(() => {
+
+      const store = TestBed.get(Store)
+      const effect = TestBed.get(AtlasViewerHistoryUseEffect)
+      subscriptions.push(
+        effect.setNewSearchString$.subscribe(obj.spiedFn)
+      )
+      const { viewerState } = defaultRootState
+  
+      store.setState({
+        ...defaultRootState,
+        viewerState: {
+          ...viewerState,
+          templateSelected: bigbrainJson,
+          parcellationSelected: bigbrainJson.parcellations[0]
+        }
+      })
+      tick(100)
+      expect(spy).toHaveBeenCalledTimes(1)
+    }))
+
+    it('should not call window.history.pushState on start', fakeAsync(() => {
+      tick(100)
+      expect(spy).toHaveBeenCalledTimes(0)
+    }))
+  
+  })
+  
+})
\ No newline at end of file

src/atlasViewer/atlasViewer.history.service.ts

@@ -2,7 +2,7 @@ import { Injectable, OnDestroy } from "@angular/core";
 import { Actions, Effect, ofType } from '@ngrx/effects'
 import { Store } from "@ngrx/store";
 import { fromEvent, merge, of, Subscription } from "rxjs";
-import { catchError, debounceTime, distinctUntilChanged, filter, map, startWith, switchMap, switchMapTo, take, withLatestFrom } from "rxjs/operators";
+import { debounceTime, distinctUntilChanged, filter, map, startWith, switchMap, switchMapTo, take, withLatestFrom, shareReplay } from "rxjs/operators";
 import { defaultRootState, GENERAL_ACTION_TYPES, IavRootStoreInterface } from "src/services/stateStore.service";
 import { AtlasViewerConstantsServices } from "src/ui/databrowserModule/singleDataset/singleDataset.base";
 import { cvtSearchParamToState, cvtStateToSearchParam } from "./atlasViewer.urlUtil";
@@ -79,53 +79,56 @@ export class AtlasViewerHistoryUseEffect implements OnDestroy {
   private subscriptions: Subscription[] = []
 
   private currentStateSearchParam$ = this.store$.pipe(
-    map(getSearchParamStringFromState),
-    catchError((err, _obs) => {
-      // TODO error parsing current state search param. let user know
-      return of(null)
+    map(s => {
+      try {
+        return getSearchParamStringFromState(s)
+      } catch (e) {
+        // TODO parsing state to search param error
+        return null
+      }
     }),
     filter(v => v !==  null),
   )
 
+  // GENERAL_ACTION_TYPES.APPLY_STATE is triggered by pop state or initial
+  // conventiently, the action has a state property
+  public setNewSearchString$ = this.actions$.pipe(
+    ofType(GENERAL_ACTION_TYPES.APPLY_STATE),
+    // subscribe to inner obs on init
+    startWith({}),
+    switchMap(({ state }: any) =>
+      this.currentStateSearchParam$.pipe(
+        shareReplay(1),
+        distinctUntilChanged(),
+        debounceTime(100),
+
+        // compares the searchParam triggerd by change of state with the searchParam generated by GENERAL_ACTION_TYPES.APPLY_STATE
+        // if the same, the change is induced by GENERAL_ACTION_TYPES.APPLY_STATE, and should NOT be pushed to history
+        filter((newSearchParam, index) => {
+          try {
+            const oldSearchParam = (state && getSearchParamStringFromState(state)) || ''
+
+            // in the unlikely event that user returns to the exact same state without use forward/back button
+            return index > 0 || newSearchParam !== oldSearchParam
+          } catch (e) {
+            return index > 0 || newSearchParam !== ''
+          }
+        })
+      )
+    )
+  )
+
   constructor(
     private store$: Store<IavRootStoreInterface>,
     private actions$: Actions,
-    private constantService: AtlasViewerConstantsServices,
+    private constantService: AtlasViewerConstantsServices
   ) {
-    this.subscriptions.push(
-
-      // GENERAL_ACTION_TYPES.APPLY_STATE is triggered by pop state or initial
-      // conventiently, the action has a state property
-      this.actions$.pipe(
-        ofType(GENERAL_ACTION_TYPES.APPLY_STATE),
-        // subscribe to inner obs on init
-        startWith({}),
-        switchMap(({ state }: any) =>
-          this.currentStateSearchParam$.pipe(
-            distinctUntilChanged(),
-            debounceTime(100),
 
-            // compares the searchParam triggerd by change of state with the searchParam generated by GENERAL_ACTION_TYPES.APPLY_STATE
-            // if the same, the change is induced by GENERAL_ACTION_TYPES.APPLY_STATE, and should NOT be pushed to history
-            filter((newSearchParam, index) => {
-              try {
-
-                const oldSearchParam = (state && getSearchParamStringFromState(state)) || ''
-
-                // in the unlikely event that user returns to the exact same state without use forward/back button
-                return index > 0 || newSearchParam !== oldSearchParam
-              } catch (e) {
-                return index > 0 || newSearchParam !== ''
-              }
-            }),
-          ),
-        ),
-      ).subscribe(newSearchString => {
-        const url = new URL(window.location.toString())
-        url.search = newSearchString
-        window.history.pushState(newSearchString, '', url.toString())
-      }),
-    )
+    this.setNewSearchString$.subscribe(newSearchString => {
+      const url = new URL(window.location.toString())
+      url.search = newSearchString
+      window.history.pushState(newSearchString, '', url.toString())
+    })
   }
 
   public ngOnDestroy() {

src/atlasViewer/atlasViewer.urlUtil.spec.ts

@@ -2,7 +2,7 @@
 
 import {} from 'jasmine'
 import { defaultRootState } from 'src/services/stateStore.service'
-import { cvtSearchParamToState, PARSING_SEARCHPARAM_ERROR } from './atlasViewer.urlUtil'
+import { cvtSearchParamToState, PARSING_SEARCHPARAM_ERROR, cvtStateToSearchParam } from './atlasViewer.urlUtil'
 
 const bigbrainJson = require('!json-loader!src/res/ext/bigbrain.json')
 const colin = require('!json-loader!src/res/ext/colin.json')
@@ -19,6 +19,7 @@ const fetchedTemplateRootState = {
   },
 }
 
+// TODO finish writing tests
 describe('atlasViewer.urlService.service.ts', () => {
   describe('cvtSearchParamToState', () => {
     it('convert empty search param to empty state', () => {
@@ -67,5 +68,34 @@ describe('atlasViewer.urlService.service.ts', () => {
 
   describe('cvtStateToSearchParam', () => {
 
+    it('should convert template selected', () => {
+      const { viewerState } = defaultRootState
+      const searchParam = cvtStateToSearchParam({
+        ...defaultRootState,
+        viewerState: {
+          ...viewerState,
+          templateSelected: bigbrainJson,
+        }
+      })
+
+      const stringified = searchParam.toString()
+      expect(stringified).toBe('templateSelected=Big+Brain+%28Histology%29')
+    })
+  })
+
+  it('should convert template selected and parcellation selected', () => {
+
+    const { viewerState } = defaultRootState
+    const searchParam = cvtStateToSearchParam({
+      ...defaultRootState,
+      viewerState: {
+        ...viewerState,
+        templateSelected: bigbrainJson,
+        parcellationSelected: bigbrainJson.parcellations[0]
+      }
+    })
+
+    const stringified = searchParam.toString()
+    expect(stringified).toBe('templateSelected=Big+Brain+%28Histology%29&parcellationSelected=Cytoarchitectonic+Maps')
   })
 })

src/atlasViewer/atlasViewer.urlUtil.ts

@@ -28,7 +28,7 @@ export const cvtStateToSearchParam = (state: IavRootStoreInterface): URLSearchPa
 
   // encoding states
   searchParam.set('templateSelected', templateSelected.name)
-  searchParam.set('parcellationSelected', parcellationSelected.name)
+  if (!!parcellationSelected) searchParam.set('parcellationSelected', parcellationSelected.name)
 
   // encoding selected regions
   const accumulatorMap = new Map<string, number[]>()
@@ -41,28 +41,34 @@ export const cvtStateToSearchParam = (state: IavRootStoreInterface): URLSearchPa
   for (const [key, arr] of accumulatorMap) {
     cRegionObj[key] = arr.map(n => encodeNumber(n)).join(separator)
   }
-  searchParam.set('cRegionsSelected', JSON.stringify(cRegionObj))
+  if (Object.keys(cRegionObj).length > 0) searchParam.set('cRegionsSelected', JSON.stringify(cRegionObj))
 
   // encoding navigation
-  const { orientation, perspectiveOrientation, perspectiveZoom, position, zoom } = navigation
-  const cNavString = [
-    orientation.map(n => encodeNumber(n, {float: true})).join(separator),
-    perspectiveOrientation.map(n => encodeNumber(n, {float: true})).join(separator),
-    encodeNumber(Math.floor(perspectiveZoom)),
-    Array.from(position).map((v: number) => Math.floor(v)).map(n => encodeNumber(n)).join(separator),
-    encodeNumber(Math.floor(zoom)),
-  ].join(`${separator}${separator}`)
-  searchParam.set('cNavigation', cNavString)
+  if (navigation) {
+    const { orientation, perspectiveOrientation, perspectiveZoom, position, zoom } = navigation
+    if (orientation && perspectiveOrientation && perspectiveZoom && position && zoom) {
+      const cNavString = [
+        orientation.map(n => encodeNumber(n, {float: true})).join(separator),
+        perspectiveOrientation.map(n => encodeNumber(n, {float: true})).join(separator),
+        encodeNumber(Math.floor(perspectiveZoom)),
+        Array.from(position).map((v: number) => Math.floor(v)).map(n => encodeNumber(n)).join(separator),
+        encodeNumber(Math.floor(zoom)),
+      ].join(`${separator}${separator}`)
+      searchParam.set('cNavigation', cNavString)
+    }
+  }
 
   // encode nifti layers
-  const initialNgState = templateSelected.nehubaConfig.dataset.initialNgState
-  const { layers } = ngViewerState
-  const additionalLayers = layers.filter(layer =>
-    /^blob:/.test(layer.name) &&
-    Object.keys(initialNgState.layers).findIndex(layerName => layerName === layer.name) < 0,
-  )
-  const niftiLayers = additionalLayers.filter(layer => /^nifti:\/\//.test(layer.source))
-  if (niftiLayers.length > 0) { searchParam.set('niftiLayers', niftiLayers.join('__')) }
+  if (!!templateSelected.nehubaConfig) {
+    const initialNgState = templateSelected.nehubaConfig.dataset.initialNgState
+    const { layers } = ngViewerState
+    const additionalLayers = layers.filter(layer =>
+      /^blob:/.test(layer.name) &&
+      Object.keys(initialNgState.layers).findIndex(layerName => layerName === layer.name) < 0,
+    )
+    const niftiLayers = additionalLayers.filter(layer => /^nifti:\/\//.test(layer.source))
+    if (niftiLayers.length > 0) { searchParam.set('niftiLayers', niftiLayers.join('__')) }
+  }
 
   // plugin state
   const { initManifests } = pluginState