diff --git a/.changeset/renders-validation-errors.md b/.changeset/renders-validation-errors.md new file mode 100644 index 00000000..4b67d528 --- /dev/null +++ b/.changeset/renders-validation-errors.md @@ -0,0 +1,5 @@ +--- +"@serverlessworkflow/diagram-editor": patch +--- + +Renders SDK validation errors on the diagram and in the sidepanel diff --git a/packages/serverless-workflow-diagram-editor/src/core/index.ts b/packages/serverless-workflow-diagram-editor/src/core/index.ts index 2a342176..ba4bb9f8 100644 --- a/packages/serverless-workflow-diagram-editor/src/core/index.ts +++ b/packages/serverless-workflow-diagram-editor/src/core/index.ts @@ -15,6 +15,7 @@ */ export * from "./workflowSdk"; +export * from "./validationErrors"; export * from "./graph"; export * from "./taskDetails"; export * from "./taskSubType"; diff --git a/packages/serverless-workflow-diagram-editor/src/core/validationErrors.ts b/packages/serverless-workflow-diagram-editor/src/core/validationErrors.ts new file mode 100644 index 00000000..2fa8fb34 --- /dev/null +++ b/packages/serverless-workflow-diagram-editor/src/core/validationErrors.ts @@ -0,0 +1,133 @@ +/* + * Copyright 2021-Present The Serverless Workflow Specification Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { SdkError, ValidationError } from "./workflowSdk"; + +/* workflowSdk produces a flat array of errors, but the UI needs them split into two categories: errors that attach to a specific node, and workflow-level errors that don't. This file provides helper functions to filter, sort, and slice that error list. + */ + +/* The SDK reports an invalid task as "missing" every other task type, which is + * noise. These are the missing-type errors to filter out. + * + * `catch` is intentionally excluded: a missing-property error on a `catch` is a genuine problem worth surfacing. + */ +const MISSING_PROP_TASK_TYPES = new Set([ + "call", + "do", + "emit", + "for", + "fork", + "listen", + "raise", + "run", + "set", + "switch", + "try", + "wait", +]); + +type NodeError = ValidationError & { path: string }; + +export function isValidationError(error: SdkError): error is ValidationError { + return !(error instanceof Error); +} + +/* Get the last segment as keyword eg #/oneOf/2/allOf/1/properties/with/required returns 'required' */ +function getErrorKeyword(error: ValidationError): string | undefined { + return error.errorType?.split("/").pop(); +} + +function isNoiseError(error: ValidationError): boolean { + const keyword = getErrorKeyword(error); + + if (keyword === "oneOf") { + return true; + } + + const missingProperty = error.object?.["missingProperty"]; + if (typeof missingProperty === "string" && MISSING_PROP_TASK_TYPES.has(missingProperty)) { + return true; + } + + /* `call` is a special case: it has a subtype, so the SDK reports errors complaining it must be one of `grpc`, `http`, etc. These are noise*/ + if ((keyword === "const" || keyword === "not") && error.errorType?.includes("/properties/call")) { + return true; + } + + return false; +} + +function isNodeError(error: SdkError): error is NodeError { + return isValidationError(error) && error.path !== undefined && !isNoiseError(error); +} + +/* Match the longest id prefix to find owning node */ +function findOwningNode(path: string, nodeIds: Set): string | undefined { + let owner: string | undefined; + for (const id of nodeIds) { + if (path === id || path.startsWith(`${id}/`)) { + if (owner === undefined || id.length > owner.length) { + owner = id; + } + } + } + return owner; +} + +/* returns errors for a particular node and removes noise */ +export function getNodeErrors( + errors: SdkError[], + nodeId: string, + nodeIds: Set, +): ValidationError[] { + return errors.filter((error): error is NodeError => { + return isNodeError(error) && findOwningNode(error.path, nodeIds) === nodeId; + }); +} + +export function getNodeErrorField(error: ValidationError, nodeId: string): string | undefined { + const prefix = `${nodeId}/`; + if (error.path === undefined || !error.path.startsWith(prefix)) { + return undefined; + } + + return error.path.slice(prefix.length).split("/").join("."); +} + +/* To quickly lookup nodeIds that should display badge and outline */ +export function getErrorNodeIds(errors: SdkError[], nodeIds: Set): Set { + const ids = new Set(); + for (const error of errors) { + if (!isNodeError(error)) { + continue; + } + const owner = findOwningNode(error.path, nodeIds); + if (owner !== undefined) { + ids.add(owner); + } + } + return ids; +} + +/* Errors not associated with node (workflow-level errors) */ +export function getGeneralErrors(errors: SdkError[], nodeIds: Set): SdkError[] { + return errors.filter((error) => { + if (!isValidationError(error) || error.path === undefined) { + return true; + } + return findOwningNode(error.path, nodeIds) === undefined; + }); +} diff --git a/packages/serverless-workflow-diagram-editor/src/core/workflowSdk.ts b/packages/serverless-workflow-diagram-editor/src/core/workflowSdk.ts index 88201d76..0645c10d 100644 --- a/packages/serverless-workflow-diagram-editor/src/core/workflowSdk.ts +++ b/packages/serverless-workflow-diagram-editor/src/core/workflowSdk.ts @@ -39,7 +39,7 @@ function sanitizeObject(obj: Record): Record { } export type ValidationError = { - taskId?: string; + path?: string; errorType?: string; message: string; object?: Record; @@ -59,7 +59,7 @@ export type WorkflowParseResult = { * * **Format 1: Pipe-delimited with 4 fields (task-specific errors)** * ``` - * - taskId | errorType | message | object + * - path | errorType | message | object * ``` * Example: * ``` @@ -78,7 +78,7 @@ export type WorkflowParseResult = { * @param message - The raw error message string from the SDK, typically containing multiple lines * @returns Array of structured ValidationError objects. Each error is guaranteed to have: * - `message`: The error description (always present) - * - `taskId`: The workflow task path (present only in Format 1) + * - `path`: The workflow task path (present only in Format 1) * - `errorType`: The error type/schema reference (present in both formats) * - `object`: Additional error context as a sanitized object with null prototype (present only in Format 1; * empty object if JSON parsing fails or if the JSON is not a plain object) @@ -98,7 +98,7 @@ export type WorkflowParseResult = { * * const errors = parseValidationErrorMessage(sdkError); * // [ - * // { taskId: "/do/0/call", errorType: "#/required", message: "must have property", + * // { path: "/do/0/call", errorType: "#/required", message: "must have property", * // object: { missingProperty: "http" } }, * // { errorType: "#/document", message: "must have required property 'document'" } * // ] @@ -134,7 +134,7 @@ export function parseValidationErrorMessage(message: string): ValidationError[] const firstPipe = pipePositions[0]!; const secondPipe = pipePositions[1]!; - const taskId = content.substring(0, firstPipe).trim(); + const path = content.substring(0, firstPipe).trim(); const errorType = content.substring(firstPipe + 1, secondPipe).trim(); // Try to find the last pipe that separates valid JSON @@ -182,12 +182,12 @@ export function parseValidationErrorMessage(message: string): ValidationError[] } // Validate all required fields are non-empty - if (!taskId || !errorType || !errorMessage || !objectStr) { + if (!path || !errorType || !errorMessage || !objectStr) { continue; } errors.push({ - taskId, + path, errorType, message: errorMessage, object: parsedObject, @@ -212,13 +212,31 @@ export function parseValidationErrorMessage(message: string): ValidationError[] return errors; } +/* + * The SDK repeats its full error block within a single message, so the parsed list + * contains duplicates (identical path + errorType + message). Those carry no + * extra information and would otherwise surface the same error twice on a node and + * inflate error counts, so collapse exact duplicates here at the produce point. + */ +function dedupeValidationErrors(errors: ValidationError[]): ValidationError[] { + const seen = new Set(); + return errors.filter((error) => { + const signature = `${error.path ?? ""}|${error.errorType}|${error.message}`; + if (seen.has(signature)) { + return false; + } + seen.add(signature); + return true; + }); +} + export function validateWorkflow(model: sdk.Specification.Workflow): SdkError[] { try { sdk.validate("Workflow", model); return []; } catch (err) { const message = err instanceof Error ? err.message : String(err); - const parsedErrors = parseValidationErrorMessage(message); + const parsedErrors = dedupeValidationErrors(parseValidationErrorMessage(message)); // If parsing succeeded and returned errors, use them if (parsedErrors.length > 0) { @@ -237,7 +255,9 @@ export function parseWorkflow(text: string): WorkflowParseResult { let raw: Partial; try { - raw = yaml.load(text, { schema: yaml.DEFAULT_SCHEMA }) as Partial; + raw = yaml.load(text, { + schema: yaml.DEFAULT_SCHEMA, + }) as Partial; } catch (err) { return { model: null, diff --git a/packages/serverless-workflow-diagram-editor/src/i18n/locales/en.ts b/packages/serverless-workflow-diagram-editor/src/i18n/locales/en.ts index c5868d8a..80c88cbb 100644 --- a/packages/serverless-workflow-diagram-editor/src/i18n/locales/en.ts +++ b/packages/serverless-workflow-diagram-editor/src/i18n/locales/en.ts @@ -24,6 +24,7 @@ export const en = { "sidebar.selectNode": "Select a node to view its details.", "sidebar.sectionDocument": "Document", "sidebar.sectionMetadata": "Metadata", + "sidebar.sectionErrors": "Errors", "sidebar.name": "Name", "sidebar.version": "Version", "sidebar.namespace": "Namespace", @@ -38,6 +39,7 @@ export const en = { "sidebar.noDetails": "No additional details for this node", "node.entry": "Entry", "node.exit": "Exit", + "node.errorBadge": "This node has validation errors", "sidebar.exportMermaid.copy": "Copy Mermaid Code", "sidebar.exportMermaid.download": "Download as Mermaid File", "sidebar.exportMermaid.copied": "Copied!", diff --git a/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/Diagram.css b/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/Diagram.css index c5efb451..9f52b4f6 100644 --- a/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/Diagram.css +++ b/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/Diagram.css @@ -358,6 +358,50 @@ @apply dec:truncate; } /* end terminal markers */ + /* Validation errors */ + .dec-root .dec-leaf-node.has-error, + .dec-root .dec-container-node.has-error { + box-shadow: + 0 0 0 1px var(--dec-error-accent), + 0 0 10px 1px var(--dec-error-glow); + } + + .dec-root.dark .dec-leaf-node.has-error, + .dec-root.dark .dec-container-node.has-error { + box-shadow: + 0 0 0 1px var(--dec-error-accent), + 0 0 12px 2px rgba(248, 113, 113, 0.4); + } + + .dec-root .dec-leaf-node.has-error.selected, + .dec-root .dec-container-node.has-error.selected { + box-shadow: + 0 0 0 2px var(--dec-error-accent), + 0 0 0 4px rgb(59 130 246), + 0 0 12px 4px rgba(59, 130, 246, 0.45); + } + + .dec-root.dark .dec-leaf-node.has-error.selected, + .dec-root.dark .dec-container-node.has-error.selected { + box-shadow: + 0 0 0 2px var(--dec-error-accent), + 0 0 0 4px rgb(96 165 250), + 0 0 16px 4px rgba(96, 165, 250, 0.5); + } + + .dec-root .dec-leaf-node.has-error { + @apply dec:relative; + } + .dec-root .dec-node-error-badge { + @apply dec:absolute dec:z-10 dec:h-[18px] dec:w-[18px] dec:rounded-full; + top: -8px; + left: -8px; + fill: var(--dec-error-accent); + color: #ffffff; + stroke-width: 2.25; + } + + /* end validation errors */ } /* custom edges */ @@ -368,8 +412,8 @@ } .dec-root .edge-line.error { - @apply dec:stroke-red-500 - dec:[stroke-dasharray:5_5]; + stroke: var(--dec-error-accent); + @apply dec:[stroke-dasharray:5_5]; } .dec-root .edge-line.condition { diff --git a/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/Diagram.tsx b/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/Diagram.tsx index 7ff812ac..34dd9caf 100644 --- a/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/Diagram.tsx +++ b/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/Diagram.tsx @@ -23,8 +23,8 @@ import { ResolvedColorMode } from "../../types/colorMode"; import { ReactFlowEdgeTypes } from "../edges/Edges"; import { useDiagramEditorContext } from "../../store/DiagramEditorContext"; import { buildDiagramElements } from "./diagramBuilder"; -import { SidebarTrigger } from "@/components/ui/sidebar"; import { applyAutoLayout } from "./autoLayout"; +import { SidePanelTrigger } from "@/side-panel/SidePanelTrigger"; const FIT_VIEW_OPTIONS: RF.FitViewOptions = { maxZoom: 1, @@ -47,7 +47,7 @@ export type DiagramProps = { export const Diagram = ({ divRef, ref, colorMode = "light" }: DiagramProps) => { const reactFlowInstance: RF.ReactFlowInstance = RF.useReactFlow(); - const { model, nodes, edges, isReadOnly, setNodes, setEdges, setSelectedNodeId } = + const { model, errors, nodes, edges, isReadOnly, setNodes, setEdges, setSelectedNodeId } = useDiagramEditorContext(); const [minimapVisible, setMinimapVisible] = React.useState(false); @@ -87,7 +87,7 @@ export const Diagram = ({ divRef, ref, colorMode = "light" }: DiagramProps) => { // Create abort controller for this layout operation abortController = new AbortController(); - const graph = buildDiagramElements(model); + const graph = buildDiagramElements(model, errors); applyAutoLayout(graph, abortController.signal) .then(({ nodes, edges }) => { // Only update if this effect is still active (not cancelled by cleanup) @@ -128,7 +128,7 @@ export const Diagram = ({ divRef, ref, colorMode = "light" }: DiagramProps) => { abortController.abort(); } }; - }, [model, reactFlowInstance, setNodes, setEdges]); + }, [model, errors, reactFlowInstance, setNodes, setEdges]); return (
{ {minimapVisible && } - + , + erroringNodeIds: Set, ): RF.Node { const type = resolveNodeType(graphNode, catchContainerIds); // There is no corresponding react flow component implemented @@ -91,9 +92,8 @@ function buildReactFlowNode( type, data: { label: graphNode.label ?? "", - ...(graphNode.task !== undefined && { - task: structuredClone(graphNode.task), - }), + ...(graphNode.task !== undefined && { task: structuredClone(graphNode.task) }), + ...(erroringNodeIds.has(graphNode.id) && { hasError: true }), }, height: size.height, width: size.width, @@ -124,16 +124,21 @@ function buildReactFlowEdge( }; } -export function buildDiagramElements(model: sdk.Specification.Workflow | null): ReactFlowGraph { +export function buildDiagramElements( + model: sdk.Specification.Workflow | null, + errors: SdkError[] = [], +): ReactFlowGraph { const nodes: RF.Node[] = []; const edges: RF.Edge[] = []; if (model) { const graph = buildFlatGraph(model); const catchContainerIds = getCatchContainerNodeIds(graph); + const nodeIds = new Set(graph.nodes.map((graphNode) => graphNode.id)); + const nodeIdsWithErrors = getErrorNodeIds(errors, nodeIds); graph.nodes.forEach((graphNode) => - nodes.push(buildReactFlowNode(graphNode, catchContainerIds)), + nodes.push(buildReactFlowNode(graphNode, catchContainerIds, nodeIdsWithErrors)), ); // Precompute node ID set for O(1) membership checks diff --git a/packages/serverless-workflow-diagram-editor/src/react-flow/nodes/Nodes.tsx b/packages/serverless-workflow-diagram-editor/src/react-flow/nodes/Nodes.tsx index d1cfbcbd..9a9efe60 100644 --- a/packages/serverless-workflow-diagram-editor/src/react-flow/nodes/Nodes.tsx +++ b/packages/serverless-workflow-diagram-editor/src/react-flow/nodes/Nodes.tsx @@ -28,6 +28,7 @@ import { terminalNodeConfigMap, } from "./taskNodeConfig"; import { getCallSubType, getListenSubType, getRunSubType } from "../../core"; +import { CircleAlert } from "lucide-react"; export const ReactFlowNodeTypes: RF.NodeTypes = { [GraphNodeType.Start]: StartNode, @@ -72,6 +73,7 @@ const KNOWN_BADGES = new Set([ export type BaseNodeData = { label: string; task?: T; + hasError?: boolean; }; interface NodeContentProps { @@ -106,15 +108,20 @@ function TaskNodeBadge({ badge, testId }: BadgeProps) { ); } +function NodeErrorBadge({ testId }: { testId: string }) { + return ; +} + function LeafNodeContent({ id, data, selected, type, badge }: NodeContentProps) { const config = leafNodeConfigMap[type as LeafNodeType]; const Icon = config.icon; return (
+ {data.hasError && }
@@ -136,10 +143,11 @@ function ContainerNodeContent({ id, data, selected, type, badge }: NodeContentPr const Icon = config.icon; return (
+ {data.hasError && }
diff --git a/packages/serverless-workflow-diagram-editor/src/side-panel/ErrorsSection.tsx b/packages/serverless-workflow-diagram-editor/src/side-panel/ErrorsSection.tsx new file mode 100644 index 00000000..d9a469a7 --- /dev/null +++ b/packages/serverless-workflow-diagram-editor/src/side-panel/ErrorsSection.tsx @@ -0,0 +1,55 @@ +/* + * Copyright 2021-Present The Serverless Workflow Specification Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { useI18n } from "@serverlessworkflow/i18n"; +import { CircleAlert } from "lucide-react"; + +export type ErrorItem = { + message: string; + field?: string; +}; + +type ErrorSectionProps = { + items: ErrorItem[]; +}; + +export function ErrorSection({ items }: ErrorSectionProps) { + const { t } = useI18n(); + + if (items.length === 0) { + return null; + } + + return ( +
+
+ +

{t("sidebar.sectionErrors")}

+ {items.length} +
+
    + {items.map((item) => ( +
  • + {item.field !== undefined && ( + {item.field} + )} + {item.message} +
  • + ))} +
+
+ ); +} diff --git a/packages/serverless-workflow-diagram-editor/src/side-panel/NodeDetailsView.tsx b/packages/serverless-workflow-diagram-editor/src/side-panel/NodeDetailsView.tsx index 9d073c3b..760220be 100644 --- a/packages/serverless-workflow-diagram-editor/src/side-panel/NodeDetailsView.tsx +++ b/packages/serverless-workflow-diagram-editor/src/side-panel/NodeDetailsView.tsx @@ -20,6 +20,9 @@ import { useI18n } from "@serverlessworkflow/i18n"; import { getTaskDetails, type DetailField } from "@/core/taskDetails"; import type { BaseNodeData } from "@/react-flow/nodes/Nodes"; import { YamlField, PropertyField, SectionHeader } from "./Fields"; +import { useDiagramEditorContext } from "@/store/DiagramEditorContext"; +import { getNodeErrorField, getNodeErrors } from "@/core"; +import { ErrorSection } from "./ErrorsSection"; type NodeDetailsViewProps = { node: RF.Node; @@ -48,23 +51,34 @@ function FieldRow({ label, field }: { label: string; field: DetailField }) { export function NodeDetailsView({ node }: NodeDetailsViewProps) { const { t } = useI18n(); + const { errors, nodeIds } = useDiagramEditorContext(); const task = node.data.task; + const nodeErrors = getNodeErrors(errors, node.id, nodeIds); + const errorItems = nodeErrors.map((error) => { + const field = getNodeErrorField(error, node.id); + return field !== undefined ? { message: error.message, field } : { message: error.message }; + }); const fields = task ? getTaskDetails(task) : []; - if (fields.length === 0) { + if (nodeErrors.length === 0 && fields.length === 0) { return

{t("sidebar.noDetails")}

; } /* TODO FUTURE: Once we have a synced text -> diagram view, re-look at the source JSON block, it becomes redundant with dual view but if user wants standalone diagram without text then it is still valid so look at conditionally displaying it */ return (
- -
- {fields.map((field) => ( - - ))} -
+ + {fields.length > 0 && ( + <> + +
+ {fields.map((field) => ( + + ))} +
+ + )} {task !== undefined && ( <>
diff --git a/packages/serverless-workflow-diagram-editor/src/side-panel/SidePanel.css b/packages/serverless-workflow-diagram-editor/src/side-panel/SidePanel.css index d57621c8..837d8d23 100644 --- a/packages/serverless-workflow-diagram-editor/src/side-panel/SidePanel.css +++ b/packages/serverless-workflow-diagram-editor/src/side-panel/SidePanel.css @@ -270,4 +270,123 @@ .dec-root.dark .dec-sidebar-yaml-pre { @apply dec:bg-gray-800 dec:text-gray-200; } + + /* Validation errors */ + .dec-root .dec-sidebar-errors { + @apply dec:mb-4 dec:rounded-md dec:border dec:border-red-200 dec:bg-red-50; + } + + .dec-root.dark .dec-sidebar-errors { + @apply dec:border-red-900 dec:bg-red-950/40; + } + + .dec-root .dec-sidebar-errors-header { + @apply dec:flex dec:items-center dec:gap-2 dec:px-3 dec:py-2 + dec:border-b dec:border-red-200; + } + + .dec-root.dark .dec-sidebar-errors-header { + @apply dec:border-red-900; + } + + .dec-root .dec-sidebar-errors-icon { + @apply dec:h-4 dec:w-4 dec:shrink-0 dec:text-red-600; + } + + .dec-root.dark .dec-sidebar-errors-icon { + @apply dec:text-red-400; + } + + .dec-root .dec-sidebar-errors-title { + @apply dec:text-[11px] dec:font-semibold dec:uppercase dec:tracking-wider dec:text-red-700; + } + + .dec-root.dark .dec-sidebar-errors-title { + @apply dec:text-red-300; + } + + .dec-root .dec-sidebar-errors-count { + @apply dec:ml-auto dec:inline-flex dec:items-center dec:justify-center + dec:min-w-5 dec:rounded-full dec:px-1.5 dec:py-0.5 + dec:text-[10px] dec:font-semibold dec:bg-red-100 dec:text-red-700; + } + + .dec-root.dark .dec-sidebar-errors-count { + @apply dec:bg-red-900/60 dec:text-red-200; + } + + .dec-root .dec-sidebar-errors-list { + @apply dec:flex dec:flex-col; + } + + .dec-root .dec-sidebar-error-item { + @apply dec:px-3 dec:py-2 dec:border-b dec:border-red-100 dec:last:border-b-0; + } + + .dec-root.dark .dec-sidebar-error-item { + @apply dec:border-red-900/50; + } + + .dec-root .dec-sidebar-error-message { + @apply dec:text-sm dec:text-gray-700; + overflow-wrap: anywhere; + } + + .dec-root.dark .dec-sidebar-error-message { + @apply dec:text-gray-300; + } + + .dec-root .dec-sidebar-error-field { + @apply dec:text-sm dec:font-semibold dec:text-red-800; + font-family: ui-monospace, SFMono-Regular, Menlo, monospace; + } + + .dec-root .dec-sidebar-error-field::after { + @apply dec:mr-1 dec:font-normal dec:text-gray-500; + content: ":"; + } + + .dec-root.dark .dec-sidebar-error-field { + @apply dec:text-red-200; + } + + .dec-root.dark .dec-sidebar-error-field::after { + @apply dec:text-gray-400; + } + + .dec-root .dec-sidebar-trigger { + @apply dec:relative dec:inline-flex; + } + .dec-root .dec-sidebar-trigger-badge { + @apply dec:absolute dec:z-10 dec:flex dec:items-center dec:justify-center + dec:min-w-[16px] dec:h-4 dec:px-1 dec:rounded-full + dec:text-[10px] dec:font-semibold dec:leading-none dec:tabular-nums + dec:text-white dec:cursor-pointer dec:transition-colors; + background-color: var(--dec-error-accent); + top: -5px; + right: -5px; + box-shadow: + 0 0 0 2px #e5e4e2, + 0 1px 2px rgba(0, 0, 0, 0.3); + } + + .dec-root .dec-sidebar-trigger-badge:hover { + @apply dec:bg-red-600; + } + + .dec-root .dec-sidebar-trigger-badge:focus-visible { + outline: 2px solid var(--dec-error-accent); + outline-offset: 2px; + } + + .dec-root.dark .dec-sidebar-trigger-badge { + box-shadow: + 0 0 0 2px #141414, + 0 1px 2px rgba(0, 0, 0, 0.5); + } + + .dec-root.dark .dec-sidebar-trigger-badge:hover { + @apply dec:bg-red-300; + } + /* end validation errors */ } diff --git a/packages/serverless-workflow-diagram-editor/src/side-panel/SidePanelTrigger.tsx b/packages/serverless-workflow-diagram-editor/src/side-panel/SidePanelTrigger.tsx new file mode 100644 index 00000000..66db1b1a --- /dev/null +++ b/packages/serverless-workflow-diagram-editor/src/side-panel/SidePanelTrigger.tsx @@ -0,0 +1,47 @@ +/* + * Copyright 2021-Present The Serverless Workflow Specification Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { useDiagramEditorContext } from "@/store/DiagramEditorContext"; +import { SidebarTrigger, useSidebar } from "@/components/ui/sidebar"; +import { getGeneralErrors } from "@/core"; + +export function SidePanelTrigger() { + const { errors, nodeIds, setSelectedNodeId } = useDiagramEditorContext(); + const { setOpen } = useSidebar(); + + const count = getGeneralErrors(errors, nodeIds).length; + + const showWorkflowErrors = () => { + setSelectedNodeId(null); + setOpen(true); + }; + + return ( +
+ + {count > 0 && ( + + )} +
+ ); +} diff --git a/packages/serverless-workflow-diagram-editor/src/side-panel/WorkflowInfoView.tsx b/packages/serverless-workflow-diagram-editor/src/side-panel/WorkflowInfoView.tsx index 3e7c6222..f609a0a1 100644 --- a/packages/serverless-workflow-diagram-editor/src/side-panel/WorkflowInfoView.tsx +++ b/packages/serverless-workflow-diagram-editor/src/side-panel/WorkflowInfoView.tsx @@ -17,6 +17,9 @@ import type { Specification } from "@serverlessworkflow/sdk"; import { useI18n } from "@serverlessworkflow/i18n"; import { InlineField, SectionHeader, StackedField } from "./Fields"; +import { useDiagramEditorContext } from "@/store/DiagramEditorContext"; +import { getGeneralErrors } from "@/core"; +import { ErrorSection } from "./ErrorsSection"; type WorkflowInfoViewProps = { document: Specification.Document; @@ -24,6 +27,11 @@ type WorkflowInfoViewProps = { export function WorkflowInfoView({ document }: WorkflowInfoViewProps) { const { t } = useI18n(); + const { errors, nodeIds } = useDiagramEditorContext(); + + const generalErrors = getGeneralErrors(errors, nodeIds).map((error) => ({ + message: error.message, + })); const hasMetadata = document.title !== undefined || document.summary !== undefined || document.tags !== undefined; @@ -31,37 +39,40 @@ export function WorkflowInfoView({ document }: WorkflowInfoViewProps) { const tagEntries = tags ? Object.entries(tags) : []; return ( -
- - - - - + <> + +
+ + + + + - {hasMetadata && ( - <> -
- - {document.title !== undefined && ( - - )} - {document.summary !== undefined && ( - - )} - {tagEntries.length > 0 && ( -
-
{t("sidebar.tags")}
-
- {tagEntries.map(([key, value]) => ( - - {key}: {value} - - ))} -
-
- )} - - )} -
+ {hasMetadata && ( + <> +
+ + {document.title !== undefined && ( + + )} + {document.summary !== undefined && ( + + )} + {tagEntries.length > 0 && ( +
+
{t("sidebar.tags")}
+
+ {tagEntries.map(([key, value]) => ( + + {key}: {value} + + ))} +
+
+ )} + + )} +
+ ); } diff --git a/packages/serverless-workflow-diagram-editor/src/store/DiagramEditorContext.tsx b/packages/serverless-workflow-diagram-editor/src/store/DiagramEditorContext.tsx index 00f92ed9..d8b8de5c 100644 --- a/packages/serverless-workflow-diagram-editor/src/store/DiagramEditorContext.tsx +++ b/packages/serverless-workflow-diagram-editor/src/store/DiagramEditorContext.tsx @@ -26,6 +26,7 @@ export type DiagramEditorContextType = { errors: SdkError[]; nodes: RF.Node[]; edges: RF.Edge[]; + nodeIds: Set; selectedNodeId: string | null; setIsReadOnly: React.Dispatch>; diff --git a/packages/serverless-workflow-diagram-editor/src/store/DiagramEditorContextProvider.tsx b/packages/serverless-workflow-diagram-editor/src/store/DiagramEditorContextProvider.tsx index fde69614..68968278 100644 --- a/packages/serverless-workflow-diagram-editor/src/store/DiagramEditorContextProvider.tsx +++ b/packages/serverless-workflow-diagram-editor/src/store/DiagramEditorContextProvider.tsx @@ -15,7 +15,7 @@ */ import * as React from "react"; -import { parseWorkflow } from "../core"; +import { buildFlatGraph, parseWorkflow } from "../core"; import { DiagramEditorProps } from "../diagram-editor/DiagramEditor"; import { DiagramEditorContext, DiagramEditorContextType } from "./DiagramEditorContext"; import type * as RF from "@xyflow/react"; @@ -34,6 +34,11 @@ export const DiagramEditorContextProvider = ( const { model, errors } = React.useMemo(() => parseWorkflow(props.content), [props.content]); + const nodeIds = React.useMemo( + () => (model ? new Set(buildFlatGraph(model).nodes.map((node) => node.id)) : new Set()), + [model], + ); + // Update states on props changes React.useEffect(() => { setIsReadOnly(props.isReadOnly); @@ -54,6 +59,7 @@ export const DiagramEditorContextProvider = ( errors, nodes, edges, + nodeIds, selectedNodeId, setIsReadOnly, setLocale, @@ -68,6 +74,7 @@ export const DiagramEditorContextProvider = ( errors, nodes, edges, + nodeIds, selectedNodeId, setIsReadOnly, setLocale, diff --git a/packages/serverless-workflow-diagram-editor/src/styles.css b/packages/serverless-workflow-diagram-editor/src/styles.css index b0feed9a..3980adc4 100644 --- a/packages/serverless-workflow-diagram-editor/src/styles.css +++ b/packages/serverless-workflow-diagram-editor/src/styles.css @@ -44,6 +44,14 @@ @layer base { .dec-root { @apply dec:h-full; + + --dec-error-accent: #ef4444; + --dec-error-glow: rgba(239, 68, 68, 0.35); + } + + .dec-root.dark{ + --dec-error-accent: #f87171; + --dec-error-glow: rgba(248, 113, 113, 0.4); } .dec-root .dec-diagram-content { diff --git a/packages/serverless-workflow-diagram-editor/stories/features/ValidationErrors.stories.tsx b/packages/serverless-workflow-diagram-editor/stories/features/ValidationErrors.stories.tsx new file mode 100644 index 00000000..00b0ebec --- /dev/null +++ b/packages/serverless-workflow-diagram-editor/stories/features/ValidationErrors.stories.tsx @@ -0,0 +1,116 @@ +/* + * Copyright 2021-Present The Serverless Workflow Specification Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import type { Meta, StoryObj } from "@storybook/react-vite"; +import { DiagramEditor } from "./DiagramEditor"; + +const meta = { + title: "Features/Validation Errors", + component: DiagramEditor, + parameters: { + layout: "fullscreen", + }, + render: (args, { globals }) => { + return ; + }, +} satisfies Meta; + +export default meta; +type Story = StoryObj; + +const DEFAULT_STORY_ARGS = { + isReadOnly: true, + locale: "en" as const, +} as const; + +const createWorkflowStory = (content: string): Story => ({ + args: { ...DEFAULT_STORY_ARGS, content }, +}); + +export const DocumentError: Story = createWorkflowStory( + ` + document: + dsl: 9.9.8 + name: unsupported-dsl-version + version: 1.0.0 + namespace: default + do: + - greet: + set: + message: hello +`, +); + +export const ContainerError: Story = createWorkflowStory( + ` + document: + dsl: 1.0.3 + name: invalid-container + version: 1.0.0 + namespace: default + do: + - processItems: + for: + each: item + do: + - greet: + set: + message: hello +`, +); + +export const NodeError: Story = createWorkflowStory( + ` + document: + dsl: 1.0.3 + name: invalid-node + version: 1.0.0 + namespace: default + do: + - validationOrder: + set: + valid: true + - chargePayment: + call: http + with: + method: post + - sendReceipt: + emit: + with: + source: + type: com.shop.receipt.sent +`, +); + +export const NestedNodeError: Story = createWorkflowStory( + ` + document: + dsl: 1.0.3 + name: invalid-nested-node + version: 1.0.0 + namespace: default + do: + - processItems: + for: + each: item + in: \${ .items} + do: + - chargePayment: + call: http + with: + method: post +`, +); diff --git a/packages/serverless-workflow-diagram-editor/tests/core/__snapshots__/workflowSdk.integration.test.ts.snap b/packages/serverless-workflow-diagram-editor/tests/core/__snapshots__/workflowSdk.integration.test.ts.snap index 7ea1d0c7..933b0c96 100644 --- a/packages/serverless-workflow-diagram-editor/tests/core/__snapshots__/workflowSdk.integration.test.ts.snap +++ b/packages/serverless-workflow-diagram-editor/tests/core/__snapshots__/workflowSdk.integration.test.ts.snap @@ -363,7 +363,7 @@ exports[`parseValidationErrorMessage > parses validation errors from workflow wi "object": { "missingProperty": "call", }, - "taskId": "/do/0/checkup", + "path": "/do/0/checkup", }, { "errorType": "#/oneOf/1/required", @@ -371,7 +371,7 @@ exports[`parseValidationErrorMessage > parses validation errors from workflow wi "object": { "missingProperty": "call", }, - "taskId": "/do/0/checkup", + "path": "/do/0/checkup", }, { "errorType": "#/oneOf/2/required", @@ -379,7 +379,7 @@ exports[`parseValidationErrorMessage > parses validation errors from workflow wi "object": { "missingProperty": "call", }, - "taskId": "/do/0/checkup", + "path": "/do/0/checkup", }, { "errorType": "#/oneOf/3/required", @@ -387,7 +387,7 @@ exports[`parseValidationErrorMessage > parses validation errors from workflow wi "object": { "missingProperty": "call", }, - "taskId": "/do/0/checkup", + "path": "/do/0/checkup", }, { "errorType": "#/oneOf/4/required", @@ -395,7 +395,7 @@ exports[`parseValidationErrorMessage > parses validation errors from workflow wi "object": { "missingProperty": "call", }, - "taskId": "/do/0/checkup", + "path": "/do/0/checkup", }, { "errorType": "#/oneOf/5/required", @@ -403,7 +403,7 @@ exports[`parseValidationErrorMessage > parses validation errors from workflow wi "object": { "missingProperty": "call", }, - "taskId": "/do/0/checkup", + "path": "/do/0/checkup", }, { "errorType": "#/oneOf/6/required", @@ -411,7 +411,7 @@ exports[`parseValidationErrorMessage > parses validation errors from workflow wi "object": { "missingProperty": "call", }, - "taskId": "/do/0/checkup", + "path": "/do/0/checkup", }, { "errorType": "#/oneOf", @@ -419,7 +419,7 @@ exports[`parseValidationErrorMessage > parses validation errors from workflow wi "object": { "passingSchemas": null, }, - "taskId": "/do/0/checkup", + "path": "/do/0/checkup", }, { "errorType": "#/oneOf/0/required", @@ -427,7 +427,7 @@ exports[`parseValidationErrorMessage > parses validation errors from workflow wi "object": { "missingProperty": "call", }, - "taskId": "/do/0/checkup/do/1/checkup", + "path": "/do/0/checkup/do/1/checkup", }, { "errorType": "#/oneOf/1/required", @@ -435,7 +435,7 @@ exports[`parseValidationErrorMessage > parses validation errors from workflow wi "object": { "missingProperty": "call", }, - "taskId": "/do/0/checkup/do/1/checkup", + "path": "/do/0/checkup/do/1/checkup", }, { "errorType": "#/oneOf/2/required", @@ -443,7 +443,7 @@ exports[`parseValidationErrorMessage > parses validation errors from workflow wi "object": { "missingProperty": "call", }, - "taskId": "/do/0/checkup/do/1/checkup", + "path": "/do/0/checkup/do/1/checkup", }, { "errorType": "#/oneOf/3/required", @@ -451,7 +451,7 @@ exports[`parseValidationErrorMessage > parses validation errors from workflow wi "object": { "missingProperty": "call", }, - "taskId": "/do/0/checkup/do/1/checkup", + "path": "/do/0/checkup/do/1/checkup", }, { "errorType": "#/oneOf/4/required", @@ -459,7 +459,7 @@ exports[`parseValidationErrorMessage > parses validation errors from workflow wi "object": { "missingProperty": "call", }, - "taskId": "/do/0/checkup/do/1/checkup", + "path": "/do/0/checkup/do/1/checkup", }, { "errorType": "#/oneOf/5/required", @@ -467,7 +467,7 @@ exports[`parseValidationErrorMessage > parses validation errors from workflow wi "object": { "missingProperty": "call", }, - "taskId": "/do/0/checkup/do/1/checkup", + "path": "/do/0/checkup/do/1/checkup", }, { "errorType": "#/oneOf/6/required", @@ -475,7 +475,7 @@ exports[`parseValidationErrorMessage > parses validation errors from workflow wi "object": { "missingProperty": "call", }, - "taskId": "/do/0/checkup/do/1/checkup", + "path": "/do/0/checkup/do/1/checkup", }, { "errorType": "#/oneOf", @@ -483,7 +483,7 @@ exports[`parseValidationErrorMessage > parses validation errors from workflow wi "object": { "passingSchemas": null, }, - "taskId": "/do/0/checkup/do/1/checkup", + "path": "/do/0/checkup/do/1/checkup", }, { "errorType": "#/type", @@ -491,7 +491,7 @@ exports[`parseValidationErrorMessage > parses validation errors from workflow wi "object": { "type": "array", }, - "taskId": "/do/0/checkup/do/1/checkup/do", + "path": "/do/0/checkup/do/1/checkup/do", }, { "errorType": "#/required", @@ -499,7 +499,7 @@ exports[`parseValidationErrorMessage > parses validation errors from workflow wi "object": { "missingProperty": "fork", }, - "taskId": "/do/0/checkup/do/1/checkup", + "path": "/do/0/checkup/do/1/checkup", }, { "errorType": "#/required", @@ -507,15 +507,7 @@ exports[`parseValidationErrorMessage > parses validation errors from workflow wi "object": { "missingProperty": "emit", }, - "taskId": "/do/0/checkup/do/1/checkup", - }, - { - "errorType": "#/type", - "message": "must be array", - "object": { - "type": "array", - }, - "taskId": "/do/0/checkup/do/1/checkup/do", + "path": "/do/0/checkup/do/1/checkup", }, { "errorType": "#/required", @@ -523,7 +515,7 @@ exports[`parseValidationErrorMessage > parses validation errors from workflow wi "object": { "missingProperty": "listen", }, - "taskId": "/do/0/checkup/do/1/checkup", + "path": "/do/0/checkup/do/1/checkup", }, { "errorType": "#/required", @@ -531,7 +523,7 @@ exports[`parseValidationErrorMessage > parses validation errors from workflow wi "object": { "missingProperty": "raise", }, - "taskId": "/do/0/checkup/do/1/checkup", + "path": "/do/0/checkup/do/1/checkup", }, { "errorType": "#/required", @@ -539,7 +531,7 @@ exports[`parseValidationErrorMessage > parses validation errors from workflow wi "object": { "missingProperty": "run", }, - "taskId": "/do/0/checkup/do/1/checkup", + "path": "/do/0/checkup/do/1/checkup", }, { "errorType": "#/required", @@ -547,7 +539,7 @@ exports[`parseValidationErrorMessage > parses validation errors from workflow wi "object": { "missingProperty": "set", }, - "taskId": "/do/0/checkup/do/1/checkup", + "path": "/do/0/checkup/do/1/checkup", }, { "errorType": "#/required", @@ -555,7 +547,7 @@ exports[`parseValidationErrorMessage > parses validation errors from workflow wi "object": { "missingProperty": "switch", }, - "taskId": "/do/0/checkup/do/1/checkup", + "path": "/do/0/checkup/do/1/checkup", }, { "errorType": "#/required", @@ -563,7 +555,7 @@ exports[`parseValidationErrorMessage > parses validation errors from workflow wi "object": { "missingProperty": "try", }, - "taskId": "/do/0/checkup/do/1/checkup", + "path": "/do/0/checkup/do/1/checkup", }, { "errorType": "#/required", @@ -571,15 +563,7 @@ exports[`parseValidationErrorMessage > parses validation errors from workflow wi "object": { "missingProperty": "wait", }, - "taskId": "/do/0/checkup/do/1/checkup", - }, - { - "errorType": "#/oneOf", - "message": "must match exactly one schema in oneOf", - "object": { - "passingSchemas": null, - }, - "taskId": "/do/0/checkup/do/1/checkup", + "path": "/do/0/checkup/do/1/checkup", }, { "errorType": "#/required", @@ -587,7 +571,7 @@ exports[`parseValidationErrorMessage > parses validation errors from workflow wi "object": { "missingProperty": "fork", }, - "taskId": "/do/0/checkup", + "path": "/do/0/checkup", }, { "errorType": "#/required", @@ -595,167 +579,7 @@ exports[`parseValidationErrorMessage > parses validation errors from workflow wi "object": { "missingProperty": "emit", }, - "taskId": "/do/0/checkup", - }, - { - "errorType": "#/oneOf/0/required", - "message": "must have required property 'call'", - "object": { - "missingProperty": "call", - }, - "taskId": "/do/0/checkup/do/1/checkup", - }, - { - "errorType": "#/oneOf/1/required", - "message": "must have required property 'call'", - "object": { - "missingProperty": "call", - }, - "taskId": "/do/0/checkup/do/1/checkup", - }, - { - "errorType": "#/oneOf/2/required", - "message": "must have required property 'call'", - "object": { - "missingProperty": "call", - }, - "taskId": "/do/0/checkup/do/1/checkup", - }, - { - "errorType": "#/oneOf/3/required", - "message": "must have required property 'call'", - "object": { - "missingProperty": "call", - }, - "taskId": "/do/0/checkup/do/1/checkup", - }, - { - "errorType": "#/oneOf/4/required", - "message": "must have required property 'call'", - "object": { - "missingProperty": "call", - }, - "taskId": "/do/0/checkup/do/1/checkup", - }, - { - "errorType": "#/oneOf/5/required", - "message": "must have required property 'call'", - "object": { - "missingProperty": "call", - }, - "taskId": "/do/0/checkup/do/1/checkup", - }, - { - "errorType": "#/oneOf/6/required", - "message": "must have required property 'call'", - "object": { - "missingProperty": "call", - }, - "taskId": "/do/0/checkup/do/1/checkup", - }, - { - "errorType": "#/oneOf", - "message": "must match exactly one schema in oneOf", - "object": { - "passingSchemas": null, - }, - "taskId": "/do/0/checkup/do/1/checkup", - }, - { - "errorType": "#/type", - "message": "must be array", - "object": { - "type": "array", - }, - "taskId": "/do/0/checkup/do/1/checkup/do", - }, - { - "errorType": "#/required", - "message": "must have required property 'fork'", - "object": { - "missingProperty": "fork", - }, - "taskId": "/do/0/checkup/do/1/checkup", - }, - { - "errorType": "#/required", - "message": "must have required property 'emit'", - "object": { - "missingProperty": "emit", - }, - "taskId": "/do/0/checkup/do/1/checkup", - }, - { - "errorType": "#/type", - "message": "must be array", - "object": { - "type": "array", - }, - "taskId": "/do/0/checkup/do/1/checkup/do", - }, - { - "errorType": "#/required", - "message": "must have required property 'listen'", - "object": { - "missingProperty": "listen", - }, - "taskId": "/do/0/checkup/do/1/checkup", - }, - { - "errorType": "#/required", - "message": "must have required property 'raise'", - "object": { - "missingProperty": "raise", - }, - "taskId": "/do/0/checkup/do/1/checkup", - }, - { - "errorType": "#/required", - "message": "must have required property 'run'", - "object": { - "missingProperty": "run", - }, - "taskId": "/do/0/checkup/do/1/checkup", - }, - { - "errorType": "#/required", - "message": "must have required property 'set'", - "object": { - "missingProperty": "set", - }, - "taskId": "/do/0/checkup/do/1/checkup", - }, - { - "errorType": "#/required", - "message": "must have required property 'switch'", - "object": { - "missingProperty": "switch", - }, - "taskId": "/do/0/checkup/do/1/checkup", - }, - { - "errorType": "#/required", - "message": "must have required property 'try'", - "object": { - "missingProperty": "try", - }, - "taskId": "/do/0/checkup/do/1/checkup", - }, - { - "errorType": "#/required", - "message": "must have required property 'wait'", - "object": { - "missingProperty": "wait", - }, - "taskId": "/do/0/checkup/do/1/checkup", - }, - { - "errorType": "#/oneOf", - "message": "must match exactly one schema in oneOf", - "object": { - "passingSchemas": null, - }, - "taskId": "/do/0/checkup/do/1/checkup", + "path": "/do/0/checkup", }, { "errorType": "#/required", @@ -763,7 +587,7 @@ exports[`parseValidationErrorMessage > parses validation errors from workflow wi "object": { "missingProperty": "listen", }, - "taskId": "/do/0/checkup", + "path": "/do/0/checkup", }, { "errorType": "#/required", @@ -771,7 +595,7 @@ exports[`parseValidationErrorMessage > parses validation errors from workflow wi "object": { "missingProperty": "raise", }, - "taskId": "/do/0/checkup", + "path": "/do/0/checkup", }, { "errorType": "#/required", @@ -779,7 +603,7 @@ exports[`parseValidationErrorMessage > parses validation errors from workflow wi "object": { "missingProperty": "run", }, - "taskId": "/do/0/checkup", + "path": "/do/0/checkup", }, { "errorType": "#/required", @@ -787,7 +611,7 @@ exports[`parseValidationErrorMessage > parses validation errors from workflow wi "object": { "missingProperty": "set", }, - "taskId": "/do/0/checkup", + "path": "/do/0/checkup", }, { "errorType": "#/required", @@ -795,7 +619,7 @@ exports[`parseValidationErrorMessage > parses validation errors from workflow wi "object": { "missingProperty": "switch", }, - "taskId": "/do/0/checkup", + "path": "/do/0/checkup", }, { "errorType": "#/required", @@ -803,7 +627,7 @@ exports[`parseValidationErrorMessage > parses validation errors from workflow wi "object": { "missingProperty": "try", }, - "taskId": "/do/0/checkup", + "path": "/do/0/checkup", }, { "errorType": "#/required", @@ -811,15 +635,7 @@ exports[`parseValidationErrorMessage > parses validation errors from workflow wi "object": { "missingProperty": "wait", }, - "taskId": "/do/0/checkup", - }, - { - "errorType": "#/oneOf", - "message": "must match exactly one schema in oneOf", - "object": { - "passingSchemas": null, - }, - "taskId": "/do/0/checkup", + "path": "/do/0/checkup", }, ] `; diff --git a/packages/serverless-workflow-diagram-editor/tests/core/validationErrors.test.ts b/packages/serverless-workflow-diagram-editor/tests/core/validationErrors.test.ts new file mode 100644 index 00000000..22fd69bc --- /dev/null +++ b/packages/serverless-workflow-diagram-editor/tests/core/validationErrors.test.ts @@ -0,0 +1,213 @@ +/* + * Copyright 2021-Present The Serverless Workflow Specification Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { describe, it, expect } from "vitest"; +import { + isValidationError, + getNodeErrors, + getNodeErrorField, + getErrorNodeIds, + getGeneralErrors, +} from "../../src/core"; +import type { SdkError, ValidationError } from "../../src/core"; + +const vErr = (partial: Partial & { message?: string }): ValidationError => ({ + message: "boom", + ...partial, +}); + +describe("isValidationError", () => { + it.each([ + { + description: "a raw Error instance", + error: new Error("syntax error"), + expected: false, + }, + { + description: "a plain ValidationError object", + error: vErr({ path: "/do/0/call" }), + expected: true, + }, + { + description: "a ValidationError with no path", + error: vErr({ errorType: "#/required" }), + expected: true, + }, + ])("returns $expected for $description", ({ error, expected }) => { + expect(isValidationError(error as SdkError)).toBe(expected); + }); +}); + +describe("getNodeErrors", () => { + const nodeIds = new Set(["/do/0/call", "/do/1/set"]); + + it("returns only errors owned by the given node", () => { + const errors: SdkError[] = [ + vErr({ path: "/do/0/call", message: "call problem" }), + vErr({ path: "/do/1/set", message: "set problem" }), + ]; + + const result = getNodeErrors(errors, "/do/0/call", nodeIds); + expect(result).toHaveLength(1); + expect(result[0]!.message).toBe("call problem"); + }); + + it("attributes field errors to their owning node", () => { + const errors: SdkError[] = [vErr({ path: "/do/0/call/with", message: "missing endpoint" })]; + + const result = getNodeErrors(errors, "/do/0/call", nodeIds); + expect(result).toHaveLength(1); + expect(result[0]!.path).toBe("/do/0/call/with"); + }); + + it.each([ + { + description: "oneOf umbrella keyword", + error: vErr({ path: "/do/0/call", errorType: "#/oneOf" }), + }, + { + description: "missing property naming another task type", + error: vErr({ path: "/do/0/call", object: { missingProperty: "do" } }), + }, + { + description: "call subtype discriminator (const)", + error: vErr({ + path: "/do/0/call", + errorType: "#/oneOf/0/properties/call/const", + }), + }, + { + description: "call subtype discriminator (not)", + error: vErr({ + path: "/do/0/call", + errorType: "#/oneOf/0/properties/call/not", + }), + }, + ])("filters out noise error: $description", ({ error }) => { + expect(getNodeErrors([error], "/do/0/call", nodeIds)).toHaveLength(0); + }); + + it("keeps a missing 'catch' property error (genuine, not noise as it catch cannot be missing)", () => { + const errors: SdkError[] = [vErr({ path: "/do/0/call", object: { missingProperty: "catch" } })]; + + expect(getNodeErrors(errors, "/do/0/call", nodeIds)).toHaveLength(1); + }); + + it("excludes raw (non-validation) Errors", () => { + const errors: SdkError[] = [new Error("yaml broke")]; + + expect(getNodeErrors(errors, "/do/0/call", nodeIds)).toHaveLength(0); + }); + + it("attributes a nested-child error to the longest matching node id", () => { + const nested = new Set(["/do/0/try", "/do/0/try/do/0/call"]); + const errors: SdkError[] = [vErr({ path: "/do/0/try/do/0/call/with", message: "nested" })]; + + expect(getNodeErrors(errors, "/do/0/try/do/0/call", nested)).toHaveLength(1); + expect(getNodeErrors(errors, "/do/0/try", nested)).toHaveLength(0); + }); + + it("does not match a node id that is only a string prefix without a path boundary", () => { + const ids = new Set(["/do/0/try"]); + // "/do/0/tryThings" starts with "/do/0/try" as a substring but not as a path segment. + const errors: SdkError[] = [vErr({ path: "/do/0/tryThings", message: "unrelated" })]; + + expect(getNodeErrors(errors, "/do/0/try", ids)).toHaveLength(0); + }); +}); + +describe("getNodeErrorField", () => { + it.each([ + { path: "/do/0/call/with", nodeId: "/do/0/call", expected: "with" }, + { + path: "/do/0/call/with/endpoint", + nodeId: "/do/0/call", + expected: "with.endpoint", + }, + { path: "/do/0/call", nodeId: "/do/0/call", expected: undefined }, + { path: "/do/1/set", nodeId: "/do/0/call", expected: undefined }, + { path: undefined, nodeId: "/do/0/call", expected: undefined }, + ])("path=$path nodeId=$nodeId -> $expected", ({ path, nodeId, expected }) => { + expect(getNodeErrorField(vErr({ path }), nodeId)).toBe(expected); + }); +}); + +describe("getErrorNodeIds", () => { + const nodeIds = new Set(["/do/0/call", "/do/1/set"]); + + it("returns the set of node ids that own at least one error", () => { + const errors: SdkError[] = [ + vErr({ path: "/do/0/call/with", message: "missing endpoint" }), + vErr({ path: "/do/1/set", message: "set problem" }), + ]; + + expect(getErrorNodeIds(errors, nodeIds)).toEqual(new Set(["/do/0/call", "/do/1/set"])); + }); + + it("excludes nodes whose only errors are noise", () => { + const errors: SdkError[] = [vErr({ path: "/do/0/call", errorType: "#/oneOf" })]; + + expect(getErrorNodeIds(errors, nodeIds)).toEqual(new Set()); + }); + + it("ignores raw errors owned by no node", () => { + const errors: SdkError[] = [ + new Error("yaml broke"), + vErr({ path: "/document", message: "missing version" }), + ]; + + expect(getErrorNodeIds(errors, nodeIds)).toEqual(new Set()); + }); +}); + +describe("getGeneralErrors", () => { + const nodeIds = new Set(["/do/0/call", "/do/1/set"]); + + it("includes raw Errors", () => { + const err = new Error("yaml broke"); + expect(getGeneralErrors([err], nodeIds)).toEqual([err]); + }); + + it("includes validation errors with no path", () => { + const errors: SdkError[] = [vErr({ errorType: "#/required", message: "missing document" })]; + expect(getGeneralErrors(errors, nodeIds)).toEqual(errors); + }); + + it("includes validation errors whose path has no node owns (e.g. /document)", () => { + const errors: SdkError[] = [vErr({ path: "/document", message: "missing version" })]; + expect(getGeneralErrors(errors, nodeIds)).toEqual(errors); + }); + + it("excludes errors owned by a node", () => { + const errors: SdkError[] = [ + vErr({ path: "/do/0/call", message: "owned" }), + vErr({ path: "/do/0/call/with", message: "owned field" }), + ]; + expect(getGeneralErrors(errors, nodeIds)).toEqual([]); + }); + + it("mixed list into node vs general", () => { + const owned = vErr({ + path: "/do/0/call/with", + message: "missing endpoint", + }); + const documentErr = vErr({ path: "/document", message: "missing version" }); + const rawErr = new Error("yaml broke"); + const errors: SdkError[] = [owned, documentErr, rawErr]; + + expect(getGeneralErrors(errors, nodeIds)).toEqual([documentErr, rawErr]); + }); +}); diff --git a/packages/serverless-workflow-diagram-editor/tests/core/workflowSdk.integration.test.ts b/packages/serverless-workflow-diagram-editor/tests/core/workflowSdk.integration.test.ts index 53fcbdbc..191f6480 100644 --- a/packages/serverless-workflow-diagram-editor/tests/core/workflowSdk.integration.test.ts +++ b/packages/serverless-workflow-diagram-editor/tests/core/workflowSdk.integration.test.ts @@ -101,7 +101,7 @@ describe("parseValidationErrorMessage", () => { const message = '- /do/0/task | #/required | message with | pipes | {"key": "value"}'; const errors = parseValidationErrorMessage(message); expect(errors).toHaveLength(1); - expect(errors[0].taskId).toBe("/do/0/task"); + expect(errors[0].path).toBe("/do/0/task"); expect(errors[0].errorType).toBe("#/required"); expect(errors[0].message).toBe("message with | pipes"); expect(errors[0].object).toEqual({ key: "value" }); @@ -112,7 +112,7 @@ describe("parseValidationErrorMessage", () => { '- /do/0/task | #/required | must have property | {"message": "value | with | pipes"}'; const errors = parseValidationErrorMessage(message); expect(errors).toHaveLength(1); - expect(errors[0].taskId).toBe("/do/0/task"); + expect(errors[0].path).toBe("/do/0/task"); expect(errors[0].errorType).toBe("#/required"); expect(errors[0].message).toBe("must have property"); expect(errors[0].object).toEqual({ message: "value | with | pipes" }); @@ -181,6 +181,27 @@ describe("validateWorkflow", () => { expect(errors).toMatchSnapshot(); }); + it("does not return exact-duplicate validation errors", () => { + const invalid = new Classes.Workflow({ + document: { dsl: "1.0.3", name: "nested-invalid", version: "1.0.0", namespace: "default" }, + do: [ + { + processItems: { + for: { each: "item", in: "${ .items }" }, + do: [{ chargePayment: { call: "http", with: { method: "post" } } }], + }, + }, + ], + }) as Specification.Workflow; + + const errors = validateWorkflow(invalid); + const signatures = errors + .filter((e): e is Exclude => !(e instanceof Error)) + .map((e) => `${e.path} ${e.errorType} ${e.message}`); + + expect(signatures.length).toBe(new Set(signatures).size); + }); + it("returns original Error when validation throws unstructured error message", () => { // This test verifies the fallback branch in validateWorkflow (lines 228-232) // where an error is thrown that doesn't match either supported format. @@ -199,7 +220,7 @@ describe("validateWorkflow", () => { // When parsing yields no structured errors, the fallback should return the original error let result: ( | Error - | { taskId?: string; errorType?: string; message: string; object?: Record } + | { path?: string; errorType?: string; message: string; object?: Record } )[]; if (parsedFromError.length > 0) { result = parsedFromError; @@ -214,7 +235,7 @@ describe("validateWorkflow", () => { expect(result[0]).toBeInstanceOf(Error); // Verify it's a plain Error, not a ValidationError - expect(result[0]).not.toHaveProperty("taskId"); + expect(result[0]).not.toHaveProperty("path"); expect(result[0]).not.toHaveProperty("errorType"); expect(result[0]).not.toHaveProperty("object"); diff --git a/packages/serverless-workflow-diagram-editor/tests/diagram-editor/DiagramEditor.test.tsx b/packages/serverless-workflow-diagram-editor/tests/diagram-editor/DiagramEditor.test.tsx index d2c2a8b4..7908ada8 100644 --- a/packages/serverless-workflow-diagram-editor/tests/diagram-editor/DiagramEditor.test.tsx +++ b/packages/serverless-workflow-diagram-editor/tests/diagram-editor/DiagramEditor.test.tsx @@ -18,6 +18,11 @@ import { render, screen } from "@testing-library/react"; import { DiagramEditor } from "../../src/diagram-editor"; import { vi, expect, afterEach, describe, it } from "vitest"; import { BASIC_VALID_WORKFLOW_YAML } from "../fixtures/workflows"; +import { t } from "../test-utils"; + +/* When js-yaml throws a YAMLException, parseWorkflow + returns a null model and the editor must fall back to the parsing error page. */ +const UNPARSEABLE_CONTENT = "{ invalid"; describe("DiagramEditor Component", () => { afterEach(() => { @@ -37,6 +42,13 @@ describe("DiagramEditor Component", () => { expect(reactFlowContainer).toBeInTheDocument(); }); + it("Renders the parsing error page instead of the diagram when content is unparseable", () => { + render(); + + expect(screen.getByText(t("workflowError.parsing.title"))).toBeInTheDocument(); + expect(screen.queryByTestId("diagram-container")).not.toBeInTheDocument(); + }); + it.each([ { colorMode: "light" as const, expectedDark: false }, { colorMode: "dark" as const, expectedDark: true }, diff --git a/packages/serverless-workflow-diagram-editor/tests/diagram-editor/error-pages/ParsingErrorPage.test.tsx b/packages/serverless-workflow-diagram-editor/tests/diagram-editor/error-pages/ParsingErrorPage.test.tsx index 62763dc5..a9d21956 100644 --- a/packages/serverless-workflow-diagram-editor/tests/diagram-editor/error-pages/ParsingErrorPage.test.tsx +++ b/packages/serverless-workflow-diagram-editor/tests/diagram-editor/error-pages/ParsingErrorPage.test.tsx @@ -34,12 +34,12 @@ const createMockYAMLException = (reason?: string, snippet?: string): Error => { }; const createMockValidationError = ( - taskId: string, + path: string, errorType: string, message: string, ): ValidationError => { return { - taskId, + path, errorType, message, object: { missingProperty: "call" }, diff --git a/packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/diagramBuilder.test.ts b/packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/diagramBuilder.test.ts index b0d25e7a..2a0c08c7 100644 --- a/packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/diagramBuilder.test.ts +++ b/packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/diagramBuilder.test.ts @@ -29,7 +29,7 @@ import { TERMINAL_NODE_SIZE, getNodeSize, } from "../../../src/react-flow/diagram/autoLayout"; -import { parseWorkflow } from "../../../src/core"; +import { parseWorkflow, type SdkError } from "../../../src/core"; import { BASIC_VALID_WORKFLOW_JSON, BASIC_VALID_WORKFLOW_JSON_TASKS, @@ -605,6 +605,50 @@ describe("diagramBuilder", () => { }); }); + describe("validation error highlighting", () => { + const { model } = parseWorkflow(BASIC_VALID_WORKFLOW_JSON_TASKS); + const baseline = buildDiagramElements(model); + const targetId = baseline.nodes.find((node) => node.data.task !== undefined)!.id; + + it("does not set hasError on any node when no errors are passed", () => { + const diagram = buildDiagramElements(model); + diagram.nodes.forEach((node) => expect(node.data.hasError).toBeUndefined()); + }); + + it("sets hasError only on the node owning a error", () => { + const errors: SdkError[] = [{ path: targetId, message: "missing endpoint" }]; + const diagram = buildDiagramElements(model, errors); + + const target = diagram.nodes.find((node) => node.id === targetId)!; + expect(target.data.hasError).toBe(true); + + diagram.nodes + .filter((node) => node.id !== targetId) + .forEach((node) => expect(node.data.hasError).toBeUndefined()); + }); + + it("attributes a field error to its owning node", () => { + const errors: SdkError[] = [{ path: `${targetId}/with`, message: "missing endpoint" }]; + const diagram = buildDiagramElements(model, errors); + + expect(diagram.nodes.find((node) => node.id === targetId)!.data.hasError).toBe(true); + }); + + it("does not set hasError for noise errors", () => { + const errors: SdkError[] = [{ path: targetId, errorType: "#/oneOf", message: "noise" }]; + const diagram = buildDiagramElements(model, errors); + + expect(diagram.nodes.find((node) => node.id === targetId)!.data.hasError).toBeUndefined(); + }); + + it("does not set hasError for general unowned errors", () => { + const errors: SdkError[] = [{ path: "/document", message: "missing version" }]; + const diagram = buildDiagramElements(model, errors); + + diagram.nodes.forEach((node) => expect(node.data.hasError).toBeUndefined()); + }); + }); + describe("graph structure", () => { it("creates nodes from workflow", () => { const diagram: DiagramElements = buildDiagramFromWorkflow(BASIC_VALID_WORKFLOW_JSON_TASKS); diff --git a/packages/serverless-workflow-diagram-editor/tests/react-flow/nodes/Nodes.test.tsx b/packages/serverless-workflow-diagram-editor/tests/react-flow/nodes/Nodes.test.tsx index bed946b1..7298ec3e 100644 --- a/packages/serverless-workflow-diagram-editor/tests/react-flow/nodes/Nodes.test.tsx +++ b/packages/serverless-workflow-diagram-editor/tests/react-flow/nodes/Nodes.test.tsx @@ -147,7 +147,11 @@ describe("React Flow custom node types", () => { }); describe("should render container nodes with ContainerNodeContent", () => { - const containerNodes: { id: string; type: ContainerNodeType; testId: string }[] = [ + const containerNodes: { + id: string; + type: ContainerNodeType; + testId: string; + }[] = [ { id: "n2", type: GraphNodeType.Do, testId: "do" }, { id: "n5", type: GraphNodeType.For, testId: "for" }, { id: "n6", type: GraphNodeType.Fork, testId: "fork" }, @@ -182,8 +186,18 @@ describe("React Flow custom node types", () => { testId: string; handleType: "source" | "target"; }[] = [ - { id: "entry1", type: GraphNodeType.Entry, testId: "entry", handleType: "source" }, - { id: "exit1", type: GraphNodeType.Exit, testId: "exit", handleType: "target" }, + { + id: "entry1", + type: GraphNodeType.Entry, + testId: "entry", + handleType: "source", + }, + { + id: "exit1", + type: GraphNodeType.Exit, + testId: "exit", + handleType: "target", + }, ]; it.each(terminalNodes)( @@ -216,7 +230,9 @@ describe("React Flow custom node types", () => { it("should render a text badge for known subtypes", () => { const nodesWithBadges = [ testNode("n1", GraphNodeType.Call, 10, "CallNode", { call: "http" }), - testNode("n2", GraphNodeType.Listen, 100, "ListenNode", { listen: { to: { any: [] } } }), + testNode("n2", GraphNodeType.Listen, 100, "ListenNode", { + listen: { to: { any: [] } }, + }), ]; render(
@@ -235,7 +251,9 @@ describe("React Flow custom node types", () => { it("should render the raw value as a custom badge for an unknown subtype", () => { const nodesWithUnknownBadges = [ - testNode("n1", GraphNodeType.Call, 100, "CallNode", { call: "customCall" }), + testNode("n1", GraphNodeType.Call, 100, "CallNode", { + call: "customCall", + }), ]; render(
@@ -300,4 +318,62 @@ describe("React Flow custom node types", () => { expect(badge).not.toBeInTheDocument(); }); }); + + describe("error badge rendering", () => { + const withError = (node: RF.Node): RF.Node => ({ + ...node, + data: { ...node.data, hasError: true }, + }); + + it.each([ + { + kind: "leaf", + id: "n1", + type: GraphNodeType.Call, + testId: "call", + nodeClass: "dec-leaf-node", + }, + { + kind: "container", + id: "n2", + type: GraphNodeType.Do, + testId: "do", + nodeClass: "dec-container-node", + }, + ])( + "renders the error badge and has-error class on a $kind node when data.hasError is set", + ({ id, type, testId, nodeClass }) => { + const nodes = [withError(testNode(id, type, 0, "Node"))]; + render( +
+ +
, + ); + + const node = screen.getByTestId(`${testId}-node-${id}`); + expect(node).toHaveClass(nodeClass); + expect(node).toHaveClass("has-error"); + expect(screen.getByTestId(`${testId}-node-${id}-error`)).toBeInTheDocument(); + }, + ); + + it.each([ + { kind: "leaf", id: "n1", type: GraphNodeType.Call, testId: "call" }, + { kind: "container", id: "n2", type: GraphNodeType.Do, testId: "do" }, + ])( + "does not render the error badge or has-error class on a $kind node without hasError", + ({ id, type, testId }) => { + const nodes = [testNode(id, type, 0, "Node")]; + render( +
+ +
, + ); + + const node = screen.getByTestId(`${testId}-node-${id}`); + expect(node).not.toHaveClass("has-error"); + expect(screen.queryByTestId(`${testId}-node-${id}-error`)).not.toBeInTheDocument(); + }, + ); + }); }); diff --git a/packages/serverless-workflow-diagram-editor/tests/side-panel/ErrorsSection.test.tsx b/packages/serverless-workflow-diagram-editor/tests/side-panel/ErrorsSection.test.tsx new file mode 100644 index 00000000..a07b9348 --- /dev/null +++ b/packages/serverless-workflow-diagram-editor/tests/side-panel/ErrorsSection.test.tsx @@ -0,0 +1,63 @@ +/* + * Copyright 2021-Present The Serverless Workflow Specification Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { describe, it, expect } from "vitest"; +import { screen } from "@testing-library/react"; +import { ErrorSection, type ErrorItem } from "../../src/side-panel/ErrorsSection"; +import { renderWithProviders, t } from "../test-utils"; + +describe("ErrorSection", () => { + it("renders nothing when there are no items", () => { + renderWithProviders(); + + expect(screen.queryByTestId("sidebar-errors")).not.toBeInTheDocument(); + }); + + it("renders the section header and item count", () => { + const items: ErrorItem[] = [{ message: "first error" }, { message: "second error" }]; + renderWithProviders(); + + expect(screen.getByText(t("sidebar.sectionErrors"))).toBeInTheDocument(); + expect(screen.getByText("2")).toBeInTheDocument(); + }); + + it("renders one row per item, with messages", () => { + const items: ErrorItem[] = [{ message: "first error" }, { message: "second error" }]; + const { container } = renderWithProviders(); + + expect(container.querySelectorAll(".dec-sidebar-error-item")).toHaveLength(2); + expect(screen.getByText("first error")).toBeInTheDocument(); + expect(screen.getByText("second error")).toBeInTheDocument(); + }); + + it("renders the field label before the message when an item has a field", () => { + const items: ErrorItem[] = [ + { field: "with", message: "must have required property 'endpoint'" }, + ]; + const { container } = renderWithProviders(); + + const field = container.querySelector(".dec-sidebar-error-field"); + expect(field?.textContent).toBe("with"); + expect(screen.getByText("must have required property 'endpoint'")).toBeInTheDocument(); + }); + + it("does not render the field label when an item has no field", () => { + const items: ErrorItem[] = [{ message: "document-level error" }]; + const { container } = renderWithProviders(); + + expect(container.querySelector(".dec-sidebar-error-field")).toBeNull(); + }); +}); diff --git a/packages/serverless-workflow-diagram-editor/tests/side-panel/NodeDetailsView.test.tsx b/packages/serverless-workflow-diagram-editor/tests/side-panel/NodeDetailsView.test.tsx index 13aa17c1..c11bd15a 100644 --- a/packages/serverless-workflow-diagram-editor/tests/side-panel/NodeDetailsView.test.tsx +++ b/packages/serverless-workflow-diagram-editor/tests/side-panel/NodeDetailsView.test.tsx @@ -33,7 +33,11 @@ describe("NodeDetailsView", () => { const node = makeNode({ label: "getPets", // eslint-disable-next-line unicorn/no-thenable -- 'then' is a real SWF directive - task: { call: "http", with: { endpoint: "https://api.example.com" }, then: "continue" }, + task: { + call: "http", + with: { endpoint: "https://api.example.com" }, + then: "continue", + }, }); renderWithProviders(); @@ -73,7 +77,10 @@ describe("NodeDetailsView", () => { }); it("renders a collapsed Source section with full yaml task", () => { - const task = { call: "http", with: { endpoint: "https://api.example.com" } }; + const task = { + call: "http", + with: { endpoint: "https://api.example.com" }, + }; const node = makeNode({ label: "getPets", task }); const { container } = renderWithProviders(); @@ -95,4 +102,64 @@ describe("NodeDetailsView", () => { expect(screen.queryByText("Source")).not.toBeInTheDocument(); expect(screen.getByText("No additional details for this node")).toBeInTheDocument(); }); + + describe("validation errors", () => { + const nodeIds = new Set(["node-1"]); + + it("renders the node's errors above the Properties section, with field labels", () => { + const node = makeNode({ + label: "getPets", + task: { call: "http", with: {} }, + }); + + renderWithProviders(, { + nodeIds, + errors: [ + { + path: "node-1/with", + message: "must have required property 'endpoint'", + }, + ], + }); + + expect(screen.getByTestId("sidebar-errors")).toBeInTheDocument(); + // field label derived relative to the node id + const field = document.querySelector(".dec-sidebar-error-field"); + expect(field?.textContent).toBe("with"); + expect(screen.getByText("must have required property 'endpoint'")).toBeInTheDocument(); + // Properties still render alongside the errors + expect(screen.getByText("Properties")).toBeInTheDocument(); + }); + + it("renders node details (errors only) when a task-less node has errors", () => { + const node = makeNode({ label: "start" }, "start"); + + renderWithProviders(, { + nodeIds, + errors: [{ path: "node-1", message: "something is wrong" }], + }); + + expect(screen.getByTestId("node-details")).toBeInTheDocument(); + expect(screen.getByTestId("sidebar-errors")).toBeInTheDocument(); + expect(screen.getByText("something is wrong")).toBeInTheDocument(); + // No task -> no Properties, no Source, and not the empty hint + expect(screen.queryByText("Properties")).not.toBeInTheDocument(); + expect(screen.queryByText("No additional details for this node")).not.toBeInTheDocument(); + }); + + it("does not render the errors section when the node has no errors", () => { + const node = makeNode({ + label: "getPets", + task: { call: "http", with: { endpoint: "x" } }, + }); + + renderWithProviders(, { + nodeIds, + errors: [], + }); + + expect(screen.queryByTestId("sidebar-errors")).not.toBeInTheDocument(); + expect(screen.getByText("Properties")).toBeInTheDocument(); + }); + }); }); diff --git a/packages/serverless-workflow-diagram-editor/tests/side-panel/SidePanelTrigger.test.tsx b/packages/serverless-workflow-diagram-editor/tests/side-panel/SidePanelTrigger.test.tsx new file mode 100644 index 00000000..07a28a85 --- /dev/null +++ b/packages/serverless-workflow-diagram-editor/tests/side-panel/SidePanelTrigger.test.tsx @@ -0,0 +1,80 @@ +/* + * Copyright 2021-Present The Serverless Workflow Specification Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { describe, it, expect, vi } from "vitest"; +import { screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { SidePanelTrigger } from "../../src/side-panel/SidePanelTrigger"; +import { renderWithProviders } from "../test-utils/render-helpers"; +import type { SdkError } from "../../src/core"; + +const nodeIds = new Set(["/do/0/call", "/do/1/set"]); + +describe("SidePanelTrigger", () => { + it("does not render the badge when there are no general errors", () => { + renderWithProviders(, { errors: [], nodeIds }); + + expect(screen.queryByTestId("sidebar-errors-badge")).not.toBeInTheDocument(); + }); + + it("does not render the badge when all errors are owned by nodes", () => { + const errors: SdkError[] = [{ path: "/do/0/call", message: "owned" }]; + renderWithProviders(, { errors, nodeIds }); + + expect(screen.queryByTestId("sidebar-errors-badge")).not.toBeInTheDocument(); + }); + + it.each([ + { + description: "a single document-level validation error", + errors: [{ path: "/document", message: "missing version" }] as SdkError[], + expectedCount: "1", + }, + { + description: "raw and unowned validation errors combined", + errors: [ + new Error("yaml broke"), + { errorType: "#/required", message: "missing document" }, + { path: "/document", message: "missing version" }, + ] as SdkError[], + expectedCount: "3", + }, + ])( + "renders the badge with the general error count for $description", + ({ errors, expectedCount }) => { + renderWithProviders(, { errors, nodeIds }); + + expect(screen.getByTestId("sidebar-errors-badge")).toHaveTextContent(expectedCount); + }, + ); + + it("clears the selected node when the badge is clicked", async () => { + const user = userEvent.setup(); + const setSelectedNodeId = vi.fn(); + const errors: SdkError[] = [{ path: "/document", message: "missing version" }]; + + renderWithProviders(, { + errors, + nodeIds, + selectedNodeId: "/do/0/call", + setSelectedNodeId, + }); + + await user.click(screen.getByTestId("sidebar-errors-badge")); + + expect(setSelectedNodeId).toHaveBeenCalledWith(null); + }); +}); diff --git a/packages/serverless-workflow-diagram-editor/tests/side-panel/WorkflowInfoView.test.tsx b/packages/serverless-workflow-diagram-editor/tests/side-panel/WorkflowInfoView.test.tsx index 45e771ba..10815701 100644 --- a/packages/serverless-workflow-diagram-editor/tests/side-panel/WorkflowInfoView.test.tsx +++ b/packages/serverless-workflow-diagram-editor/tests/side-panel/WorkflowInfoView.test.tsx @@ -66,4 +66,44 @@ describe("WorkflowInfoView", () => { expect(screen.getByText("env: production")).toBeInTheDocument(); expect(screen.getByText("team: platform")).toBeInTheDocument(); }); + + describe("general errors", () => { + it("renders general errors above the document section", () => { + renderWithProviders(, { + nodeIds: new Set(["/do/0/call"]), + errors: [ + { + path: "/document", + message: "must have required property 'version'", + }, + new Error("Unsupported DSL version"), + ], + }); + + expect(screen.getByTestId("sidebar-errors")).toBeInTheDocument(); + expect(screen.getByText("must have required property 'version'")).toBeInTheDocument(); + expect(screen.getByText("Unsupported DSL version")).toBeInTheDocument(); + // document section still renders below the errors + expect(screen.getByTestId("workflow-info")).toBeInTheDocument(); + }); + + it("does not render node-owned errors in the workflow view", () => { + renderWithProviders(, { + nodeIds: new Set(["/do/0/call"]), + errors: [{ path: "/do/0/call", message: "node-owned error" }], + }); + + expect(screen.queryByTestId("sidebar-errors")).not.toBeInTheDocument(); + expect(screen.queryByText("node-owned error")).not.toBeInTheDocument(); + }); + + it("does not render the errors section when there are no general errors", () => { + renderWithProviders(, { + nodeIds: new Set(), + errors: [], + }); + + expect(screen.queryByTestId("sidebar-errors")).not.toBeInTheDocument(); + }); + }); }); diff --git a/packages/serverless-workflow-diagram-editor/tests/test-utils/render-helpers.tsx b/packages/serverless-workflow-diagram-editor/tests/test-utils/render-helpers.tsx index e0483995..3aba721c 100644 --- a/packages/serverless-workflow-diagram-editor/tests/test-utils/render-helpers.tsx +++ b/packages/serverless-workflow-diagram-editor/tests/test-utils/render-helpers.tsx @@ -38,6 +38,7 @@ export const createMockContextValue = ( errors: [], nodes: [], edges: [], + nodeIds: new Set(), selectedNodeId: null, setIsReadOnly: noop, setLocale: noop,