From 360c80a4e521dad47a6ba652d70034b5055de73c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 09:58:54 +0000 Subject: [PATCH 1/2] fix(plugin-form): honour the declared submitHandler seam in every form variant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ObjectFormSchema.submitHandler` is documented as the seam a host uses to own persistence: the form validates and hands the collected values over INSTEAD of calling dataSource.create / dataSource.update. `ObjectForm` forwards the key into every variant it routes to, but only `SimpleObjectForm` ever read it. TabbedForm, WizardForm, SplitForm, DrawerForm and ModalForm called dataSource.create directly. MasterDetailForm supplies `submitHandler: submitViaBatch` so the parent and its child collections commit as ONE batchTransaction (#2679 / ADR-0034 item 4). With the parent half rendered `tabbed`, the measured reading was `batchTransaction 0 / create 1` with args ["po", {"ref":"PO-1"}] — and the child leg was never attempted at all: the parent committed alone, the entered line items were discarded, no compensation ran, and a success toast confirmed it. `split` measured identically. Each variant now checks `schema.submitHandler` first, with the same precedence SimpleObjectForm uses, and declares the key on its own schema interface. The write payload is hoisted to one `writePayload` per handler so the host-owned route and the direct route cannot diverge. WizardForm additionally guards its default success arms with `!schema.submitHandler`, mirroring ObjectForm, so a host that owns the write also owns the outcome. The `object-master-detail-form.formType` vocabulary is deliberately unchanged and stays `simple | tabbed`. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Mn4BZ5AVDM81pvfij1WwM9 --- packages/plugin-form/src/DrawerForm.tsx | 41 ++- packages/plugin-form/src/ModalForm.tsx | 42 ++- packages/plugin-form/src/SplitForm.tsx | 43 ++- packages/plugin-form/src/TabbedForm.tsx | 43 ++- packages/plugin-form/src/WizardForm.tsx | 52 +++- .../masterDetailFormTypeVocabulary.test.tsx | 24 +- .../src/submitHandlerSeam.test.tsx | 271 ++++++++++++++++++ 7 files changed, 465 insertions(+), 51 deletions(-) create mode 100644 packages/plugin-form/src/submitHandlerSeam.test.tsx diff --git a/packages/plugin-form/src/DrawerForm.tsx b/packages/plugin-form/src/DrawerForm.tsx index 79f20f74c3..82095600c0 100644 --- a/packages/plugin-form/src/DrawerForm.tsx +++ b/packages/plugin-form/src/DrawerForm.tsx @@ -156,6 +156,20 @@ export interface DrawerFormSchema { readOnly?: boolean; layout?: 'vertical' | 'horizontal'; columns?: number; + /** + * Override persistence — the seam a host uses to own the write. Mirrors + * `ObjectFormSchema.submitHandler`, the key `ObjectForm` forwards into this + * variant. When supplied, the form validates and hands the collected values + * to this handler INSTEAD of calling `dataSource.create` / + * `dataSource.update`; the returned record is passed on to `onSuccess`. + * + * `MasterDetailForm` supplies it to route the parent AND its child + * collections through one atomic `batchTransaction` (#2679 / ADR-0034 + * item 4). A renderer that does not read it writes the parent on its own and + * escapes that transaction — objectui#6176. + */ + submitHandler?: (values: Record) => any | Promise; + onSuccess?: (data: any) => void | Promise; onError?: (error: Error) => void; onCancel?: () => void; @@ -394,14 +408,25 @@ export const DrawerForm: React.FC = ({ let result; const payload = sanitizeFormData(data, objectSchema); - if (schema.mode === 'create') { - // Omit the fields the producer owns (#4069) — see - // `omitServerResolvedDefaults` for why an empty key is not the same as - // no key at insert time. - result = await dataSource.create( - schema.objectName, - omitServerResolvedDefaults(payload, objectSchema), - ); + // Omit the fields the producer owns (#4069) — see + // `omitServerResolvedDefaults` for why an empty key is not the same as + // no key at insert time. Create only: on an edit form a cleared column is + // a real removal. Computed ONCE so every persistence route below — the + // host-owned seam included — writes the identical payload. + const writePayload = schema.mode === 'create' + ? omitServerResolvedDefaults(payload, objectSchema) + : payload; + + if (schema.submitHandler) { + // The host owns persistence (e.g. MasterDetailForm batching the parent + // + its child collections into ONE atomic transaction). The form + // validates and hands the values over; it does NOT create/update + // itself. Same seam and same precedence as SimpleObjectForm — every + // renderer `ObjectForm` routes to must check it FIRST, or a declared + // host-owned write silently becomes an independent one (objectui#6176). + result = await schema.submitHandler(writePayload); + } else if (schema.mode === 'create') { + result = await dataSource.create(schema.objectName, writePayload); } else if (schema.mode === 'edit' && schema.recordId) { // OCC-guarded: sends `ifMatch` from the record we read; a 409 asks the // user to keep editing (drawer stays open, draft intact) or overwrite. diff --git a/packages/plugin-form/src/ModalForm.tsx b/packages/plugin-form/src/ModalForm.tsx index d8d7999a84..109b260738 100644 --- a/packages/plugin-form/src/ModalForm.tsx +++ b/packages/plugin-form/src/ModalForm.tsx @@ -152,6 +152,20 @@ export interface ModalFormSchema { readOnly?: boolean; layout?: 'vertical' | 'horizontal'; columns?: number; + /** + * Override persistence — the seam a host uses to own the write. Mirrors + * `ObjectFormSchema.submitHandler`, the key `ObjectForm` forwards into this + * variant. When supplied, the form validates and hands the collected values + * to this handler INSTEAD of calling `dataSource.create` / + * `dataSource.update`; the returned record is passed on to `onSuccess`. + * + * `MasterDetailForm` supplies it to route the parent AND its child + * collections through one atomic `batchTransaction` (#2679 / ADR-0034 + * item 4). A renderer that does not read it writes the parent on its own and + * escapes that transaction — objectui#6176. + */ + submitHandler?: (values: Record) => any | Promise; + onSuccess?: (data: any) => void | Promise; onError?: (error: Error) => void; onCancel?: () => void; @@ -446,14 +460,26 @@ export const ModalForm: React.FC = ({ } payload = stripped; } - if (schema.mode === 'create') { - // Omit the fields the producer owns (#4069) — see - // `omitServerResolvedDefaults` for why an empty key is not the same as - // no key at insert time. - result = await dataSource.create( - schema.objectName, - omitServerResolvedDefaults(payload, objectSchema), - ); + // Omit the fields the producer owns (#4069) — see + // `omitServerResolvedDefaults` for why an empty key is not the same as + // no key at insert time. Create only: on an edit form a cleared column is + // a real removal. Computed ONCE (after the FLS strip above) so every + // persistence route below — the host-owned seam included — writes the + // identical payload. + const writePayload = schema.mode === 'create' + ? omitServerResolvedDefaults(payload, objectSchema) + : payload; + + if (schema.submitHandler) { + // The host owns persistence (e.g. MasterDetailForm batching the parent + // + its child collections into ONE atomic transaction). The form + // validates and hands the values over; it does NOT create/update + // itself. Same seam and same precedence as SimpleObjectForm — every + // renderer `ObjectForm` routes to must check it FIRST, or a declared + // host-owned write silently becomes an independent one (objectui#6176). + result = await schema.submitHandler(writePayload); + } else if (schema.mode === 'create') { + result = await dataSource.create(schema.objectName, writePayload); } else if (schema.mode === 'edit' && schema.recordId) { // OCC-guarded: sends `ifMatch` from the record we read; a 409 asks the // user to keep editing (modal stays open, draft intact) or overwrite. diff --git a/packages/plugin-form/src/SplitForm.tsx b/packages/plugin-form/src/SplitForm.tsx index 393416d6d5..d8dbd83930 100644 --- a/packages/plugin-form/src/SplitForm.tsx +++ b/packages/plugin-form/src/SplitForm.tsx @@ -105,6 +105,20 @@ export interface SplitFormSchema { initialValues?: Record; initialData?: Record; readOnly?: boolean; + /** + * Override persistence — the seam a host uses to own the write. Mirrors + * `ObjectFormSchema.submitHandler`, the key `ObjectForm` forwards into this + * variant. When supplied, the form validates and hands the collected values + * to this handler INSTEAD of calling `dataSource.create` / + * `dataSource.update`; the returned record is passed on to `onSuccess`. + * + * `MasterDetailForm` supplies it to route the parent AND its child + * collections through one atomic `batchTransaction` (#2679 / ADR-0034 + * item 4). A renderer that does not read it writes the parent on its own and + * escapes that transaction — objectui#6176. + */ + submitHandler?: (values: Record) => any | Promise; + onSuccess?: (data: any) => void | Promise; onError?: (error: Error) => void; onCancel?: () => void; @@ -232,14 +246,25 @@ export const SplitForm: React.FC = ({ try { let result; - if (schema.mode === 'create') { - // Omit the fields the producer owns (#4069) — see - // `omitServerResolvedDefaults` for why an empty key is not the same as - // no key at insert time. - result = await dataSource.create( - schema.objectName, - omitServerResolvedDefaults(data, objectSchema), - ); + // Omit the fields the producer owns (#4069) — see + // `omitServerResolvedDefaults` for why an empty key is not the same as + // no key at insert time. Create only: on an edit form a cleared column is + // a real removal. Computed ONCE so every persistence route below — the + // host-owned seam included — writes the identical payload. + const writePayload = schema.mode === 'create' + ? omitServerResolvedDefaults(data, objectSchema) + : data; + + if (schema.submitHandler) { + // The host owns persistence (e.g. MasterDetailForm batching the parent + // + its child collections into ONE atomic transaction). The form + // validates and hands the values over; it does NOT create/update + // itself. Same seam and same precedence as SimpleObjectForm — every + // renderer `ObjectForm` routes to must check it FIRST, or a declared + // host-owned write silently becomes an independent one (objectui#6176). + result = await schema.submitHandler(writePayload); + } else if (schema.mode === 'create') { + result = await dataSource.create(schema.objectName, writePayload); } else if (schema.mode === 'edit' && schema.recordId) { // OCC-guarded: sends `ifMatch` from the record we read; a 409 asks the // user to keep editing (skip the success path) or overwrite. @@ -247,7 +272,7 @@ export const SplitForm: React.FC = ({ dataSource, objectName: schema.objectName, recordId: schema.recordId, - payload: data, + payload: writePayload, baseRecord: formData, }); if (outcome.status === 'cancelled') return; diff --git a/packages/plugin-form/src/TabbedForm.tsx b/packages/plugin-form/src/TabbedForm.tsx index 4f173c02f3..d5fd378be0 100644 --- a/packages/plugin-form/src/TabbedForm.tsx +++ b/packages/plugin-form/src/TabbedForm.tsx @@ -145,6 +145,20 @@ export interface TabbedFormSchema { */ readOnly?: boolean; + /** + * Override persistence — the seam a host uses to own the write. Mirrors + * `ObjectFormSchema.submitHandler`, the key `ObjectForm` forwards into this + * variant. When supplied, the form validates and hands the collected values + * to this handler INSTEAD of calling `dataSource.create` / + * `dataSource.update`; the returned record is passed on to `onSuccess`. + * + * `MasterDetailForm` supplies it to route the parent AND its child + * collections through one atomic `batchTransaction` (#2679 / ADR-0034 + * item 4). A renderer that does not read it writes the parent on its own and + * escapes that transaction — objectui#6176. + */ + submitHandler?: (values: Record) => any | Promise; + /** * Callbacks */ @@ -302,14 +316,25 @@ export const TabbedForm: React.FC = ({ try { let result; - if (schema.mode === 'create') { - // Omit the fields the producer owns (#4069) — see - // `omitServerResolvedDefaults` for why an empty key is not the same as - // no key at insert time. - result = await dataSource.create( - schema.objectName, - omitServerResolvedDefaults(data, objectSchema), - ); + // Omit the fields the producer owns (#4069) — see + // `omitServerResolvedDefaults` for why an empty key is not the same as + // no key at insert time. Create only: on an edit form a cleared column is + // a real removal. Computed ONCE so every persistence route below — the + // host-owned seam included — writes the identical payload. + const writePayload = schema.mode === 'create' + ? omitServerResolvedDefaults(data, objectSchema) + : data; + + if (schema.submitHandler) { + // The host owns persistence (e.g. MasterDetailForm batching the parent + // + its child collections into ONE atomic transaction). The form + // validates and hands the values over; it does NOT create/update + // itself. Same seam and same precedence as SimpleObjectForm — every + // renderer `ObjectForm` routes to must check it FIRST, or a declared + // host-owned write silently becomes an independent one (objectui#6176). + result = await schema.submitHandler(writePayload); + } else if (schema.mode === 'create') { + result = await dataSource.create(schema.objectName, writePayload); } else if (schema.mode === 'edit' && schema.recordId) { // OCC-guarded: sends `ifMatch` from the record we read; a 409 asks the // user to keep editing (skip the success path) or overwrite. @@ -317,7 +342,7 @@ export const TabbedForm: React.FC = ({ dataSource, objectName: schema.objectName, recordId: schema.recordId, - payload: data, + payload: writePayload, baseRecord: formData, }); if (outcome.status === 'cancelled') return; diff --git a/packages/plugin-form/src/WizardForm.tsx b/packages/plugin-form/src/WizardForm.tsx index 4817f4c1d8..b3a96f8169 100644 --- a/packages/plugin-form/src/WizardForm.tsx +++ b/packages/plugin-form/src/WizardForm.tsx @@ -203,6 +203,20 @@ export interface WizardFormSchema { */ readOnly?: boolean; + /** + * Override persistence — the seam a host uses to own the write. Mirrors + * `ObjectFormSchema.submitHandler`, the key `ObjectForm` forwards into this + * variant. When supplied, the form validates and hands the collected values + * to this handler INSTEAD of calling `dataSource.create` / + * `dataSource.update`; the returned record is passed on to `onSuccess`. + * + * `MasterDetailForm` supplies it to route the parent AND its child + * collections through one atomic `batchTransaction` (#2679 / ADR-0034 + * item 4). A renderer that does not read it writes the parent on its own and + * escapes that transaction — objectui#6176. + */ + submitHandler?: (values: Record) => any | Promise; + /** * Callbacks */ @@ -550,14 +564,26 @@ export const WizardForm: React.FC = ({ } let result; - if (schema.mode === 'create') { - // Omit the fields the producer owns (#4069) — see - // `omitServerResolvedDefaults` for why an empty key is not the same - // as no key at insert time. - result = await dataSource.create( - schema.objectName, - omitServerResolvedDefaults(mergedData, objectSchema), - ); + // Omit the fields the producer owns (#4069) — see + // `omitServerResolvedDefaults` for why an empty key is not the same + // as no key at insert time. Create only: on an edit form a cleared + // column is a real removal. Computed ONCE so every persistence route + // below — the host-owned seam included — writes the identical payload. + const writePayload = schema.mode === 'create' + ? omitServerResolvedDefaults(mergedData, objectSchema) + : mergedData; + + if (schema.submitHandler) { + // The host owns persistence (e.g. MasterDetailForm batching the + // parent + its child collections into ONE atomic transaction). The + // form validates and hands the values over; it does NOT create/update + // itself. Same seam and same precedence as SimpleObjectForm — every + // renderer `ObjectForm` routes to must check it FIRST, or a declared + // host-owned write silently becomes an independent one + // (objectui#6176). + result = await schema.submitHandler(writePayload); + } else if (schema.mode === 'create') { + result = await dataSource.create(schema.objectName, writePayload); } else if (schema.mode === 'edit' && schema.recordId) { // OCC-guarded: sends `ifMatch` from the record we read; a 409 asks // the user to keep editing (skip the success path) or overwrite. @@ -567,7 +593,7 @@ export const WizardForm: React.FC = ({ dataSource, objectName: schema.objectName, recordId: schema.recordId, - payload: mergedData, + payload: writePayload, baseRecord: formData, }); if (outcome.status === 'cancelled') return; @@ -576,7 +602,7 @@ export const WizardForm: React.FC = ({ if (schema.onSuccess) { await schema.onSuccess(result); - } else if (schema.submitBehavior) { + } else if (!schema.submitHandler && schema.submitBehavior) { const behavior = schema.submitBehavior; switch (behavior.kind) { case 'redirect': { @@ -636,8 +662,12 @@ export const WizardForm: React.FC = ({ break; } } - } else { + } else if (!schema.submitHandler) { // Legacy declarative success behaviors for metadata-only wizards. + // Skipped when a `submitHandler` owns persistence: the host already + // reports the outcome, so a second toast/redirect here would + // double-confirm (the guard SimpleObjectForm applies for the same + // reason). const nav = resolveSuccessNavigate(schema.navigateOnSuccess, result); if (nav) { // Landing on the saved record is the confirmation — no toast needed. diff --git a/packages/plugin-form/src/masterDetailFormTypeVocabulary.test.tsx b/packages/plugin-form/src/masterDetailFormTypeVocabulary.test.tsx index 7ae18963b4..8f0fa0683b 100644 --- a/packages/plugin-form/src/masterDetailFormTypeVocabulary.test.tsx +++ b/packages/plugin-form/src/masterDetailFormTypeVocabulary.test.tsx @@ -36,6 +36,9 @@ * step's fields, and the master-detail's single bottom Save bar * drives the wizard's `Next` instead of saving. * `split` → ObjectForm.tsx:287 (SplitForm) — renders two panels inline. + * (It ALSO escaped the atomic batch when this file was written; + * objectui#6176 fixed that, so only the presentation difference + * is left. The set below is unchanged regardless.) * `drawer` → ObjectForm.tsx:316 (DrawerForm) — hosts the parent half in a * PORTAL dialog, outside the master-detail container, so the Save * bar has no `
` to submit. @@ -210,15 +213,24 @@ describe('the measurement the vocabulary is derived from', () => { expect(create).not.toHaveBeenCalled(); }); - it('`split` is excluded: it renders inline but persists AROUND the atomic batch', async () => { + it('`split` renders inline and now saves through the ATOMIC batch — its persistence rationale is spent (objectui#6176)', async () => { const { container, batchTransaction, create } = mountWith('split'); await waitFor(() => expect(container.querySelector('input[name="ref"]')).toBeTruthy()); expect(await clickHostSave(container)).toBe(true); - // `submitHandler` — the hook MasterDetailForm hands the parent form to route - // the save through `batchTransaction` — is read only by SimpleObjectForm - // (ObjectForm.tsx:820). SplitForm writes the parent directly instead. - await waitFor(() => expect(create).toHaveBeenCalledTimes(1)); - expect(batchTransaction).not.toHaveBeenCalled(); + // This row used to read `create 1 / batchTransaction 0`. `submitHandler` — + // the hook MasterDetailForm hands the parent form so the save routes + // through `batchTransaction` — was read only by SimpleObjectForm, and + // SplitForm wrote the parent directly instead. objectui#6176 made every + // renderer honour the declared seam, so this row now reads like `simple`. + await waitFor(() => expect(batchTransaction).toHaveBeenCalledTimes(1)); + expect(create).not.toHaveBeenCalled(); + // ⚠️ The VOCABULARY is deliberately unchanged and stays `simple | tabbed` + // (asserted above). Admitting `split` would be a contract change, and + // objectui#6176 — a declared-vs-enforced restoration — did not make it. + // What this case now records is narrower than before: the PERSISTENCE + // reason for excluding `split` no longer holds, so whatever keeps it out is + // presentational and is an open question for triage, not a fact this file + // may assert. }); it('the harm the closed vocabulary guards: an out-of-vocabulary value silently drops the sections', async () => { diff --git a/packages/plugin-form/src/submitHandlerSeam.test.tsx b/packages/plugin-form/src/submitHandlerSeam.test.tsx new file mode 100644 index 0000000000..d6aa40b738 --- /dev/null +++ b/packages/plugin-form/src/submitHandlerSeam.test.tsx @@ -0,0 +1,271 @@ +/** + * 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. + * + * `submitHandler` is the DECLARED seam a host uses to own persistence, and + * EVERY renderer `ObjectForm` routes to must honour it (objectui#6176). + * + * ## What went wrong + * + * `ObjectFormSchema.submitHandler` is documented as "the form validates and + * hands the collected values to the host INSTEAD of calling dataSource.create / + * dataSource.update". `ObjectForm` forwards the key into every variant it + * routes to — the `{...schema}` spread carries it — but only `SimpleObjectForm` + * ever read it. `TabbedForm`, `WizardForm`, `SplitForm`, `DrawerForm` and + * `ModalForm` called `dataSource.create` directly instead. + * + * That is not cosmetic. `MasterDetailForm` supplies `submitHandler: + * submitViaBatch` precisely so the parent AND its child collections commit as + * ONE `batchTransaction` (#2679 / ADR-0034 item 4). With the parent half + * rendered `tabbed`, the measured reading on `main` was: + * + * batchTransaction 0 · dataSource.create 1 · args ["po", {"ref":"PO-1"}] + * + * and the consequence is worse than a bypassed transaction: the child leg was + * never ATTEMPTED at all. The parent committed alone, the operator's entered + * line items were silently discarded, no compensation ran, and a SUCCESS toast + * confirmed it. `split` measured identically. + * + * ## What each block below pins + * + * 1. The data-integrity claim — a failing child leg must leave NO committed + * parent. This is the assertion that distinguishes the two worlds; "the + * parent was created" is true in both. + * 2. The seam itself, per renderer — all six, because a fix landing on four of + * five still passes a suite that only exercises four. + * 3. The master-detail composition reading the card measured. + * + * ## Scope, stated so it is not over-read + * + * This restores an implementation to a decision already taken; it does NOT + * change the `object-master-detail-form.formType` vocabulary, which stays + * `simple | tabbed` (objectui#5939). See `masterDetailFormTypeVocabulary.test.tsx` + * — one exclusion rationale there is updated because this fix falsifies it, but + * the SET is untouched. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, waitFor, fireEvent, screen } from '@testing-library/react'; +import React from 'react'; +import { registerAllFields } from '@object-ui/fields'; +import { MasterDetailForm } from './MasterDetailForm'; +import { ObjectForm } from './ObjectForm'; +import './index'; + +const toastSuccess = vi.fn(); +const toastError = vi.fn(); +vi.mock('@object-ui/components', async (orig) => { + const actual = await (orig as any)(); + return { + ...actual, + toast: { + success: (...a: any[]) => toastSuccess(...a), + error: (...a: any[]) => toastError(...a), + }, + }; +}); + +registerAllFields(); + +/** The full vocabulary `object-form` declares — every renderer `ObjectForm` routes to. */ +const OBJECT_FORM_SIX = ['simple', 'tabbed', 'wizard', 'split', 'drawer', 'modal'] as const; + +/** + * The variants that render the parent half INLINE, so the master-detail host's + * single Save bar can actually reach a ``. `wizard` (mounts one step at a + * time, and the Save bar drives its `Next`) and `drawer` / `modal` (parent half + * lands in a portal dialog) are excluded by LAYOUT, not by the seam — that + * exclusion is measured in `masterDetailFormTypeVocabulary.test.tsx`. + */ +const INLINE_PARENT_VARIANTS = ['simple', 'tabbed', 'split'] as const; + +const parentObject = { + name: 'po', + fields: { ref: { type: 'text', label: 'Ref' }, memo: { type: 'text', label: 'Memo' } }, +}; + +const masterDetailSchema = (formType: string) => ({ + objectName: 'po', + mode: 'create', + formType, + sections: [ + { name: 's1', label: 'Sec One', fields: ['ref'] }, + { name: 's2', label: 'Sec Two', fields: ['memo'] }, + ], + details: [ + { + childObject: 'po_line', + relationshipField: 'po', + columns: [{ name: 'qty', label: 'Qty', type: 'number' }], + }, + ], +}); + +/** + * Fill the parent header AND the first (ghost) line, so the save carries BOTH + * legs. Without a real child row the batch would be a single-op list and the + * atomicity claim would have nothing to be atomic about. + */ +async function fillHeaderAndLine(container: HTMLElement) { + const ref = await waitFor(() => { + const el = container.querySelector('input[name="ref"]') as HTMLInputElement | null; + if (!el) throw new Error('parent form not ready'); + return el; + }); + fireEvent.change(ref, { target: { value: 'PO-1' } }); + const qty = await waitFor(() => screen.getAllByLabelText('Qty')[0] as HTMLInputElement); + fireEvent.change(qty, { target: { value: '5' } }); +} + +const clickHostSave = () => + fireEvent.click(screen.getByRole('button', { name: /^create$/i })); + +beforeEach(() => { + toastSuccess.mockClear(); + toastError.mockClear(); +}); + +describe('a failing child leg leaves NO committed parent, whichever variant renders the parent half (objectui#6176)', () => { + // A dataSource with NO `batchTransaction`: `runBatchTransaction` falls back to + // `emulateBatchTransaction`, which drives the ops in order and COMPENSATES on + // failure. The compensating delete is the observable stand-in for a rollback, + // which is why this shape is used rather than a rejecting atomic stub — a + // rejecting stub can only show that nothing was written, never that an + // already-written parent gets removed. + it.each(INLINE_PARENT_VARIANTS)( + 'formType `%s`: the child create fails → the just-created parent is compensated away, and the operator is told', + async (formType) => { + const create = vi.fn(async (object: string, data: any) => { + if (object === 'po_line') throw new Error('child create failed'); + return { id: 'po1', ...data }; + }); + const del = vi.fn(async () => true); + const dataSource = { + getObjectSchema: vi.fn().mockResolvedValue(parentObject), + find: vi.fn().mockResolvedValue({ data: [] }), + create, + update: vi.fn(), + delete: del, + bulk: vi.fn(), + // deliberately NO batchTransaction + } as any; + + const { container } = render( + , + ); + await fillHeaderAndLine(container); + clickHostSave(); + + // POSITIVE PROBE — the child leg was actually attempted. Without this the + // assertions below pass vacuously on a form that never submitted at all, + // which is exactly how the broken `tabbed` path read: parent written, the + // child collection silently dropped, nothing to fail. + await waitFor(() => + expect(create).toHaveBeenCalledWith('po_line', expect.objectContaining({ po: 'po1' })), + ); + + // THE PIN — no committed parent survives the failed leg. + await waitFor(() => expect(del).toHaveBeenCalledWith('po', 'po1')); + // …and the failure is surfaced. On the broken path the operator got a + // SUCCESS toast over a half-written document. + await waitFor(() => expect(toastError).toHaveBeenCalled()); + expect(toastSuccess).not.toHaveBeenCalled(); + }, + ); +}); + +describe('every renderer ObjectForm routes to honours the declared `submitHandler` seam', () => { + // Mounted through `ObjectForm` itself — the component that OWNS the routing — + // so this exercises the same forwarding path a real host uses, not each + // variant in isolation. + it.each(OBJECT_FORM_SIX)( + 'formType `%s`: hands the values to the host and does NOT persist on its own', + async (formType) => { + const submitHandler = vi.fn().mockResolvedValue({ id: 'p1' }); + const create = vi.fn().mockResolvedValue({ id: 'p1' }); + const update = vi.fn().mockResolvedValue({ id: 'p1' }); + const dataSource = { + getObjectSchema: vi.fn().mockResolvedValue(parentObject), + find: vi.fn().mockResolvedValue({ data: [] }), + create, + update, + delete: vi.fn(), + bulk: vi.fn(), + } as any; + + render( + , + ); + + const ref = await waitFor(() => { + const el = document.querySelector('input[name="ref"]') as HTMLInputElement | null; + if (!el) throw new Error('form not ready'); + return el; + }); + fireEvent.change(ref, { target: { value: 'PO-1' } }); + fireEvent.click(screen.getByRole('button', { name: /save now/i })); + + // POSITIVE — the host was handed the collected values. A bare + // "create was not called" would also hold for a form that never submitted. + await waitFor(() => expect(submitHandler).toHaveBeenCalledTimes(1)); + expect(submitHandler).toHaveBeenCalledWith(expect.objectContaining({ ref: 'PO-1' })); + // NEGATIVE — the form did not write on its own behind the host's back. + expect(create).not.toHaveBeenCalled(); + expect(update).not.toHaveBeenCalled(); + }, + ); +}); + +describe('master-detail: the parent commits INSIDE the atomic batch, not beside it', () => { + // The card's own measurement, re-expressed as a pin. On `main` the `tabbed` + // and `split` rows read `batchTransaction 0 / create 1`. + it.each(INLINE_PARENT_VARIANTS)( + 'formType `%s`: one batchTransaction carrying BOTH legs, and zero independent creates', + async (formType) => { + const batchTransaction = vi.fn().mockResolvedValue({ results: [{ id: 'po1' }] }); + const create = vi.fn().mockResolvedValue({ id: 'po1' }); + const dataSource = { + getObjectSchema: vi.fn().mockResolvedValue(parentObject), + find: vi.fn().mockResolvedValue({ data: [] }), + create, + update: vi.fn(), + delete: vi.fn(), + bulk: vi.fn(), + batchTransaction, + } as any; + + const { container } = render( + , + ); + await fillHeaderAndLine(container); + clickHostSave(); + + await waitFor(() => expect(batchTransaction).toHaveBeenCalledTimes(1)); + const ops = batchTransaction.mock.calls[0][0]; + // BOTH legs in one operation list — the parent at index 0 and the child + // pointing at it through `$ref: 0`, which is what makes the commit atomic. + expect(ops).toHaveLength(2); + expect(ops[0]).toMatchObject({ object: 'po', action: 'create', data: { ref: 'PO-1' } }); + expect(ops[1]).toMatchObject({ object: 'po_line', action: 'create', data: { po: { $ref: 0 } } }); + // The escape this card closed. + expect(create).not.toHaveBeenCalled(); + }, + ); +}); From 403488be26c463f058e77ee10155de0d612dc7e0 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 10:07:07 +0000 Subject: [PATCH 2/2] refactor(plugin-form): declare the variant submitHandler key by reference, add changeset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The five variant schemas restated `submitHandler`'s signature verbatim. Declaring it as `ObjectFormSchema['submitHandler']` instead makes the variant key and the canonical key `ObjectForm` forwards provably the same type — they cannot drift, and the restated `any`s (3 per site) stop being new lint warnings. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Mn4BZ5AVDM81pvfij1WwM9 --- .changeset/submithandler-variant-forms.md | 13 +++++++++++++ packages/plugin-form/src/DrawerForm.tsx | 11 ++++++----- packages/plugin-form/src/ModalForm.tsx | 11 ++++++----- packages/plugin-form/src/SplitForm.tsx | 11 ++++++----- packages/plugin-form/src/TabbedForm.tsx | 11 ++++++----- packages/plugin-form/src/WizardForm.tsx | 11 ++++++----- 6 files changed, 43 insertions(+), 25 deletions(-) create mode 100644 .changeset/submithandler-variant-forms.md diff --git a/.changeset/submithandler-variant-forms.md b/.changeset/submithandler-variant-forms.md new file mode 100644 index 0000000000..84007e9a29 --- /dev/null +++ b/.changeset/submithandler-variant-forms.md @@ -0,0 +1,13 @@ +--- +"@object-ui/plugin-form": patch +--- + +Honour the declared `submitHandler` seam in every form variant, not just the simple one. + +`ObjectFormSchema.submitHandler` is documented as the seam a host uses to own persistence: the form validates and hands the collected values over instead of calling `dataSource.create` / `dataSource.update`. `ObjectForm` forwarded the key into every variant it routes to, but only `SimpleObjectForm` read it — `TabbedForm`, `WizardForm`, `SplitForm`, `DrawerForm` and `ModalForm` persisted directly. + +**Behaviour change on a persistence path.** A master-detail parent half rendered `tabbed` (or `split`) now commits through the atomic `batchTransaction` together with its child collections, instead of writing the parent independently through `dataSource.create`. Previously the child leg was never attempted on those layouts: the parent was committed alone, the entered line items were silently discarded, no compensation ran, and a success toast confirmed the save. A failing child leg now leaves no committed parent, on every layout that renders the parent half inline. + +`WizardForm` additionally skips its own default success toast / redirect arms when a `submitHandler` is present, matching `ObjectForm`, so a host that owns the write also owns the outcome. + +The `object-master-detail-form.formType` vocabulary is unchanged and stays `simple | tabbed`. diff --git a/packages/plugin-form/src/DrawerForm.tsx b/packages/plugin-form/src/DrawerForm.tsx index 82095600c0..2b76bfea0e 100644 --- a/packages/plugin-form/src/DrawerForm.tsx +++ b/packages/plugin-form/src/DrawerForm.tsx @@ -14,7 +14,7 @@ */ import React, { useState, useCallback, useEffect, useMemo, useRef, useId } from 'react'; -import type { FormField, DataSource } from '@object-ui/types'; +import type { FormField, DataSource, ObjectFormSchema } from '@object-ui/types'; import { Sheet, SheetContent, @@ -157,9 +157,10 @@ export interface DrawerFormSchema { layout?: 'vertical' | 'horizontal'; columns?: number; /** - * Override persistence — the seam a host uses to own the write. Mirrors - * `ObjectFormSchema.submitHandler`, the key `ObjectForm` forwards into this - * variant. When supplied, the form validates and hands the collected values + * Override persistence — the seam a host uses to own the write. Declared as + * `ObjectFormSchema['submitHandler']` rather than restated, so this variant + * and the canonical key `ObjectForm` forwards can never drift apart. + * When supplied, the form validates and hands the collected values * to this handler INSTEAD of calling `dataSource.create` / * `dataSource.update`; the returned record is passed on to `onSuccess`. * @@ -168,7 +169,7 @@ export interface DrawerFormSchema { * item 4). A renderer that does not read it writes the parent on its own and * escapes that transaction — objectui#6176. */ - submitHandler?: (values: Record) => any | Promise; + submitHandler?: ObjectFormSchema['submitHandler']; onSuccess?: (data: any) => void | Promise; onError?: (error: Error) => void; diff --git a/packages/plugin-form/src/ModalForm.tsx b/packages/plugin-form/src/ModalForm.tsx index 109b260738..9c1cda6970 100644 --- a/packages/plugin-form/src/ModalForm.tsx +++ b/packages/plugin-form/src/ModalForm.tsx @@ -14,7 +14,7 @@ */ import React, { useState, useCallback, useEffect, useMemo, useId, useRef } from 'react'; -import type { FormField, DataSource } from '@object-ui/types'; +import type { FormField, DataSource, ObjectFormSchema } from '@object-ui/types'; import { Dialog, MobileDialogContent, @@ -153,9 +153,10 @@ export interface ModalFormSchema { layout?: 'vertical' | 'horizontal'; columns?: number; /** - * Override persistence — the seam a host uses to own the write. Mirrors - * `ObjectFormSchema.submitHandler`, the key `ObjectForm` forwards into this - * variant. When supplied, the form validates and hands the collected values + * Override persistence — the seam a host uses to own the write. Declared as + * `ObjectFormSchema['submitHandler']` rather than restated, so this variant + * and the canonical key `ObjectForm` forwards can never drift apart. + * When supplied, the form validates and hands the collected values * to this handler INSTEAD of calling `dataSource.create` / * `dataSource.update`; the returned record is passed on to `onSuccess`. * @@ -164,7 +165,7 @@ export interface ModalFormSchema { * item 4). A renderer that does not read it writes the parent on its own and * escapes that transaction — objectui#6176. */ - submitHandler?: (values: Record) => any | Promise; + submitHandler?: ObjectFormSchema['submitHandler']; onSuccess?: (data: any) => void | Promise; onError?: (error: Error) => void; diff --git a/packages/plugin-form/src/SplitForm.tsx b/packages/plugin-form/src/SplitForm.tsx index d8dbd83930..0195312925 100644 --- a/packages/plugin-form/src/SplitForm.tsx +++ b/packages/plugin-form/src/SplitForm.tsx @@ -24,7 +24,7 @@ */ import React, { useState, useCallback, useEffect, useMemo, useRef } from 'react'; -import type { FormField, DataSource } from '@object-ui/types'; +import type { FormField, DataSource, ObjectFormSchema } from '@object-ui/types'; import { cn } from '@object-ui/components'; import { SchemaRenderer, useSafeFieldLabel } from '@object-ui/react'; import { buildSectionFields as buildSectionFieldsShared } from './sectionFields'; @@ -106,9 +106,10 @@ export interface SplitFormSchema { initialData?: Record; readOnly?: boolean; /** - * Override persistence — the seam a host uses to own the write. Mirrors - * `ObjectFormSchema.submitHandler`, the key `ObjectForm` forwards into this - * variant. When supplied, the form validates and hands the collected values + * Override persistence — the seam a host uses to own the write. Declared as + * `ObjectFormSchema['submitHandler']` rather than restated, so this variant + * and the canonical key `ObjectForm` forwards can never drift apart. + * When supplied, the form validates and hands the collected values * to this handler INSTEAD of calling `dataSource.create` / * `dataSource.update`; the returned record is passed on to `onSuccess`. * @@ -117,7 +118,7 @@ export interface SplitFormSchema { * item 4). A renderer that does not read it writes the parent on its own and * escapes that transaction — objectui#6176. */ - submitHandler?: (values: Record) => any | Promise; + submitHandler?: ObjectFormSchema['submitHandler']; onSuccess?: (data: any) => void | Promise; onError?: (error: Error) => void; diff --git a/packages/plugin-form/src/TabbedForm.tsx b/packages/plugin-form/src/TabbedForm.tsx index d5fd378be0..3ee782c965 100644 --- a/packages/plugin-form/src/TabbedForm.tsx +++ b/packages/plugin-form/src/TabbedForm.tsx @@ -14,7 +14,7 @@ */ import React, { useState, useCallback, useRef } from 'react'; -import type { FormField, DataSource } from '@object-ui/types'; +import type { FormField, DataSource, ObjectFormSchema } from '@object-ui/types'; import { cn } from '@object-ui/components'; import { SchemaRenderer, useSafeFieldLabel } from '@object-ui/react'; import { buildSectionFields as buildSectionFieldsShared } from './sectionFields'; @@ -146,9 +146,10 @@ export interface TabbedFormSchema { readOnly?: boolean; /** - * Override persistence — the seam a host uses to own the write. Mirrors - * `ObjectFormSchema.submitHandler`, the key `ObjectForm` forwards into this - * variant. When supplied, the form validates and hands the collected values + * Override persistence — the seam a host uses to own the write. Declared as + * `ObjectFormSchema['submitHandler']` rather than restated, so this variant + * and the canonical key `ObjectForm` forwards can never drift apart. + * When supplied, the form validates and hands the collected values * to this handler INSTEAD of calling `dataSource.create` / * `dataSource.update`; the returned record is passed on to `onSuccess`. * @@ -157,7 +158,7 @@ export interface TabbedFormSchema { * item 4). A renderer that does not read it writes the parent on its own and * escapes that transaction — objectui#6176. */ - submitHandler?: (values: Record) => any | Promise; + submitHandler?: ObjectFormSchema['submitHandler']; /** * Callbacks diff --git a/packages/plugin-form/src/WizardForm.tsx b/packages/plugin-form/src/WizardForm.tsx index b3a96f8169..113b8f4347 100644 --- a/packages/plugin-form/src/WizardForm.tsx +++ b/packages/plugin-form/src/WizardForm.tsx @@ -14,7 +14,7 @@ */ import React, { useState, useCallback, useMemo } from 'react'; -import type { FormField, DataSource } from '@object-ui/types'; +import type { FormField, DataSource, ObjectFormSchema } from '@object-ui/types'; import { Button, cn, toast } from '@object-ui/components'; import { AlertCircle, Check, ChevronLeft, ChevronRight, Loader2 } from 'lucide-react'; import { resolveFieldRuleState, evalFieldPredicate, isMissingForRequired, isServerOwnedValue } from '@object-ui/core'; @@ -204,9 +204,10 @@ export interface WizardFormSchema { readOnly?: boolean; /** - * Override persistence — the seam a host uses to own the write. Mirrors - * `ObjectFormSchema.submitHandler`, the key `ObjectForm` forwards into this - * variant. When supplied, the form validates and hands the collected values + * Override persistence — the seam a host uses to own the write. Declared as + * `ObjectFormSchema['submitHandler']` rather than restated, so this variant + * and the canonical key `ObjectForm` forwards can never drift apart. + * When supplied, the form validates and hands the collected values * to this handler INSTEAD of calling `dataSource.create` / * `dataSource.update`; the returned record is passed on to `onSuccess`. * @@ -215,7 +216,7 @@ export interface WizardFormSchema { * item 4). A renderer that does not read it writes the parent on its own and * escapes that transaction — objectui#6176. */ - submitHandler?: (values: Record) => any | Promise; + submitHandler?: ObjectFormSchema['submitHandler']; /** * Callbacks