From 100bb396e84801a844042a50c668f85c5742cf1c Mon Sep 17 00:00:00 2001
From: Katie Stahl <kathryn.stahl@nationwidechildrens.org>
Date: Thu, 26 Sep 2024 13:03:47 -0400
Subject: [PATCH] feat: add better error messaging (#335)

* feat: add better error messaging

* feat: add better error messaging

* feat: print all validation errors for fusion

* update fusor version with error message fixes
---
 .../GeneElementInput/GeneElementInput.tsx     |  8 ++++-
 .../LinkerElementInput/LinkerElementInput.tsx |  1 +
 .../RegulatoryElementInput.tsx                | 11 +++++--
 .../Input/StructuralElementInputAccordion.tsx | 26 ++++++++++------
 .../TemplatedSequenceElementInput.tsx         |  4 +++
 .../TxSegmentElementInput.tsx                 |  9 +++++-
 .../Pages/Summary/Invalid/Invalid.tsx         |  7 ++---
 .../components/Pages/Summary/Main/Summary.tsx |  8 +----
 requirements.txt                              | 30 +++++++++++++++++--
 server/pyproject.toml                         |  2 +-
 server/src/curfu/routers/validate.py          |  2 +-
 11 files changed, 79 insertions(+), 29 deletions(-)

diff --git a/client/src/components/Pages/Structure/Input/GeneElementInput/GeneElementInput.tsx b/client/src/components/Pages/Structure/Input/GeneElementInput/GeneElementInput.tsx
index 1db935cf..a17a90a2 100644
--- a/client/src/components/Pages/Structure/Input/GeneElementInput/GeneElementInput.tsx
+++ b/client/src/components/Pages/Structure/Input/GeneElementInput/GeneElementInput.tsx
@@ -22,11 +22,13 @@ const GeneElementInput: React.FC<GeneElementInputProps> = ({
   handleDelete,
   icon,
 }) => {
+  const [errors, setErrors] = useState<string[]>([]);
   const [gene, setGene] = useState<string>(element.gene?.label || "");
   const [geneText, setGeneText] = useState<string>("");
   const validated = gene !== "" && geneText == "";
   const [expanded, setExpanded] = useState<boolean>(!validated);
   const [pendingResponse, setPendingResponse] = useState(false);
+  const geneNotFound = "Gene not found";
 
   useEffect(() => {
     if (validated) buildGeneElement();
@@ -39,11 +41,14 @@ const GeneElementInput: React.FC<GeneElementInputProps> = ({
         geneElementResponse.warnings &&
         geneElementResponse.warnings.length > 0
       ) {
-        setGeneText("Gene not found");
+        setGeneText(geneNotFound);
+        setErrors([geneNotFound]);
+        setPendingResponse(false);
       } else if (
         geneElementResponse.element &&
         geneElementResponse.element.gene
       ) {
+        setErrors([]);
         getGeneNomenclature(geneElementResponse.element).then(
           (nomenclatureResponse: NomenclatureResponse) => {
             if (
@@ -81,6 +86,7 @@ const GeneElementInput: React.FC<GeneElementInputProps> = ({
     handleDelete,
     inputElements,
     validated,
+    errors,
     icon,
     pendingResponse,
   });
diff --git a/client/src/components/Pages/Structure/Input/LinkerElementInput/LinkerElementInput.tsx b/client/src/components/Pages/Structure/Input/LinkerElementInput/LinkerElementInput.tsx
index e35f4252..120dfd14 100644
--- a/client/src/components/Pages/Structure/Input/LinkerElementInput/LinkerElementInput.tsx
+++ b/client/src/components/Pages/Structure/Input/LinkerElementInput/LinkerElementInput.tsx
@@ -70,6 +70,7 @@ const LinkerElementInput: React.FC<LinkerElementInputProps> = ({
     handleDelete,
     inputElements,
     validated,
+    errors: linkerError ? ["Invalid linker sequence"] : [],
     icon,
   });
 };
diff --git a/client/src/components/Pages/Structure/Input/RegulatoryElementInput/RegulatoryElementInput.tsx b/client/src/components/Pages/Structure/Input/RegulatoryElementInput/RegulatoryElementInput.tsx
index fa52cce7..bf383260 100644
--- a/client/src/components/Pages/Structure/Input/RegulatoryElementInput/RegulatoryElementInput.tsx
+++ b/client/src/components/Pages/Structure/Input/RegulatoryElementInput/RegulatoryElementInput.tsx
@@ -65,6 +65,8 @@ const RegulatoryElementInput: React.FC<RegulatoryElementInputProps> = ({
   const validated = gene !== "" && geneText == "" && elementClass !== "default";
   const [expanded, setExpanded] = useState<boolean>(!validated);
 
+  const [errors, setErrors] = useState<string[]>([]);
+
   useEffect(() => {
     if (validated) handleAdd();
   }, [gene, geneText, elementClass]);
@@ -73,7 +75,8 @@ const RegulatoryElementInput: React.FC<RegulatoryElementInputProps> = ({
     if (elementClass === "default") return;
     getRegulatoryElement(elementClass, gene).then((reResponse) => {
       if (reResponse.warnings && reResponse.warnings.length > 0) {
-        throw new Error(reResponse.warnings[0]);
+        setErrors(reResponse.warnings);
+        return;
       }
       getRegElementNomenclature(reResponse.regulatoryElement).then(
         (nomenclatureResponse) => {
@@ -81,8 +84,10 @@ const RegulatoryElementInput: React.FC<RegulatoryElementInputProps> = ({
             nomenclatureResponse.warnings &&
             nomenclatureResponse.warnings.length > 0
           ) {
-            throw new Error(nomenclatureResponse.warnings[0]);
+            setErrors(nomenclatureResponse.warnings);
+            return;
           }
+          setErrors([]);
           const newRegElement: ClientRegulatoryElement = {
             ...reResponse.regulatoryElement,
             elementId: element.elementId,
@@ -104,6 +109,7 @@ const RegulatoryElementInput: React.FC<RegulatoryElementInputProps> = ({
     setElementClass("default");
     setGene("");
     setGeneText("");
+    setErrors([]);
   };
 
   const inputElements = (
@@ -127,6 +133,7 @@ const RegulatoryElementInput: React.FC<RegulatoryElementInputProps> = ({
     handleDelete: handleDeleteElement,
     inputElements,
     validated,
+    errors,
     icon,
   });
 };
diff --git a/client/src/components/Pages/Structure/Input/StructuralElementInputAccordion.tsx b/client/src/components/Pages/Structure/Input/StructuralElementInputAccordion.tsx
index 24fcf0b9..c0bed313 100644
--- a/client/src/components/Pages/Structure/Input/StructuralElementInputAccordion.tsx
+++ b/client/src/components/Pages/Structure/Input/StructuralElementInputAccordion.tsx
@@ -1,4 +1,3 @@
-import { Tooltip, makeStyles, CircularProgress } from "@material-ui/core";
 import { styled } from "@mui/material/styles";
 import IconButton, { IconButtonProps } from "@mui/material/IconButton";
 import Card from "@mui/material/Card";
@@ -8,11 +7,13 @@ import CardActions from "@mui/material/CardActions";
 import Collapse from "@mui/material/Collapse";
 import DeleteIcon from "@material-ui/icons/Delete";
 import CheckCircleIcon from "@mui/icons-material/CheckCircle";
-import ErrorIcon from "@mui/icons-material/Error";
 import EditIcon from "@material-ui/icons/Edit";
 import { red, green } from "@material-ui/core/colors";
 import "./StructuralElementInputAccordion.scss";
 import { BaseStructuralElementProps } from "./StructuralElementInputProps";
+import React from "react";
+import { Alert, CircularProgress, Tooltip } from "@mui/material";
+import { makeStyles, Typography } from "@material-ui/core";
 
 const useStyles = makeStyles(() => ({
   cardHeaderAction: {
@@ -42,6 +43,7 @@ interface StructuralElementInputAccordionProps
   setExpanded?: React.Dispatch<React.SetStateAction<boolean>>;
   inputElements?: JSX.Element;
   validated: boolean;
+  errors?: string[];
   icon: JSX.Element;
   pendingResponse?: boolean;
 }
@@ -71,6 +73,7 @@ const StructuralElementInputAccordion: React.FC<
   handleDelete,
   inputElements,
   validated,
+  errors,
   icon,
   pendingResponse,
 }) => {
@@ -121,7 +124,7 @@ const StructuralElementInputAccordion: React.FC<
           root: classes.cardActionsRoot,
         }}
       >
-        {validated ? (
+        {errors?.length === 0 && validated ? (
           <Tooltip title="Validation successful">
             <CheckCircleIcon
               className="input-correct"
@@ -129,12 +132,17 @@ const StructuralElementInputAccordion: React.FC<
             />
           </Tooltip>
         ) : (
-          <Tooltip title="Invalid component">
-            <ErrorIcon
-              className="input-incorrect"
-              style={{ color: red[500] }}
-            />
-          </Tooltip>
+          <>
+            {errors && errors.length ? (
+              <Alert severity="error">
+                {errors?.map((e) => {
+                  return <Typography variant="body2">{e}</Typography>;
+                })}
+              </Alert>
+            ) : (
+              <></>
+            )}
+          </>
         )}
         {inputElements ? (
           <ExpandMore
diff --git a/client/src/components/Pages/Structure/Input/TemplatedSequenceElementInput/TemplatedSequenceElementInput.tsx b/client/src/components/Pages/Structure/Input/TemplatedSequenceElementInput/TemplatedSequenceElementInput.tsx
index 8611f7af..b4ef0f3f 100644
--- a/client/src/components/Pages/Structure/Input/TemplatedSequenceElementInput/TemplatedSequenceElementInput.tsx
+++ b/client/src/components/Pages/Structure/Input/TemplatedSequenceElementInput/TemplatedSequenceElementInput.tsx
@@ -18,6 +18,7 @@ interface TemplatedSequenceElementInputProps
 const TemplatedSequenceElementInput: React.FC<
   TemplatedSequenceElementInputProps
 > = ({ element, handleSave, handleDelete, icon }) => {
+  const [errors, setErrors] = useState<string[]>([]);
   const [chromosome, setChromosome] = useState<string>(
     element.inputChromosome || ""
   );
@@ -72,10 +73,12 @@ const TemplatedSequenceElementInput: React.FC<
       ) {
         // TODO visible error handling
         setInputError("element validation unsuccessful");
+        setErrors(templatedSequenceResponse.warnings);
         setPendingResponse(false);
         return;
       } else if (templatedSequenceResponse.element) {
         setInputError("");
+        setErrors([]);
         getTemplatedSequenceNomenclature(
           templatedSequenceResponse.element
         ).then((nomenclatureResponse) => {
@@ -175,6 +178,7 @@ const TemplatedSequenceElementInput: React.FC<
     handleDelete,
     inputElements,
     validated,
+    errors,
     icon,
     pendingResponse,
   });
diff --git a/client/src/components/Pages/Structure/Input/TxSegmentElementInput/TxSegmentElementInput.tsx b/client/src/components/Pages/Structure/Input/TxSegmentElementInput/TxSegmentElementInput.tsx
index e83895e1..0b531e04 100644
--- a/client/src/components/Pages/Structure/Input/TxSegmentElementInput/TxSegmentElementInput.tsx
+++ b/client/src/components/Pages/Structure/Input/TxSegmentElementInput/TxSegmentElementInput.tsx
@@ -33,7 +33,6 @@ interface TxSegmentElementInputProps extends StructuralElementInputProps {
 
 const TxSegmentCompInput: React.FC<TxSegmentElementInputProps> = ({
   element,
-  index,
   handleSave,
   handleDelete,
   icon,
@@ -51,6 +50,8 @@ const TxSegmentCompInput: React.FC<TxSegmentElementInputProps> = ({
         : null
     );
 
+  const [errors, setErrors] = useState<string[]>([]);
+
   // "Text" variables refer to helper or warning text to set under input fields
   // TODO: this needs refactored so badly
   const [txAc, setTxAc] = useState(element.inputTx || "");
@@ -234,6 +235,7 @@ const TxSegmentCompInput: React.FC<TxSegmentElementInputProps> = ({
             txSegmentResponse.warnings &&
             txSegmentResponse.warnings?.length > 0
           ) {
+            setErrors(txSegmentResponse.warnings);
             CheckGenomicCoordWarning(txSegmentResponse.warnings);
           } else {
             const inputParams = {
@@ -244,6 +246,7 @@ const TxSegmentCompInput: React.FC<TxSegmentElementInputProps> = ({
               inputGenomicStart: txStartingGenomic,
               inputGenomicEnd: txEndingGenomic,
             };
+            setErrors([]);
             handleTxElementResponse(txSegmentResponse, inputParams);
           }
         });
@@ -278,10 +281,13 @@ const TxSegmentCompInput: React.FC<TxSegmentElementInputProps> = ({
                 setEndingExonText("Invalid");
               }
             }
+            setPendingResponse(false);
+            setErrors(txSegmentResponse.warnings);
           } else {
             setTxAcText("");
             setStartingExonText("");
             setEndingExonText("");
+            setErrors([]);
             const inputParams = {
               inputType: txInputType,
               inputTx: txAc,
@@ -678,6 +684,7 @@ const TxSegmentCompInput: React.FC<TxSegmentElementInputProps> = ({
     handleDelete,
     inputElements,
     validated,
+    errors,
     icon,
     pendingResponse,
   });
diff --git a/client/src/components/Pages/Summary/Invalid/Invalid.tsx b/client/src/components/Pages/Summary/Invalid/Invalid.tsx
index 757bdf02..e1978ade 100644
--- a/client/src/components/Pages/Summary/Invalid/Invalid.tsx
+++ b/client/src/components/Pages/Summary/Invalid/Invalid.tsx
@@ -101,10 +101,9 @@ export const Invalid: React.FC<Props> = ({
 
   const unknownError = (
     <>
-      <ListItemText>
-        We were unable to parse the validation failure for this fusion:
-      </ListItemText>
-      <blockquote>{validationErrors}</blockquote>
+      {validationErrors.map((error) => (
+        <blockquote>{error}</blockquote>
+      ))}
     </>
   );
 
diff --git a/client/src/components/Pages/Summary/Main/Summary.tsx b/client/src/components/Pages/Summary/Main/Summary.tsx
index 14b0605e..976938ea 100644
--- a/client/src/components/Pages/Summary/Main/Summary.tsx
+++ b/client/src/components/Pages/Summary/Main/Summary.tsx
@@ -104,13 +104,7 @@ export const Summary: React.FC<Props> = ({ setVisibleTab }) => {
     // make request
     validateFusion(formattedFusion).then((response) => {
       if (response.warnings && response.warnings?.length > 0) {
-        if (
-          validationErrors !== null &&
-          JSON.stringify(response.warnings.sort()) !==
-            JSON.stringify(validationErrors.sort())
-        ) {
-          setValidationErrors(response.warnings);
-        }
+        setValidationErrors(response.warnings);
       } else {
         setValidationErrors([]);
         setValidatedFusion(
diff --git a/requirements.txt b/requirements.txt
index 0c7c0fdf..310c4d88 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -8,46 +8,69 @@ asyncpg==0.29.0
 attrs==24.2.0
 biocommons.seqrepo==0.6.9
 bioutils==0.5.8.post1
-boto3==1.35.4
-botocore==1.35.4
+black==24.8.0
+boto3==1.35.3
+botocore==1.35.3
 canonicaljson==2.0.0
 certifi==2024.7.4
+cfgv==3.4.0
 charset-normalizer==3.3.2
 click==8.1.7
 coloredlogs==15.0.1
 configparser==7.1.0
 cool_seq_tool==0.7.0
+coverage==7.6.1
 decorator==5.1.1
+distlib==0.3.8
 executing==2.0.1
 fastapi==0.112.1
-fusor==0.4.2
+filelock==3.15.4
+fusor==0.4.3
 ga4gh.vrs==2.0.0a10
 gene-normalizer==0.4.0
 h11==0.14.0
 hgvs==1.5.4
+httpcore==1.0.5
+httpx==0.27.0
 humanfriendly==10.0
+identify==2.6.0
 idna==3.7
 importlib_metadata==8.4.0
+iniconfig==2.0.0
 ipython==8.26.0
 jedi==0.19.1
 Jinja2==3.1.4
 jmespath==1.0.1
 MarkupSafe==2.1.5
 matplotlib-inline==0.1.7
+mypy-extensions==1.0.0
+nodeenv==1.9.1
+packaging==24.1
 Parsley==1.3
 parso==0.8.4
+pathspec==0.12.1
 pexpect==4.9.0
+platformdirs==4.2.2
+pluggy==1.5.0
 polars==1.5.0
+pre-commit==3.8.0
 prompt_toolkit==3.0.47
 psycopg2==2.9.9
+psycopg2-binary==2.9.9
 ptyprocess==0.7.0
 pure_eval==0.2.3
 pydantic==2.4.2
+pydantic-to-typescript2==1.0.4
 pydantic_core==2.10.1
 Pygments==2.18.0
 pysam==0.22.1
+pytest==8.3.2
+pytest-asyncio==0.24.0
+pytest-cov==5.0.0
 python-dateutil==2.9.0.post0
+PyYAML==6.0.2
 requests==2.32.3
+ruff==0.5.0
 s3transfer==0.10.2
 six==1.16.0
 sniffio==1.3.1
@@ -60,6 +83,7 @@ traitlets==5.14.3
 typing_extensions==4.12.2
 urllib3==1.26.19
 uvicorn==0.30.6
+virtualenv==20.26.3
 wags_tails==0.1.4
 wcwidth==0.2.13
 yoyo-migrations==8.2.0
diff --git a/server/pyproject.toml b/server/pyproject.toml
index 80bda160..60b7021b 100644
--- a/server/pyproject.toml
+++ b/server/pyproject.toml
@@ -28,7 +28,7 @@ dependencies = [
     "click",
     "boto3",
     "botocore",
-    "fusor ~= 0.4.2",
+    "fusor ~= 0.4.3",
     "cool-seq-tool ~= 0.7.0",
     "pydantic == 2.4.2", # validation errors with more recent versions, so don't remove this specific pin
     "gene-normalizer ~= 0.4.0",
diff --git a/server/src/curfu/routers/validate.py b/server/src/curfu/routers/validate.py
index 8ee72c54..38a85fec 100644
--- a/server/src/curfu/routers/validate.py
+++ b/server/src/curfu/routers/validate.py
@@ -32,7 +32,7 @@ def validate_fusion(request: Request, fusion: dict = Body()) -> ResponseDict:
     try:
         verified_fusion = fusor.fusion(**fusion)
     except FUSORParametersException as e:
-        response["warnings"] = [str(e)]
+        response["warnings"] = str(e).split("\n")
     else:
         response["fusion"] = verified_fusion
     return response