From b00041b0a8b95f0ab59ee147d309091f926bc3d7 Mon Sep 17 00:00:00 2001 From: lornakelly Date: Mon, 21 Sep 2026 13:54:45 +0100 Subject: [PATCH 1/4] Add changes for raise task type Signed-off-by: lornakelly --- .changeset/raiseTask-form.md | 5 + .../src/core/schemaToFormFields.ts | 39 ++-- .../src/core/taskDraft.ts | 13 +- .../src/side-panel/EditFormFooter.tsx | 12 +- .../src/side-panel/forms/FormField.tsx | 45 +++-- .../src/side-panel/forms/TaskForm.tsx | 6 +- .../nested-editing/NestedEditing.stories.tsx | 1 + .../stories/nested-editing/index.ts | 1 + .../workflows/raise-error-shapes.yaml | 49 +++++ .../tests/core/schemaToFormFields.test.ts | 120 +++++++++++++ .../tests/core/taskDraft.test.ts | 21 +++ .../tests/fixtures/workflows.ts | 38 ++++ .../EditFormFooter.emitTask.test.tsx | 8 +- .../EditFormFooter.raiseTask.test.tsx | 170 ++++++++++++++++++ .../side-panel/forms/FormFields.test.tsx | 61 +++++++ .../tests/test-utils/combobox-stub.tsx | 88 +++++++++ 16 files changed, 627 insertions(+), 50 deletions(-) create mode 100644 .changeset/raiseTask-form.md create mode 100644 packages/open-workflow-diagram-editor/stories/nested-editing/workflows/raise-error-shapes.yaml create mode 100644 packages/open-workflow-diagram-editor/tests/side-panel/EditFormFooter.raiseTask.test.tsx create mode 100644 packages/open-workflow-diagram-editor/tests/side-panel/forms/FormFields.test.tsx create mode 100644 packages/open-workflow-diagram-editor/tests/test-utils/combobox-stub.tsx diff --git a/.changeset/raiseTask-form.md b/.changeset/raiseTask-form.md new file mode 100644 index 00000000..a9efe9ec --- /dev/null +++ b/.changeset/raiseTask-form.md @@ -0,0 +1,5 @@ +--- +"@openworkflowspec/diagram-editor": minor +--- + +Add full field support to raiseTask form generation. diff --git a/packages/open-workflow-diagram-editor/src/core/schemaToFormFields.ts b/packages/open-workflow-diagram-editor/src/core/schemaToFormFields.ts index 8c089b1c..8f6c4129 100644 --- a/packages/open-workflow-diagram-editor/src/core/schemaToFormFields.ts +++ b/packages/open-workflow-diagram-editor/src/core/schemaToFormFields.ts @@ -249,12 +249,15 @@ function isFlowDirectiveSchema( /** Derive a human-readable label from a schema node and the property key. */ function deriveLabel(schema: Record, key: string): string { if (typeof schema.title === "string") { - // Strip any CamelCase prefix from composite titles like "ForTaskDo" → "Do" const words = schema.title .replace(/([A-Z])/g, " $1") .trim() .split(" "); - return words[words.length - 1] ?? key; + const lastWord = words[words.length - 1]; + // Use the last word only if it matches the property key (case-insensitive). + if (lastWord !== undefined && lastWord.toLowerCase() === key.toLowerCase()) { + return lastWord; + } } return key; } @@ -284,6 +287,15 @@ function formatVariantLabel(title: string): string { .trim(); } +const API_ENDPOINT_PLACEHOLDER = "https://example.com/api/{id}"; +const ADDRESS_PATH_SUFFIXES = ["endpoint", "uri", "source"] as const; + +/* Whether a path holds the address of a service in workflow calls - and should get the API-endpoint example */ +function isApiEndpointPath(path: string): boolean { + const lower = path.toLowerCase(); + return ADDRESS_PATH_SUFFIXES.some((suffix) => lower.endsWith(suffix)); +} + /** Only include the `description` key when it has a value (exactOptionalPropertyTypes). */ function withDesc(description: string | undefined): { description?: string } { return description !== undefined ? { description } : {}; @@ -639,6 +651,7 @@ function buildOneOfVariants( format: "json" | "yaml" = "yaml", ): OneOfVariant[] { const leafPath = parentPath || "__leaf__"; + const isApiEndpoint = isApiEndpointPath(parentPath); // First pass: resolve candidate refs and build raw variant list const resolvedList = candidates.flatMap((candidate, idx): ResolvedVariant[] => { @@ -752,17 +765,8 @@ function buildOneOfVariants( ]; } else { // plain string (or uriTemplate anyOf or runtimeExpression) - const isUriOrTemplate = - (typeof c.$ref === "string" && c.$ref.includes("uriTemplate")) || - resolved.title === "UriTemplate" || - parentPath.toLowerCase().endsWith("endpoint") || - parentPath.toLowerCase().endsWith("uri"); const isRe = isRuntimeExpressionSchema(c, resolved); - const placeholder = isUriOrTemplate - ? "https://example.com/api/{id}" - : isRe - ? "${...}" - : undefined; + const placeholder = isRe ? "${...}" : isApiEndpoint ? API_ENDPOINT_PLACEHOLDER : undefined; leafField = { kind: "string", @@ -839,8 +843,7 @@ function buildOneOfVariants( for (const item of resolvedList) { if (item.kind === "string") { const isUriContext = - parentPath.toLowerCase().endsWith("endpoint") || - parentPath.toLowerCase().endsWith("uri") || + isApiEndpoint || (typeof item.c.$ref === "string" && item.c.$ref.includes("uriTemplate")) || item.resolved.title === "UriTemplate"; @@ -863,8 +866,8 @@ function buildOneOfVariants( required: false, multiline: false, isRuntimeExpression: isRe, - ...(isUriContext - ? { placeholder: "https://example.com/api/{id}" } + ...(isApiEndpoint + ? { placeholder: API_ENDPOINT_PLACEHOLDER } : inheritedPlaceholder !== undefined ? { placeholder: inheritedPlaceholder } : {}), @@ -879,7 +882,9 @@ function buildOneOfVariants( const merged = mergedStringVariant.fields[0] as StringField; if (isUriContext) { mergedStringVariant.label = "URI"; - merged.placeholder = "https://example.com/api/{id}"; + } + if (isApiEndpoint) { + merged.placeholder = API_ENDPOINT_PLACEHOLDER; } else if (isRe && merged.placeholder === undefined && inheritedPlaceholder !== undefined) { merged.placeholder = inheritedPlaceholder; } diff --git a/packages/open-workflow-diagram-editor/src/core/taskDraft.ts b/packages/open-workflow-diagram-editor/src/core/taskDraft.ts index 48fbde8c..c49ab45b 100644 --- a/packages/open-workflow-diagram-editor/src/core/taskDraft.ts +++ b/packages/open-workflow-diagram-editor/src/core/taskDraft.ts @@ -78,15 +78,14 @@ export function applyDirtyValues( } } - // For sentinel-derived paths: delete from the model unless the same path (or - // a leaf under it) is independently dirty in dirtyPaths — which means the - // user actually edited the field after switching back to it. + // For sentinel-derived paths: delete from the model unless a dirty field supplied a value - the path itself, a leaf under it or an ancestor + // (Ancestor because RHF reports that when a whole shape has changed like raise.error does when an error name becomes an inline definition) for (const sentinelPath of sentinelPaths) { const prefix = sentinelPath + "."; - const independentlyDirty = - dirtyPaths.has(sentinelPath) || - [...dirtyPaths].some((p) => p === sentinelPath || p.startsWith(prefix)); - if (!independentlyDirty) { + const suppliedByEdit = [...dirtyPaths].some( + (p) => p === sentinelPath || p.startsWith(prefix) || sentinelPath.startsWith(p + "."), + ); + if (!suppliedByEdit) { deletePath(result, sentinelPath.split(".")); } } diff --git a/packages/open-workflow-diagram-editor/src/side-panel/EditFormFooter.tsx b/packages/open-workflow-diagram-editor/src/side-panel/EditFormFooter.tsx index a2860c30..6e3f94d1 100644 --- a/packages/open-workflow-diagram-editor/src/side-panel/EditFormFooter.tsx +++ b/packages/open-workflow-diagram-editor/src/side-panel/EditFormFooter.tsx @@ -24,7 +24,12 @@ import { useFormState } from "react-hook-form"; import { updateTask } from "@/core/workflowEditing"; import { applyDirtyValues } from "@/core/taskDraft"; import { flattenTask } from "@/side-panel/forms/TaskForm"; -import { computeSentinelDefaults } from "@/side-panel/forms/FormField"; +import { + computeSentinelDefaults, + SENTINEL_KEY, + SENTINEL_PREFIX, + SENTINEL_SUFFIX, +} from "@/side-panel/forms/FormField"; import { getFormFieldsForNodeType } from "@/core"; import { useDiagramEditorContext } from "@/store/DiagramEditorContext"; import { useEditSession } from "./EditSession"; @@ -34,9 +39,6 @@ import type { Specification } from "@openworkflowspec/sdk"; /* How long the applied message stays in footer */ const APPLIED_MESSAGE_MS = 2400; -const SENTINEL_KEY = "__oneof__"; -const SENTINEL_PREFIX = `${SENTINEL_KEY}.`; - type DraftStatusProps = { changedCount: number; isDirty: boolean; @@ -120,7 +122,7 @@ export function EditFormFooter({ node }: { node: RF.Node }) { const sentinelPaths = new Set(); for (const path of rawFlatDirty) { if (path.startsWith(SENTINEL_PREFIX)) { - sentinelPaths.add(path.slice(SENTINEL_PREFIX.length)); + sentinelPaths.add(path.slice(SENTINEL_PREFIX.length, -SENTINEL_SUFFIX.length)); } else { flatDirty.add(path); } diff --git a/packages/open-workflow-diagram-editor/src/side-panel/forms/FormField.tsx b/packages/open-workflow-diagram-editor/src/side-panel/forms/FormField.tsx index 745fa855..71bf0c78 100644 --- a/packages/open-workflow-diagram-editor/src/side-panel/forms/FormField.tsx +++ b/packages/open-workflow-diagram-editor/src/side-panel/forms/FormField.tsx @@ -16,7 +16,7 @@ import * as React from "react"; import { HelpCircle, ChevronDown, ChevronRight } from "lucide-react"; -import { useFormContext } from "react-hook-form"; +import { useFormContext, useWatch } from "react-hook-form"; import { useI18n } from "@openworkflowspec/i18n"; import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip"; import type { FormFieldDescriptor, ObjectField, OneOfField } from "../../core/schemaToFormFields"; @@ -31,6 +31,15 @@ import { } from "./ui/combobox"; import { KeyValueMapField } from "./customFields/KeyValueMapField"; +// --------------------------------------------------------------------------- +// Variant Sentinels +// --------------------------------------------------------------------------- + +export const SENTINEL_KEY = "__oneof__"; +export const SENTINEL_SELF_KEY = "__self__"; +export const SENTINEL_PREFIX = `${SENTINEL_KEY}.`; +export const SENTINEL_SUFFIX = `.${SENTINEL_SELF_KEY}`; + // --------------------------------------------------------------------------- // FormField — single form row (label + optional tooltip + control) // --------------------------------------------------------------------------- @@ -196,15 +205,25 @@ function ObjectFieldRow({ field }: { field: ObjectField }) { function OneOfFieldRow({ field }: { field: OneOfField }) { const { isReadOnly, taskData } = useTaskFormContext(); + const {control, getValues, setValue, register} = useFormContext>(); + const sentinelPath = `${SENTINEL_PREFIX}${field.path}${SENTINEL_SUFFIX}`; + + // Watched so the row follows a reset as well as switch + const sentinelLabel = useWatch({control, name: sentinelPath as never}) as unknown - // Derive the initial variant index from the actual task data in both modes. const derivedIdx = React.useMemo(() => { + if(typeof sentinelLabel === "string" && sentinelLabel !==""){ + const chosen = field.variants.findIndex((v)=> v.label === sentinelLabel) + if(chosen !== -1){ + return chosen + } + } // For the root one-of the relevant data is the whole task object; // for property-level one-ofs it's the value at the field's path. const dataAtPath = field.path === "__root__" ? taskData : getNestedValue(taskData, field.path); const idx = field.variants.findIndex((v) => v.matchesData(dataAtPath)); return idx === -1 ? 0 : idx; - }, [field.path, field.variants, taskData]); + }, [field.path, field.variants, taskData, sentinelLabel]); const [selectedVariantIdx, setSelectedVariantIdx] = React.useState(derivedIdx); @@ -218,8 +237,6 @@ function OneOfFieldRow({ field }: { field: OneOfField }) { // Per-variant saved values — preserves field data when switching variants // and then switching back, so the user does not have to re-type values. const savedVariantValues = React.useRef>>(new Map()); - const { getValues, setValue, register } = useFormContext>(); - const sentinelPath = `__oneof__.${field.path}`; const sentinelRef = register(sentinelPath as never); // The initial sentinel value is the committed variant label (derived from @@ -245,11 +262,8 @@ function OneOfFieldRow({ field }: { field: OneOfField }) { setSelectedVariantIdx(newIdx); const newLabel = field.variants[newIdx]?.label ?? ""; - // Update sentinel: compare against the committed variant label so that - // switching back to the original variant marks the sentinel clean. - setValue(sentinelPath as never, newLabel as never, { - shouldDirty: newLabel !== commitedVariantLabel, - }); + // Update sentinel: always dirty - the default is the committed variants label so returning it clears the flag + setValue(sentinelPath as never, newLabel as never, { shouldDirty: true }); // Restore saved values for the new variant if previously stored; // otherwise clear its leaf paths so stale values from the old variant. @@ -281,14 +295,17 @@ function OneOfFieldRow({ field }: { field: OneOfField }) { // Paths exclusive to the old variant: clear them silently (no dirty // needed — dirty is tracked via the sentinel). + const newPaths = [...newKindByPath.keys()]; for (const path of currentKindByPath.keys()) { - if (!newKindByPath.has(path)) { + const overlapsNewVariant = + newKindByPath.has(path) || + newPaths.some((p) => p.startsWith(`${path}.`) || path.startsWith(`${p}.`)); + if (!overlapsNewVariant) { setValue(path, undefined, { shouldDirty: false }); } } }, - // eslint-disable-next-line react-hooks/exhaustive-deps - [selectedVariantIdx, field.variants, getValues, setValue, sentinelPath, commitedVariantLabel], + [selectedVariantIdx, field.variants, getValues, setValue, sentinelPath], ); const variantLabels = React.useMemo(() => field.variants.map((v) => v.label), [field.variants]); @@ -419,7 +436,7 @@ function collectSentinelDefaults( /** Writes `label` at a dot-notation path */ function setNestedSentinel(result: Record, path: string, label: string): void { - const parts = path.split("."); + const parts = [...path.split("."), SENTINEL_SELF_KEY]; let obj = result; for (let i = 0; i < parts.length - 1; i++) { const part = parts[i]!; diff --git a/packages/open-workflow-diagram-editor/src/side-panel/forms/TaskForm.tsx b/packages/open-workflow-diagram-editor/src/side-panel/forms/TaskForm.tsx index acc8db2b..75c1d23e 100644 --- a/packages/open-workflow-diagram-editor/src/side-panel/forms/TaskForm.tsx +++ b/packages/open-workflow-diagram-editor/src/side-panel/forms/TaskForm.tsx @@ -19,7 +19,7 @@ import "./forms.css"; import type { Specification } from "@openworkflowspec/sdk"; import { useI18n } from "@openworkflowspec/i18n"; import { getFormFieldsForNodeType, structuralEqual } from "@/core"; -import { FormField, computeSentinelDefaults } from "./FormField"; +import { FormField, SENTINEL_KEY, computeSentinelDefaults } from "./FormField"; import { useSiblingTaskNames } from "./useSiblingTaskNames"; import { useDiagramEditorContext } from "@/store/DiagramEditorContext"; import { TaskFormContext, filterReadOnlyFields } from "./taskFormContext"; @@ -101,7 +101,7 @@ export function TaskForm({ nodeType, task, nodeId, taskReference }: TaskFormProp const sentinelDefaults = computeSentinelDefaults(allFields, task as Record); form.reset({ ...(task as Record), - ...(Object.keys(sentinelDefaults).length > 0 ? { __oneof__: sentinelDefaults } : {}), + ...(Object.keys(sentinelDefaults).length > 0 ? { [SENTINEL_KEY]: sentinelDefaults } : {}), }); // `task` is intentionally excluded: on node change we always reset to the // current task snapshot. External task mutations (undo/redo) are handled @@ -121,7 +121,7 @@ export function TaskForm({ nodeType, task, nodeId, taskReference }: TaskFormProp const sentinelDefaults = computeSentinelDefaults(allFields, task as Record); form.reset({ ...(task as Record), - ...(Object.keys(sentinelDefaults).length > 0 ? { __oneof__: sentinelDefaults } : {}), + ...(Object.keys(sentinelDefaults).length > 0 ? { [SENTINEL_KEY]: sentinelDefaults } : {}), }); }, [task, form, allFields]); diff --git a/packages/open-workflow-diagram-editor/stories/nested-editing/NestedEditing.stories.tsx b/packages/open-workflow-diagram-editor/stories/nested-editing/NestedEditing.stories.tsx index 6ae8d228..ccfa5556 100644 --- a/packages/open-workflow-diagram-editor/stories/nested-editing/NestedEditing.stories.tsx +++ b/packages/open-workflow-diagram-editor/stories/nested-editing/NestedEditing.stories.tsx @@ -49,6 +49,7 @@ export const CallEndpointUnion: Story = createWorkflowStory(workflows.callEndpoi export const CallHeadersMap: Story = createWorkflowStory(workflows.callHeadersMap); export const ListenDeepNesting: Story = createWorkflowStory(workflows.listenDeepNesting); export const NestedValidation: Story = createWorkflowStory(workflows.nestedValidation); +export const RaiseErrorShapes: Story = createWorkflowStory(workflows.raiseErrorShapes); export const RunTaskArray: Story = createWorkflowStory(workflows.runTaskArray); export const SetOpenMap: Story = createWorkflowStory(workflows.setOpenMap); export const SwitchLockedCases: Story = { diff --git a/packages/open-workflow-diagram-editor/stories/nested-editing/index.ts b/packages/open-workflow-diagram-editor/stories/nested-editing/index.ts index 7eefa954..246be314 100644 --- a/packages/open-workflow-diagram-editor/stories/nested-editing/index.ts +++ b/packages/open-workflow-diagram-editor/stories/nested-editing/index.ts @@ -19,6 +19,7 @@ export { default as callEndpointUnion } from "./workflows/call-endpoint-union.ya export { default as callHeadersMap } from "./workflows/call-headers-map.yaml?raw"; export { default as listenDeepNesting } from "./workflows/listen-deep-nesting.yaml?raw"; export { default as nestedValidation } from "./workflows/nested-validation.yaml?raw"; +export { default as raiseErrorShapes } from "./workflows/raise-error-shapes.yaml?raw"; export { default as runTaskArray } from "./workflows/run-task-array.yaml?raw"; export { default as setOpenMap } from "./workflows/set-open-map.yaml?raw"; export { default as switchLockedCases } from "./workflows/switch-locked-cases.yaml?raw"; diff --git a/packages/open-workflow-diagram-editor/stories/nested-editing/workflows/raise-error-shapes.yaml b/packages/open-workflow-diagram-editor/stories/nested-editing/workflows/raise-error-shapes.yaml new file mode 100644 index 00000000..506680a2 --- /dev/null +++ b/packages/open-workflow-diagram-editor/stories/nested-editing/workflows/raise-error-shapes.yaml @@ -0,0 +1,49 @@ +# +# Copyright 2021-Present The Open 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. +# +# Editing focus: the raise task's error, which is the only one-of in the schema +# whose branches nest — an inline definition holds four further one-ofs (type, +# instance, title and detail are each a literal or an expression), while the other +# branch is a plain name from `use.errors`. +# +# One task per branch and nothing else, so the variant selector can be switched in +# either direction without hunting for the node. `raiseInline` deliberately mixes +# literal and expression values so the nested selectors do not all read the same. +document: + dsl: "1.0.3" + namespace: examples + name: raise-error-shapes + version: "0.1.0" +use: + errors: + notImplemented: + type: https://open-workflow-specification.org/errors/not-implemented + status: 500 + title: Not Implemented +do: + # Reference branch: the name must match a key under `use.errors` above. + - raiseByReference: + raise: + error: notImplemented + # Inline branch, with a literal type/instance/title and an expression detail, + # so each nested selector starts on a different mode. + - raiseByDefinition: + raise: + error: + type: https://open-workflow-specification.org/errors/validation + status: 400 + instance: /do/0/raiseByReference + title: Invalid reading + detail: ${ .message } diff --git a/packages/open-workflow-diagram-editor/tests/core/schemaToFormFields.test.ts b/packages/open-workflow-diagram-editor/tests/core/schemaToFormFields.test.ts index 91caaf17..527ba6c0 100644 --- a/packages/open-workflow-diagram-editor/tests/core/schemaToFormFields.test.ts +++ b/packages/open-workflow-diagram-editor/tests/core/schemaToFormFields.test.ts @@ -21,6 +21,7 @@ import type { StringField, ObjectField, JsonField, + FormFieldDescriptor, } from "../../src/core/schemaToFormFields"; describe("schemaToFormFields endpoint and oneOf unwrapping", () => { @@ -303,3 +304,122 @@ describe("schemaToFormFields emitTask transparent-wrapper elimination", () => { expect(withChild?.kind).toBe("object"); }); }); + +// --------------------------------------------------------------------------- +// Labels and placeholders +// --------------------------------------------------------------------------- + +/** Every field as `path kind "label" placeholder`, variants inlined, depth-first. */ +function describeFields(fields: FormFieldDescriptor[], trail = ""): string[] { + return fields.flatMap((f) => { + const placeholder = f.kind === "string" && f.placeholder ? ` ph=${f.placeholder}` : ""; + const line = `${trail}${f.path} ${f.kind} "${f.label}"${placeholder}`; + if (f.kind === "object") return [line, ...describeFields(f.children, trail)]; + if (f.kind === "one-of") + return [ + line, + ...f.variants.flatMap((v) => [ + `${trail} variant "${v.label}"`, + ...describeFields(v.fields, `${trail} `), + ]), + ]; + return [line]; + }); +} + +/** The task's own property, without the seven shared `taskBase` fields after it. */ +function ownFields(nodeType: string): FormFieldDescriptor[] { + const [own] = getFormFieldsForNodeType(nodeType); + return own ? [own] : []; +} + +describe("schemaToFormFields labels", () => { + it.each([ + ["raise"], + ["emit"], + ["for"], + ["fork"], + ["listen"], + ["run"], + ["set"], + ["switch"], + ["try"], + ["wait"], + ])("labels the %s task's own property with its key, not its title", (nodeType) => { + expect(getFormFieldsForNodeType(nodeType)[0]?.label).toBe(nodeType); + }); + + it("describes the raise task's error in full", () => { + expect(describeFields(ownFields("raise"))).toMatchInlineSnapshot(` + [ + "raise object "raise"", + "raise.error one-of "Error"", + " variant "Raise Error Definition"", + " raise.error.type one-of "Type"", + " variant "Literal Error Type"", + " raise.error.type string "Literal Error Type"", + " variant "Expression Error Type"", + " raise.error.type string "Expression Error Type" ph=\${...}", + " raise.error.status number "Status"", + " raise.error.instance one-of "Instance"", + " variant "Literal Error Instance"", + " raise.error.instance string "Literal Error Instance"", + " variant "Expression Error Instance"", + " raise.error.instance string "Expression Error Instance" ph=\${...}", + " raise.error.title one-of "Title"", + " variant "Expression Error Title"", + " raise.error.title string "Expression Error Title" ph=\${...}", + " variant "Literal Error Title"", + " raise.error.title string "Literal Error Title"", + " raise.error.detail one-of "detail"", + " variant "Expression Error Details"", + " raise.error.detail string "Expression Error Details" ph=\${...}", + " variant "Literal Error Details"", + " raise.error.detail string "Literal Error Details"", + " variant "Raise Error Reference"", + " raise.error string "Raise Error Reference"", + ] + `); + }); + + it("describes a task whose own property is a one-of", () => { + expect(describeFields(ownFields("wait"))).toMatchInlineSnapshot(` + [ + "wait one-of "wait"", + " variant "Duration Inline"", + " wait.days number "Days"", + " wait.hours number "Hours"", + " wait.minutes number "Minutes"", + " wait.seconds number "Seconds"", + " wait.milliseconds number "Milliseconds"", + " variant "Duration Expression"", + " wait string "Duration Expression" ph=\${...}", + ] + `); + }); +}); + +describe("schemaToFormFields URI placeholders", () => { + const API_ENDPOINT_EXAMPLE = "https://example.com/api/{id}"; + + /** Every placeholder in the task, so an absence assertion cannot pass vacuously. */ + const placeholdersIn = (fields: FormFieldDescriptor[]): string[] => + describeFields(fields) + .filter((line) => line.includes(" ph=")) + .map((line) => line.slice(line.indexOf(" ph=") + 5)); + + it("suggests an API endpoint where the path is one", () => { + const endpoint = getFormFieldsForNodeType("set").find((f) => f.path === "input"); + expect(endpoint).toBeDefined(); + + expect(placeholdersIn([endpoint!])).toContain(API_ENDPOINT_EXAMPLE); + }); + + it("does not suggest one for an error type, which is a URI but not one to call", () => { + const placeholders = placeholdersIn(ownFields("raise")); + + // Non-empty, so `not.toContain` is a real assertion rather than a vacuous one. + expect(placeholders.length).toBeGreaterThan(0); + expect(placeholders).not.toContain(API_ENDPOINT_EXAMPLE); + }); +}); diff --git a/packages/open-workflow-diagram-editor/tests/core/taskDraft.test.ts b/packages/open-workflow-diagram-editor/tests/core/taskDraft.test.ts index 59a9f24a..26b98ab9 100644 --- a/packages/open-workflow-diagram-editor/tests/core/taskDraft.test.ts +++ b/packages/open-workflow-diagram-editor/tests/core/taskDraft.test.ts @@ -163,6 +163,27 @@ describe("applyDirtyValues", () => { expect(result).toEqual({}); }); + it("keeps a nested selectors value when an ancestor path has the change", () => { + // Scenario: raise.error held an error name and the user switched it to an inline + // definition, so react-hook-form marks the ancestor `raise.error` dirty - its type changed from string to object. + // The nested type/title selectors mounted with the switch and are sentinel-dirty too, + // but the values beneath them arrived with the ancestor's change and must survive. + + const original = { raise: { error: "notImplemented" } }; + const allValues = { + "raise.error.type": "https://example.com/errors/nope", + "raise.error.status": 418, + }; + + const dirtyPaths = new Set(["raise.error"]); + const sentinelPaths = new Set(["raise.error", "raise.error.type", "raise.error.title"]); + const result = applyDirtyValues(original, allValues, dirtyPaths, sentinelPaths); + + expect(result).toEqual({ + raise: { error: { type: "https://example.com/errors/nope", status: 418 } }, + }); + }); + it("does not mutate the original object", () => { const original = { set: { startEvent: "${x}" } }; const allValues = { "set.startEvent": "${changed}" }; diff --git a/packages/open-workflow-diagram-editor/tests/fixtures/workflows.ts b/packages/open-workflow-diagram-editor/tests/fixtures/workflows.ts index 68740449..217eae8d 100644 --- a/packages/open-workflow-diagram-editor/tests/fixtures/workflows.ts +++ b/packages/open-workflow-diagram-editor/tests/fixtures/workflows.ts @@ -584,3 +584,41 @@ export const NESTED_CONTAINERS_WORKFLOW = { { doTask: { do: [{ storeProfile: { set: { stored: true } } }] } }, ], }; + +/** + * Both shapes a `raise` task's error can take, side by side. + * `raise.error` is a one-of: an inline error *definition* (an object) or a *reference* + * (a string naming a key in `use.errors`). + */ + +export const RAISE_BOTH_ERROR_SHAPES_WORKFLOW = { + document: { dsl: "1.0.3", name: "raise-shapes", version: "1.0.0", namespace: "default" }, + use: { + errors: { + notImplemented: { + type: "https://open-workflow-specification.org/errors/not-implemented", + status: 500, + }, + serviceUnavailable: { + type: "https://open-workflow-specification.org/errors/service-unavailable", + status: 503, + }, + }, + }, + do: [ + { raiseByReference: { raise: { error: "notImplemented" } } }, + { + raiseInline: { + raise: { + error: { + type: "https://open-workflow-specification.org/errors/validation", + status: 400, + instance: "/do/0/raiseByReference", + title: "Invalid reading", + detail: "${ .message }", + }, + }, + }, + }, + ], +}; diff --git a/packages/open-workflow-diagram-editor/tests/side-panel/EditFormFooter.emitTask.test.tsx b/packages/open-workflow-diagram-editor/tests/side-panel/EditFormFooter.emitTask.test.tsx index 6a9e91c2..43391e04 100644 --- a/packages/open-workflow-diagram-editor/tests/side-panel/EditFormFooter.emitTask.test.tsx +++ b/packages/open-workflow-diagram-editor/tests/side-panel/EditFormFooter.emitTask.test.tsx @@ -22,7 +22,7 @@ * simulate exactly what OneOfFieldRow.handleVariantChange does: * * handleVariantChange (Data → Expression): - * 1. setValue("__oneof__.emit.event.with.data", "Expression", { shouldDirty: true }) + * 1. setValue("__oneof__.emit.event.with.data.__self__", "Expression", { shouldDirty: true }) * 2. setValue("emit.event.with.data", undefined, { shouldDirty: false }) * (kind boundary: json ≠ string) * @@ -80,7 +80,7 @@ const exprEmitNode = nodeAt(EXPR_WORKFLOW, EXPR_NODE_ID); // Sentinel path prefix used by OneOfFieldRow const SENTINEL_PREFIX = "__oneof__." as const; const DATA_PATH = "emit.event.with.data" as const; -const SENTINEL_PATH = `${SENTINEL_PREFIX}${DATA_PATH}` as const; +const SENTINEL_PATH = `${SENTINEL_PREFIX}${DATA_PATH}.__self__` as const; /** * FormSpy: rendered as a sibling of TaskForm inside the same EditSessionProvider. @@ -397,7 +397,7 @@ describe("EditFormFooter — full round-trip: Data(obj)→Expression(ok)→Data( await act(async () => { // Simulate handleVariantChange: Data(1) → Expression(0) // Sentinel dirty; data path cleared (kind boundary) - formRef.current!.setValue("__oneof__.emit.event.with.data" as never, "Expression" as never, { + formRef.current!.setValue(SENTINEL_PATH as never, "Expression" as never, { shouldDirty: true, }); formRef.current!.setValue("emit.event.with.data" as never, undefined as never, { @@ -431,7 +431,7 @@ describe("EditFormFooter — full round-trip: Data(obj)→Expression(ok)→Data( // Step 4: switch Expression(0) → Data(1), leave textarea empty, Apply await act(async () => { - formRef.current!.setValue("__oneof__.emit.event.with.data" as never, "Data" as never, { + formRef.current!.setValue(SENTINEL_PATH as never, "Data" as never, { shouldDirty: true, }); formRef.current!.setValue("emit.event.with.data" as never, undefined as never, { diff --git a/packages/open-workflow-diagram-editor/tests/side-panel/EditFormFooter.raiseTask.test.tsx b/packages/open-workflow-diagram-editor/tests/side-panel/EditFormFooter.raiseTask.test.tsx new file mode 100644 index 00000000..a2250421 --- /dev/null +++ b/packages/open-workflow-diagram-editor/tests/side-panel/EditFormFooter.raiseTask.test.tsx @@ -0,0 +1,170 @@ +/* + * Copyright 2021-Present The Open 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. + */ + +/** + * The raise task's Definition/Reference variant switch, driven through the UI. + * + * `raise.error` is the only one-of in the schema whose selected variant contains further + * one-ofs at child paths (`type`, `instance`, `title`, `detail`), which makes it the only + * place a parent and its children interact. + * + */ + +import { describe, it, expect, vi } from "vitest"; +import { act, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import type { Specification } from "@openworkflowspec/sdk"; + +vi.mock("../../src/side-panel/forms/ui/combobox", async () => { + const { createComboboxStub } = await import("../test-utils/combobox-stub"); + return createComboboxStub(); +}); + +const { EditFormFooter } = await import("../../src/side-panel/EditFormFooter"); +const { TaskForm } = await import("../../src/side-panel/forms/TaskForm"); +const { RAISE_BOTH_ERROR_SHAPES_WORKFLOW } = await import("../fixtures/workflows"); +const { renderWithProviders } = await import("../test-utils/render-helpers"); +const { nodeAt, parseFixture } = await import("../test-utils"); + +const REFERENCE_NODE_ID = "/do/raiseByReference"; +const INLINE_NODE_ID = "/do/raiseInline"; +const model = parseFixture(RAISE_BOTH_ERROR_SHAPES_WORKFLOW); + +function renderRaiseFooter(nodeId: string) { + const commitWorkflow = vi.fn(); + const node = nodeAt(model, nodeId); + + renderWithProviders( + <> + + + , + { isReadOnly: false, contentFormat: "yaml", model, commitWorkflow }, + ); + + return { commitWorkflow, user: userEvent.setup() }; +} + +type User = ReturnType; + +const apply = () => screen.getByRole("button", { name: "Apply" }); +const choose = (user: User, label: string) => + user.click(screen.getByRole("button", { name: label })); +const fieldValue = (label: string) => + (screen.queryByLabelText(label) as HTMLInputElement | null)?.value ?? "(not rendered)"; +const selector = (label: string) => (screen.getByLabelText(label) as HTMLInputElement).value; + +/** The raise block of the task the footer committed. */ +function committedRaise( + commitWorkflow: ReturnType, + nodeId: string, +): Record { + const task = nodeAt(commitWorkflow.mock.calls[0]![0] as Specification.Workflow, nodeId).data + .task as { raise: Record }; + return task.raise; +} + +describe("raise task error variant switch", () => { + it.each([ + ["a committed error reference", REFERENCE_NODE_ID], + ["a committed inline error definition", INLINE_NODE_ID], + ])("leaves Apply disabled on %s", async (_label, nodeId) => { + renderRaiseFooter(nodeId); + await act(async () => {}); + + expect(apply()).toBeDisabled(); + }); + + it("commits an inline definition when switching away from a reference", async () => { + const { commitWorkflow, user } = renderRaiseFooter(REFERENCE_NODE_ID); + await act(async () => {}); + + await choose(user, "Raise Error Definition"); + await user.type(screen.getByLabelText("Literal Error Type"), "https://example.com/errors/nope"); + await user.type(screen.getByLabelText("Status"), "418"); + await user.click(apply()); + + expect(committedRaise(commitWorkflow, REFERENCE_NODE_ID)).toEqual({ + error: { type: "https://example.com/errors/nope", status: 418 }, + }); + }); + + it("commits a reference when switching away from an inline definition", async () => { + const { commitWorkflow, user } = renderRaiseFooter(INLINE_NODE_ID); + await act(async () => {}); + + await choose(user, "Raise Error Reference"); + await user.type(screen.getByLabelText("Raise Error Reference"), "serviceUnavailable"); + await user.click(apply()); + + expect(committedRaise(commitWorkflow, INLINE_NODE_ID)).toEqual({ + error: "serviceUnavailable", + }); + }); + + it("keeps the error name when the variant is switched away and back", async () => { + // Clearing the outgoing variant's paths must not rebuild `raise.error` as an object + // of empty keys, which would destroy the name restored a moment earlier. + const { user } = renderRaiseFooter(REFERENCE_NODE_ID); + await act(async () => {}); + + await choose(user, "Raise Error Definition"); + await choose(user, "Raise Error Reference"); + + expect(fieldValue("Raise Error Reference")).toBe("notImplemented"); + }); + + it("keeps the inline fields when the variant is switched away and back", async () => { + const { user } = renderRaiseFooter(INLINE_NODE_ID); + await act(async () => {}); + + await choose(user, "Raise Error Reference"); + await choose(user, "Raise Error Definition"); + + expect(fieldValue("Literal Error Type")).toBe( + "https://open-workflow-specification.org/errors/validation", + ); + expect(fieldValue("Status")).toBe("400"); + }); + + it("keeps a nested selector in step with its value across a parent switch", async () => { + // The nested one-of unmounts with its parent. On remount its selection must follow + // the draft, not the committed task, or the row is labelled as one mode while + // holding — and committing — the other. + const { commitWorkflow, user } = renderRaiseFooter(INLINE_NODE_ID); + await act(async () => {}); + + await choose(user, "Expression Error Type"); + await user.clear(screen.getByLabelText("Expression Error Type")); + await user.type(screen.getByLabelText("Expression Error Type"), "${{ .boom }"); + + await choose(user, "Raise Error Reference"); + await choose(user, "Raise Error Definition"); + + expect(selector("Type")).toBe("Expression Error Type"); + expect(fieldValue("Expression Error Type")).toBe("${ .boom }"); + + await user.click(apply()); + expect(committedRaise(commitWorkflow, INLINE_NODE_ID)).toMatchObject({ + error: { type: "${ .boom }" }, + }); + }); +}); diff --git a/packages/open-workflow-diagram-editor/tests/side-panel/forms/FormFields.test.tsx b/packages/open-workflow-diagram-editor/tests/side-panel/forms/FormFields.test.tsx new file mode 100644 index 00000000..917073c6 --- /dev/null +++ b/packages/open-workflow-diagram-editor/tests/side-panel/forms/FormFields.test.tsx @@ -0,0 +1,61 @@ +/* + * Copyright 2021-Present The Open 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, expect, it } from "vitest"; +import { getFormFieldsForNodeType } from "../../../src/core"; +import { computeSentinelDefaults } from "../../../src/side-panel/forms/FormField"; +import { RAISE_BOTH_ERROR_SHAPES_WORKFLOW } from "../../fixtures/workflows"; +import { nodeAt, parseFixture } from "../../test-utils/workflow-helpers"; + +const model = parseFixture(RAISE_BOTH_ERROR_SHAPES_WORKFLOW); +const raiseFields = getFormFieldsForNodeType("raise"); +const taskAt = (nodeId: string) => nodeAt(model, nodeId).data.task as Record; + +// --------------------------------------------------------------------------- +// Variant sentinels +// +// `computeSentinelDefaults` records which variant each one-of has committed, so the +// footer can tell "the user moved the selector" apart from "the user edited a field". +// A raise task is the only place where a one-of's selected variant contains further +// one-ofs at child paths, so a parent and its children compete for the same slot. +// +// The whole `raise` subtree is asserted rather than one key, so a parent label being +// overwritten by its children shows up as a missing key rather than passing unnoticed. +// --------------------------------------------------------------------------- + +describe("computeSentinelDefaults", () => { + it("records a one-of and the one-ofs nested inside its selected variant", () => { + const sentinels = computeSentinelDefaults(raiseFields, taskAt("/do/raiseInline")); + + expect(sentinels.raise).toEqual({ + error: { + __self__: "Raise Error Definition", + type: { __self__: "Literal Error Type" }, + instance: { __self__: "Literal Error Instance" }, + title: { __self__: "Literal Error Title" }, + detail: { __self__: "Expression Error Details" }, + }, + }); + }); + + it("records only the chosen variant when it has no one-ofs inside it", () => { + const sentinels = computeSentinelDefaults(raiseFields, taskAt("/do/raiseByReference")); + + expect(sentinels.raise).toEqual({ + error: { __self__: "Raise Error Reference" }, + }); + }); +}); diff --git a/packages/open-workflow-diagram-editor/tests/test-utils/combobox-stub.tsx b/packages/open-workflow-diagram-editor/tests/test-utils/combobox-stub.tsx new file mode 100644 index 00000000..7b424239 --- /dev/null +++ b/packages/open-workflow-diagram-editor/tests/test-utils/combobox-stub.tsx @@ -0,0 +1,88 @@ +/* + * Copyright 2021-Present The Open 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 * as React from "react"; +/** + * A stand-in for `side-panel/forms/ui/combobox` that works under jsdom. + * + * base-ui opens its popup through layout APIs jsdom does not implement, so its items can + * never be clicked in a unit test. This keeps the same prop contract and renders each item + * as a plain button, so a test drives the *real* `handleVariantChange` instead of + * transcribing what it does + * + * ```ts + * vi.mock("../../src/side-panel/forms/ui/combobox", async () => { + * const { createComboboxStub } = await import("../test-utils/combobox-stub"); + * return createComboboxStub(); + * }); + * ``` + */ +export function createComboboxStub(): Record { + type Ctx = { value?: string; onValueChange?: (value: string) => void; disabled?: boolean }; + + const ComboboxCtx = React.createContext({}); + const Passthrough = ({ children }: { children?: React.ReactNode }) => <>{children}; + + return { + Combobox: ({ children, ...ctx }: Ctx & { children?: React.ReactNode }) => ( + {children} + ), + + ComboboxInput: ({ + id, + value, + readOnly, + disabled, + onBlur, + name, + ...rest + }: Record) => { + const ctx = React.useContext(ComboboxCtx); + return ( + {}} + onBlur={onBlur as React.FocusEventHandler | undefined} + aria-label={rest["aria-label"] as string | undefined} + aria-invalid={rest["aria-invalid"] as boolean | undefined} + /> + ); + }, + + ComboboxItem: ({ value, children }: { value: string; children?: React.ReactNode }) => { + const ctx = React.useContext(ComboboxCtx); + return ( + + ); + }, + + ComboboxContent: Passthrough, + ComboboxList: Passthrough, + ComboboxGroup: Passthrough, + ComboboxLabel: Passthrough, + ComboboxSeparator: () => null, + }; +} From dc7806f81657c611e78bfb5e84063f5b49f6e771 Mon Sep 17 00:00:00 2001 From: lornakelly Date: Tue, 22 Sep 2026 13:57:09 +0100 Subject: [PATCH 2/4] trigger deploy Signed-off-by: lornakelly --- .../stories/examples/workflows/raise-reusable.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/open-workflow-diagram-editor/stories/examples/workflows/raise-reusable.yaml b/packages/open-workflow-diagram-editor/stories/examples/workflows/raise-reusable.yaml index 425d7404..031dae10 100644 --- a/packages/open-workflow-diagram-editor/stories/examples/workflows/raise-reusable.yaml +++ b/packages/open-workflow-diagram-editor/stories/examples/workflows/raise-reusable.yaml @@ -13,6 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. # + document: dsl: "1.0.3" namespace: test From 7d9b18868408b84040fb82202eef7fca563e46be Mon Sep 17 00:00:00 2001 From: lornakelly Date: Wed, 23 Sep 2026 15:59:21 +0100 Subject: [PATCH 3/4] Fix for deleting values Signed-off-by: lornakelly --- .../src/core/taskDraft.ts | 43 ++++++++++++++++--- .../tests/core/taskDraft.test.ts | 24 ++++++++++- 2 files changed, 60 insertions(+), 7 deletions(-) diff --git a/packages/open-workflow-diagram-editor/src/core/taskDraft.ts b/packages/open-workflow-diagram-editor/src/core/taskDraft.ts index c49ab45b..c439fcbc 100644 --- a/packages/open-workflow-diagram-editor/src/core/taskDraft.ts +++ b/packages/open-workflow-diagram-editor/src/core/taskDraft.ts @@ -14,6 +14,26 @@ * limitations under the License. */ +const TASK_TYPE_KEYS = new Set([ + "call", + "do", + "emit", + "for", + "fork", + "listen", + "raise", + "run", + "set", + "switch", + "try", + "wait", +]); + +/* Returns the key that gives the task its type or undefined when it has none i.e object not a task */ +function getTaskTypeKey(task: Record): string | undefined { + return Object.keys(task).find((key) => TASK_TYPE_KEYS.has(key)); +} + /** * Reconstructs a nested task object from the flat dot-notation form values * produced by `flattenTask` in TaskForm. Arrays (child-task-list values) are @@ -65,6 +85,7 @@ export function applyDirtyValues( ): Record { // Deep clone the original so we never mutate the store value. const result = deepClone(original); + const taskTypeKey = getTaskTypeKey(original); for (const [dotPath, value] of Object.entries(allValues)) { if (!isDirtyPath(dotPath, dirtyPaths)) continue; @@ -72,7 +93,10 @@ export function applyDirtyValues( // A dirty path with an empty / null value means the user cleared the // field — delete it from the clone rather than writing an empty string. if (value === undefined || value === null || value === "") { - deletePath(result, dotPath.split(".")); + deletePath(result, dotPath.split("."), { + prune: !sentinelPaths.has(dotPath), + protectedKey: taskTypeKey, + }); } else { setPath(result, dotPath.split("."), value); } @@ -86,7 +110,7 @@ export function applyDirtyValues( (p) => p === sentinelPath || p.startsWith(prefix) || sentinelPath.startsWith(p + "."), ); if (!suppliedByEdit) { - deletePath(result, sentinelPath.split(".")); + deletePath(result, sentinelPath.split("."), { prune: false }); } } @@ -150,8 +174,15 @@ function setPath(obj: Record, parts: string[], value: unknown): current[parts[parts.length - 1]!] = value; } -/** Removes a key at a dot-notation path within `obj`. Cleans up empty parent objects. */ -function deletePath(obj: Record, parts: string[]): void { +/** Removes a key at a dot-notation path within `obj`. Cleans up empty parent objects. + * @param options.prune - Whether parents emptied by the deletion are removed too. + * @param options.protectedKey - A top-level key that is never pruned + */ +function deletePath( + obj: Record, + parts: string[], + options: { prune: boolean; protectedKey?: string | undefined } = { prune: true }, +): void { if (parts.length === 0) return; if (parts.some((p) => !isSafeKey(p))) { throw new Error(`Unsafe path segment in: ${parts.join(".")}`); @@ -163,9 +194,9 @@ function deletePath(obj: Record, parts: string[]): void { const head = parts[0]!; const child = obj[head]; if (child !== null && typeof child === "object" && !Array.isArray(child)) { - deletePath(child as Record, parts.slice(1)); + deletePath(child as Record, parts.slice(1), { prune: options.prune }); // Remove the parent if it became empty after deletion. - if (Object.keys(child).length === 0) { + if (options.prune && head !== options.protectedKey && Object.keys(child).length === 0) { delete obj[head]; } } diff --git a/packages/open-workflow-diagram-editor/tests/core/taskDraft.test.ts b/packages/open-workflow-diagram-editor/tests/core/taskDraft.test.ts index 26b98ab9..e3aafcce 100644 --- a/packages/open-workflow-diagram-editor/tests/core/taskDraft.test.ts +++ b/packages/open-workflow-diagram-editor/tests/core/taskDraft.test.ts @@ -160,7 +160,29 @@ describe("applyDirtyValues", () => { const sentinelPaths = new Set(["emit.event.with.data"]); const result = applyDirtyValues(original, allValues, dirtyPaths, sentinelPaths); expect(result).not.toHaveProperty("emit.event.with.data"); - expect(result).toEqual({}); + // Only the switched key goes: the containers around it stay, so the task is still an emit task. + expect(result).toEqual({ emit: { event: { with: {} } } }); + }); + + it("keeps the containers around a switched variant that commits no value", () => { + const original = { raise: { error: { type: "${ .errorType }" } } }; + const result = applyDirtyValues( + original, + { "raise.error.type": undefined }, + new Set(), + new Set(["raise.error.type"]), + ); + expect(result).toEqual({ raise: { error: {} } }); + }); + + it("never removes the key that gives the task its type", () => { + const original = { raise: { error: { type: "https://example.com/errors/boom" } } }; + const result = applyDirtyValues( + original, + { "raise.error.type": "" }, + new Set(["raise.error.type"]), + ); + expect(result).toEqual({ raise: {} }); }); it("keeps a nested selectors value when an ancestor path has the change", () => { From 25a14568a7c23b622a708363c6f650a73372581e Mon Sep 17 00:00:00 2001 From: lornakelly Date: Thu, 24 Sep 2026 14:35:10 +0100 Subject: [PATCH 4/4] PR fix for centralising task types Signed-off-by: lornakelly --- .../src/core/index.ts | 1 + .../src/core/taskDraft.ts | 20 +------ .../src/core/taskTypes.ts | 59 +++++++++++++++++++ .../src/core/validationErrors.ts | 24 +------- 4 files changed, 63 insertions(+), 41 deletions(-) create mode 100644 packages/open-workflow-diagram-editor/src/core/taskTypes.ts diff --git a/packages/open-workflow-diagram-editor/src/core/index.ts b/packages/open-workflow-diagram-editor/src/core/index.ts index 015e1470..c6e39a09 100644 --- a/packages/open-workflow-diagram-editor/src/core/index.ts +++ b/packages/open-workflow-diagram-editor/src/core/index.ts @@ -20,6 +20,7 @@ export * from "./validationErrors"; export * from "./graph"; export * from "./taskDraft"; export * from "./taskSubType"; +export * from "./taskTypes"; export * from "./elkjs"; export * from "./mermaidExport"; export * from "./schemaFilter"; diff --git a/packages/open-workflow-diagram-editor/src/core/taskDraft.ts b/packages/open-workflow-diagram-editor/src/core/taskDraft.ts index c439fcbc..1b07cff6 100644 --- a/packages/open-workflow-diagram-editor/src/core/taskDraft.ts +++ b/packages/open-workflow-diagram-editor/src/core/taskDraft.ts @@ -14,25 +14,7 @@ * limitations under the License. */ -const TASK_TYPE_KEYS = new Set([ - "call", - "do", - "emit", - "for", - "fork", - "listen", - "raise", - "run", - "set", - "switch", - "try", - "wait", -]); - -/* Returns the key that gives the task its type or undefined when it has none i.e object not a task */ -function getTaskTypeKey(task: Record): string | undefined { - return Object.keys(task).find((key) => TASK_TYPE_KEYS.has(key)); -} +import { getTaskTypeKey } from "./taskTypes"; /** * Reconstructs a nested task object from the flat dot-notation form values diff --git a/packages/open-workflow-diagram-editor/src/core/taskTypes.ts b/packages/open-workflow-diagram-editor/src/core/taskTypes.ts new file mode 100644 index 00000000..adfe75f9 --- /dev/null +++ b/packages/open-workflow-diagram-editor/src/core/taskTypes.ts @@ -0,0 +1,59 @@ +/* +* Copyright 2021-Present The Open 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 { GraphNodeType } from "@openworkflowspec/sdk"; + +/* The key that gives a task its type — the twelve task keywords the DSL defines. +* +* `GraphNodeType.Catch` is deliberately NOT here. `catch` is a property of a +* try task, not a task type +*/ +export type TaskTypeKey = + | typeof GraphNodeType.Call + | typeof GraphNodeType.Do + | typeof GraphNodeType.Emit + | typeof GraphNodeType.For + | typeof GraphNodeType.Fork + | typeof GraphNodeType.Listen + | typeof GraphNodeType.Raise + | typeof GraphNodeType.Run + | typeof GraphNodeType.Set + | typeof GraphNodeType.Switch + | typeof GraphNodeType.Try + | typeof GraphNodeType.Wait; + +export const TASK_TYPE_KEYS: ReadonlySet = new Set([ + GraphNodeType.Call, + GraphNodeType.Do, + GraphNodeType.Emit, + GraphNodeType.For, + GraphNodeType.Fork, + GraphNodeType.Listen, + GraphNodeType.Raise, + GraphNodeType.Run, + GraphNodeType.Set, + GraphNodeType.Switch, + GraphNodeType.Try, + GraphNodeType.Wait, +]); + +/* Returns the key that gives the task its type or undefined when it has none i.e object not a task */ +export function getTaskTypeKey(task: Record): string | undefined { + return Object.keys(task).find((key) => TASK_TYPE_KEYS.has(key)); +} + + + diff --git a/packages/open-workflow-diagram-editor/src/core/validationErrors.ts b/packages/open-workflow-diagram-editor/src/core/validationErrors.ts index 7f84075b..96be7ef9 100644 --- a/packages/open-workflow-diagram-editor/src/core/validationErrors.ts +++ b/packages/open-workflow-diagram-editor/src/core/validationErrors.ts @@ -14,31 +14,11 @@ * limitations under the License. */ +import { TASK_TYPE_KEYS } from "./taskTypes"; 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 { @@ -58,7 +38,7 @@ function isNoiseError(error: ValidationError): boolean { } const missingProperty = error.object?.["missingProperty"]; - if (typeof missingProperty === "string" && MISSING_PROP_TASK_TYPES.has(missingProperty)) { + if (typeof missingProperty === "string" && TASK_TYPE_KEYS.has(missingProperty)) { return true; }