#3831 Add validation support for sub pipelines

src/blocks/renderers/documentView/DocumentView.tsx

@@ -21,7 +21,7 @@ import ReactShadowRoot from "react-shadow-root";
 import BootstrapStylesheet from "@/blocks/renderers/BootstrapStylesheet";
 import { DocumentViewProps } from "./DocumentViewProps";
 import DocumentContext from "@/components/documentBuilder/render/DocumentContext";
-import { joinElementName } from "@/components/documentBuilder/utils";
+import { joinPathParts } from "@/utils";
 
 const DocumentView: React.FC<DocumentViewProps> = ({ body, options, meta }) => {
   // Track style loading to avoid a FOUC
@@ -52,7 +52,7 @@ const DocumentView: React.FC<DocumentViewProps> = ({ body, options, meta }) => {
             body.map((documentElement, index) => {
               const { Component, props } = buildDocumentBranch(
                 documentElement,
-                { staticId: joinElementName("body", "children"), branches: [] }
+                { staticId: joinPathParts("body", "children"), branches: [] }
               );
               return <Component key={index} {...props} />;
             })}

src/blocks/renderers/markdown.ts

@@ -19,11 +19,14 @@ import { Renderer } from "@/types";
 import { propertiesToSchema } from "@/validators/generic";
 import { BlockArg, SafeHTML } from "@/core";
 import safeMarkdown from "@/utils/safeMarkdown";
+import { validateRegistryId } from "@/types/helpers";
 
 export class MarkdownRenderer extends Renderer {
+  static BLOCK_ID = validateRegistryId("@pixiebrix/markdown");
+
   constructor() {
     super(
-      "@pixiebrix/markdown",
+      MarkdownRenderer.BLOCK_ID,
       "Render Markdown",
       "Render Markdown to sanitized HTML"
     );

src/components/documentBuilder/documentTree.tsx

@@ -30,7 +30,7 @@ import {
 import ButtonElement from "@/components/documentBuilder/render/ButtonElement";
 import ListElement from "@/components/documentBuilder/render/ListElement";
 import { BusinessError } from "@/errors/businessErrors";
-import { joinElementName } from "@/components/documentBuilder/utils";
+import { joinPathParts } from "@/utils";
 
 const headerComponents = {
   header_1: "h1",
@@ -181,7 +181,7 @@ export const buildDocumentBranch: BuildDocumentBranch = (root, tracePath) => {
   if (root.children?.length > 0) {
     componentDefinition.props.children = root.children.map((child, index) => {
       const { Component, props } = buildDocumentBranch(child, {
-        staticId: joinElementName(staticId, root.type, "children"),
+        staticId: joinPathParts(staticId, root.type, "children"),
         branches: [...branches, { staticId, index }],
       });
       return <Component key={index} {...props} />;

src/components/documentBuilder/hooks/useMoveWithinParent.ts

@@ -21,7 +21,7 @@ import { actions } from "@/pageEditor/slices/editorSlice";
 import { useField } from "formik";
 import { DocumentElement } from "@/components/documentBuilder/documentBuilderTypes";
 import getElementCollectionName from "@/components/documentBuilder/edit/getElementCollectionName";
-import { joinElementName } from "@/components/documentBuilder/utils";
+import { joinPathParts } from "@/utils";
 
 type MoveWithinParent = {
   canMoveUp: boolean;
@@ -38,7 +38,7 @@ function useMoveWithinParent(documentBodyName: string): MoveWithinParent {
   const { collectionName, elementIndex } =
     getElementCollectionName(activeElement);
 
-  const fullCollectionName = joinElementName(documentBodyName, collectionName);
+  const fullCollectionName = joinPathParts(documentBodyName, collectionName);
 
   const [{ value: elementsCollection }, , { setValue: setElementsCollection }] =
     useField<DocumentElement[]>(fullCollectionName);
@@ -58,7 +58,7 @@ function useMoveWithinParent(documentBodyName: string): MoveWithinParent {
     /* eslint-enable security/detect-object-injection */
 
     setElementsCollection(newElementsCollection);
-    setActiveElement(joinElementName(collectionName, toIndex));
+    setActiveElement(joinPathParts(collectionName, toIndex));
   };
 
   return {

src/components/documentBuilder/outline/outlineHelpers.ts

@@ -22,7 +22,7 @@ import {
 import { TreeExpandedState } from "@/components/jsonTree/JsonTree";
 import { ItemId, TreeData } from "@atlaskit/tree";
 import { TreeItem } from "@atlaskit/tree/types";
-import { joinElementName } from "@/components/documentBuilder/utils";
+import { joinPathParts } from "@/utils";
 import { PARENT_ELEMENT_TYPES } from "@/components/documentBuilder/allowedElementTypes";
 
 type ElementArgs = {
@@ -39,7 +39,7 @@ function getChildren(
     return [
       {
         element: element.config.element.__value__,
-        elementName: joinElementName(
+        elementName: joinPathParts(
           elementName,
           "config",
           "element",
@@ -53,7 +53,7 @@ function getChildren(
 
   return children.map((child, index) => ({
     element: child,
-    elementName: joinElementName(elementName, "children", String(index)),
+    elementName: joinPathParts(elementName, "children", String(index)),
   }));
 }
 

src/components/documentBuilder/render/ListElement.tsx

@@ -30,10 +30,9 @@ import { produce } from "immer";
 import ErrorBoundary from "@/components/ErrorBoundary";
 import { getErrorMessage } from "@/errors/errorHelpers";
 import { runMapArgs } from "@/contentScript/messenger/api";
-import { isNullOrBlank } from "@/utils";
+import { isNullOrBlank, joinPathParts } from "@/utils";
 import apiVersionOptions from "@/runtime/apiVersionOptions";
 import { whoAmI } from "@/background/messenger/api";
-import { joinElementName } from "@/components/documentBuilder/utils";
 
 type DocumentListProps = {
   array: UnknownObject[];
@@ -130,7 +129,7 @@ const ListElementInternal: React.FC<DocumentListProps> = ({
         const { Component, props } = buildDocumentBranch(
           documentElement as DocumentElement,
           {
-            staticId: joinElementName(staticId, "list", "children"),
+            staticId: joinPathParts(staticId, "list", "children"),
             branches: [...branches, { staticId, index }],
           }
         );

src/components/documentBuilder/utils.ts

@@ -18,15 +18,6 @@
 import { DynamicPath } from "@/components/documentBuilder/documentBuilderTypes";
 import { Branch } from "@/blocks/types";
 
-/**
- * Join parts of a document builder element name, ignoring null/blank parts.
- * @param nameParts the parts of the name
- */
-export function joinElementName(...nameParts: Array<string | number>): string {
-  // Don't use lodash.compact and lodash.isEmpty since they treat 0 as falsy
-  return nameParts.filter((x) => x != null && x !== "").join(".");
-}
-
 export function mapPathToTraceBranches(tracePath: DynamicPath): Branch[] {
   return tracePath.branches.map(({ staticId, index }) => ({
     key: staticId,

src/pageEditor/extensionPoints/pipelineMapping.ts

@@ -32,7 +32,7 @@ export function normalizePipelineForEditor(
 ): BlockPipeline {
   return produce(pipeline, (pipeline: WritableDraft<BlockPipeline>) => {
     traversePipeline({
-      blockPipeline: pipeline,
+      pipeline,
       visitBlock({ blockConfig }) {
         blockConfig.instanceId = uuidv4();
       },
@@ -55,7 +55,7 @@ export function normalizePipelineForEditor(
 export function omitEditorMetadata(pipeline: BlockPipeline): BlockPipeline {
   return produce(pipeline, (pipeline: WritableDraft<BlockPipeline>) => {
     traversePipeline({
-      blockPipeline: pipeline,
+      pipeline,
       visitBlock({ blockConfig }) {
         delete blockConfig.instanceId;
       },

src/pageEditor/hooks/usePipelineErrors.ts

@@ -30,30 +30,34 @@ import validateStringTemplates from "@/pageEditor/validation/validateStringTempl
 import { PIPELINE_BLOCKS_FIELD_NAME } from "@/pageEditor/consts";
 import { FormState } from "@/pageEditor/pageEditorTypes";
 import useAllBlocks from "./useAllBlocks";
+import { selectPipelineMap } from "@/pageEditor/slices/editorSelectors";
 
 function usePipelineErrors() {
   const [allBlocks] = useAllBlocks();
   const traceErrors = useSelector(selectTraceErrors);
   const formikContext = useFormikContext<FormState>();
   const extensionPointType = formikContext.values.type;
+  const pipelineMap = useSelector(selectPipelineMap);
 
   const validatePipelineBlocks = useCallback(
     (pipeline: BlockPipeline): void | FormikErrorTree => {
       const formikErrors: FormikErrorTree = {};
 
+      // TODO move this to the OutputKey field level
       validateOutputKey(formikErrors, pipeline, allBlocks);
       validateRenderers(formikErrors, pipeline, allBlocks, extensionPointType);
+      // TODO move this to the TextField level
       validateStringTemplates(formikErrors, pipeline);
-      applyTraceErrors(formikErrors, traceErrors, pipeline);
+      applyTraceErrors(formikErrors, traceErrors, pipelineMap);
 
       return isEmpty(formikErrors) ? undefined : formikErrors;
     },
-    [allBlocks, extensionPointType, traceErrors]
+    [allBlocks, extensionPointType, pipelineMap, traceErrors]
   );
 
   useField<BlockPipeline>({
     name: PIPELINE_BLOCKS_FIELD_NAME,
-    // @ts-expect-error working with nested errors
+    // @ts-expect-error -- validatePipelineBlocks can return an object b/c we're working with nested errors
     validate: validatePipelineBlocks,
   });
 

src/pageEditor/panes/EditorPane.test.tsx

@@ -16,7 +16,7 @@
  */
 
 import React from "react";
-import { render, screen } from "@/pageEditor/testHelpers";
+import { fireEvent, render, screen } from "@/pageEditor/testHelpers";
 import EditorPane from "./EditorPane";
 import { actions as editorActions } from "@/pageEditor/slices/editorSlice";
 import { selectActiveElement } from "@/pageEditor/slices/editorSelectors";
@@ -40,8 +40,12 @@ import {
 import { PipelineExpression } from "@/runtime/mapArgs";
 import { act } from "react-dom/test-utils";
 import { OutputKey } from "@/core";
-
-jest.setTimeout(30_000); // This test is flaky with the default timeout of 5000 ms
+import {
+  fireTextInput,
+  RJSF_SCHEMA_PROPERTY_NAME,
+  selectSchemaFieldType,
+} from "@/testUtils/formHelpers";
+import { PIPELINE_BLOCKS_FIELD_NAME } from "../consts";
 
 const jqBlock = new JQTransformer();
 const forEachBlock = new ForEach();
@@ -50,23 +54,15 @@ const forEachBlock = new ForEach();
 const immediateUserEvent = userEvent.setup({ delay: null });
 
 beforeAll(async () => {
-  jest.useFakeTimers();
-
+  jest.setTimeout(30_000); // This test is flaky with the default timeout of 5000 ms
   registerDefaultWidgets();
   blockRegistry.clear();
   blockRegistry.register(echoBlock, teapotBlock, jqBlock, forEachBlock);
   await blockRegistry.allTyped();
 });
 
 afterAll(() => {
-  jest.useRealTimers();
-});
-
-beforeEach(() => {
-  jest.clearAllTimers();
-});
-afterEach(() => {
-  jest.runOnlyPendingTimers();
+  jest.setTimeout(5_000);
 });
 
 const getPlainFormState = (): FormState =>
@@ -143,6 +139,20 @@ describe("renders", () => {
 });
 
 describe("can add a node", () => {
+  beforeAll(() => {
+    jest.useFakeTimers();
+  });
+  afterAll(() => {
+    jest.useRealTimers();
+  });
+
+  beforeEach(() => {
+    jest.clearAllTimers();
+  });
+  afterEach(() => {
+    jest.runOnlyPendingTimers();
+  });
+
   async function addABlock(addButton: Element, blockName: string) {
     await immediateUserEvent.click(addButton);
 
@@ -319,3 +329,36 @@ describe("can remove a node", () => {
     expect(nodes[2]).toHaveTextContent(/for-each loop/i);
   });
 });
+
+describe("validation", () => {
+  test("validates string templates", async () => {
+    const formState = getFormStateWithSubPipelines();
+    const subEchoNode = (
+      formState.extension.blockPipeline[1].config.body as PipelineExpression
+    ).__value__[0];
+    const rendered = render(<EditorPane />, {
+      setupRedux(dispatch) {
+        dispatch(editorActions.addElement(formState));
+        dispatch(editorActions.selectElement(formState.uuid));
+        dispatch(editorActions.setElementActiveNodeId(subEchoNode.instanceId));
+      },
+    });
+
+    await waitForEffect();
+
+    await selectSchemaFieldType(
+      `${PIPELINE_BLOCKS_FIELD_NAME}.1.config.body.__value__.0.config.message`,
+      "var"
+    );
+
+    // By some reason, the validation doesn't fire with userEvent.type
+    fireTextInput(rendered.getByLabelText("message"), "{{!");
+    await waitForEffect();
+
+    expect(
+      screen.getByText(
+        "Invalid string template. Read more about string templates: https://docs.pixiebrix.com/nunjucks-templates"
+      )
+    ).toBeInTheDocument();
+  });
+});

src/pageEditor/panes/__snapshots__/EditorPane.test.tsx.snap

@@ -1443,7 +1443,49 @@ exports[`renders an extension with sub pipeline 1`] = `
                       >
                         <ul
                           style="padding: 0px; margin: 0px; list-style: none;"
-                        />
+                        >
+                          <li
+                            style="padding-top: 0.25em; padding-right: 0px; margin-left: 0.875em; word-wrap: break-word; padding-left: 1.25em; text-indent: -0.5em; word-break: break-all; white-space: pre-wrap;"
+                          >
+                            <label
+                              style="display: inline-block; color: rgb(66, 122, 245); margin: 0px 0.5em 0px 0px;"
+                            >
+                              <div>
+                                <span>
+                                  @input
+                                </span>
+                                : 
+                                <a
+                                  aria-label="copy path"
+                                  class="copyPath p-0 btn btn-text"
+                                  href="#"
+                                  role="button"
+                                >
+                                  <svg
+                                    aria-hidden="true"
+                                    class="svg-inline--fa fa-copy fa-w-14 "
+                                    data-icon="copy"
+                                    data-prefix="fas"
+                                    focusable="false"
+                                    role="img"
+                                    viewBox="0 0 448 512"
+                                    xmlns="http://www.w3.org/2000/svg"
+                                  >
+                                    <path
+                                      d="M320 448v40c0 13.255-10.745 24-24 24H24c-13.255 0-24-10.745-24-24V120c0-13.255 10.745-24 24-24h72v296c0 30.879 25.121 56 56 56h168zm0-344V0H152c-13.255 0-24 10.745-24 24v368c0 13.255 10.745 24 24 24h272c13.255 0 24-10.745 24-24V128H344c-13.2 0-24-10.8-24-24zm120.971-31.029L375.029 7.029A24 24 0 0 0 358.059 0H352v96h96v-6.059a24 24 0 0 0-7.029-16.97z"
+                                      fill="currentColor"
+                                    />
+                                  </svg>
+                                </a>
+                              </div>
+                            </label>
+                            <span
+                              style="color: rgb(241, 89, 80);"
+                            >
+                              undefined
+                            </span>
+                          </li>
+                        </ul>
                       </li>
                     </ul>
                   </div>

src/pageEditor/tabs/editTab/editHelpers.ts

@@ -76,11 +76,11 @@ export async function generateFreshOutputKey(
   return freshIdentifier(type as SafeString, outputKeys) as OutputKey;
 }
 
-export function getPipelineMap(blockPipeline: BlockPipeline) {
+export function getPipelineMap(pipeline: BlockPipeline) {
   const pipelineMap: PipelineMap = {};
   traversePipeline({
-    blockPipeline,
-    blockPipelinePath: PIPELINE_BLOCKS_FIELD_NAME,
+    pipeline,
+    pipelinePath: PIPELINE_BLOCKS_FIELD_NAME,
     visitBlock({
       blockConfig,
       index,