From bd181061519d1f6e1be83017fcb3bf2bcb0855bd Mon Sep 17 00:00:00 2001
From: Xiao Gui <xgui3783@gmail.com>
Date: Tue, 27 Jul 2021 10:36:28 +0200
Subject: [PATCH] bugfix: roi$ should retain ngId information

---
 .../viewerCmp/viewerCmp.component.spec.ts     | 125 ++++++++++++++++++
 .../viewerCmp/viewerCmp.component.ts          |  48 ++++---
 2 files changed, 151 insertions(+), 22 deletions(-)
 create mode 100644 src/viewerModule/viewerCmp/viewerCmp.component.spec.ts

diff --git a/src/viewerModule/viewerCmp/viewerCmp.component.spec.ts b/src/viewerModule/viewerCmp/viewerCmp.component.spec.ts
new file mode 100644
index 000000000..dba48dfcf
--- /dev/null
+++ b/src/viewerModule/viewerCmp/viewerCmp.component.spec.ts
@@ -0,0 +1,125 @@
+import { TestBed } from "@angular/core/testing"
+import { MockStore, provideMockStore } from "@ngrx/store/testing"
+import { hot } from "jasmine-marbles"
+import { Observable, of, throwError } from "rxjs"
+import { viewerStateContextedSelectedRegionsSelector } from "src/services/state/viewerState/selectors"
+import { ROIFactory } from "./viewerCmp.component"
+
+describe('> viewerCmp.component.ts', () => {
+  let mockStore: MockStore
+  beforeEach(() => {
+    TestBed.configureTestingModule({
+      providers: [
+        provideMockStore()
+      ]
+    })
+    mockStore = TestBed.inject(MockStore)
+  })
+  describe('> ROIFactory', () => {
+    const mockDetail = {
+      foo: 'bar'
+    }
+    class MockPCSvc {
+      getRegionDetail(){
+        return of(mockDetail)
+      }
+    }
+    const pcsvc = new MockPCSvc()
+    let getRegionDetailSpy:  jasmine.Spy
+
+    beforeEach(() => {
+      getRegionDetailSpy = spyOn(pcsvc, 'getRegionDetail')
+      mockStore.overrideSelector(viewerStateContextedSelectedRegionsSelector, [])
+    })
+    
+    afterEach(() => {
+      getRegionDetailSpy.calls.reset()
+    })
+
+    describe('> if regoinselected is empty array', () => {
+      let returnVal: Observable<any>
+      beforeEach(() => {
+        getRegionDetailSpy.and.callThrough()
+        returnVal = ROIFactory(mockStore, pcsvc as any)
+      })
+      it('> returns null', () => {
+        expect(
+          returnVal
+        ).toBeObservable(hot('a', {
+          a: null
+        }))
+      })
+
+      it('> regionDetail not called', () => {
+        expect(getRegionDetailSpy).not.toHaveBeenCalled()
+      })
+    })
+
+    describe('> if regionselected is nonempty', () => {
+      const mockRegion = {
+        context: {
+          template: {
+            '@id': 'template-id'
+          },
+          parcellation: {
+            '@id': 'parcellation-id'
+          },
+          atlas: {
+            '@id': 'atlas-id'
+          }
+        },
+        ngId: 'foo-bar',
+        labelIndex: 123
+      }
+      const returnDetail = {
+        map: {
+          hello: 'world'
+        }
+      }
+      let returnVal: Observable<any>
+      beforeEach(() => {
+        getRegionDetailSpy.and.callFake(() => of(returnDetail))
+        mockStore.overrideSelector(viewerStateContextedSelectedRegionsSelector, [mockRegion])
+        returnVal = ROIFactory(mockStore, pcsvc as any)
+      })
+
+      // TODO check why marble is acting weird
+      // and that null is not emitted
+      it('> returns as expected', () => {
+        expect(returnVal).toBeObservable(
+          hot('b', {
+            a: null,
+            b: {
+              ...mockRegion,
+              ...returnDetail
+            }
+          })
+        )
+        const { context } = mockRegion
+        expect(getRegionDetailSpy).toHaveBeenCalledWith(
+          context.atlas["@id"],
+          context.parcellation["@id"],
+          context.template["@id"],
+          mockRegion
+        )
+      })
+
+      it('> if getRegionDetail throws, at least return original region', () => {
+        getRegionDetailSpy.and.callFake(() => throwError('blabla'))
+        expect(returnVal).toBeObservable(
+          hot('b', {
+            a: null,
+            b: mockRegion
+          })
+        )
+        const { context } = mockRegion
+        expect(getRegionDetailSpy).toHaveBeenCalledWith(
+          context.atlas["@id"],
+          context.parcellation["@id"],
+          context.template["@id"],
+          mockRegion
+        )
+      })
+    })
+  })
+})
diff --git a/src/viewerModule/viewerCmp/viewerCmp.component.ts b/src/viewerModule/viewerCmp/viewerCmp.component.ts
index 7bb913885..f0eb2af8c 100644
--- a/src/viewerModule/viewerCmp/viewerCmp.component.ts
+++ b/src/viewerModule/viewerCmp/viewerCmp.component.ts
@@ -28,6 +28,31 @@ type TCStoreViewerCmp = {
   overlaySideNav: any
 }
 
+export function ROIFactory(store: Store<any>, svc: PureContantService){
+  return store.pipe(
+    select(viewerStateContextedSelectedRegionsSelector),
+    switchMap(r => {
+      if (!r[0]) return of(null)
+      const { context } = r[0]
+      const { atlas, template, parcellation } = context || {}
+      return merge(
+        of(null),
+        svc.getRegionDetail(atlas['@id'], parcellation['@id'], template['@id'], r[0]).pipe(
+          map(det => {
+            return {
+              ...r[0],
+              ...det,
+            }
+          }),
+          // in case detailed requests fails
+          catchError((_err, _obs) => of(r[0])),
+        )
+      )
+    }),
+    shareReplay(1)
+  )
+}
+
 @Component({
   selector: 'iav-cmp-viewer-container',
   templateUrl: './viewerCmp.template.html',
@@ -70,28 +95,7 @@ type TCStoreViewerCmp = {
   providers: [
     {
       provide: REGION_OF_INTEREST,
-      useFactory: (store: Store<any>, svc: PureContantService) => store.pipe(
-        select(viewerStateContextedSelectedRegionsSelector),
-        switchMap(r => {
-          if (!r[0]) return of(null)
-          const { context } = r[0]  
-          const { atlas, template, parcellation } = context || {}
-          return merge(
-            of(null),
-            svc.getRegionDetail(atlas['@id'], parcellation['@id'], template['@id'], r[0]).pipe(
-              map(det => {
-                return {
-                  ...det,
-                  context
-                }
-              }),
-              // in case detailed requests 
-              catchError((_err, _obs) => of(r[0])),
-            )
-          )
-        }),
-        shareReplay(1)
-      ),
+      useFactory: ROIFactory,
       deps: [ Store, PureContantService ]
     },
     {
-- 
GitLab