From 2c685a2222d6ff8ad81073c1169e48ca20b3ba96 Mon Sep 17 00:00:00 2001
From: Xiao Gui <xgui3783@gmail.com>
Date: Fri, 24 Apr 2020 11:36:36 +0200
Subject: [PATCH] bugfix: fixing breaking e2e tests

---
 common/constants.js                           |  1 +
 .../navigating/navigateFromRegion.e2e-spec.js | 79 +++++++++----------
 e2e/src/util.js                               | 39 ++-------
 .../databrowser.useEffect.ts                  | 25 ++++--
 src/ui/parcellationRegion/region.base.ts      |  1 +
 .../regionMenu/regionMenu.template.html       |  4 +-
 6 files changed, 69 insertions(+), 80 deletions(-)

diff --git a/common/constants.js b/common/constants.js
index bfb603d31..b3643c815 100644
--- a/common/constants.js
+++ b/common/constants.js
@@ -12,5 +12,6 @@
     SHOW_ORIGIN_DATASET: `Show probabilistic map`,
     SHOW_CONNECTIVITY_DATA: `Show connectivity data`,
     SHOW_IN_OTHER_REF_SPACE: `Show in other reference space`,
+    AVAILABILITY_IN_OTHER_REF_SPACE: 'Availability in other reference spaces'
   }
 })(typeof exports === 'undefined' ? module.exports : exports)
diff --git a/e2e/src/navigating/navigateFromRegion.e2e-spec.js b/e2e/src/navigating/navigateFromRegion.e2e-spec.js
index d6a1ba4ab..6462c0401 100644
--- a/e2e/src/navigating/navigateFromRegion.e2e-spec.js
+++ b/e2e/src/navigating/navigateFromRegion.e2e-spec.js
@@ -1,4 +1,6 @@
-const {AtlasPage} = require('../util')
+const { AtlasPage } = require('../util')
+const { ARIA_LABELS } = require('../../../common/constants')
+const { SHOW_IN_OTHER_REF_SPACE, AVAILABILITY_IN_OTHER_REF_SPACE } = ARIA_LABELS
 
 const TEST_DATA = [
   {
@@ -21,34 +23,34 @@ const TEST_DATA = [
       },
     ],
   },
