diff --git a/.changeset/11168-button-undoable.md b/.changeset/11168-button-undoable.md new file mode 100644 index 0000000000..70fd89d6f6 --- /dev/null +++ b/.changeset/11168-button-undoable.md @@ -0,0 +1,15 @@ +--- +'@object-ui/components': minor +'@object-ui/types': patch +--- + +`action:button` delivers `undoable` where a record is in scope, and publishes it (objectui#11168, ruling B on objectui#11754). + +An `action:button` that declares `undoable: true` on an `operation: update` now offers Undo in its success toast. Undo writes back the prior values of the fields the update wrote, read off the record in scope: the record page's record, or the row the host binds to the node through `data` (a table's row, `DetailView`'s header, an `action:bar` member). Before, the block forwarded `undoable` but handed the runner no record, so the update ran and no Undo was offered anywhere the block was used. + +- **What the block now sends.** For an `undoable` `operation: update`, the button hands the runner the record in scope under `params._rowRecord`, the spelling the record page's header, the declared-actions bar, the related-record bridge and the grid's rows already use. The route dispatch strips it before it POSTs. It is attached only when the update writes that record, so where no explicit `recordId` is given, the shared route dispatch (`createServerActionHandler`) now takes the record id from it, as it does for those hosts; the record page's own dispatch already wrote to its record. +- **The one limit.** A button with no record in scope offers no Undo, because there is no row to restore. The same holds for a button whose `recordId` names a record other than the one in scope: its Undo would restore another record's values. A record that does not carry every written field offers no Undo, as before. +- **Unchanged.** A button that is not `undoable`, and an `undoable` action that is not an `operation: update`, dispatch exactly as before, with no record attached. +- **Published.** `undoable` is a published input of `action:button` (a boolean, with a description that states the limit), so the page validator stops reporting it as an unknown prop. Nothing is refused that was accepted before. + +`@object-ui/types`: the `UIActionSchema.undoable` doc comment no longer says `action:button` never hands the runner a record. No type changes. diff --git a/apps/console/src/__tests__/registry-inputs-spec-parity.test.ts b/apps/console/src/__tests__/registry-inputs-spec-parity.test.ts index 8433facf42..e816b81175 100644 --- a/apps/console/src/__tests__/registry-inputs-spec-parity.test.ts +++ b/apps/console/src/__tests__/registry-inputs-spec-parity.test.ts @@ -846,24 +846,13 @@ const FIELD_SECURITY_TRIPLE = ['enforceFieldSecurity', 'redactFields', 'required /** Every entry the ruling books starts with this, so the caps can count them. */ const OWED_PREFIX = 'OWED TO '; -/** One ruled entry's reason: owner first, then what is owed, then the ruling and the expiry. */ -const OWED_TO = (owner: Objectui11111Owner, what: string): string => { - const { bump, bookedBy, expires } = OBJECTUI_11111_BOOKINGS[owner]; - return ( - `${OWED_PREFIX}${owner}. ${what} Booked by ${bookedBy}: ` + - `the ${bump} bump re-pins and declares nothing; ${owner} decides it by its own measurement. ` + - `Expires ${expires}, or when ${owner} lands, whichever is first.` - ); -}; - -/** `BLOCK.KEY` entries for every listed key of one block, all with the same owner and reason. */ -const owedEntries = ( - type: string, - keys: readonly string[], - owner: Objectui11111Owner, - what: string, -): Record => - Object.fromEntries(keys.map((key) => [`${type}.${key}`, OWED_TO(owner, what)])); +// The two helpers that wrote a booked entry — `OWED_TO` (the reason: owner, +// what is owed, the booking record and the expiry) and `owedEntries` (one +// entry per key of a block) — left with the last entry they wrote: +// `action:button.undoable`, struck by objectui#11168's last slice. Every ledger +// is empty and every cap below is 0. The cap test still reads each ledger for +// the `OWED TO ` prefix, so anything booked again is counted against a cap of 0 +// and goes red until a ruling books it with an owner and an expiry. /** Which owner the ruling routes an entry id to — asserted against every entry's reason. */ function objectui11111OwnerOf(id: string): Objectui11111Owner { @@ -892,7 +881,7 @@ const owedIdsOf = (ledger: Record): string[] => const OBJECTUI_11111_LEDGER_CAPS = { unjudgedBlocks: 0, // objectui#11168 loaded and judged all four: slice 3 object-map and object-tree, slice 4 object-gantt, slice 5 object-timeline offSpecInputs: 0, // objectui#11168 slice 1 retired action:group.name - unpublishedKeys: 1, // objectui#11168: 1 (action:button undoable; the two `endpoint` entries left at the 17.6.0 bump, objectui#11438, when the spec stopped declaring the key); objectui#8652: 0, objectui#8649: 0, objectui#11536: 0 and objectui#11068: 0 (each struck by its landing; objectui#11536 declared all ten record:line_items keys, objectui#11068's build published object-grid keyboardNavigation) + unpublishedKeys: 0, // objectui#11168: 0 (its last slice declared action:button undoable, ruling B on objectui#11754; the two `endpoint` entries had left at the 17.6.0 bump, objectui#11438, when the spec stopped declaring the key); objectui#8652: 0, objectui#8649: 0, objectui#11536: 0 and objectui#11068: 0 (each struck by its landing; objectui#11536 declared all ten record:line_items keys, objectui#11068's build published object-grid keyboardNavigation) refusedArms: 0, // objectui#11168: slice 2 narrowed element:definition-list.columns, slice 3 object-form.layout memberPins: 0, // objectui#11168 slice 2 pinned element:definition-list.items and element:repeater ×3; objectui#11536 pinned record:line_items columns and dataSource } as const; @@ -1529,13 +1518,13 @@ const UNPUBLISHED_EXEMPTIONS: Record = { // at the 17.6.0 bump (objectui#11438): 17.6.0 refuses `endpoint` on both // blocks (objectstack `b3917d90`, the rename to `target`), so the entries no // longer named a key the spec declares and `every unpublished-key exemption - // names a key the spec really declares` went red on them. One is left. - ...owedEntries( - 'action:button', - ['undoable'], - 'objectui#11168', - 'A SPEC KEY HELD UNPUBLISHED AFTER MEASUREMENT (slice 1): the block forwards `undoable`, but the runner\'s `operation: update` path and the console runtime offer Undo only with a host `_rowRecord` stash this block never writes; only the record page\'s own `api` handler honours it.', - ), + // names a key the spec really declares` went red on them. The third, + // `action:button.undoable`, is STRUCK: ruling B on objectui#11754 (record + // 6030342264) had the block hand the runner the record in scope as the Undo + // baseline its `operation: update` path reads, and the key is DECLARED on + // the block's `inputs` + // (`packages/components/src/renderers/action/__tests__/action-button-undoable-11168.test.tsx`). + // It was objectui#11168's last entry, so every owner's count below is 0. // `action:group`'s `location` / `visible` and `action:menu`'s `size` / // `visible` stood here until objectui#11168 slice 1 measured each against its // renderer through the real `SchemaRenderer` and DECLARED all four — the @@ -6014,7 +6003,7 @@ describe('registry `inputs` vs `@objectstack/spec` ComponentPropsMap (repo-wide) ]), ), ).toEqual({ - 'objectui#11168': 1, + 'objectui#11168': 0, 'objectui#8652': 0, 'objectui#8649': 0, 'objectui#11536': 0, diff --git a/content/docs/guide/layout.md b/content/docs/guide/layout.md index 50eff212dd..d409f99565 100644 --- a/content/docs/guide/layout.md +++ b/content/docs/guide/layout.md @@ -684,6 +684,12 @@ registration's `inputs` lists, with a `description` per key. For an `api` action the request URL in `target`: `endpoint` is not published, because the console's `api` handler never reads it. +`undoable: true` on an `operation: update` makes the success toast offer Undo, which +writes back the prior values of the fields the update wrote. `action:button` reads those +values off the record in scope: the record page's record, or the row the host binds +through `data`. A button with no record in scope offers no Undo, because there is no row +to restore; neither does one whose `recordId` names a record other than the one in scope. + ## Responsive Behavior The shell has exactly **one** layout breakpoint, at **768px** — Tailwind's `md`, and diff --git a/packages/components/src/renderers/action/__tests__/action-button-icon-inputs-11168.test.tsx b/packages/components/src/renderers/action/__tests__/action-button-icon-inputs-11168.test.tsx index ccfde3856d..448ce88afb 100644 --- a/packages/components/src/renderers/action/__tests__/action-button-icon-inputs-11168.test.tsx +++ b/packages/components/src/renderers/action/__tests__/action-button-icon-inputs-11168.test.tsx @@ -31,12 +31,15 @@ * and `objectName` are the ones `scripts/check-action-forward-parity.mjs` * extracts from its runtime. * - * NOT published, and so not pinned here — each stays booked to objectui#11168 - * with its measurement on the card: `endpoint` on both blocks (the runner's - * built-in `api` executor reads it, the console's own `api` handler reads - * `target` and never `endpoint`) and `undoable` on `action:button` (the - * runner's update path offers Undo only with a host row stash this block never - * writes). + * Held back by slice 1, each with its measurement on the card: `endpoint` on + * both blocks (the runner's built-in `api` executor reads it, the console's own + * `api` handler reads `target` and never `endpoint`; 17.6.0 then refused the + * key in favour of `target`) and `undoable` on `action:button` (the runner's + * update path offered Undo only with a host row stash this block did not + * write). Ruling B on objectui#11754 made the block hand the runner the record + * in scope, and `undoable` is published on `action:button` since: its + * behaviour is pinned in `action-button-undoable-11168.test.tsx`, and its row + * in `DECLARED` below. * * Slice 2 added the `size` rows at the end: `action:button` publishes the five * sizes its spec row declares. The renderer hands `default`, `sm`, `lg` and @@ -85,7 +88,7 @@ const LEAF_KEYS = [ /** The keys each block now publishes, beyond the ones it published before. */ const DECLARED: Record = { - 'action:button': [...LEAF_KEYS, 'recordIdField'], + 'action:button': [...LEAF_KEYS, 'recordIdField', 'undoable'], 'action:icon': LEAF_KEYS, }; diff --git a/packages/components/src/renderers/action/__tests__/action-button-undoable-11168.test.tsx b/packages/components/src/renderers/action/__tests__/action-button-undoable-11168.test.tsx new file mode 100644 index 0000000000..7ce302c86a --- /dev/null +++ b/packages/components/src/renderers/action/__tests__/action-button-undoable-11168.test.tsx @@ -0,0 +1,302 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#11168 — `action:button.undoable` is delivered where a record is in + * scope, and published (ruling B on objectui#11754, record 6030342264). + * + * The runner's `operation: 'update'` path offers Undo only when the invoking + * surface hands it the record the update writes, under `params._rowRecord` + * (`ActionRunner.executeUpdateOperation`, which reads the prior value of every + * written field off it through `captureUpdateUndoData`). Before this change the + * block forwarded `undoable` and handed the runner no record, so a declared + * `undoable` offered no Undo anywhere the block ran. The block now attaches the + * record in scope: the row its host binds through `data`, else the record + * page's `RecordContext` record. + * + * Driven end to end through the real pieces: the real `SchemaRenderer` renders + * the real `action:button` (and, once, the real `action:bar`), the click goes + * through the real `ActionRunner`, and the `script` dispatch is the real + * `createServerActionHandler` the console builds its route dispatch on, over a + * stubbed `fetch`. What is asserted is what a user gets: whether the success + * toast offers Undo, and what the global undo stack would write back. + * + * Every "no Undo" row has a lit companion: the same node with the condition + * reversed offers Undo, so a red cannot come from a harness that never offers + * one. + */ + +import { describe, it, expect, vi, beforeEach, afterEach, type Mock } from 'vitest'; +import { render, screen, fireEvent, waitFor, cleanup } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import React from 'react'; +import { ComponentRegistry, createServerActionHandler, globalUndoManager } from '@object-ui/core'; +import type { ActionContext, ActionDef, ActionResult } from '@object-ui/core'; +import type { PublicBlockNodeOf } from '@object-ui/types'; +import { undeclaredNode } from '@object-ui/test-support'; +import { ActionProvider, RecordContextProvider, SchemaRenderer } from '@object-ui/react'; +import { ComponentPropsMap } from '@objectstack/spec/ui'; +import { manifestFromConfigs, validateTree } from '@object-ui/sdui-parser'; +import type { SchemaElement } from '@object-ui/sdui-parser'; +// Module-scope side-effect imports, so the renderers are registered before the +// first render: the light `dom` project does not load the components graph. +// Module scope, not a `beforeAll`, per AGENTS.md 测试纪律. +import '../action-button'; +import '../action-bar'; + +/** The record the action runs on, as the page or the row carries it. */ +const RECORD = { id: 't1', status: 'open', title: 'Write the report' }; + +type Props = NonNullable['properties']>; + +/** One `action:button` node, its props in the `properties` bag as the spec places them. */ +const button = (extra: Partial = {}): PublicBlockNodeOf<'action:button'> => ({ + type: 'action:button', + properties: { + name: 'close_task', + label: 'Close', + operation: 'update', + patch: { status: 'done' }, + undoable: true, + ...extra, + }, +}); + +let toast: Mock<(message: string, options?: { type?: string; undo?: { label?: string } }) => void>; +let fetchSpy: Mock<(url: string, init?: RequestInit) => Promise>; +/** Every def the `script` dispatch was handed, before it POSTs. */ +let dispatched: ActionDef[]; +let script: (action: ActionDef, ctx?: ActionContext) => Promise; + +beforeEach(() => { + toast = vi.fn(); + fetchSpy = vi.fn(async () => + new Response(JSON.stringify({ success: true, data: {} }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }), + ); + dispatched = []; + const route = createServerActionHandler({ fetch: fetchSpy as never, resolveObject: () => 'task' }); + script = async (action, ctx) => { + dispatched.push(action); + return route(action, ctx); + }; + globalUndoManager.clear(); +}); + +afterEach(() => { + cleanup(); + globalUndoManager.clear(); +}); + +/** The host's runner, publishing its object the way every console host does (`context.objectName`). */ +function Runner({ children }: { children: React.ReactNode }) { + return ( + ({ status: 'blocked' })) as never} + > + {children} + + ); +} + +/** A node `SchemaRenderer` takes. */ +type SchemaNodeInput = React.ComponentProps['schema']; + +/** On a record page: the record comes from `RecordContext`, and the node gets no `data`. */ +const onRecordPage = (node: SchemaNodeInput, record: Record = RECORD) => + render( + + + + + , + ); + +/** On a row: the host binds the row through `data`, as a table cell or `DetailView`'s header does. */ +const onRow = (node: SchemaNodeInput, row: Record = RECORD) => + render( + + + , + ); + +/** With no record anywhere. */ +const standalone = (node: SchemaNodeInput) => + render( + + + , + ); + +/** Click `label` and wait for the runner to report back through the toast. */ +async function press(label = 'Close') { + fireEvent.click(screen.getByRole('button', { name: label })); + await waitFor(() => expect(toast).toHaveBeenCalled()); +} + +/** The success toast's options; `undo` is set exactly when the toast offers Undo. */ +const successToast = () => toast.mock.calls.find(([, options]) => options?.type === 'success')?.[1]; + +/** The last def the dispatch was handed carried the record stash. */ +const carriedStash = () => + Object.prototype.hasOwnProperty.call((dispatched.at(-1)?.params ?? {}) as object, '_rowRecord'); + +/** The request the route was sent. */ +const sentBody = () => JSON.parse(String(fetchSpy.mock.calls.at(-1)?.[1]?.body)); + +describe('action:button `undoable` — an update of the record in scope offers Undo with its prior values', () => { + it('on a record page, from the page record', async () => { + onRecordPage(button()); + await press(); + expect(successToast()?.undo).toBeDefined(); + expect(globalUndoManager.peekUndo()).toMatchObject({ + type: 'update', + objectName: 'task', + recordId: 't1', + undoData: { status: 'open' }, + redoData: { status: 'done' }, + }); + // The write addressed that record, and the stash stayed on the client. + expect(sentBody()).toEqual({ recordId: 't1', params: { status: 'done' } }); + }); + + it('on a row the host binds, from the row', async () => { + onRow(button(), { id: 'r9', status: 'waiting' }); + await press(); + expect(successToast()?.undo).toBeDefined(); + expect(globalUndoManager.peekUndo()).toMatchObject({ recordId: 'r9', undoData: { status: 'waiting' } }); + }); + + it('the row the host binds outranks the page record around it', async () => { + render( + + + + + , + ); + await press(); + expect(globalUndoManager.peekUndo()).toMatchObject({ recordId: 'r1', undoData: { status: 'row' } }); + }); + + it('an `action:bar` member on a record page, which the bar draws through this block', async () => { + onRecordPage( + undeclaredNode({ + type: 'action:bar', + actions: [ + { name: 'close_task', label: 'Close', operation: 'update', patch: { status: 'done' }, undoable: true }, + ], + }), + ); + await press(); + expect(globalUndoManager.peekUndo()).toMatchObject({ recordId: 't1', undoData: { status: 'open' } }); + }); + + it('a value the user supplies is restored too, from the same record', async () => { + // The input list collects `status`; the collected value is written, so its + // prior value is what Undo restores. + onRecordPage(button({ patch: undefined, params: [{ name: 'status', type: 'text', label: 'Status' }] })); + await press(); + expect(globalUndoManager.peekUndo()).toMatchObject({ + undoData: { status: 'open' }, + redoData: { status: 'blocked' }, + }); + }); +}); + +describe('action:button `undoable` — the one limit: no record in scope, no Undo', () => { + it('a standalone button runs the update and offers no Undo, with no error', async () => { + standalone(button()); + await press(); + expect(successToast()).toBeDefined(); + expect(successToast()?.undo).toBeUndefined(); + expect(toast.mock.calls.some(([, options]) => options?.type === 'error')).toBe(false); + expect(globalUndoManager.undoCount).toBe(0); + }); + + it('a button that writes ANOTHER record gets no Undo from the record in scope', async () => { + onRecordPage(button({ params: { recordId: 'elsewhere' } })); + await press(); + expect(sentBody().recordId).toBe('elsewhere'); + expect(successToast()?.undo).toBeUndefined(); + expect(carriedStash()).toBe(false); + cleanup(); + toast.mockClear(); + // Lit: the same write addressed to the record in scope, as authors write it. + onRecordPage(button({ params: { recordId: '${record.id}' } })); + await press(); + expect(sentBody().recordId).toBe('t1'); + expect(successToast()?.undo).toBeDefined(); + }); + + it('a record that does not carry a written field offers no Undo (the runner\'s rule, unchanged)', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + onRecordPage(button(), { id: 't1', title: 'no status here' }); + await press(); + expect(successToast()?.undo).toBeUndefined(); + expect(carriedStash()).toBe(true); + } finally { + warn.mockRestore(); + } + }); +}); + +describe('action:button `undoable` — only an `undoable` update is handed the record', () => { + it('an update that is not `undoable` offers no Undo and carries no record', async () => { + onRecordPage(button({ undoable: false })); + await press(); + expect(successToast()?.undo).toBeUndefined(); + expect(carriedStash()).toBe(false); + cleanup(); + toast.mockClear(); + // Lit: the same node, `undoable`. + onRecordPage(button()); + await press(); + expect(carriedStash()).toBe(true); + }); + + it('an `undoable` action that is not an update carries no record', async () => { + onRecordPage(button({ operation: undefined, patch: undefined, actionType: 'script' })); + await press(); + expect(dispatched).toHaveLength(1); + expect(carriedStash()).toBe(false); + }); +}); + +describe('action:button `undoable` — published', () => { + it('is a published boolean input that the installed spec row declares', () => { + const input = (ComponentRegistry.getConfig('action:button')?.inputs ?? []).find((i) => i.name === 'undoable'); + expect(input?.type).toBe('boolean'); + const row = (ComponentPropsMap as unknown as Record }>)['action:button']; + expect(Object.keys(row?.shape ?? {})).toContain('undoable'); + }); + + it('the page validator no longer reports `undoable` as an unknown prop', () => { + const diagnostics = validateTree( + { type: 'action:button', undoable: true } as unknown as SchemaElement, + manifestFromConfigs( + ComponentRegistry.getAllConfigs() as unknown as Parameters[0], + ), + ).diagnostics; + expect(diagnostics.filter((d) => d.code === 'unknown-prop').map((d) => d.message)).toEqual([]); + // Lit: a key the block does not publish is still reported. + const control = validateTree( + { type: 'action:button', notAKey: true } as unknown as SchemaElement, + manifestFromConfigs( + ComponentRegistry.getAllConfigs() as unknown as Parameters[0], + ), + ).diagnostics; + expect(control.some((d) => d.code === 'unknown-prop')).toBe(true); + }); +}); diff --git a/packages/components/src/renderers/action/__tests__/action-forward-parity.test.tsx b/packages/components/src/renderers/action/__tests__/action-forward-parity.test.tsx index 2299de4b18..99b75bbecd 100644 --- a/packages/components/src/renderers/action/__tests__/action-forward-parity.test.tsx +++ b/packages/components/src/renderers/action/__tests__/action-forward-parity.test.tsx @@ -42,13 +42,16 @@ * * ## Reachability, deliberately NOT pinned here * - * `undoable` and `recordIdField` are absent from these payloads on purpose, and - * asserting they arrive would pin a fiction. Both are read only under a - * `rowRecord` guard, and `rowRecord` is `params._rowRecord` — written by the - * spread-based hosts (`DeclaredActionsBar`, `RelatedRecordActionsBridge`, - * `ObjectGrid`, `page:header`), never by these renderers. `action:button` - * forwards them INERTLY on this path; the menu omitting them costs nothing. - * That verdict is carried, with its evidence, in the gate's JUSTIFIED table. + * `undoable` and `recordIdField` are absent from these payloads on purpose. + * Both are read only under a `rowRecord` guard, and `rowRecord` is + * `params._rowRecord` — written by the spread-based hosts + * (`DeclaredActionsBar`, `RelatedRecordActionsBridge`, `ObjectGrid`, + * `page:header`) and, since objectui#11168, by `action:button` itself for an + * `undoable` `operation: 'update'` of the record in scope (pinned in + * `action-button-undoable-11168.test.tsx`). The menu, the group and the icon + * write it nowhere, so on them both keys stay unreachable and omitting them + * costs nothing. That verdict is carried, with its evidence, in the gate's + * JUSTIFIED table. */ import { describe, it, expect, vi, beforeEach, type Mock } from 'vitest'; diff --git a/packages/components/src/renderers/action/action-button.tsx b/packages/components/src/renderers/action/action-button.tsx index 174cc44bb6..354e1a84dc 100644 --- a/packages/components/src/renderers/action/action-button.tsx +++ b/packages/components/src/renderers/action/action-button.tsx @@ -21,7 +21,7 @@ import React, { forwardRef, useCallback, useState } from 'react'; import { ComponentRegistry } from '@object-ui/core'; import type { ActionDef } from '@object-ui/core'; import type { UIActionSchema } from '@object-ui/types'; -import { useAction } from '@object-ui/react'; +import { useAction, useRecordContext } from '@object-ui/react'; import { useCondition, toPredicateInput, usePredicateRecordContext } from '@object-ui/react'; import { Button } from '../../ui'; import { cn } from '../../lib/utils'; @@ -33,6 +33,53 @@ import { useAutoTriggerOnce } from './auto-trigger'; import { readStaticParamValues } from './static-params'; import { DisabledReasonTrigger, describedByWithReason, useDisabledReason } from './disabled-reason'; +/** A record value: a plain object, never `null` or an array. */ +function asRecord(value: unknown): Record | undefined { + return value != null && typeof value === 'object' && !Array.isArray(value) + ? (value as Record) + : undefined; +} + +/** + * The static values this button hands the runner, with the record in scope + * attached as the Undo baseline when the action is an `undoable` update of that + * record (objectui#11168, ruling B on objectui#11754, record 6030342264). + * + * The runner's `operation: 'update'` path offers Undo only when the invoking + * surface hands it the record the update writes, under `params._rowRecord`: it + * reads the prior value of every written field off that record + * (`captureUpdateUndoData`). The record page's declared-actions bar, the + * related-record bridge, `page:header` and the grid's rows already hand it one. + * This button handed it nothing, so a declared `undoable` was dropped one hop + * before the runner. This is the same spelling those hosts use; the dispatch + * strips the stash before it POSTs. + * + * Attached only when all of these hold, and otherwise `values` is returned + * as it came, the same object: + * + * - the action declares `undoable` and `operation: 'update'`, the path the + * ruling names. On an `api` action the console's handler reads the stash for + * more than Undo (it fills `{field}` tokens in the URL and seeds + * `recordIdParam`), and this ruling does not change that path. + * - a record is in scope. + * - the update writes THAT record. The id the dispatch resolves (an explicit + * `recordId` value, else the record's `recordIdField`, `id` by default) must + * be the record's own `id`, because the runner keys the Undo by + * `recordId ?? record.id`. A button in a record's scope that writes another + * record would otherwise get the scoped record's values as its Undo, and an + * Undo that restores the wrong values is worse than none. + */ +function withUndoBaseline( + schema: Pick, + values: Record | undefined, + record: Record | undefined, +): Record | undefined { + if (!schema.undoable || schema.operation !== 'update' || !record) return values; + const writtenId = values?.recordId ?? record[schema.recordIdField || 'id']; + if (writtenId == null || record.id == null || String(writtenId) !== String(record.id)) return values; + return { ...values, _rowRecord: record }; +} + /** * The declared props. `schema` is `UIActionSchema` (objectui#4418): every key * this renderer forwards below — `target`, `endpoint`, `bodyExtra`, @@ -115,6 +162,15 @@ const ActionButtonRenderer = forwardRef< // the fail-closed `visible` below turned that into "hidden". const recordData = usePredicateRecordContext(data); + // The record in scope for an `undoable` update's Undo baseline + // (objectui#11168, see `withUndoBaseline`): the row the host binds through + // `data` (a table's row, `DetailView`'s header, an `action:bar` member), + // else the record page's own record. An authored record page renders this + // node through `SchemaRenderer` with no `data`; its record is the + // `RecordContext` one, the record `${record.*}` in `properties` reads. + const recordContext = useRecordContext(); + const recordInScope = asRecord(data) ?? asRecord(recordContext?.data); + // Evaluate visibility and disabled conditions with record data context. // `visible` fails CLOSED on a throwing predicate (mirrors ActionEngine's // getActionsForLocation) — a precondition that can't be evaluated should @@ -185,7 +241,14 @@ const ActionButtonRenderer = forwardRef< // // The two channels are independent, so the input-list branch forwards // the static values too. - const staticValues = readStaticParamValues(schema, 'action:button'); + // + // An `undoable` update of the record in scope also carries that record + // as its Undo baseline (objectui#11168); see `withUndoBaseline`. + const staticValues = withUndoBaseline( + schema, + readStaticParamValues(schema, 'action:button'), + recordInScope, + ); const paramsPayload: ActionDef = Array.isArray(schema.params) ? { actionParams: schema.params as any, params: staticValues } : { params: staticValues }; @@ -330,7 +393,7 @@ const ActionButtonRenderer = forwardRef< } finally { setLoading(false); } - }, [schema, execute, loading, localContext]); + }, [schema, execute, loading, localContext, recordInScope]); // Client-side auto-trigger (#844): a caller (e.g. a welcome-page CTA that // deep-links into "create") can mark an action `autoTrigger: true` to run @@ -427,11 +490,14 @@ ComponentRegistry.register('button', ActionButtonRenderer, { // declare (and its renderer does not forward); the two are kept literal so // the source readers that census registrations can still name every entry. // - // Two spec keys stay unpublished, each with its measurement on objectui#11168: - // `endpoint` (the runner's built-in `api` executor reads it, but the console - // registers its own `api` handler, which reads `target` and never `endpoint`) - // and `undoable` (the runner's update path offers Undo only with a host row - // stash this block never writes). + // `undoable` was held back by slice 1 with its measurement: the runner's + // update path offers Undo only when the invoking surface hands it the + // record it writes, and this block handed it none. Ruling B on objectui#11754 + // (record 6030342264) made the block deliver it: an `undoable` update of the + // record in scope now carries that record as its Undo baseline + // (`withUndoBaseline` above), and the key is published at the end of this + // list. Pinned in `__tests__/action-button-undoable-11168.test.tsx`. + // `endpoint` is not on this row: 17.6.0 refuses it in favour of `target`. // // `outcomeMessages` is FORWARDED above but not published here // (objectui#11344): it is an `ActionSchema` key, which reaches this renderer @@ -583,6 +649,12 @@ ComponentRegistry.register('button', ActionButtonRenderer, { description: 'For a `script` action run against a single selected row: the row field whose value is sent as the record id (default `id`)', }, + { + name: 'undoable', + type: 'boolean', + description: + 'Offer an Undo affordance after an update action: once an `operation: update` succeeds, its success toast offers Undo, which writes back the values the fields it wrote held on the record in scope (the record page\'s record, or the row the host binds), provided that record carries each of them. The one limit: a button with no record in scope (standalone, or writing a record other than the one in scope) offers no Undo, because there is no row to restore', + }, ], defaultProps: { label: 'Action', diff --git a/packages/types/src/ui-action.ts b/packages/types/src/ui-action.ts index f388a0ba87..442bc6975e 100644 --- a/packages/types/src/ui-action.ts +++ b/packages/types/src/ui-action.ts @@ -788,11 +788,14 @@ export interface UIActionSchema { * along; only this read side was missing, so the forward was typed `any` in * both directions. * - * Reachability is unchanged and is not what this declares: the runtime reads - * the key only under a `rowRecord` guard that `action:button` never seeds, - * which `check:action-forward-parity`'s JUSTIFIED table records for the three - * sibling surfaces. Declaring the key does not make it reachable; it makes - * the forward compiler-checked. + * Reachability is not what this declares: the runtime reads the key only + * under a `rowRecord` guard, the record the invoking surface hands it. Since + * objectui#11168 (ruling B on objectui#11754) `action:button` hands it the + * record in scope for an `undoable` `operation: 'update'` of that record, so + * on that block the key is reachable. `action:icon`, `action:group` + * and `action:menu` hand it none, which `check:action-forward-parity`'s + * JUSTIFIED table records. Declaring the key makes the forward + * compiler-checked. */ undoable?: SpecAction['undoable']; diff --git a/scripts/check-action-forward-parity.mjs b/scripts/check-action-forward-parity.mjs index a3eb53e196..cb4f078d65 100644 --- a/scripts/check-action-forward-parity.mjs +++ b/scripts/check-action-forward-parity.mjs @@ -234,6 +234,14 @@ export const JUSTIFIED = { // and `_rowRecord` is a host stash — `isAuthoredParamKey` excludes it by its // `_` prefix (packages/core/src/actions/actionKeys.ts:326-327). So the guard // cannot hold on this path and the key is unreachable, not dropped. + // + // One exception since objectui#11168 (ruling B on objectui#11754): + // `action:button` itself writes the stash, for an `undoable` + // `operation: 'update'` of the record in scope only. That action dispatches + // to the `script` route, never to the console `api` handler that seeds + // `recordIdParam`, so `action:button:recordIdParam` below stays unreachable; + // `undoable` and `recordIdField` are forwarded by `action:button` and need no + // entry for it. ...Object.fromEntries( [ ["recordIdParam", "action:button", "action:icon", "action:group", "action:menu"], @@ -249,12 +257,14 @@ export const JUSTIFIED = { "row (`resolveRecordIdParamSeed(action, rowRecord)`, which is where " + "`recordIdField` is read since objectstack#8018), and reads " + "`action.undoable && obj && recId && rowRecord && …` — and `rowRecord` is " + - "`params._rowRecord`, written only by the spread-based hosts listed above, " + - "none of which dispatch through this renderer. objectstack#6938 made the same " + + "`params._rowRecord`, written by the spread-based hosts listed above, none of " + + "which dispatch through this renderer, and by `action:button` for an `undoable` " + + "`operation: 'update'` alone (objectui#11168), a path that never reaches the " + + "console `api` handler. objectstack#6938 made the same " + "reachability call for `recordIdParam`; this gate's own measurement extended it " + "to the two siblings behind the same guard (objectui#4192 had read the omission " + - "as a live Undo loss — it is not: `action:button` forwards `undoable` and " + - "`recordIdField` on this path INERTLY, for want of the same `rowRecord`).", + "as a live Undo loss — it is not on these surfaces, and `action:button` forwards " + + "both).", issue: 4192, }, ])