From b714c18e7543d167198d99b82a8a963a27504a9e Mon Sep 17 00:00:00 2001
From: Severiano Badajoz <sbadajoz@chanzuckerberg.com>
Date: Thu, 1 Jul 2021 15:09:50 -0700
Subject: [PATCH] feat: frontend geneset validation (#2258)

* add geneset name validation

* validate genes before addition

* display error messages
---
 client/src/actions/geneset.js                 | 21 +++++++++-
 .../menus/addGeneToGenesetDialogue.js         |  2 +-
 .../menus/createGenesetDialogue.js            | 40 ++++++++++++++-----
 .../menus/editGenesetNameDialogue.js          | 19 ++++++++-
 4 files changed, 67 insertions(+), 15 deletions(-)

diff --git a/client/src/actions/geneset.js b/client/src/actions/geneset.js
index 06965be68..49b2cf560 100644
--- a/client/src/actions/geneset.js
+++ b/client/src/actions/geneset.js
@@ -1,3 +1,4 @@
+import { postUserErrorToast } from "../components/framework/toasters";
 /*
 Action creators for gene sets
 
@@ -39,9 +40,25 @@ export const genesetDelete = (genesetName) => (dispatch, getState) => {
   });
 };
 
-export const genesetAddGenes = (genesetName, genes) => (dispatch, getState) => {
+export const genesetAddGenes = (genesetName, genes) => async (
+  dispatch,
+  getState
+) => {
   const state = getState();
-  const { obsCrossfilter: prevObsCrossfilter } = state;
+  const { obsCrossfilter: prevObsCrossfilter, annoMatrix } = state;
+  const { schema } = annoMatrix;
+  const varIndex = schema.annotations.var.index;
+  const df = await annoMatrix.fetch("var", varIndex);
+  const geneNames = df.col(varIndex).asArray();
+  genes = genes.reduce((acc, gene) => {
+    if (geneNames.indexOf(gene.geneSymbol) === -1) {
+      postUserErrorToast(
+        `${gene.geneSymbol} doesn't appear to be a valid gene name.`
+      );
+    } else acc.push(gene);
+    return acc;
+  }, []);
+
   const obsCrossfilter = dropGenesetSummaryDimension(
     prevObsCrossfilter,
     state,
diff --git a/client/src/components/geneExpression/menus/addGeneToGenesetDialogue.js b/client/src/components/geneExpression/menus/addGeneToGenesetDialogue.js
index 17330c5fb..c66ecdab0 100644
--- a/client/src/components/geneExpression/menus/addGeneToGenesetDialogue.js
+++ b/client/src/components/geneExpression/menus/addGeneToGenesetDialogue.js
@@ -61,7 +61,7 @@ class AddGeneToGenesetDialogue extends React.PureComponent {
             "data-testid": `${geneset}:submit-label`,
           }}
           title="Add genes to gene set"
-          instruction={`Add gene to ${geneset}`}
+          instruction={`Add genes to ${geneset}`}
           cancelTooltipContent="Close this dialog without adding genes to gene set."
           primaryButtonText="Add genes"
           text={genesToAdd}
diff --git a/client/src/components/geneExpression/menus/createGenesetDialogue.js b/client/src/components/geneExpression/menus/createGenesetDialogue.js
index 5d7b6fcab..17ed5647c 100644
--- a/client/src/components/geneExpression/menus/createGenesetDialogue.js
+++ b/client/src/components/geneExpression/menus/createGenesetDialogue.js
@@ -31,6 +31,7 @@ class CreateGenesetDialogue extends React.PureComponent {
       genesetName: "",
       genesToPopulateGeneset: "",
       genesetDescription: "",
+      nameErrorMessage: "",
     });
     dispatch({
       type: "geneset: disable create geneset mode",
@@ -48,7 +49,7 @@ class CreateGenesetDialogue extends React.PureComponent {
 
     dispatch({
       type: "geneset: create",
-      genesetName,
+      genesetName: genesetName.trim(),
       genesetDescription,
     });
     if (genesToPopulateGeneset) {
@@ -82,7 +83,9 @@ class CreateGenesetDialogue extends React.PureComponent {
   };
 
   handleChange = (e) => {
+    const { genesets } = this.props;
     this.setState({ genesetName: e });
+    this.validate(e, genesets);
   };
 
   handleGenesetInputChange = (e) => {
@@ -100,11 +103,32 @@ class CreateGenesetDialogue extends React.PureComponent {
   };
 
   validate = (genesetName, genesets) => {
-    return genesets.has(genesetName);
+    if (genesets.has(genesetName)) {
+      this.setState({
+        nameErrorMessage: "There is already a geneset with that name",
+      });
+      return false;
+    }
+
+    if (
+      genesetName.length > 1 &&
+      // eslint-disable-next-line no-control-regex -- unicode 0-31 127-65535
+      genesetName.match(/^[\u0000-\u001F\u007F-\uFFFF]|[ ]{2,}/g)?.length
+    ) {
+      this.setState({
+        nameErrorMessage:
+          "Gene set names can only contain alphanumeric characters and the following special characters: ! ” # $ % ’ ( ) * + , - . / : ; < = > ? @  ] ^ _ ` | ~",
+      });
+      return false;
+    }
+    this.setState({
+      nameErrorMessage: "",
+    });
+    return true;
   };
 
   render() {
-    const { genesetName } = this.state;
+    const { genesetName, nameErrorMessage } = this.state;
     const { metadataField, genesetsUI, genesets } = this.props;
 
     return (
@@ -136,13 +160,11 @@ class CreateGenesetDialogue extends React.PureComponent {
                 <p
                   style={{
                     marginTop: 7,
-                    visibility: this.validate(genesetName, genesets)
-                      ? "visible"
-                      : "hidden",
+                    visibility: nameErrorMessage !== "" ? "visible" : "hidden",
                     color: Colors.ORANGE3,
                   }}
                 >
-                  {this.genesetNameError()}
+                  {nameErrorMessage}
                 </p>
                 <p style={{ marginTop: 20 }}>
                   Optionally add a{" "}
@@ -185,9 +207,7 @@ class CreateGenesetDialogue extends React.PureComponent {
                 <Button
                   data-testid={`${metadataField}:submit-geneset`}
                   onClick={this.createGeneset}
-                  disabled={
-                    !genesetName || this.validate(genesetName, genesets)
-                  }
+                  disabled={nameErrorMessage !== ""}
                   intent="primary"
                   type="submit"
                 >
diff --git a/client/src/components/geneExpression/menus/editGenesetNameDialogue.js b/client/src/components/geneExpression/menus/editGenesetNameDialogue.js
index 276492d4d..429ed612b 100644
--- a/client/src/components/geneExpression/menus/editGenesetNameDialogue.js
+++ b/client/src/components/geneExpression/menus/editGenesetNameDialogue.js
@@ -9,6 +9,7 @@ import LabelInput from "../../labelInput";
   ontology: state.ontology,
   obsCrossfilter: state.obsCrossfilter,
   genesetsUI: state.genesetsUI,
+  genesets: state.genesets.genesets,
 }))
 class RenameGeneset extends React.PureComponent {
   constructor(props) {
@@ -61,9 +62,23 @@ class RenameGeneset extends React.PureComponent {
     this.setState({ newGenesetDescription: e });
   };
 
+  validate = (genesetName, genesets) => {
+    return (
+      !genesets.has(genesetName) &&
+      // eslint-disable-next-line no-control-regex -- unicode 0-31 127-65535
+      genesetName.match(/^\s|[\u0000-\u001F\u007F-\uFFFF]|[ ]{2,}|^$|\s$/g)
+        ?.length
+    );
+  };
+
   render() {
     const { newGenesetName, newGenesetDescription } = this.state;
-    const { genesetsUI, parentGeneset, parentGenesetDescription } = this.props;
+    const {
+      genesetsUI,
+      parentGeneset,
+      parentGenesetDescription,
+      genesets,
+    } = this.props;
 
     return (
       <>
@@ -82,7 +97,7 @@ class RenameGeneset extends React.PureComponent {
           text={newGenesetName}
           secondaryText={newGenesetDescription}
           validationError={
-            genesetsUI.isEditingGenesetName === newGenesetName &&
+            this.validate(newGenesetName, genesets) &&
             parentGenesetDescription === newGenesetDescription
           }
           annoInput={