diff --git a/.changeset/10948-flow-inspector-required-keys.md b/.changeset/10948-flow-inspector-required-keys.md new file mode 100644 index 0000000000..5073e52a0e --- /dev/null +++ b/.changeset/10948-flow-inspector-required-keys.md @@ -0,0 +1,22 @@ +--- +'@object-ui/app-shell': minor +--- + +feat(app-shell): the flow node inspector marks the config keys the installed spec refuses the node +without (objectui#10948). + +`@objectstack/spec` 17.5.0 refuses, at authoring, a flow node whose executor could not run it: a key +its config contract requires left out (a CRUD node's `objectName`, an `http` node's `url`, a +`notify` node's `title` while it has no `template`, a `script` node's `function`, a `subflow` or +`map` node's `flowName`, …), a decision branch with no `label` or `expression`, a screen field with +no `name`, and a `connector_action` whose `connectorConfig` names no connector or action. The +designer's live flow check already reports each one at the node's config path; the inspector now +marks the same keys before the author reaches that error, with the metadata form's own required +marker — the `*` in the field label, the row label of a branch or screen-field row, and +`aria-required` on the control where the inspector owns it. + +The inspector keeps no list of required keys. Each marker is the installed spec's own verdict, asked +of the node as it stands: the key is removed from a copy of the node and handed to the judges the +flow parse runs (`flowNodeConfigRefusals`, the predicate-slot walk, and `FlowNodeSchema`), so a +requirement that depends on the configuration — `notify`'s `title` without a `template`, a `loop`'s +`collection` once it has a body, a refused `end`'s `message` — is marked only while it applies. diff --git a/content/docs/guide/flow-designer.md b/content/docs/guide/flow-designer.md index 642a51830f..d70f7574ef 100644 --- a/content/docs/guide/flow-designer.md +++ b/content/docs/guide/flow-designer.md @@ -97,11 +97,16 @@ built-in defaults, so authoring still works offline. ## The node inspector -Selecting a node opens its inspector: **ID**, **Label**, **Node Type**, an -optional **Description**, and a **Configuration** section. New nodes start with -spec-valid defaults (a *Wait* node already carries a timer config, an *HTTP* -node defaults to `GET`) so a freshly dropped block is never in a broken -intermediate state. +Selecting a node opens its inspector: **ID**, **Label**, **Node Type**, and a +**Configuration** section. New nodes start with defaults where the spec allows +one: a *Wait* node already carries a timer config, and an *HTTP* node defaults to +`GET`. What the author must still supply is marked with a red `*`, the same +required marker the other metadata forms use. That covers an *HTTP* node's URL, a +record node's object, a decision branch's label and expression, and a screen +field's name. The marker follows the installed spec, so a key required only in +some configurations (a *Notify* node's title while it has no template) is marked +only while it applies. Until the value is supplied, the spec's error is shown on +that node and listed in the **Problems** panel. For node types whose engine executor publishes a `configSchema` (ADR-0018), the inspector renders a **server-driven property form** from that schema — so a diff --git a/packages/app-shell/README.md b/packages/app-shell/README.md index 14810236bf..dbcc929a45 100644 --- a/packages/app-shell/README.md +++ b/packages/app-shell/README.md @@ -567,6 +567,16 @@ variables* shape and an *email/SMS* notification shape (*Template* / *Recipients *Wait for* mode. A conditional field is never hidden while it still holds a value, so existing config is always reachable. +A config key the installed `@objectstack/spec` refuses the node without — an +`http` node's *URL*, a record node's *Object*, a decision branch's *Label* and +*Expression*, a screen field's *Name* — carries the same required marker (`*`) +`SchemaForm` draws, and a control the inspector renders itself also carries +`aria-required`. No list of required keys is kept here: `flow-required-keys.ts` +removes the key from a copy of the node and asks the spec's own judges +(`flowNodeConfigRefusals`, the predicate-slot walk, `FlowNodeSchema`). So a +rule-dependent key such as a `notify` node's *Title*, which is required only +while the node has no `template`, is marked only while the rule applies. + Config keys come in three editable shapes so authors never hand-write JSON: - **Flat object maps** — a `create_record` node's **Field values**, a diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/FlowKeyValueField.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/FlowKeyValueField.tsx index 8c612b27f3..9d385a701f 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/FlowKeyValueField.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/FlowKeyValueField.tsx @@ -26,7 +26,7 @@ import * as React from 'react'; import { Code2, Plus, X } from 'lucide-react'; import { Button, Input, Label, cn } from '@object-ui/components'; -import { uniqueId } from './_shared.js'; +import { RequiredMarker, uniqueId } from './_shared.js'; import { VariableTextInput } from './VariableTextInput.js'; import type { ScopeGroup } from './useFlowScope.js'; import { FlowExprIssue } from './FlowExprIssue.js'; @@ -211,6 +211,8 @@ export interface FlowKeyValueFieldProps { /** Placeholder of an expression row's source input. */ expressionPlaceholder: string; }; + /** The spec requires this map (objectui#10948): the label carries {@link RequiredMarker}. */ + required?: boolean; } /** @@ -242,6 +244,7 @@ export function FlowKeyValueField({ emptyLabel, scopeGroups, valueEnvelope, + required, }: FlowKeyValueFieldProps) { // Preserve whichever shape the value was authored in (object map vs the // assignment-node array form) across edits. @@ -301,7 +304,10 @@ export function FlowKeyValueField({ return (
- + {envelopeSlot && arrayShape && (

{ASSIGNMENT_ARRAY_FORM_PRESCRIPTION} diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeConfigField.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeConfigField.tsx index ac18721214..c170895b1d 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeConfigField.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeConfigField.tsx @@ -15,6 +15,7 @@ import { InspectorNumberField, InspectorSelectField, InspectorCheckboxField, + RequiredMarker, flagUnknownValue, } from './_shared.js'; import { Button, Label } from '@object-ui/components'; @@ -150,9 +151,22 @@ export interface FlowNodeConfigFieldProps { * DELIBERATELY. Omit to render the notice without the button. */ onClearInactive?: () => void; + /** + * objectui#10948 — the installed spec refuses this node without a value at + * `field.path`. Supplied by the host inspector, which owns the node, from + * `specRequiresField` (`flow-required-keys.ts`) — the spec's own verdict, + * never a list kept here. The label carries {@link RequiredMarker}; a control + * this component owns carries `aria-required`. + */ + required?: boolean; + /** + * objectui#10948 — for an `objectList` field, the column keys the spec + * requires on every row (`specRequiredColumns`), handed to the list editor. + */ + requiredColumns?: ReadonlySet; } -export function FlowNodeConfigField({ field, value, onCommit, disabled, locale, context, scopeGroups, approvalScopeGroups, triggerScope, inactiveRetained, onClearInactive }: FlowNodeConfigFieldProps) { +export function FlowNodeConfigField({ field, value, onCommit, disabled, locale, context, scopeGroups, approvalScopeGroups, triggerScope, inactiveRetained, onClearInactive, required, requiredColumns }: FlowNodeConfigFieldProps) { const refMode: 'expression' | 'template' = field.refMode ?? (field.kind === 'expression' ? 'expression' : 'template'); // objectui#6226 — the row-based condition builder, on the fields that opted in @@ -190,6 +204,7 @@ export function FlowNodeConfigField({ field, value, onCommit, disabled, locale, onCommit={(v) => onCommit(v)} disabled={disabled} context={context} + required={required} /> ); case 'keyValue': @@ -205,6 +220,7 @@ export function FlowNodeConfigField({ field, value, onCommit, disabled, locale, removeLabel={t('engine.inspector.flowNode.kv.remove', locale)} emptyLabel={t('engine.inspector.flowNode.kv.empty', locale)} scopeGroups={scopeGroups} + required={required} // objectui#7588 — the per-value text / expression toggle, offered // only on a map the spec's expression ledger declares `value`-role // for this node type (today the assignment node's `assignments`). @@ -225,6 +241,7 @@ export function FlowNodeConfigField({ field, value, onCommit, disabled, locale, value={value} onCommit={(v) => onCommit(v)} disabled={disabled} + required={required} addLabel={t('engine.inspector.flowNode.list.add', locale)} itemLabel={t('engine.inspector.flowNode.list.item', locale)} removeLabel={t('engine.inspector.flowNode.list.remove', locale)} @@ -246,6 +263,7 @@ export function FlowNodeConfigField({ field, value, onCommit, disabled, locale, onCommit(nums.length ? nums : undefined); }} disabled={disabled} + required={required} addLabel={t('engine.inspector.flowNode.list.add', locale)} itemLabel={t('engine.inspector.flowNode.list.item', locale)} removeLabel={t('engine.inspector.flowNode.list.remove', locale)} @@ -270,6 +288,8 @@ export function FlowNodeConfigField({ field, value, onCommit, disabled, locale, // objectui#10772 — with `context.node`, names a screen's `fields` // list, whose `visibleWhen` column binds the screen's own fields. fieldId={field.id} + required={required} + requiredColumns={requiredColumns} /> ); case 'number': @@ -280,6 +300,7 @@ export function FlowNodeConfigField({ field, value, onCommit, disabled, locale, placeholder={field.placeholder} onCommit={(v) => onCommit(v)} disabled={disabled} + required={required} /> ); case 'boolean': { @@ -391,6 +412,7 @@ export function FlowNodeConfigField({ field, value, onCommit, disabled, locale, } onCommit={(v) => onCommit(v)} disabled={disabled} + required={required} /> ); })(); @@ -402,6 +424,7 @@ export function FlowNodeConfigField({ field, value, onCommit, disabled, locale,

@@ -410,7 +433,10 @@ export function FlowNodeConfigField({ field, value, onCommit, disabled, locale, case 'textarea': return (
- +
); @@ -428,7 +455,10 @@ export function FlowNodeConfigField({ field, value, onCommit, disabled, locale, default: return (
- +
); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.requiredMarkers-10948.test.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.requiredMarkers-10948.test.tsx new file mode 100644 index 0000000000..dea4ab09e8 --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.requiredMarkers-10948.test.tsx @@ -0,0 +1,259 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#10948 — the flow node inspector marks the config keys the installed + * spec refuses the node without, so the author meets the requirement before the + * save-time error does. + * + * The marker is the metadata form's own (`RequiredMarker`, `SchemaForm`'s + * `data-required-marker` `*`), and its source is the spec, asked at render time + * (`flow-required-keys.ts`): no required-key list lives in the product. The + * expectations below are the MEASURED answer for each seeded node — what the + * installed spec requires of it — and each is asserted as an EQUALITY over the + * labels the form draws, so one row pins both halves: a marker where the spec + * requires the key, and none where it does not. + * + * Every row is lit by a control: the labels it expects UNMARKED are asserted + * present, so an empty marked set can never be a form that drew nothing. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, cleanup } from '@testing-library/react'; + +vi.mock('../previews/useFlowNodePalette', () => ({ + useActionConfigSchemas: () => ({}), + useFlowNodePalette: () => [], +})); +vi.mock('../previews/useObjectFields', () => ({ + useObjectFields: () => ({ fields: [], loading: false, error: null }), +})); + +import { FlowNodeInspector } from './FlowNodeInspector'; +import { fieldsForNodeType } from './flow-node-config'; +import { specRequiredColumns, specRequiresField } from './flow-required-keys'; +import type { MetadataSelection } from '../preview-registry'; +import { defaultNodeExtras, defaultNodeLabel } from '../previews/flow-canvas-parts'; + +/* ── `fetch` double: reference pickers resolve through a real `fetch` under + * happy-dom. The metadata routes answer empty and the automation routes (the + * connector registry a `connector_action` node reads) answer 404 — the degrade + * the pickers already fall back from; anything else fails the row. ── */ +const META_PREFIX = '/api/v1/meta/'; +const AUTOMATION_PREFIX = '/api/v1/automation/'; +let calls: string[] = []; +const routeOf = (url: string) => url.split('?')[0]; +const allowed = (url: string) => routeOf(url).startsWith(META_PREFIX) || routeOf(url).startsWith(AUTOMATION_PREFIX); + +beforeEach(() => { + calls = []; + vi.stubGlobal( + 'fetch', + vi.fn(async (input: unknown) => { + const url = String( + input && typeof input === 'object' && 'url' in input ? (input as { url: unknown }).url : input, + ); + calls.push(url); + const route = routeOf(url); + if (!route.startsWith(META_PREFIX)) { + return { ok: false, status: 404, headers: new Headers(), json: async () => ({}) }; + } + return { + ok: true, + status: 200, + headers: new Headers(), + json: async () => ({ type: route.slice(META_PREFIX.length), items: [] }), + }; + }), + ); +}); + +afterEach(() => { + expect(calls.filter((url) => !allowed(url))).toEqual([]); + cleanup(); + vi.unstubAllGlobals(); +}); + +function renderNode(node: Record) { + const { container } = render( + , + ); + return container; +} + +/** A label's text as the author reads it, without the marker. */ +const ownText = (el: Element) => { + const copy = el.cloneNode(true) as Element; + copy.querySelectorAll('[data-required-marker]').forEach((m) => m.remove()); + return (copy.textContent ?? '').trim(); +}; + +/** Every label the form drew that carries the required marker, by its own text. */ +const markedLabels = (container: HTMLElement) => + Array.from(container.querySelectorAll('[data-required-marker="true"]')).map((m) => { + expect(m.getAttribute('aria-hidden'), 'the marker is visual-only, as in SchemaForm').toBe('true'); + return ownText(m.parentElement!); + }); + +/** Every label (`