Undo selection appends genes from differential expression to user gene list (#1183)

* Undo selection appends diffExp genes to user gene list

Fixes https://github.com/chanzuckerberg/cellxgene/issues/1171

Need: When a user performs a differential expression from within
a sub-selection (world) of the data and then resets the selection to all
cells (universe), the differential expression results are no longer
valid.

Approach:
* When the selection is reset, move the top (maxUserDefinedGenes
- len(userDefinedGenes) from the differential expression results to the
list of user defined genes
* Raise maxUserDefinedGenes to 25 to give users more room and
accommodate the extra genes transferred in from differential expression

Other commits:
* Choose different button icons
* Add diff exp genes to user defined genes on subset too
* Respond to feedback from @liaprins-czi and @bkmartinjr

client/__tests__/e2e/e2e.test.js

@@ -20,9 +20,7 @@ beforeEach(async () => {
 });
 
 afterAll(() => {
-  if (!DEBUG) {
-    browser.close();
-  }
+  browser.close();
 });
 
 describe("did launch", () => {
@@ -111,16 +109,10 @@ describe("cell selection", () => {
 
 describe("gene entry", () => {
   test("search for single gene", async () => {
-    // blueprint's  typeahead is treating typing weird, clicking & waiting first solves this
-    await utils.typeInto("gene-search", data.genes.search);
-    await page.keyboard.press("Enter");
-    await page.waitForSelector(
-      `[data-testid='histogram-${data.genes.search}']`
-    );
+    await cxgActions.addGeneToSearch(data.genes.search);
   });
 
   test("bulk add genes", async () => {
-    await cxgActions.reset();
     const testGenes = data.genes.bulkadd;
     await utils.clickOn("section-bulk-add");
     await utils.typeInto("input-bulk-add", testGenes.join(","));
@@ -135,21 +127,9 @@ describe("gene entry", () => {
   });
 });
 
-describe("diffexp", () => {
+describe("differential expression", () => {
   test("selects cells, saves them and performs diffexp", async () => {
-    for (const select of data.diffexp.cellset1) {
-      if (select.kind === "categorical") {
-        await cxgActions.selectCategory(select.metadata, select.values, true);
-      }
-    }
-    await cxgActions.cellSet(1);
-    for (const select of data.diffexp.cellset2) {
-      if (select.kind === "categorical") {
-        await cxgActions.selectCategory(select.metadata, select.values, true);
-      }
-    }
-    await cxgActions.cellSet(2);
-    await utils.clickOn("diffexp-button");
+    await cxgActions.runDiffExp(data.diffexp.cellset1, data.diffexp.cellset2);
     const allHistograms = await cxgActions.getAllHistograms(
       "histogram-diffexp",
       data.diffexp["gene-results"]
@@ -161,7 +141,7 @@ describe("diffexp", () => {
   });
 });
 
-describe("subset/reset", () => {
+describe("subset", () => {
   test("subset - cell count matches", async () => {
     for (const select of data.subset.cellset1) {
       if (select.kind === "categorical") {
@@ -180,39 +160,6 @@ describe("subset/reset", () => {
     }
   });
 
-  test("reset after subset", async () => {
-    for (const select of data.subset.cellset1) {
-      if (select.kind === "categorical") {
-        await cxgActions.selectCategory(select.metadata, select.values, true);
-      }
-    }
-    await utils.clickOn("subset-button");
-    for (const label in data.subset.categorical) {
-      const categories = await cxgActions.getAllCategoriesAndCounts(label);
-      expect(Object.keys(categories)).toMatchObject(
-        Object.keys(data.subset.categorical[label])
-      );
-      expect(Object.values(categories)).toMatchObject(
-        Object.values(data.subset.categorical[label])
-      );
-    }
-    await cxgActions.reset();
-    for (const label in data.categorical) {
-      await utils.waitByID(`category-${label}`);
-      const categoryName = await utils.getOneElementInnerText(
-        `[data-testid="category-${label}"]`
-      );
-      expect(categoryName).toMatch(label);
-      const categories = await cxgActions.getAllCategoriesAndCounts(label);
-      expect(Object.keys(categories)).toMatchObject(
-        Object.keys(data.categorical[label])
-      );
-      expect(Object.values(categories)).toMatchObject(
-        Object.values(data.categorical[label])
-      );
-    }
-  });
-
   test("lasso after subset", async () => {
     for (const select of data.subset.cellset1) {
       if (select.kind === "categorical") {
@@ -233,11 +180,52 @@ describe("subset/reset", () => {
     const cellCount = await cxgActions.cellSet(1);
     expect(cellCount).toBe(data.subset.lasso.count);
   });
+
+  test("undo selection appends the top diff exp genes to user defined genes", async () => {
+    const userDefinedGenes = ["ACD", "AAR2", "AATF", "ARSG"];
+    const diffExpGenes = data.diffexp["gene-results"];
+    for (const userDefinedGene of userDefinedGenes) {
+      await cxgActions.addGeneToSearch(userDefinedGene);
+    }
+    const userDefinedHistograms = await cxgActions.getAllHistograms("histogram-user-gene", userDefinedGenes);
+    expect(userDefinedHistograms).toStrictEqual(userDefinedGenes);
+    await cxgActions.subset({x1: 0.15, y1: 0.10, x2: 0.98, y2: 0.98});
+    await cxgActions.runDiffExp(data.diffexp.cellset1, data.diffexp.cellset2);
+    const diffExpHistograms = await cxgActions.getAllHistograms("histogram-diffexp", diffExpGenes);
+    expect(diffExpHistograms).toStrictEqual(diffExpGenes);
+    await utils.clickOn("reset-subset-button");
+    const expected = [].concat(userDefinedGenes, diffExpGenes);
+    const userDefinedHistogramsAfterSubset = await cxgActions.getAllHistograms(
+      "histogram-user-gene",
+      expected
+    );
+    expect(userDefinedHistogramsAfterSubset).toStrictEqual(expected);
+  });
+
+  test("subset selection appends the top diff exp genes to user defined genes", async () => {
+    const userDefinedGenes = ["ACD", "AAR2", "AATF", "ARSG"];
+    const diffExpGenes = data.diffexp["gene-results"];
+    for (const userDefinedGene of userDefinedGenes) {
+      await cxgActions.addGeneToSearch(userDefinedGene);
+    }
+    const userDefinedHistograms = await cxgActions.getAllHistograms("histogram-user-gene", userDefinedGenes);
+    expect(userDefinedHistograms).toStrictEqual(userDefinedGenes);
+    await cxgActions.subset({x1: 0.15, y1: 0.10, x2: 0.98, y2: 0.98});
+    await cxgActions.runDiffExp(data.diffexp.cellset1, data.diffexp.cellset2);
+    const diffExpHistograms = await cxgActions.getAllHistograms("histogram-diffexp", diffExpGenes);
+    expect(diffExpHistograms).toStrictEqual(diffExpGenes);
+    await cxgActions.subset({x1: 0.16, y1: 0.11, x2: 0.97, y2: 0.97});
+    const expected = [].concat(userDefinedGenes, diffExpGenes);
+    const userDefinedHistogramsAfterSubset = await cxgActions.getAllHistograms(
+      "histogram-user-gene",
+      expected
+    );
+    expect(userDefinedHistogramsAfterSubset).toStrictEqual(expected);
+  });
 });
 
 describe("scatter plot", () => {
   test("scatter plot appears", async () => {
-    await cxgActions.reset();
     const testGenes = data.scatter.genes;
     await utils.clickOn("section-bulk-add");
     await utils.typeInto("input-bulk-add", Object.values(testGenes).join(","));

client/__tests__/e2e/e2eAnnotations.test.js

@@ -22,22 +22,6 @@ describe.each([
   {withSubset: false, tag: "whole"}
 ])("annotations", (config) => {
 
-  async function subset() {
-    const lassoSelection = await cxgActions.calcDragCoordinates(
-      "layout-graph",
-      {x1: 0.10, y1: 0.10, x2: 0.80, y2: 0.80}
-    );
-    await cxgActions.drag(
-      "layout-graph",
-      lassoSelection.start,
-      lassoSelection.end,
-      true
-    );
-    await utils.clickOn("subset-button");
-    const coordinate = await cxgActions.calcCoordinate("layout-graph", 0.9, 0.9);
-    await cxgActions.clickOnCoordinate("layout-graph", coordinate);
-  }
-
   const perTestCategoryName = "per-test-category";
   const perTestLabelName = "per-test-label";
 
@@ -48,7 +32,7 @@ describe.each([
     // setup the test fixtures
     await cxgActions.createCategory(perTestCategoryName);
     await cxgActions.createLabel(perTestCategoryName, perTestLabelName);
-    if (config.withSubset) await subset();
+    if (config.withSubset) await cxgActions.subset({x1: 0.10, y1: 0.10, x2: 0.80, y2: 0.80});
     await utils.waitByClass("autosave-complete");
   });
 

client/__tests__/e2e/puppeteerUtils.js

@@ -1,5 +1,6 @@
 import {DEBUG, DEV} from "./config";
 import puppeteer from "puppeteer";
+import { strict as assert } from "assert";
 
 export const puppeteerUtils = puppeteerPage => ({
   async waitByID(testid, props = {}) {
@@ -16,7 +17,7 @@ export const puppeteerUtils = puppeteerPage => ({
     );
   },
 
-  async waitForAllByIds(testids, props = {}) {
+  async waitForAllByIds(testids) {
     await Promise.all(
       testids.map(testid =>
         puppeteerPage.waitForSelector(`[data-testid='${testid}']`)
@@ -24,19 +25,16 @@ export const puppeteerUtils = puppeteerPage => ({
     );
   },
 
-  async getAllByClass(testclass, props = {}) {
+  async getAllByClass(testclass) {
     const elements = await puppeteerPage.$$eval(
       `[data-testclass=${testclass}]`,
-      els => {
-        return els.map(el => {
-          return el.dataset.testid;
-        });
-      }
+      eles => eles.map(ele => ele.dataset.testid)
     );
     return elements;
   },
 
   async typeInto(testid, text) {
+    // blueprint's  typeahead is treating typing weird, clicking & waiting first solves this
     // only works for text without special characters
     await this.waitByID(testid);
     const selector = `[data-testid='${testid}']`;
@@ -80,6 +78,7 @@ export const puppeteerUtils = puppeteerPage => ({
 });
 
 export const cellxgeneActions = puppeteerPage => ({
+
   async drag(testid, start, end, lasso = false) {
     const layout = await puppeteerUtils(puppeteerPage).waitByID(testid);
     const elBox = await layout.boxModel();
@@ -116,7 +115,7 @@ export const cellxgeneActions = puppeteerPage => ({
       testclass
     );
     return allHistograms.map(hist =>
-      hist.substr("histogram_".length, hist.length)
+      hist.substr("histogram-".length, hist.length)
     );
   },
 
@@ -196,9 +195,7 @@ export const cellxgeneActions = puppeteerPage => ({
     await puppeteerUtils(puppeteerPage).clickOn(`${category}:category-expand`);
     await puppeteerUtils(puppeteerPage).clickOn(`${category}:category-select`);
     for (const val of values) {
-      await puppeteerUtils(puppeteerPage).clickOn(
-        `categorical-value-select-${category}-${val}`
-      );
+      await puppeteerUtils(puppeteerPage).clickOn(`categorical-value-select-${category}-${val}`);
     }
   },
 
@@ -210,12 +207,6 @@ export const cellxgeneActions = puppeteerPage => ({
     }
   },
 
-  async reset() {
-    await puppeteerUtils(puppeteerPage).clickOn("reset");
-    // loading state never actually happens, reset is too fast
-    await page.waitFor(200);
-  },
-
   async clip(min = 0, max = 100) {
     await puppeteerUtils(puppeteerPage).clickOn("visualization-settings");
     await puppeteerUtils(puppeteerPage).clearInputAndTypeInto(
@@ -269,6 +260,44 @@ export const cellxgeneActions = puppeteerPage => ({
       newLabelName
     );
     await puppeteerUtils(puppeteerPage).clickOn(`${categoryName}:${oldLabelName}:submit-label-edit`);
+  },
+
+  async addGeneToSearch(geneName) {
+    await puppeteerUtils(puppeteerPage).typeInto("gene-search", geneName);
+    await puppeteerPage.keyboard.press("Enter");
+    await puppeteerPage.waitForSelector(
+      `[data-testid='histogram-${geneName}']`
+    );
+  },
+
+  async subset(coordinatesAsPercent) {
+    // In order to deselect the selection after the subset, make sure we have some clear part
+    // of the scatterplot we can click on
+    assert(coordinatesAsPercent.x2 < 0.99 || coordinatesAsPercent.y2 < 0.99);
+    const lassoSelection = await this.calcDragCoordinates( "layout-graph", coordinatesAsPercent);
+    await this.drag("layout-graph", lassoSelection.start, lassoSelection.end, true );
+    await puppeteerUtils(puppeteerPage).clickOn("subset-button");
+    const clearCoordinate = await this.calcCoordinate(
+      "layout-graph",
+      0.5,
+      0.99
+    );
+    await this.clickOnCoordinate("layout-graph", clearCoordinate);
+  },
+
+  async setSellSet(cellSet, cellSetNum) {
+    for (const selection of cellSet) {
+      if (selection.kind === "categorical") {
+        await this.selectCategory(selection.metadata, selection.values, true);
+      }
+    }
+    await this.cellSet(cellSetNum);
+  },
+
+  async runDiffExp(cellSet1, cellSet2) {
+    await this.setSellSet(cellSet1, 1);
+    await this.setSellSet(cellSet2, 2);
+    await puppeteerUtils(puppeteerPage).clickOn("diffexp-button");
   }
 });
 

client/__tests__/util/stateManager/controlsHelpers.test.js

@@ -0,0 +1,24 @@
+/*
+test controls helpers
+*/
+import { subsetAndResetGeneLists } from "../../../src/util/stateManager/controlsHelpers";
+import * as globals from "../../../src/globals";
+
+describe("controls helpers", () => {
+
+  test("subsetAndResetGeneLists", () => {
+    const geneList = [...Array(150).keys()].map(() =>
+      Math.random().toString(36).substring(2, 6) // random string of 4 characters
+    );
+    const state = {
+      userDefinedGenes: geneList.slice(0, 20),
+      diffexpGenes: geneList.slice(20),
+    };
+    const [newUserDefinedGenes, newDiffExpGenes] = subsetAndResetGeneLists(state);
+    expect(globals.maxUserDefinedGenes).toBeLessThan(globals.maxGenes);
+    expect(geneList.length).toBeGreaterThan(globals.maxGenes);
+    expect(newUserDefinedGenes).toHaveLength(globals.maxGenes);
+    expect(newUserDefinedGenes).toStrictEqual(geneList.slice(0, globals.maxGenes));
+    expect(newDiffExpGenes).toStrictEqual([]);
+  });
+});