ENG-879 A node can have no specification or format (#703)

* eng-879: Error message if neither format nor specification

Author: maparent

apps/roam/src/components/settings/DiscourseNodeSpecification.tsx

@@ -8,16 +8,19 @@ import refreshConfigTree from "~/utils/refreshConfigTree";
 import getDiscourseNodes from "~/utils/getDiscourseNodes";
 import getDiscourseNodeFormatExpression from "~/utils/getDiscourseNodeFormatExpression";
 import QueryEditor from "~/components/QueryEditor";
+import internalError from "~/utils/internalError";
 
 const NodeSpecification = ({
   parentUid,
   node,
+  parentSetEnabled,
 }: {
   parentUid: string;
   node: ReturnType<typeof getDiscourseNodes>[number];
+  parentSetEnabled?: (enabled: boolean) => void;
 }) => {
   const [migrated, setMigrated] = React.useState(false);
-  const [enabled, setEnabled] = React.useState(
+  const [enabled, setEnabled] = React.useState<string>(
     () =>
       getSubTree({ tree: getBasicTreeByParentUid(parentUid), key: "enabled" })
         ?.uid,
@@ -66,17 +69,24 @@ const NodeSpecification = ({
               },
             }),
           )
-          .then(() => setMigrated(true));
+          .then(() => setMigrated(true))
+          .catch((error) => {
+            internalError({ error });
+          });
       }
     } else {
       const tree = getBasicTreeByParentUid(parentUid);
       const scratchNode = getSubTree({ tree, key: "scratch" });
-      Promise.all(scratchNode.children.map((c) => deleteBlock(c.uid)));
+      Promise.all(scratchNode.children.map((c) => deleteBlock(c.uid))).catch(
+        (error) => {
+          internalError({ error });
+        },
+      );
     }
     return () => {
       refreshConfigTree();
     };
-  }, [parentUid, setMigrated, enabled]);
+  }, [parentUid, setMigrated, enabled, node.format, node.text]);
   return (
     <div className={"roamjs-node-specification"}>
       <style>
@@ -93,9 +103,23 @@ const NodeSpecification = ({
                 parentUid,
                 order: 2,
                 node: { text: "enabled" },
-              }).then(setEnabled);
+              })
+                .then((uid: string) => {
+                  setEnabled(uid);
+                  if (parentSetEnabled) parentSetEnabled(true);
+                })
+                .catch((error) => {
+                  internalError({ error });
+                });
             } else {
-              deleteBlock(enabled).then(() => setEnabled(""));
+              deleteBlock(enabled)
+                .then(() => {
+                  setEnabled("");
+                  if (parentSetEnabled) parentSetEnabled(false);
+                })
+                .catch((error) => {
+                  internalError({ error });
+                });
             }
           }}
         />

apps/roam/src/components/settings/NodeConfig.tsx

@@ -230,52 +230,73 @@ const NodeConfig = ({
     true,
   );
 
-  const validate = useCallback((tag: string, format: string) => {
-    const cleanTag = getCleanTagText(tag);
+  const validate = useCallback(
+    ({
+      tag,
+      format,
+      isSpecificationEnabled,
+    }: {
+      tag: string;
+      format: string;
+      isSpecificationEnabled?: boolean;
+    }) => {
+      if (isSpecificationEnabled === undefined)
+        isSpecificationEnabled = !!getSubTree({
+          tree: getBasicTreeByParentUid(specificationUid),
+          key: "enabled",
+        })?.uid?.length;
+      if (format.trim().length === 0 && !isSpecificationEnabled) {
+        setTagError("");
+        setFormatError("Error: you must set either a format or specification");
+        return;
+      }
+      const cleanTag = getCleanTagText(tag);
 
-    if (!cleanTag) {
-      setTagError("");
-      setFormatError("");
-      return;
-    }
+      if (!cleanTag) {
+        setTagError("");
+        setFormatError("");
+        return;
+      }
 
-    const roamTagRegex = /#?\[\[(.*?)\]\]|#(\S+)/g;
-    const matches = format.matchAll(roamTagRegex);
-    const formatTags: string[] = [];
-    for (const match of matches) {
-      const tagName = match[1] || match[2];
-      if (tagName) {
-        formatTags.push(tagName.toUpperCase());
+      const roamTagRegex = /#?\[\[(.*?)\]\]|#(\S+)/g;
+      const matches = format.matchAll(roamTagRegex);
+      const formatTags: string[] = [];
+      for (const match of matches) {
+        const tagName = match[1] || match[2];
+        if (tagName) {
+          formatTags.push(tagName.toUpperCase());
+        }
       }
-    }
 
-    const hasConflict = formatTags.includes(cleanTag);
+      const hasConflict = formatTags.includes(cleanTag);
 
-    if (hasConflict) {
-      setFormatError(
-        `The format references the node's tag "${tag}". Please use a different format or tag.`,
-      );
-      setTagError(
-        `The tag "${tag}" is referenced in the format. Please use a different tag or format.`,
-      );
-    } else {
-      setTagError("");
-      setFormatError("");
-    }
-  }, []);
+      if (hasConflict) {
+        setFormatError(
+          `The format references the node's tag "${tag}". Please use a different format or tag.`,
+        );
+        setTagError(
+          `The tag "${tag}" is referenced in the format. Please use a different tag or format.`,
+        );
+      } else {
+        setTagError("");
+        setFormatError("");
+      }
+    },
+    [specificationUid],
+  );
 
   useEffect(() => {
-    validate(tagValue, formatValue);
+    validate({ tag: tagValue, format: formatValue });
   }, [tagValue, formatValue, validate]);
 
   const handleTagBlur = useCallback(() => {
     handleTagBlurFromHook();
-    validate(tagValue, formatValue);
+    validate({ tag: tagValue, format: formatValue });
   }, [handleTagBlurFromHook, tagValue, formatValue, validate]);
 
   const handleFormatBlur = useCallback(() => {
     handleFormatBlurFromHook();
-    validate(tagValue, formatValue);
+    validate({ tag: tagValue, format: formatValue });
   }, [handleFormatBlurFromHook, tagValue, formatValue, validate]);
 
   return (
@@ -353,6 +374,13 @@ const NodeConfig = ({
                 <DiscourseNodeSpecification
                   node={node}
                   parentUid={specificationUid}
+                  parentSetEnabled={(isSpecificationEnabled) => {
+                    validate({
+                      tag: tagValue,
+                      format: formatValue,
+                      isSpecificationEnabled,
+                    });
+                  }}
                 />
               </Label>
             </div>