category and label name validation updates (#1056)

* update label validation with new schema

* add new error messages for new label errors; general cleanup

client/src/components/categorical/categorical.js

@@ -67,42 +67,42 @@ class Categories extends React.Component {
     return false if this is a LEGAL/acceptable category name or NULL/empty string,
     or return an error type.
     */
-    if (!name) return false;
+
+    /* allow empty string */
+    if (name === "") return false;
 
     const { categoricalSelection } = this.props;
     const allCategoryNames = Object.keys(categoricalSelection);
 
+    /* check category name syntax */
+    const error = AnnotationsHelpers.annotationNameIsErroneous(name);
+    if (error) {
+      return error;
+    }
+
+    /* disallow duplicates */
     if (allCategoryNames.indexOf(name) !== -1) {
       return "duplicate";
     }
 
-    if (!AnnotationsHelpers.isLegalAnnotationName(name)) {
+    /* otherwise, no error */
-      return "characters";
-    }
-
     return false;
   };
 
   categoryNameErrorMessage = name => {
     const err = this.categoryNameError(name);
     if (err === false) return null;
-    if (err === "duplicate") {
+
-      return (
+    const errorMessageMap = {
-        <span>
+      /* map error code to human readable error message */
-          <span style={{ fontStyle: "italic" }}>{name}</span> already exists -
+      "empty-string": "Blank names not allowed",
-          no duplicates allowed
+      duplicate: "Name must be unique",
-        </span>
+      "trim-spaces": "Leading and trailing spaces not allowed",
-      );
+      "illegal-characters": "Only alphanumeric, underscore and period allowed",
-    }
+      "multi-space-run": "Multiple consecutive spaces not allowed"
-    if (err === "characters") {
+    };
-      return (
+    const errorMessage = errorMessageMap[err] ?? "error";
-        <span>
+    return <span>{errorMessage}</span>;
-          <span style={{ fontStyle: "italic" }}>{name}</span> contains illegal
-          characters. Hint: use alpha-numeric and underscore
-        </span>
-      );
-    }
-    return err;
   };
 
   render() {

client/src/components/categorical/category.js

@@ -158,25 +158,30 @@ class Category extends React.Component {
     return false if this is a LEGAL/acceptable category name or NULL/empty string,
     or return an error type.
     */
-    let error = false;
-    if (name) {
-      const { metadataField, universe } = this.props;
-      const { obsByName } = universe.schema.annotations;
 
-      if (obsByName[metadataField].categories.indexOf(name) !== -1) {
+    /* allow empty string */
-        error = "duplicate";
+    if (name === "") return false;
-      } else if (!AnnotationsHelpers.isLegalAnnotationName(name)) {
+
-        error = "characters";
+    /* check for label syntax errors */
-      }
+    const error = AnnotationsHelpers.annotationNameIsErroneous(name);
-    }
+    if (error) return error;
-    return error;
+
+    /* disallow duplicates */
+    const { metadataField, universe } = this.props;
+    const { obsByName } = universe.schema.annotations;
+    if (obsByName[metadataField].categories.indexOf(name) !== -1)
+      return "duplicate";
+
+    /* otherwise, no error */
+    return false;
   };
 
   labelNameErrorMessage = name => {
     const { metadataField } = this.props;
     const err = this.labelNameError(name);
-    if (err === false) return null;
+
     if (err === "duplicate") {
+      /* duplicate error is special cased because it has special formatting */
       return (
         <span>
           <span style={{ fontStyle: "italic" }}>{name}</span> already exists
@@ -185,15 +190,23 @@ class Category extends React.Component {
         </span>
       );
     }
-    if (err === "characters") {
+
-      return (
+    if (err) {
-        <span>
+      /* all other errors - map code to human error message */
-          <span style={{ fontStyle: "italic" }}>{name}</span> contains illegal
+      const errorMessageMap = {
-          characters. Hint: use alpha-numeric and underscore
+        "empty-string": "Blank names not allowed",
-        </span>
+        duplicate: "Name must be unique",
-      );
+        "trim-spaces": "Leading and trailing spaces not allowed",
+        "illegal-characters":
+          "Only alphanumeric, underscore and period allowed",
+        "multi-space-run": "Multiple consecutive spaces not allowed"
+      };
+      const errorMessage = errorMessageMap[err] ?? "error";
+      return <span>{errorMessage}</span>;
     }
-    return err;
+
+    /* no error, no message generated */
+    return null;
   };
 
   categoryNameErrorMessage = () => {
@@ -201,76 +214,51 @@ class Category extends React.Component {
     const err = this.editedCategoryNameError();
     if (err === false) return null;
 
-    let markup = null;
+    const errorMessageMap = {
-
+      /* map error code to human readable error message */
-    if (err === "empty_string") {
+      "empty-string": "Blank names not allowed",
-      markup = (
+      duplicate: "Category name must be unique",
-        <span
+      "trim-spaces": "Leading and trailing spaces not allowed",
-          style={{
+      "illegal-characters": "Only alphanumeric, underscore and period allowed",
-            display: "block",
+      "multi-space-run": "Multiple consecutive spaces not allowed"
-            fontStyle: "italic",
+    };
-            fontSize: 12,
+    const errorMessage = errorMessageMap[err] ?? "error";
-            marginTop: 5,
+    return (
-            color: Colors.ORANGE3
+      <span
-          }}
+        style={{
-        >
+          display: "block",
-          {"Category name cannot be blank"}
+          fontStyle: "italic",
-        </span>
+          fontSize: 12,
-      );
+          marginTop: 5,
-    } else if (err === "already_exists") {
+          color: Colors.ORANGE3
-      markup = (
+        }}
-        <span
+      >
-          style={{
+        {errorMessage}
-            display: "block",
+      </span>
-            fontStyle: "italic",
+    );
-            fontSize: 12,
-            marginTop: 5,
-            color: Colors.ORANGE3
-          }}
-        >
-          {"Category name must be unique"}
-        </span>
-      );
-    } else if (err === "characters") {
-      markup = (
-        <span
-          style={{
-            display: "block",
-            fontStyle: "italic",
-            fontSize: 12,
-            marginTop: 5,
-            color: Colors.ORANGE3
-          }}
-        >
-          {"Only alphanumeric and underscore allowed"}
-        </span>
-      );
-    }
-
-    return markup;
   };
 
   editedCategoryNameError = () => {
     const { metadataField, categoricalSelection } = this.props;
     const { newCategoryText } = this.state;
-    const allCategoryNames = _.keys(categoricalSelection);
 
-    const isEmptyString = newCategoryText === "";
+    /* check for syntax errors in category name */
+    const error = AnnotationsHelpers.annotationNameIsErroneous(newCategoryText);
+    if (error) {
+      return error;
+    }
+
+    /* check for duplicative categories */
+    const allCategoryNames = _.keys(categoricalSelection);
     const categoryNameAlreadyExists =
       allCategoryNames.indexOf(newCategoryText) > -1;
     const sameName = newCategoryText === metadataField;
-
+    if (categoryNameAlreadyExists && !sameName) {
-    let error = false;
+      return "duplicate";
-
-    if (isEmptyString) {
-      error = "empty_string";
-    } else if (categoryNameAlreadyExists && !sameName) {
-      error = "already_exists";
-    } else if (!AnnotationsHelpers.isLegalAnnotationName(newCategoryText)) {
-      error = "characters";
     }
 
-    return error;
+    /* otherwise, no error */
+    return false;
   };
 
   toggleAll() {

client/src/components/categorical/value.js

@@ -10,7 +10,8 @@ import {
   Popover,
   Position,
   PopoverInteractionKind,
-  Tooltip
+  Tooltip,
+  Colors
 } from "@blueprintjs/core";
 import Occupancy from "./occupancy";
 import * as globals from "../../globals";
@@ -85,75 +86,58 @@ class CategoryValue extends React.Component {
   valueNameErrorMessage = () => {
     const { editedLabelText } = this.state;
     const err = this.valueNameError();
-    if (!err) return null;
+    if (err === false) return null;
 
-    let markup = null;
+    const errorMessageMap = {
-
+      /* map error code to human readable error message */
-    if (err === "empty_string") {
+      "empty-string": "Blank names not allowed",
-      markup = (
+      duplicate: "Label must be unique",
-        <span
+      "trim-spaces": "Leading and trailing spaces not allowed",
-          style={{
+      "illegal-characters": "Only alphanumeric, underscore and period allowed",
-            fontStyle: "italic",
+      "multi-space-run": "Multiple consecutive spaces not allowed"
-            fontSize: 12,
+    };
-            marginTop: 5,
+    const errorMessage = errorMessageMap[err] ?? "error";
-            color: Colors.ORANGE3
+    return (
-          }}
+      <span
-        >
+        style={{
-          {"Label cannot be blank"}
+          fontStyle: "italic",
-        </span>
+          fontSize: 12,
-      );
+          marginTop: 5,
-    } else if (err === "duplicate") {
+          color: Colors.ORANGE3
-      markup = (
+        }}
-        <span
+      >
-          style={{
+        {errorMessage}
-            fontStyle: "italic",
+      </span>
-            fontSize: 12,
+    );
-            marginTop: 5,
-            color: Colors.ORANGE3
-          }}
-        >
-          {"Label must be unique"}
-        </span>
-      );
-    } else if (err === "characters") {
-      markup = (
-        <span
-          style={{
-            fontStyle: "italic",
-            fontSize: 12,
-            marginTop: 5,
-            color: Colors.ORANGE3
-          }}
-        >
-          {"Only alphanumeric and underscore allowed"}
-        </span>
-      );
-    }
-    return markup;
   };
 
   valueNameError = () => {
     const { editedLabelText } = this.state;
     const { categoricalSelection, metadataField, categoryIndex } = this.props;
 
-    let err = null;
+    /* 
+    check label syntax
+    */
+    const err = AnnotationsHelpers.annotationNameIsErroneous(editedLabelText);
+    if (err) return err;
 
+    /*
+    disallow duplicates
+    */
     const category = categoricalSelection[metadataField];
     const displayString = String(
       category.categoryValues[categoryIndex]
     ).valueOf();
-
+    if (
-    if (editedLabelText === "") {
-      err = "empty_string";
-    } else if (
       category.categoryValues.indexOf(editedLabelText) > -1 &&
       editedLabelText !== displayString
-    ) {
+    )
-      err = "duplicate";
+      return "duplicate";
-    } else if (!AnnotationsHelpers.isLegalAnnotationName(editedLabelText)) {
+
-      err = "characters";
+    /*
-    }
+    otherwise, all good!
-    return err;
+    */
+    return false;
   };
 
   activateEditLabelMode = () => {
@@ -221,6 +205,21 @@ class CategoryValue extends React.Component {
     );
   };
 
+  componentDidUpdate(prevProps) {
+    const { categoricalSelection, metadataField, categoryIndex } = this.props;
+    if (
+      prevProps.categoricalSelection !== categoricalSelection ||
+      prevProps.metadataField !== metadataField ||
+      prevProps.categoryIndex !== categoryIndex
+    ) {
+      this.setState({
+        editedLabelText: String(
+          categoricalSelection[metadataField].categoryValues[categoryIndex]
+        ).valueOf()
+      });
+    }
+  }
+
   toggleOn = () => {
     const { dispatch, metadataField, categoryIndex } = this.props;
     dispatch({
@@ -546,6 +545,7 @@ class CategoryValue extends React.Component {
                         data-testclass="handleEditValue"
                         data-testid={`handleEditValue-${metadataField}`}
                         onClick={this.activateEditLabelMode}
+                        disabled={annotations.isEditingLabelName}
                       />
                     ) : null}
                     {displayString !== globals.unassignedCategoryLabel ? (

client/src/util/stateManager/annotationsHelpers.js

@@ -183,7 +183,35 @@ export function createWritableAnnotationDimensions(world, crossfilter) {
 	return crossfilter;
 }
 
-const legalNames = /^\w+$/;
+const legalCharacters = /^(\w|[ .])+$/;
-export function isLegalAnnotationName(name) {
+export function annotationNameIsErroneous(name) {
-	return legalNames.test(name);
+	/*
+	Validate the name - return:
+	* false - a valid name
+	* string - a named error, indicating why it was invalid.
+
+	Tests:
+	0. must be string, non-null
+	1. no leading or trailing spaces
+	2. only accept alpha, numeric, underscore, period and space
+	3. no runs of multiple spaces
+	*/
+
+	if (name === "") {
+		return "empty-string";
+	}
+	if (name[0] === " " || name[name.length - 1] === " ") {
+		return "trim-spaces";
+	}
+	if (!legalCharacters.test(name)) {
+		return "illegal-characters";
+	}
+	for (let i = 1, l = name.length; i < l; i += 1) {
+		if (name[i] === " " && name[i - 1] === " ") {
+			return "multi-space-run";
+		}
+	}
+
+	/* all is well!  Indicte not erroneous with a false */
+	return false;
 }