diff --git a/.changeset/11660-escalation-off-removes-block.md b/.changeset/11660-escalation-off-removes-block.md new file mode 100644 index 0000000000..8bb56bf217 --- /dev/null +++ b/.changeset/11660-escalation-off-removes-block.md @@ -0,0 +1,16 @@ +--- +'@object-ui/app-shell': patch +--- + +The flow designer's SLA escalation switch on an approval node no longer writes a refused `escalation: { enabled: false }` block, and switching it off keeps the values the author entered (objectui#11660). + +`ApprovalNodeConfigSchema` requires `escalation.timeoutHours` whether or not `enabled` is `false`. So `{ enabled: false, timeoutHours: 24 }` is a valid way to switch escalation off, and the runtime skips escalation for it. Having no block at all is also valid. The bare `{ enabled: false }` is not. The inspector wrote that bare block when an author switched off a node with nothing entered, because the switch drew on over a node that had no block. The flow saved and then failed at the approval node on every run, or, once the platform judges the block at save time, the save is refused. + +- **No block reads off.** A node without an `escalation` block used to draw the switch on, with its four fields showing. The inspector applied the declared default (`true`) although there was no block for it to apply to. It now draws the switch off, and rendering writes nothing. A block that omits `enabled` still reads on, and a stored `enabled: false` still reads off. +- **Switching off with nothing entered writes no block.** Switching on writes `{ enabled: true }` alone. Switching back off before anything is entered removes the block instead of writing `{ enabled: false }`. A `config` left empty is dropped, as it is for any other cleared field. +- **Switching off a block that holds values keeps them.** The block gets `enabled: false`, and every entered value stays exactly as stored. They show as kept but not in effect, with a Clear action (objectui#6499). A block that has values but no `timeoutHours` yet keeps them too: no hours are filled in, and the block stays refused at `escalation.timeoutHours` until hours are entered, as it was before the switch. +- **Clearing the last kept value removes the block**, instead of leaving `{ enabled: false }` behind. A block that still holds another value is kept. + +The offline field table and the `configSchema` the engine publishes for the approval node behave the same. A stored block is never rewritten just because the node is opened. + +**Clause-②: no.** Nothing on the package entry changes. The new helpers (`switchedBlockOf`, `readFieldValue`, `isBareSwitchedOffBlock`) live in the inspector's own module, which `@object-ui/app-shell` does not export. diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.declaredDefault.test.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.declaredDefault.test.tsx index 768719ba0e..16f7255268 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.declaredDefault.test.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.declaredDefault.test.tsx @@ -1070,7 +1070,15 @@ describe('the declaration surface this card names', () => { ]); for (const c of cases) { - renderInspector(draftWith(c.type, { config: {} })); + // objectui#11660 — a field stored inside the approval escalation block is + // on screen only while that block EXISTS: the spec makes the block itself + // the switch, so no block reads off and hides its fields. Its case renders + // on the smallest block the spec accepts, `{ timeoutHours: 24 }` (the + // #6620 rows' fixture), whose own `enabled` is omitted and so reads on — + // the key under measurement is still unset. Every other case keeps the + // empty config. + const config = c.id.startsWith('approval.escalation.') ? { escalation: { timeoutHours: 24 } } : {}; + renderInspector(draftWith(c.type, { config })); expect( triggerText(c.label), `${c.id}: an unset key states the declared default on the trigger`, diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.escalationOff-11660.test.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.escalationOff-11660.test.tsx new file mode 100644 index 0000000000..118ec69c38 --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.escalationOff-11660.test.tsx @@ -0,0 +1,385 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#11660 — the SLA escalation switch never writes the bare + * `{ enabled: false }` block `ApprovalNodeConfigSchema` refuses, and switching + * it off keeps every value the author entered. + * + * The governing text is the spec's own: `ApprovalEscalationSchema.enabled` is + * described as "an escalation block carrying timeoutHours is live unless this + * is explicitly false — the feature-level switch is whether the escalation + * block exists at all", and the block's `timeoutHours` is required whatever + * `enabled` says. So `{ enabled: false, timeoutHours: N }` and no block are both + * a conforming OFF; the bare stub is not. Triage's amended direction (comment + * 6003792818 on the card, replacing 6000059097 under objectui#6499 Option C): + * + * 1. no block reads OFF, and rendering writes nothing; + * 2. switching off with no entered value writes no block; + * 3. switching off a block that holds values writes `enabled: false` and keeps + * every value — the "inactive values retained" display carries them; + * 4. clearing the last retained value removes the block; + * 5. a block with values but no `timeoutHours` keeps its values on switch-off: + * nothing filled in, nothing deleted, and its refusal is neither created + * nor hidden by the switch. + * + * A block that omits `enabled` reads ON (the objectui#6620 pin), and a stored + * `enabled: false` reads OFF. + * + * Every case runs twice, once per DESCRIPTOR SOURCE, because the inspector + * renders `serverFields ?? fieldsForNodeType(...)`: + * + * - `offline` — the hand-written table in `flow-node-config.ts`; + * - `engine` — `getApprovalNodeConfigJsonSchema()` from the installed + * `@objectstack/spec/automation`, fed as the approval node's published + * `configSchema`. That function is what objectstack's `plugin-approvals` + * hands its approval node descriptor as `configSchema` (`approval-node.ts`, + * read at objectstack `6afb1b55`), so this is the shape a live backend + * publishes, not a hand-written imitation of it. It labels the fields from + * the schema (`Escalation`, `Timeout Hours`, …), which is why the labels + * below are per source. + * + * Round-trips are judged by `ApprovalNodeConfigSchema` imported from the + * `@objectstack/spec/automation` SUBPATH — `ApprovalEscalationSchema` is not on + * the package root. + */ + +import * as React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, cleanup, fireEvent, within } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +const stubs = vi.hoisted(() => ({ configSchemas: {} as Record })); + +vi.mock('../previews/useFlowNodePalette', () => ({ + useActionConfigSchemas: () => stubs.configSchemas, + useFlowNodePalette: () => [], +})); +vi.mock('../previews/useObjectFields', () => ({ + useObjectFields: () => ({ fields: [], loading: false, error: null }), +})); + +import { FlowNodeInspector } from './FlowNodeInspector'; +import type { MetadataSelection } from '../preview-registry'; +import { ApprovalNodeConfigSchema, getApprovalNodeConfigJsonSchema } from '@objectstack/spec/automation'; + +/* ── The `meta/*` double (objectui#7307), as the sibling inspector files + * serve it: an empty registry in the `{ type, items: [] }` envelope, and an + * `afterEach` that fails on any URL outside the metadata routes. ── */ +const META_PREFIX = '/api/v1/meta/'; +let calls: string[] = []; +const routeOf = (url: string) => url.split('?')[0]; + +beforeEach(() => { + stubs.configSchemas = {}; + 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) => !routeOf(url).startsWith(META_PREFIX))).toEqual([]); + cleanup(); + vi.unstubAllGlobals(); +}); + +type Config = Record; +type Draft = { nodes: Array>; edges: unknown[] }; + +const APPROVERS = [{ type: 'user', value: 'usr_approver' }]; + +function draftWith(config: Config): Draft { + return { nodes: [{ id: 'gate', type: 'approval', label: 'Gate', config }], edges: [] }; +} + +const configOf = (draft: Draft): Config | undefined => draft.nodes[0].config as Config | undefined; + +/** + * The editor loop: every patch is applied to the draft the inspector renders + * next, as the host does. `latest()` is the draft a save would send. + */ +function mount(initial: Draft) { + let current = initial; + const patches: unknown[] = []; + function Host() { + const [draft, setDraft] = React.useState(initial); + return ( + { + patches.push(patch); + setDraft((d) => { + current = { ...d, ...(patch as Partial) }; + return current; + }); + }} + onClearSelection={vi.fn()} + readOnly={false} + locale="en-US" + /> + ); + } + const utils = render(); + return { ...utils, patches, latest: () => current }; +} + +interface Labels { + gate: string; + timeout: string; + action: string; + escalateTo: string; + notify: string; + reassign: string; +} + +const SOURCES: Array<{ name: 'offline' | 'engine'; labels: Labels; install: () => void }> = [ + { + name: 'offline', + labels: { + gate: 'SLA escalation', + timeout: 'Timeout (hours)', + action: 'On timeout', + escalateTo: 'Escalate to', + notify: 'Notify submitter', + reassign: 'Reassign', + }, + install: () => { + stubs.configSchemas = {}; + }, + }, + { + name: 'engine', + labels: { + gate: 'Escalation', + timeout: 'Timeout Hours', + action: 'Action', + escalateTo: 'Escalate To', + notify: 'Notify Submitter', + reassign: 'Reassign', + }, + install: () => { + stubs.configSchemas = { approval: getApprovalNodeConfigJsonSchema() }; + }, + }, +]; + +const gateBox = (l: Labels) => screen.queryByLabelText(l.gate) as HTMLInputElement | null; +const subLabels = (l: Labels) => [l.timeout, l.action, l.escalateTo, l.notify]; +const shownSubFields = (l: Labels) => subLabels(l).filter((label) => screen.queryByText(label) !== null); +const retainedNotices = () => screen.queryAllByTestId('inactive-retained'); + +/** The free-text input of the `Escalate to` reference field (no data adapter is mounted, so it is a plain input). */ +function escalateToInput(l: Labels): HTMLInputElement { + const label = screen.getByText(l.escalateTo); + const input = label.parentElement?.querySelector('input'); + if (!input) throw new Error(`no input beside '${l.escalateTo}'`); + return input as HTMLInputElement; +} + +for (const source of SOURCES) { + const l = source.labels; + + describe(`${source.name} descriptors: a node with no escalation block reads OFF (objectui#11660)`, () => { + beforeEach(source.install); + + it('the toggle draws unchecked and no escalation field is revealed', () => { + const draft = draftWith({ approvers: APPROVERS }); + const before = JSON.stringify(draft); + const { patches } = mount(draft); + const box = gateBox(l); + expect(box, 'the switch is on screen').not.toBeNull(); + expect(box!.checked, 'no block is OFF — the spec calls the block itself the switch').toBe(false); + expect(shownSubFields(l), 'none of the four escalation fields is revealed over no block').toEqual([]); + // A read: rendering the corrected toggle writes nothing. + expect(patches, 'rendering patches the draft zero times').toEqual([]); + expect(JSON.stringify(draft), 'and the draft is unchanged byte for byte').toBe(before); + }); + + it('lit control: a block that omits `enabled` still reads ON and reveals its fields (objectui#6620)', () => { + mount(draftWith({ approvers: APPROVERS, escalation: { timeoutHours: 24 } })); + expect(gateBox(l)!.checked, 'inside a block, the spec default `true` applies').toBe(true); + expect(shownSubFields(l)).toEqual(subLabels(l)); + }); + + it('a stored `enabled: false` reads OFF', () => { + mount(draftWith({ approvers: APPROVERS, escalation: { enabled: false } })); + expect(gateBox(l)!.checked).toBe(false); + expect(shownSubFields(l)).toEqual([]); + }); + }); + + describe(`${source.name} descriptors: switching escalation off (objectui#11660, triage 6003792818)`, () => { + beforeEach(source.install); + + it('rule 3: a block holding every value gains `enabled: false`, every value and every sibling key byte-identical', () => { + const siblings: Config = { + approvers: APPROVERS, + behavior: 'quorum', + minApprovals: 1, + onEmptyApprovers: 'fail', + lockRecord: false, + maxRevisions: 2, + // An Advanced (JSON) extra — a key no form field owns. + custom_note: { kept: ['as', 'is'] }, + }; + const config: Config = { + ...siblings, + escalation: { enabled: true, timeoutHours: 24, action: 'reassign', escalateTo: 'sre_lead', notifySubmitter: false }, + }; + const { latest } = mount(draftWith(config)); + expect(gateBox(l)!.checked, 'premise: escalation is on').toBe(true); + + gateBox(l)!.click(); + + const { escalation, ...rest } = configOf(latest())!; + // Byte-identical, key order included: the switch is the one key that moved. + expect(JSON.stringify(escalation)).toBe( + JSON.stringify({ enabled: false, timeoutHours: 24, action: 'reassign', escalateTo: 'sre_lead', notifySubmitter: false }), + ); + expect(JSON.stringify(rest), 'the sibling config keys are untouched').toBe(JSON.stringify(siblings)); + const parsed = ApprovalNodeConfigSchema.safeParse({ approvers: APPROVERS, escalation }); + expect(parsed.success, parsed.success ? '' : JSON.stringify(parsed.error.issues)).toBe(true); + + // What the author sees next: the switch off, and the four values on screen + // as retained, not in effect (objectui#6499 Option C). + expect(gateBox(l)!.checked, 'the stored false reads off').toBe(false); + expect(shownSubFields(l)).toEqual(subLabels(l)); + expect(retainedNotices()).toHaveLength(4); + }); + + it('rule 3: a block that omits `enabled` keeps its value and gains `enabled: false`', () => { + const { latest } = mount(draftWith({ approvers: APPROVERS, escalation: { timeoutHours: 8 } })); + gateBox(l)!.click(); + expect(configOf(latest())).toEqual({ approvers: APPROVERS, escalation: { timeoutHours: 8, enabled: false } }); + }); + + it('rule 2: switching on writes only the switch, and switching off with nothing entered leaves no `escalation` key', () => { + const { latest } = mount(draftWith({ approvers: APPROVERS })); + gateBox(l)!.click(); + // Nothing the author did not enter is written by switching on: no + // `action`, no `notifySubmitter` default (shown, never written). + expect(configOf(latest())!.escalation, 'switching on writes the switch alone').toEqual({ enabled: true }); + gateBox(l)!.click(); + expect(configOf(latest()), 'no `{ enabled: false }` stub is written').toEqual({ approvers: APPROVERS }); + expect(gateBox(l)!.checked).toBe(false); + }); + + it('rule 2: an emptied `config` is pruned with the block', () => { + const { latest } = mount(draftWith({ escalation: { enabled: true } })); + gateBox(l)!.click(); + expect(latest().nodes[0], 'removing the only config key leaves no empty `config` object').not.toHaveProperty('config'); + }); + + it('rule 5: a block with values but no `timeoutHours` keeps them, gains no `timeoutHours`, and stays refused where it was', () => { + const before: Config = { approvers: APPROVERS, escalation: { enabled: true, action: 'reassign' } }; + const refusedAt = (cfg: Config) => { + const r = ApprovalNodeConfigSchema.safeParse(cfg); + return r.success ? [] : r.error.issues.map((i) => i.path.join('.')); + }; + expect(refusedAt(before), 'premise: refused at the missing hours before the switch').toEqual(['escalation.timeoutHours']); + const { latest } = mount(draftWith(before)); + gateBox(l)!.click(); + const after = configOf(latest())!; + expect(after.escalation, 'the value stays; nothing is filled in').toEqual({ enabled: false, action: 'reassign' }); + expect(refusedAt(after), 'the switch neither creates nor hides that refusal').toEqual(['escalation.timeoutHours']); + }); + + it('rule 4 on a block rule 3 wrote: clearing its last retained value leaves no block', () => { + const { latest } = mount(draftWith({ approvers: APPROVERS, escalation: { enabled: true, timeoutHours: 24 } })); + gateBox(l)!.click(); + expect(configOf(latest())!.escalation, 'premise: rule 3 kept the hours').toEqual({ enabled: false, timeoutHours: 24 }); + fireEvent.click(within(retainedNotices()[0]).getByRole('button', { name: /clear value/i })); + expect(configOf(latest())).toEqual({ approvers: APPROVERS }); + }); + }); + + describe(`${source.name} descriptors: an inspector-authored node round-trips through ApprovalNodeConfigSchema (objectui#11660)`, () => { + beforeEach(source.install); + + it('escalation ON with every value parses and keeps every value; OFF keeps every value beside `enabled: false` and parses', async () => { + const { latest } = mount(draftWith({ approvers: APPROVERS })); + expect(gateBox(l)!.checked, 'premise: a fresh node starts off').toBe(false); + + // ON, authored control by control through the inspector. + gateBox(l)!.click(); + expect(gateBox(l)!.checked, 'switching on draws the switch on').toBe(true); + fireEvent.change(screen.getByLabelText(l.timeout), { target: { value: '24' } }); + await userEvent.click(screen.getByRole('combobox', { name: l.action })); + await userEvent.click(await screen.findByRole('option', { name: l.reassign })); + fireEvent.change(escalateToInput(l), { target: { value: 'sre_lead' } }); + (screen.getByLabelText(l.notify) as HTMLInputElement).click(); + + const onConfig = configOf(latest())!; + const authored = { enabled: true, timeoutHours: 24, action: 'reassign', escalateTo: 'sre_lead', notifySubmitter: false }; + expect(onConfig.escalation, 'the inspector stored exactly what the author entered').toEqual(authored); + const on = ApprovalNodeConfigSchema.safeParse(onConfig); + expect(on.success, on.success ? '' : JSON.stringify(on.error.issues)).toBe(true); + expect((on.data as { escalation?: unknown }).escalation, 'and the contract keeps every value').toEqual(authored); + + // OFF, values retained. + gateBox(l)!.click(); + const offConfig = configOf(latest())!; + const retained = { ...authored, enabled: false }; + expect(JSON.stringify(offConfig.escalation), 'every value identical, the switch false').toBe(JSON.stringify(retained)); + const off = ApprovalNodeConfigSchema.safeParse(offConfig); + expect(off.success, off.success ? '' : JSON.stringify(off.error.issues)).toBe(true); + expect((off.data as { escalation?: unknown }).escalation, 'and the contract keeps them, switched off').toEqual(retained); + }); + }); + + describe(`${source.name} descriptors: a stored switched-off block (objectui#11660)`, () => { + beforeEach(source.install); + + it('`{ enabled: false, timeoutHours: 24 }` reads OFF, shows the retained value, and writes nothing on render', () => { + // A shape the spec ACCEPTS, and the one rule 3 writes. It is read, never + // rewritten on sight. + expect(ApprovalNodeConfigSchema.safeParse({ approvers: APPROVERS, escalation: { enabled: false, timeoutHours: 24 } }).success).toBe(true); + const draft = draftWith({ approvers: APPROVERS, escalation: { enabled: false, timeoutHours: 24 } }); + const before = JSON.stringify(draft); + const { patches } = mount(draft); + expect(gateBox(l)!.checked).toBe(false); + expect(shownSubFields(l), 'the stored value is re-shown (objectui#6499)').toEqual([l.timeout]); + expect(retainedNotices(), 'and flagged as kept, not in effect').toHaveLength(1); + expect(patches).toEqual([]); + expect(JSON.stringify(draft)).toBe(before); + }); + + it('clearing its last retained value leaves no `{ enabled: false }` stub behind', () => { + const { latest } = mount(draftWith({ approvers: APPROVERS, escalation: { enabled: false, timeoutHours: 24 } })); + fireEvent.click(within(retainedNotices()[0]).getByRole('button', { name: /clear value/i })); + const after = configOf(latest())!; + expect('escalation' in after, 'the bare stub the spec refuses is not written').toBe(false); + expect(ApprovalNodeConfigSchema.safeParse(after).success).toBe(true); + }); + + it('clearing ONE of two retained values keeps the other — only the bare stub is removed', () => { + // The boundary of the stub rule: it removes a block holding nothing but + // the switch, never a value the author did not clear. + const { latest } = mount( + draftWith({ approvers: APPROVERS, escalation: { enabled: false, timeoutHours: 24, escalateTo: 'sre_lead' } }), + ); + expect(retainedNotices(), 'premise: both stored values are retained').toHaveLength(2); + fireEvent.change(escalateToInput(l), { target: { value: '' } }); + expect(configOf(latest())!.escalation).toEqual({ enabled: false, timeoutHours: 24 }); + }); + }); +} diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.inactiveRetained.test.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.inactiveRetained.test.tsx index 86b5c8f8c3..357c15ef65 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.inactiveRetained.test.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.inactiveRetained.test.tsx @@ -175,8 +175,14 @@ describe('clearing is deliberate, author-initiated, and goes through the ordinar const clear = within(notices()[0]).getByRole('button', { name: /clear value/i }); fireEvent.click(clear); const patched = onPatch.mock.calls.at(-1)![0] as any; - // The retained key is gone; the controller the author actually set stays. - expect(patched.nodes[0].config.escalation).toEqual({ enabled: false }); + // The retained key is gone. This row used to go on "the controller the + // author actually set stays" and pin `{ enabled: false }` — the stub + // `ApprovalNodeConfigSchema` refuses for its missing `timeoutHours`. + // objectui#11660 (triage 6003792818, rule 4): with its last retained value + // cleared, the block would hold nothing but its switch, and the author's + // OFF survives as no block — which the spec reads as off too. A block that + // still held another value would be kept, `enabled: false` and all. + expect(patched.nodes[0].config?.escalation).toBeUndefined(); }); it('does not clear anything until the author clicks — rendering is a read', () => { diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.tsx index 5aa4488e05..3e9fcf287d 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.tsx @@ -37,6 +37,9 @@ import { isFieldVisible, inactiveRetainedKind, getFieldValue, + readFieldValue, + switchedBlockOf, + isBareSwitchedOffBlock, configKeyOf, FLOW_NODE_TYPE_OPTIONS, type FlowConfigField, @@ -365,7 +368,22 @@ export function FlowNodeInspector({ selection, draft, onPatch, onClearSelection, nextEdges = syncDecisionEdgesByOrder(node.id, value, draftEdges); } } + // objectui#11660 — a block whose EXISTENCE is its switch (`switchedBlockOf`: + // the approval node's SLA escalation). The write itself is the ordinary + // one, so switching off a block that holds entered values stores + // `enabled: false` beside them, every value kept (objectui#6499 Option C, + // triage 6003792818) — including a block that has no `timeoutHours` yet: + // nothing is filled in, nothing deleted. What is never stored is the bare + // `{ enabled: false }` stub `ApprovalNodeConfigSchema` refuses: a write + // that would leave it — switching off with nothing entered, or clearing the + // last value retained under a switched-off block — removes the block + // instead, which says the same OFF. `setAtPath` deletes only the block — + // sibling config keys are not rebuilt — and prunes a `config` left empty. + const switched = switchedBlockOf(node, field); let nextNode = setAtPath(node, path, stored); + if (switched && isBareSwitchedOffBlock(nextNode, switched)) { + nextNode = setAtPath(nextNode, [...switched.block], undefined); + } // Migrate-on-edit: writing the canonical path drops any looser fallback // location, so the node never carries a stale duplicate (engine + designer agree). if (field.fallbackPath) nextNode = setAtPath(nextNode, field.fallbackPath, undefined); @@ -489,7 +507,9 @@ export function FlowNodeInspector({ selection, draft, onPatch, onClearSelection, // #4305 — show this repeater only the keys the typed sibling // fields do not own; `setField` merges its commit back. ? connectorInputExtras(getFieldValue(node, effField), effField.omitKeys) - : getFieldValue(node, effField); + // objectui#11660 — `readFieldValue`: a block switch over an + // absent block draws OFF, not its declared default. + : readFieldValue(node, effField); return ( = { // `flow-node-config.spec-reconciliation.test.ts` derives every default in // this block from the installed `ApprovalEscalationSchema`, so the next // upstream flip reddens there rather than diverging silently again. + // + // That default applies INSIDE a block. Whether the block exists is the + // switch itself (objectui#11660, `BLOCK_SWITCHES` below): no block reads + // off, and switching off never writes a bare `{ enabled: false }` — with + // nothing entered no block is written; entered values stay beside + // `enabled: false`. The engine-published copy of this gate behaves the + // same: the rule is keyed by path, not declared here. { id: 'escalation.enabled', path: ['config', 'escalation', 'enabled'], label: 'SLA escalation', kind: 'boolean', defaultValue: 'true', help: 'Escalate when a decision is not recorded within the timeout.' }, { id: 'escalation.timeoutHours', path: ['config', 'escalation', 'timeoutHours'], label: 'Timeout (hours)', kind: 'number', placeholder: '24', showWhen: { field: 'escalation.enabled', equals: ['true'] } }, { @@ -1332,6 +1339,118 @@ export function getFieldValue(node: Record | null | undefined, return field.fallbackPath ? read(field.fallbackPath) : undefined; } +/** + * Boolean gates whose spec meaning is "the enclosing block EXISTS" + * (objectui#11660), keyed by node type and the gate's path. + * + * The approval node's SLA escalation is the one instance, and the spec states + * it in the declaration itself: `ApprovalEscalationSchema.enabled` is + * described as "an escalation block carrying timeoutHours is live unless this + * is explicitly false — the feature-level switch is whether the escalation + * block exists at all", and the block's `timeoutHours` is REQUIRED whatever + * `enabled` says. So OFF has two conforming spellings: NO BLOCK, and a block + * that keeps its `timeoutHours` beside `enabled: false` (the runtime skips the + * escalation sweep on an explicit `false`). What it refuses is the bare + * `{ enabled: false }` stub — and the inspector used to write exactly that when + * an author switched off a node with nothing entered, because the switch drew + * ON over a node with no block. The flow saved and then failed at the approval + * node on every run, or, at a door that judges the block, did not save. + * + * Keyed by PATH, not by a descriptor member: the offline table and the + * engine-published `configSchema` (`jsonSchemaToFlowFields`) both emit this + * path for the gate, so the rule holds whichever of the two produced the field + * on screen. Triage's ruling on objectui#11660 (6003792818, amending its + * first grade under objectui#6499 Option C) is what this list carries out: no + * block reads off; switching off with nothing entered writes no block; + * switching off a block holding values writes `enabled: false` and keeps every + * value; clearing the last retained value removes the block. + */ +const BLOCK_SWITCHES: ReadonlyArray<{ nodeType: string; path: readonly string[] }> = [ + { nodeType: 'approval', path: ['config', 'escalation', 'enabled'] }, +]; + +/** Where a field sits relative to a {@link BLOCK_SWITCHES} block. */ +export interface SwitchedBlock { + /** The block whose existence is the switch, e.g. `['config', 'escalation']`. */ + block: readonly string[]; + /** The switch's own key inside that block, e.g. `enabled`. */ + switchKey: string; + /** Whether the field IS the switch, rather than a value stored inside the block. */ + isSwitch: boolean; +} + +/** + * The switched block `field` belongs to on `node` — as its switch or as a + * value inside it — or `undefined` for every other field (objectui#11660). + */ +export function switchedBlockOf( + node: Record | null | undefined, + field: Pick, +): SwitchedBlock | undefined { + const type = node?.type; + if (typeof type !== 'string') return undefined; + for (const s of BLOCK_SWITCHES) { + if (s.nodeType !== type) continue; + const block = s.path.slice(0, -1); + if (field.path.length <= block.length || !block.every((seg, i) => field.path[i] === seg)) continue; + const isSwitch = + field.kind === 'boolean' && field.path.length === s.path.length && s.path.every((seg, i) => field.path[i] === seg); + return { block, switchKey: s.path[s.path.length - 1], isSwitch }; + } + return undefined; +} + +function blockAt(node: Record | null | undefined, block: readonly string[]): unknown { + let cur: unknown = node; + for (const seg of block) { + if (cur && typeof cur === 'object' && !Array.isArray(cur)) cur = (cur as Record)[seg]; + else return undefined; + } + return cur; +} + +const isBlockObject = (v: unknown): v is Record => + !!v && typeof v === 'object' && !Array.isArray(v); + +/** + * The value a field's control draws and a `showWhen` controller resolves + * through: the stored value ({@link getFieldValue}), except that a block + * switch over an ABSENT block reads `false` (objectui#11660). + * + * The switch's declared default (`'true'`, the spec's `.default(true)`) is what + * the runtime applies to an `enabled` key omitted from a block that EXISTS — + * the objectui#6620 reading, unchanged: `{ timeoutHours: 24 }` draws checked. + * With no block there is nothing for that default to apply to, and the spec + * calls the block's absence OFF; reading the default there drew the toggle + * checked, and revealed the four escalation fields, on every approval node + * that has no escalation at all. + * + * A READ: it writes nothing, so a node that stores no block keeps storing none. + */ +export function readFieldValue(node: Record | null | undefined, field: FlowConfigField): unknown { + const switched = switchedBlockOf(node, field); + if (switched?.isSwitch && !isBlockObject(blockAt(node, switched.block))) return false; + return getFieldValue(node, field); +} + +/** + * Whether `node` holds `switched`'s block as the bare `{ : false }` + * stub — OFF spelt as a block that holds no entered value, which + * `ApprovalNodeConfigSchema` refuses for its missing `timeoutHours` and which + * absence already says (objectui#11660). A switched-off block that still holds + * any other key is NOT a stub: its values are the author's, kept (objectui#6499 + * Option C). + */ +export function isBareSwitchedOffBlock( + node: Record | null | undefined, + switched: SwitchedBlock, +): boolean { + const block = blockAt(node, switched.block); + if (!isBlockObject(block)) return false; + const keys = Object.keys(block); + return keys.length === 1 && keys[0] === switched.switchKey && block[switched.switchKey] === false; +} + /** * The `config` key this field owns, or `undefined` for fields stored outside * `config` (spec-structured blocks, top-level `timeoutMs`). Used by the @@ -1438,7 +1557,9 @@ function controllerAdmits( if (!field.showWhen) return true; const controller = fields.find((f) => f.id === field.showWhen!.field); if (!controller) return false; - const raw = getFieldValue(node, controller); + // `readFieldValue`, not the bare stored read: a block switch over an absent + // block resolves OFF rather than through its declared default (objectui#11660). + const raw = readFieldValue(node, controller); const resolved = isUnsetFieldValue(raw) ? controller.defaultValue : raw; // Boolean controllers (e.g. `escalation.enabled`) compare against 'true'/'false'. const value = typeof resolved === 'boolean' ? String(resolved) : resolved;