-  {
-    url: "/?templateSelected=Big+Brain+%28Histology%29&parcellationSelected=Cytoarchitectonic+Maps&cNavigation=0.0.0.-W000.._eCwg.2-FUe3._-s_W.2_evlu..7LIx..1n5q~.1FYC.2Is-..1B9C",
-    templateName: 'Big Brain (Histology)',
-    position: [691,678], // [370, 150],
-    expectedRegion: 'Area STS1 (STS)',
-    expectedTemplateLabels: [
-      {
-        name: 'MNI Colin 27',
-        hemisphere: 'Left',
-        expectedPosition: [-54514755, -16753913, -5260713]
-      },
-      {
-        name: 'MNI Colin 27',
-        hemisphere: 'Right',
-        expectedPosition: [54536567, -17992636, -5712544]
-      },
-      {
-        name: 'MNI 152 ICBM 2009c Nonlinear Asymmetric',
-        hemisphere: 'Left',
-        expectedPosition: [-55442669, -18314601, -6381831]
-      },
-      {
-        name: 'MNI 152 ICBM 2009c Nonlinear Asymmetric',
-        hemisphere: 'Right',
-        expectedPosition: [52602966, -18339402, -5666868]
-      },
-    ],
-  },
+  // {
+  //   url: "/?templateSelected=Big+Brain+%28Histology%29&parcellationSelected=Cytoarchitectonic+Maps&cNavigation=0.0.0.-W000.._eCwg.2-FUe3._-s_W.2_evlu..7LIx..1n5q~.1FYC.2Is-..1B9C",
+  //   templateName: 'Big Brain (Histology)',
+  //   position: [691,678], // [370, 150],
+  //   expectedRegion: 'Area STS1 (STS)',
+  //   expectedTemplateLabels: [
+  //     {
+  //       name: 'MNI Colin 27',
+  //       hemisphere: 'Left',
+  //       expectedPosition: [-54514755, -16753913, -5260713]
+  //     },
+  //     {
+  //       name: 'MNI Colin 27',
+  //       hemisphere: 'Right',
+  //       expectedPosition: [54536567, -17992636, -5712544]
+  //     },
+  //     {
+  //       name: 'MNI 152 ICBM 2009c Nonlinear Asymmetric',
+  //       hemisphere: 'Left',
+  //       expectedPosition: [-55442669, -18314601, -6381831]
+  //     },
+  //     {
+  //       name: 'MNI 152 ICBM 2009c Nonlinear Asymmetric',
+  //       hemisphere: 'Right',
+  //       expectedPosition: [52602966, -18339402, -5666868]
+  //     },
+  //   ],
+  // },
 ]
 
 const getBeforeEachFn = iavPage => ({ 
@@ -72,7 +74,7 @@ const getBeforeEachFn = iavPage => ({
   await iavPage.waitUntilAllChunksLoaded()
   await iavPage.cursorMoveToAndClick({ position })
 
-  await iavPage.showOtherTemplateMenu()
+  await iavPage.click(`[aria-label="${SHOW_IN_OTHER_REF_SPACE}"]`)
   await iavPage.wait(500)
 }
 
@@ -104,18 +106,13 @@ describe('> explore same region in different templates', () => {
           describe(`> moving to ${name}`, () => {
             it('> works as expected', async () => {
 
-              const otherTemplates = await iavPage.getAllOtherTemplates()
-              const idx = otherTemplates.findIndex(template => {
-                if (hemisphere) {
-                  if (template.indexOf(hemisphere) < 0) return false
-                }
-                return template.indexOf(name) >= 0
-              })
-      
-              expect(idx).toBeGreaterThanOrEqual(0)
-      
-              await iavPage.clickNthItemAllOtherTemplates(idx)
+              const otherTemplates = await iavPage.getText(`[aria-label="${SHOW_IN_OTHER_REF_SPACE}: ${name}${hemisphere ? (' - ' + hemisphere) : ''}"]`)
+              if (hemisphere) {
+                expect(otherTemplates.indexOf(hemisphere)).toBeGreaterThanOrEqual(0)
+              }
+              expect(otherTemplates.indexOf(name)).toBeGreaterThanOrEqual(0)
       
+              await iavPage.click(`[aria-label="${SHOW_IN_OTHER_REF_SPACE}: ${name}${hemisphere ? (' - ' + hemisphere) : ''}"]`)
               await iavPage.wait(500)
               await iavPage.waitUntilAllChunksLoaded()
               
diff --git a/e2e/src/util.js b/e2e/src/util.js
index 2c53dd6e2..a5157471f 100644
--- a/e2e/src/util.js
+++ b/e2e/src/util.js
@@ -105,6 +105,14 @@ class WdBase{
     await this._browser.findElement( By.css(cssSelector) ).click()
   }
 
+  async getText(cssSelector){
+    if (!cssSelector) throw new Error(`getText needs to define css selector`)
+    const el = await this._browser.findElement( By.css(cssSelector) )
+    
+    const text = await el.getText()
+    return text
+  }
+
   historyBack() {
     return this._browser.navigate().back()
   }
@@ -558,37 +566,6 @@ class WdLayoutPage extends WdBase{
     else await menuItems[index].click()
   }
 
-  // other templates
-  async showOtherTemplateMenu(){
-    await this._driver
-      .findElement( By.css('[aria-label="Show availability in other reference spaces"]') )
-      .click()
-  }
-
-  _getOtherTemplateMenu(){
-    return this._driver
-      .findElement( By.css('[aria-label="Availability in other reference spaces"]') )
-  }
-
-  _getAllOtherTemplates(){
-    return this._getOtherTemplateMenu().findElements( By.css('[mat-menu-item]') )
-  }
-
-  async getAllOtherTemplates(){
-    const els = await this._getAllOtherTemplates()
-    const returnArr = []
-    for (const el of els) {
-      returnArr.push(await _getTextFromWebElement(el))
-    }
-    return returnArr
-  }
-
-  async clickNthItemAllOtherTemplates(index){
-    const arr = await this._getAllOtherTemplates()
-    if (!arr[index]) throw new Error(`index out of bound: trying to access ${index} from arr with length ${arr.length}`)
-    await arr[index].click()
-  }
-
   _getFavDatasetIcon(){
     return this._driver
       .findElement( By.css('[aria-label="Show pinned datasets"]') )
diff --git a/src/ui/databrowserModule/databrowser.useEffect.ts b/src/ui/databrowserModule/databrowser.useEffect.ts
index c107a7d27..cfacc8906 100644
--- a/src/ui/databrowserModule/databrowser.useEffect.ts
+++ b/src/ui/databrowserModule/databrowser.useEffect.ts
@@ -2,7 +2,7 @@ import { Injectable, OnDestroy } from "@angular/core";
 import { Actions, Effect, ofType } from "@ngrx/effects";
 import { select, Store } from "@ngrx/store";
 import { from, merge, Observable, of, Subscription, forkJoin, combineLatest } from "rxjs";
-import { filter, map, scan, switchMap, withLatestFrom, mapTo, shareReplay, startWith, distinctUntilChanged, concatMap } from "rxjs/operators";
+import { filter, map, scan, switchMap, withLatestFrom, mapTo, shareReplay, startWith, distinctUntilChanged, concatMap, pairwise } from "rxjs/operators";
 import { LoggingService } from "src/logging";
 import { DATASETS_ACTIONS_TYPES, IDataEntry, ViewerPreviewFile, DatasetPreview } from "src/services/state/dataStore.store";
 import { IavRootStoreInterface, ADD_NG_LAYER, CHANGE_NAVIGATION } from "src/services/stateStore.service";
@@ -78,17 +78,30 @@ export class DataBrowserUseEffect implements OnDestroy {
     )
 
     this.removePreviewDataset$ = ngViewerStateLayers$.pipe(
-      map(layers => layers.filter(({ annotation }) => annotation && annotation.indexOf(DATASET_PREVIEW_ANNOTATION) >= 0)),
-      withLatestFrom(this.datasetPreviews$),
-      map(([ ngViewerLayers, datasetPreviews ]) => {
+      distinctUntilChanged(),
+      pairwise(),
+      map(([o, n]: [INgLayerInterface[], INgLayerInterface[]]) => {
+        const nNameSet = new Set(n.map(({ name }) => name))
+        const oNameSet = new Set(o.map(({ name }) => name))
+        return {
+          add: n.filter(({ name: nName }) => !oNameSet.has(nName)),
+          remove: o.filter(({ name: oName }) => !nNameSet.has(oName)),
+        }
+      }),
+      map(({ remove }) => remove),
+      withLatestFrom(
+        this.datasetPreviews$,
+      ),
+      map(([ removedLayers, datasetPreviews ]) => {
+        const removeLayersAnnotation = removedLayers.map(({ annotation }) => annotation)
         return datasetPreviews.filter(({ filename }) => {
-          return ngViewerLayers.findIndex(({ annotation }) => annotation.indexOf(filename) >= 0) < 0
+          return removeLayersAnnotation.findIndex(annnoation => annnoation.indexOf(filename) >= 0) >= 0
         })
       }),
       filter(arr => arr.length > 0),
       concatMap(arr => from(arr).pipe(
         map(item => {
-          const { filename, datasetId } = item
+          const { datasetId, filename } = item
           return {
             type: DATASETS_ACTIONS_TYPES.CLEAR_PREVIEW_DATASET,
             payload: {
diff --git a/src/ui/parcellationRegion/region.base.ts b/src/ui/parcellationRegion/region.base.ts
index 17b84af7f..0333e86ed 100644
--- a/src/ui/parcellationRegion/region.base.ts
+++ b/src/ui/parcellationRegion/region.base.ts
@@ -153,4 +153,5 @@ export class RegionBase {
   public SHOW_CONNECTIVITY_DATA = ARIA_LABELS.SHOW_CONNECTIVITY_DATA
   public SHOW_IN_OTHER_REF_SPACE = ARIA_LABELS.SHOW_IN_OTHER_REF_SPACE
   public SHOW_ORIGIN_DATASET = ARIA_LABELS.SHOW_ORIGIN_DATASET
+  public AVAILABILITY_IN_OTHER_REF_SPACE = ARIA_LABELS.AVAILABILITY_IN_OTHER_REF_SPACE
 }
diff --git a/src/ui/parcellationRegion/regionMenu/regionMenu.template.html b/src/ui/parcellationRegion/regionMenu/regionMenu.template.html
index 7ba29a29e..4e343862e 100644
--- a/src/ui/parcellationRegion/regionMenu/regionMenu.template.html
+++ b/src/ui/parcellationRegion/regionMenu/regionMenu.template.html
@@ -73,7 +73,6 @@
 
         <mat-grid-tile>
           <iav-v-button *ngIf="sameRegionTemplate.length"
-            [attr.aria-label]="'Show availability in other reference spaces'"
             class="h-100 w-100"
             mat-ripple
             [matMenuTriggerFor]="additionalActions"
@@ -175,8 +174,9 @@
 
         <!-- change template items -->
         <div *ngIf="changeTmplSwitch.switchState"
-          aria-label="Availability in other reference spaces">
+          [attr.aria-label]="AVAILABILITY_IN_OTHER_REF_SPACE">
           <mat-list-item *ngFor="let sameRegion of sameRegionTemplate; let i = index"
+            [attr.aria-label]="SHOW_IN_OTHER_REF_SPACE + ': ' + sameRegion.template.name + (sameRegion.hemisphere ? (' - ' + sameRegion.hemisphere) : '') "
             (click)="changeView(i)"
             mat-ripple>
             <mat-icon fontSet="fas" fontIcon="fa-none" mat-list-icon></mat-icon>
-- 
GitLab