diff --git a/.changeset/simple-form-consults-declared-submit-handler.md b/.changeset/simple-form-consults-declared-submit-handler.md new file mode 100644 index 0000000000..74dfa9e5ef --- /dev/null +++ b/.changeset/simple-form-consults-declared-submit-handler.md @@ -0,0 +1,9 @@ +--- +"@object-ui/plugin-form": patch +--- + +`SimpleObjectForm`: consult a declared `submitHandler` before the inline-fields carve-out + +`ObjectFormSchema.submitHandler` is documented as handing the collected values to the host INSTEAD of calling `dataSource.create` / `dataSource.update`, so a form that declares it has a submit target with or without an adapter. `SimpleObjectForm.handleSubmit` nevertheless opened with the inline-fields carve-out (`hasInlineFields && !dataSource`), which returned before the persistence chain: a host that had declared it owns the write was never asked, and `onSuccess` confirmed a write that never happened (measured `onSuccess 1 / submitHandler 0`). + +The carve-out now fires only when no `submitHandler` is declared, and the "no submit target" refusal moved into the persistence chain after the seam — the shape the five variant renderers already use, reusing their shared refusal from `submitTarget.ts` rather than a private copy. A form with inline fields and no seam is unchanged: its `onSuccess` is still the write. diff --git a/packages/plugin-form/src/ObjectForm.tsx b/packages/plugin-form/src/ObjectForm.tsx index 379c0a535c..b109875548 100644 --- a/packages/plugin-form/src/ObjectForm.tsx +++ b/packages/plugin-form/src/ObjectForm.tsx @@ -42,6 +42,7 @@ import { } from './autoLayout'; import { deriveFieldGroupSections } from './fieldGroups'; import { sanitizeFormData } from './sanitize'; +import { noSubmitTargetError } from './submitTarget'; import { schemaDefaultValues, isCreateFormMode, @@ -781,18 +782,33 @@ const SimpleObjectForm: React.FC = ({ } } - // For inline fields without a dataSource, just call the success callback - if (hasInlineFields && !dataSource) { + // No submit TARGET: a declared `submitHandler` owns the write and needs no + // adapter of its own (objectui#6176's seam), so only a form with NEITHER it + // nor a `dataSource` is target-less. The one target-less form that is still + // legitimate is the inline-fields collector, whose `onSuccess` IS the write. + // This arm used to be `hasInlineFields && !dataSource` alone, checked BEFORE + // the persistence chain: a declared `submitHandler` was never reached, so a + // host that had said it owns the write got a success signal for a write it + // was never asked to perform (objectui#6388). Same rule and same precedence + // as the five variant renderers — see `submitTarget.ts` for the whole rule. + // + // The predicate stays this component's own `hasInlineFields` (non-empty + // `customFields`) rather than the shared `hasInlineFieldSource`. That + // helper's second limb — sections whose every field is an inline runtime + // `FormField` — is how the SECTIONED variants express an inline field + // source, and this renderer does not read it: here `sections[].fields` only + // SELECT (and override) fields already resolved from `customFields` or the + // object schema, so a sections-only form with no adapter resolves zero + // fields. Treating that as inline would widen the carve-out into a success + // signal for a form that collected nothing — this card's own defect class. + // Limb (a) is identical, and the refusal below is the shared one, verbatim. + if (!dataSource && !schema.submitHandler && hasInlineFields) { if (schema.onSuccess) { await schema.onSuccess(formData); } return formData; } - if (!dataSource) { - throw new Error('DataSource is required for form submission (inline mode not configured)'); - } - // Strip server-managed and computed / read-only fields from the payload // before persisting. react-hook-form retains state for unmounted/disabled // fields (see ModalForm), so an edit form seeded from a full record read @@ -830,6 +846,13 @@ const SimpleObjectForm: React.FC = ({ // + children into one atomic transaction). The form just validates and // hands over the values; it does NOT create/update itself. result = await schema.submitHandler(payload); + } else if (!dataSource) { + // No route left: no host seam and no adapter. Refuse instead of + // reporting success — the `catch` below hands this to `schema.onError` + // and rethrows. Expressing it here rather than in a pre-`try` guard is + // what lets the `submitHandler` branch above run first, and it is also + // what narrows `dataSource` for the routes below with no assertion. + throw noSubmitTargetError(); } else if (schema.mode === 'create') { result = await dataSource.create(schema.objectName, payload); } else if (schema.mode === 'edit' && schema.recordId) { diff --git a/packages/plugin-form/src/submitTargetRefusal.test.tsx b/packages/plugin-form/src/submitTargetRefusal.test.tsx index 17c15b0f43..166453315e 100644 --- a/packages/plugin-form/src/submitTargetRefusal.test.tsx +++ b/packages/plugin-form/src/submitTargetRefusal.test.tsx @@ -45,6 +45,20 @@ * `submitViaBatch` (which would have said `dataSource is required`) never * called. That is the half this file pins in `describe` block 5. * + * ## The sixth renderer (objectui#6388) + * + * `SimpleObjectForm` had the same hole in its own dialect. Its carve-out reads + * `hasInlineFields && !dataSource` — non-empty `customFields`, its own inline + * field source — and it too opened `handleSubmit`, ahead of the persistence + * chain. Re-derived on the merged tree (faa863dce) with `customFields`, a + * `submitHandler` and NO `dataSource`: `onSuccess 1 / submitHandler 0`. Its + * cases live in blocks 1 and 3 beside the family's, on a `customFields` + * fixture: under `simple`, `sections[].fields` only SELECT fields already + * resolved from `customFields` or the object schema, so the sectioned fixture + * above renders no fields at all there — which is also why `simple` keeps its + * own predicate rather than `hasInlineFieldSource` (block 3's `simple` + * BOUNDARY case pins that). + * * ## The blocks below * * 1. the declared `submitHandler` seam is consulted with no `dataSource`; @@ -108,6 +122,24 @@ const baseSchema = (formType: string, extra: Record = {}) => ({ ...extra, }); +/** + * `simple` (`SimpleObjectForm`) — the sixth renderer. Its inline field source is + * `customFields`, so it gets its own fixture rather than `baseSchema`'s + * `sections`: a bare field NAME in a section is resolved against fields that + * only `customFields` or an object schema can supply, so `baseSchema('simple')` + * with no `dataSource` renders zero fields and every assertion on it would pass + * vacuously. + */ +const simpleSchema = (extra: Record = {}) => ({ + type: 'object-form', + objectName: 'po', + mode: 'create', + formType: 'simple', + submitText: 'Save Now', + customFields: INLINE_FIELDS, + ...extra, +}); + /** Type into the one field and press the form's own submit button. */ async function fillAndSubmit(value = 'PO-1') { const ref = await waitFor(() => { @@ -151,6 +183,63 @@ describe('1. a declared `submitHandler` is consulted even with NO dataSource', ( expect(onError).not.toHaveBeenCalled(); }, ); + + it('formType `simple`: inline fields + a declared seam — the seam is what runs', async () => { + const submitHandler = vi.fn().mockResolvedValue({ id: 'p1' }); + const onSuccess = vi.fn(); + const onError = vi.fn(); + + render( + , + ); + await fillAndSubmit(); + + // THE PIN (objectui#6388). Measured on the merged tree: `onSuccess 1 / + // submitHandler 0` — the inline-fields carve-out opened `handleSubmit`, so a + // host that had DECLARED it owns the write was never asked, and got a + // success signal for a write that never happened. + await waitFor(() => expect(submitHandler).toHaveBeenCalledTimes(1)); + expect(submitHandler).toHaveBeenCalledWith(expect.objectContaining({ ref: 'PO-1' })); + // Success still reported — after the host wrote, carrying ITS result. + await waitFor(() => expect(onSuccess).toHaveBeenCalledTimes(1)); + expect(onSuccess).toHaveBeenCalledWith({ id: 'p1' }); + expect(onError).not.toHaveBeenCalled(); + }); + + it('formType `simple`: the seam wins over a PRESENT dataSource too (objectui#6176)', async () => { + const create = vi.fn().mockResolvedValue({ id: 'written-by-adapter' }); + const submitHandler = vi.fn().mockResolvedValue({ id: 'written-by-host' }); + const onSuccess = vi.fn(); + const onError = vi.fn(); + const dataSource = { + getObjectSchema: vi.fn().mockResolvedValue(parentObject), + find: vi.fn().mockResolvedValue({ data: [] }), + findOne: vi.fn().mockResolvedValue({}), + create, + update: vi.fn(), + delete: vi.fn(), + bulk: vi.fn(), + } as any; + + // The inverse of the pin above: `submitHandler` is documented as handing the + // values to the host INSTEAD of calling `dataSource.create` / `update`, so + // the adapter being available changes nothing about who writes. Passes on + // the merged tree as well — the ordering defect was reachable only through + // the `!dataSource` carve-out, and this case is what says so. + render( + , + ); + await fillAndSubmit(); + + await waitFor(() => expect(submitHandler).toHaveBeenCalledTimes(1)); + expect(create).not.toHaveBeenCalled(); + await waitFor(() => expect(onSuccess).toHaveBeenCalledTimes(1)); + expect(onSuccess).toHaveBeenCalledWith({ id: 'written-by-host' }); + expect(onError).not.toHaveBeenCalled(); + }); }); describe('2. no seam, no adapter, no inline fields → refuse loudly', () => { @@ -280,6 +369,59 @@ describe('3. CARVE-OUT: a legitimate inline-fields form still works', () => { expect(onSuccess).not.toHaveBeenCalled(); }, ); + + it('formType `simple`: CONTROL — no seam declared, the carve-out still fires', async () => { + const onSuccess = vi.fn(); + const onError = vi.fn(); + + // THE DEGENERATE CONTROL for objectui#6388. `simple`'s inline collector is + // legitimate and must survive the reordering untouched: with no + // `submitHandler` to consult, `onSuccess` IS the write and it receives the + // RAW collected values, not an adapter's result. Passes on the merged tree + // and after the fix, deliberately — it is what makes block 1's `simple` pin + // attributable to the declared seam rather than to the carve-out having + // been narrowed or removed. + render(); + await fillAndSubmit(); + + await waitFor(() => expect(onSuccess).toHaveBeenCalledTimes(1)); + expect(onSuccess).toHaveBeenCalledWith(expect.objectContaining({ ref: 'PO-1' })); + expect(onError).not.toHaveBeenCalled(); + }); + + it('formType `simple`: BOUNDARY — sections of inline fields are NOT its field source', async () => { + const onSuccess = vi.fn(); + const onError = vi.fn(); + + // `hasInlineFieldSource`'s second limb (all-inline `sections`) is how the + // SECTIONED variants declare an inline field source — block 3's case above + // pins it for them. `SimpleObjectForm` does not read it: a section field + // here only SELECTS a field already resolved from `customFields` or the + // object schema, so this form resolves ZERO fields and collected nothing. + // Adopting the shared predicate for `simple` while "aligning" it would + // therefore turn this into a success signal for an empty submit — the very + // defect class of objectui#6300. It refuses instead. + render( + , + ); + const submit = await waitFor(() => screen.getByRole('button', { name: /save now/i })); + fireEvent.click(submit); + + await waitFor(() => expect(onError).toHaveBeenCalledTimes(1)); + expect((onError.mock.calls[0][0] as Error).message).toBe(NO_SUBMIT_TARGET_MESSAGE); + expect(onSuccess).not.toHaveBeenCalled(); + }); }); describe('4. DEGENERATE CONTROL: with a dataSource the write really happens', () => {