From 65d90fc8d0918b1ed4103ed0e77b4d8de250bd51 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 25 Sep 2026 12:13:57 +0000 Subject: [PATCH 1/3] fix(plugin-form): a mounted master-detail edit form advances its child-row baseline after a save After a committed edit batch, the rows it created take the ids the server echoed and each collection's `original` takes on what the batch wrote, laid over the rows as read (the child-row form of `advanceLoadedRecord`). A second save from the same mounted form therefore no longer re-creates a row, re-sends a delete, or drops a cell changed back to its first-read value. A batch that rejects advances neither the child rows nor the parent. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_014mXUNuFomfj24w7s1pZzhN --- ...terDetailForm.editBaselineAdvance.test.tsx | 343 ++++++++++++++++++ packages/plugin-form/src/MasterDetailForm.tsx | 226 +++++++++++- 2 files changed, 552 insertions(+), 17 deletions(-) create mode 100644 packages/plugin-form/src/MasterDetailForm.editBaselineAdvance.test.tsx diff --git a/packages/plugin-form/src/MasterDetailForm.editBaselineAdvance.test.tsx b/packages/plugin-form/src/MasterDetailForm.editBaselineAdvance.test.tsx new file mode 100644 index 0000000000..6edafab30d --- /dev/null +++ b/packages/plugin-form/src/MasterDetailForm.editBaselineAdvance.test.tsx @@ -0,0 +1,343 @@ +/** + * 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. + */ + +/** + * A master-detail EDIT form that stays mounted after a save diffs its next + * save against the child rows as they now stand on the server, not as first + * read (objectui#10564). + * + * The child-row twin of the parent-record baseline advance (objectui#10156, + * `advanceLoadedRecord`). Every row drives the real `MasterDetailForm` with the + * real line-item grid, and a host `onSuccess` that stays on the form — the + * only kind of host for which a second save from the same mounted form exists. + * + * The batch double answers the way the `batchTransaction` contract says a + * server does: `results` index-aligned with the operations, a create echoing + * the written record with the id the server minted. + * + * Two of these rows cannot go red on the pre-fix component, and are here for + * the fix, not for the defect: a FAILED save must leave both baselines where + * they were (a baseline that advanced on a write that did not land would drop + * that write from the retry), and create mode's reset is the control. + */ +import { describe, it, expect, vi } from 'vitest'; +import { render, waitFor, fireEvent, screen, act } from '@testing-library/react'; +import React from 'react'; + +import { registerAllFields } from '@object-ui/fields'; +import { MasterDetailForm } from './MasterDetailForm'; + +registerAllFields(); + +const PO_SCHEMA = { name: 'po', fields: { ref: { type: 'text', label: 'Ref' } } }; +const LINE_SCHEMA = { + name: 'po_line', + fields: { + qty: { type: 'number', label: 'Qty' }, + po: { type: 'master_detail', label: 'PO', reference: 'po' }, + }, +}; + +type Op = { object: string; action?: string; id?: string; data?: Record }; + +/** + * A batch that answers per the contract: creates echo the written record under + * a freshly minted id, updates echo the record, deletes echo `true`. + */ +function echoBatch() { + let minted = 0; + return vi.fn(async (ops: Op[]) => ({ + results: ops.map((op) => { + if (op.action === 'create') return { id: `new${++minted}`, ...op.data }; + if (op.action === 'delete') return true; + return { id: op.id, ...op.data }; + }), + })); +} + +function renderForm(opts: { + mode?: 'create' | 'edit'; + lines?: Record[]; + batchTransaction?: any; + ds?: Record; +}) { + const mode = opts.mode ?? 'edit'; + const lines = opts.lines ?? []; + // A host that keeps the form mounted after the save (no navigation). + const onSuccess = vi.fn(); + const onError = vi.fn(); + const ds: any = { + getObjectSchema: vi.fn(async (o: string) => (o === 'po_line' ? LINE_SCHEMA : PO_SCHEMA)), + findOne: vi.fn().mockResolvedValue({ id: 'po1', ref: 'PO-1' }), + find: vi.fn(async (o: string) => ({ data: o === 'po_line' ? lines.map((l) => ({ ...l })) : [] })), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + ...(opts.batchTransaction ? { batchTransaction: opts.batchTransaction } : {}), + ...opts.ds, + }; + const view = render( + , + ); + return { ...view, ds, onSuccess, onError }; +} + +const saveButton = () => screen.getByTestId('md-form-submit') as HTMLButtonElement; +const qtyInputs = () => screen.getAllByLabelText('Qty') as HTMLInputElement[]; + +/** The parent form has its values and the stored lines have landed. */ +async function ready(container: HTMLElement, mode: 'create' | 'edit', storedLines: number) { + await waitFor(() => { + const ref = container.querySelector('input[name="ref"]') as HTMLInputElement | null; + if (!ref) throw new Error('parent form not ready'); + if (mode === 'edit' && ref.value !== 'PO-1') throw new Error('parent record not loaded'); + }); + // Every stored line plus the grid's trailing ghost row. + await waitFor(() => expect(qtyInputs()).toHaveLength(storedLines + 1)); + await waitFor(() => expect(saveButton().disabled).toBe(false)); +} + +const change = async (el: HTMLElement, value: string) => { + await act(async () => { + fireEvent.change(el, { target: { value } }); + }); +}; + +/** Press Save and wait until the attempt has settled, whatever its outcome. */ +async function save(calls: () => number) { + const before = calls(); + await act(async () => { + fireEvent.click(saveButton()); + }); + await waitFor(() => expect(calls()).toBeGreaterThan(before)); + await waitFor(() => expect(saveButton().disabled).toBe(false)); +} + +/** The child operations of the batch sent on save number `n` (0-based). */ +const childOps = (batch: any, n: number): Op[] => (batch.mock.calls[n][0] as Op[]).slice(1); + +describe('a mounted master-detail edit form: the next save diffs against what the last save wrote', () => { + it('a row the first save created is not created again, and a later edit updates it by its new id', async () => { + const batch = echoBatch(); + const { container, onSuccess } = renderForm({ batchTransaction: batch }); + await ready(container, 'edit', 0); + + await change(qtyInputs()[0], '5'); // type into the ghost row + await save(() => batch.mock.calls.length); + expect(onSuccess).toHaveBeenCalledTimes(1); + expect(childOps(batch, 0)).toEqual([ + { object: 'po_line', action: 'create', data: { qty: 5, po: 'po1' } }, + ]); + + // Nothing changed: no child operation at all — not a second create. + await save(() => batch.mock.calls.length); + expect(childOps(batch, 1)).toEqual([]); + expect(qtyInputs()).toHaveLength(2); // the one line, and the ghost row + + // The created row carries the server's id now: editing it is an update. + await change(qtyInputs()[0], '6'); + await save(() => batch.mock.calls.length); + expect(childOps(batch, 2)).toEqual([ + { object: 'po_line', action: 'update', id: 'new1', data: { qty: 6 } }, + ]); + }); + + it('a cell changed and then changed back to its first-read value is written, not dropped', async () => { + const batch = echoBatch(); + const { container } = renderForm({ + batchTransaction: batch, + lines: [{ id: 'L1', qty: 1, po: 'po1' }], + }); + await ready(container, 'edit', 1); + expect(qtyInputs()[0].value).toBe('1'); + + await change(qtyInputs()[0], '2'); + await save(() => batch.mock.calls.length); + expect(childOps(batch, 0)).toEqual([ + { object: 'po_line', action: 'update', id: 'L1', data: { qty: 2 } }, + ]); + + await change(qtyInputs()[0], '1'); + await save(() => batch.mock.calls.length); + expect(childOps(batch, 1)).toEqual([ + { object: 'po_line', action: 'update', id: 'L1', data: { qty: 1 } }, + ]); + }); + + it('a created row edited while its save was in flight is not duplicated: the next save replaces the created record', async () => { + const echo = echoBatch(); + let land: () => void = () => {}; + const batch = vi.fn(async (ops: Op[]) => { + if (batch.mock.calls.length === 1) await new Promise((r) => { land = r; }); + return echo(ops); + }); + const { container, onSuccess } = renderForm({ batchTransaction: batch }); + await ready(container, 'edit', 0); + + await change(qtyInputs()[0], '5'); + await act(async () => { + fireEvent.click(saveButton()); + }); + await waitFor(() => expect(batch).toHaveBeenCalledTimes(1)); + await change(qtyInputs()[0], '8'); // the batch has not answered yet + await act(async () => { + land(); + }); + await waitFor(() => expect(onSuccess).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(saveButton().disabled).toBe(false)); + + await save(() => batch.mock.calls.length); + expect(childOps(batch, 1)).toEqual([ + { object: 'po_line', action: 'create', data: { qty: 8, po: 'po1' } }, + { object: 'po_line', action: 'delete', id: 'new1' }, + ]); + }); + + it('a row the first save deleted is not deleted again', async () => { + const batch = echoBatch(); + const { container } = renderForm({ + batchTransaction: batch, + lines: [ + { id: 'L1', qty: 1, po: 'po1' }, + { id: 'L2', qty: 2, po: 'po1' }, + ], + }); + await ready(container, 'edit', 2); + + await act(async () => { + fireEvent.click(screen.getAllByRole('button', { name: 'Remove row' })[1]); + }); + await save(() => batch.mock.calls.length); + expect(childOps(batch, 0)).toEqual([{ object: 'po_line', action: 'delete', id: 'L2' }]); + + await save(() => batch.mock.calls.length); + expect(childOps(batch, 1)).toEqual([]); + }); +}); + +describe('a FAILED save advances neither baseline: the retry carries every operation again', () => { + it('a refused batch: the retry sends the same parent and child operations', async () => { + const echo = echoBatch(); + const batch = vi.fn(async (ops: Op[]) => { + if (batch.mock.calls.length === 1) throw new Error('BATCH_ERROR: rolled back'); + return echo(ops); + }); + const { container, onSuccess, onError } = renderForm({ + batchTransaction: batch, + lines: [{ id: 'L1', qty: 1, po: 'po1' }], + }); + await ready(container, 'edit', 1); + + await change(container.querySelector('input[name="ref"]') as HTMLElement, 'PO-2'); + await change(qtyInputs()[0], '2'); + await change(qtyInputs()[1], '7'); // the ghost row + await save(() => batch.mock.calls.length); + expect(onError).toHaveBeenCalledTimes(1); + expect(onSuccess).not.toHaveBeenCalled(); + const first = batch.mock.calls[0][0]; + expect(first).toEqual([ + { object: 'po', action: 'update', id: 'po1', data: { ref: 'PO-2' } }, + { object: 'po_line', action: 'create', data: { qty: 7, po: 'po1' } }, + { object: 'po_line', action: 'update', id: 'L1', data: { qty: 2 } }, + ]); + + // The retry is the same batch: nothing was taken as written. + await save(() => batch.mock.calls.length); + expect(onSuccess).toHaveBeenCalledTimes(1); + expect(batch.mock.calls[1][0]).toEqual(first); + + // And once the retry has landed, a third save carries no child operation. + await save(() => batch.mock.calls.length); + expect(childOps(batch, 2)).toEqual([]); + }); + + it('a partially applied save (no atomic endpoint: the parent write landed, a child write failed) advances neither', async () => { + let failChildCreate = true; + let minted = 0; + const create = vi.fn(async (object: string, data: any) => { + if (object === 'po_line' && failChildCreate) { + failChildCreate = false; + throw new Error('child create failed'); + } + return { id: `new${++minted}`, ...data }; + }); + const update = vi.fn(async (_o: string, id: string, data: any) => ({ id, ...data })); + const del = vi.fn(async () => true); + // No `batchTransaction`: the emulation runs the operations one by one. + const { container, onSuccess, onError } = renderForm({ + lines: [{ id: 'L1', qty: 1, po: 'po1' }], + ds: { create, update, delete: del }, + }); + await ready(container, 'edit', 1); + + await change(container.querySelector('input[name="ref"]') as HTMLElement, 'PO-2'); + await change(qtyInputs()[0], '2'); + await change(qtyInputs()[1], '7'); + await save(() => create.mock.calls.length); + expect(onError).toHaveBeenCalledTimes(1); + // The parent write went through before the child create failed. + expect(update.mock.calls).toEqual([['po', 'po1', { ref: 'PO-2' }]]); + + await save(() => create.mock.calls.length); + expect(onSuccess).toHaveBeenCalledTimes(1); + // The retry re-sends the parent write AND both child writes. + expect(update.mock.calls.slice(1)).toEqual([ + ['po', 'po1', { ref: 'PO-2' }], + ['po_line', 'L1', { qty: 2 }], + ]); + expect(create.mock.calls[1]).toEqual(['po_line', { qty: 7, po: 'po1' }]); + }); +}); + +describe('create mode is unchanged (control)', () => { + it('a successful create still empties the lines and the header for the next entry', async () => { + const batch = echoBatch(); + const { container, onSuccess } = renderForm({ mode: 'create', batchTransaction: batch }); + await ready(container, 'create', 0); + + await change(container.querySelector('input[name="ref"]') as HTMLElement, 'PO-9'); + await change(qtyInputs()[0], '5'); + await save(() => batch.mock.calls.length); + expect(onSuccess).toHaveBeenCalledTimes(1); + expect(batch.mock.calls[0][0]).toEqual([ + { object: 'po', action: 'create', data: { ref: 'PO-9' } }, + { object: 'po_line', action: 'create', data: { qty: 5, po: { $ref: 0 } } }, + ]); + + // Lines back to the lone ghost row, header cleared. + await waitFor(() => expect(qtyInputs()).toHaveLength(1)); + expect(qtyInputs()[0].value).toBe(''); + await waitFor(() => + expect((container.querySelector('input[name="ref"]') as HTMLInputElement).value).toBe(''), + ); + + // The next entry is a new document: its create carries no line. + await change(container.querySelector('input[name="ref"]') as HTMLElement, 'PO-10'); + await save(() => batch.mock.calls.length); + expect(batch.mock.calls[1][0]).toEqual([ + { object: 'po', action: 'create', data: { ref: 'PO-10' } }, + ]); + }); +}); diff --git a/packages/plugin-form/src/MasterDetailForm.tsx b/packages/plugin-form/src/MasterDetailForm.tsx index 0a156d7978..9c4d859641 100644 --- a/packages/plugin-form/src/MasterDetailForm.tsx +++ b/packages/plugin-form/src/MasterDetailForm.tsx @@ -31,7 +31,7 @@ */ import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'; -import type { DataSource } from '@object-ui/types'; +import type { BatchTransactionOperation, DataSource } from '@object-ui/types'; import { runBatchTransaction } from '@object-ui/core'; import { LineItemsField, type GridColumn } from '@object-ui/fields'; import { Button, Card, CardContent, CardHeader, CardTitle, cn, toast } from '@object-ui/components'; @@ -40,7 +40,15 @@ import { usePermissions } from '@object-ui/permissions'; import { ObjectForm } from './ObjectForm'; import { applyColumnPermissions } from './fieldWriteGate'; import { useUploadGate, UploadGateProvider, UploadInFlightNotice } from './uploadGate'; -import { buildMasterDetailBatch, buildMasterDetailEditBatch, sumRows } from './masterDetailTx'; +import { + buildMasterDetailBatch, + buildMasterDetailEditBatch, + idOf, + isBlankRow, + sumRows, + type BatchEditDetailInput, +} from './masterDetailTx'; +import { isSameStoredValue } from './sanitize'; import { deriveDetail, hydrateColumns, type InlineMode } from './deriveMasterDetail'; export interface MasterDetailDetailConfig { @@ -125,10 +133,154 @@ export interface MasterDetailFormSchema { /** Rows keyed by their persisted id (when known), for edit-mode diffing. */ interface RowState { rows: Record[]; - /** Snapshot of the persisted rows (edit mode) for diffing on submit. */ + /** + * Snapshot of the persisted rows (edit mode) for diffing on submit. Two + * writers: the children-fetch effect sets it from the read, and every + * successful edit save advances it to what that save wrote + * ({@link childRowsAfterSave}, objectui#10564). + */ original: Record[]; } +/** One detail collection as the edit batch was built from it. */ +interface SavedDetailInput { + entryId: string; + /** The exact input handed to `buildMasterDetailEditBatch`. */ + input: BatchEditDetailInput; +} + +/** + * What one SUCCESSFUL edit batch wrote to one detail collection: the child-row + * form of `advanceLoadedRecord`, the parent record's baseline advance + * (objectui#10156 / objectui#10564). + */ +interface SavedChildRows { + entryId: string; + /** + * The `original` the batch diffed against. The advance lands only while it + * is still the collection's baseline: a reload that replaced the row state + * while the save was in flight has set a newer one. + */ + diffedAgainst: Record[]; + /** Each row the batch CREATED (the row object it was built from) → the id the server gave it. */ + createdIds: Map, unknown>; + /** The baseline after the save: every persisted row, laid over with what this batch wrote to it. */ + original: Record[]; +} + +/** + * Whether a `create` operation's payload was built from `row`: every field it + * writes, other than the parent link the builder sets, holds the row's value. + * The builder only drops keys from a row (sanitize) and never rewrites one. + */ +function isBuiltFrom( + data: Record | undefined, + row: Record, + relationshipField: string, +): boolean { + return Object.entries(data ?? {}).every( + ([k, v]) => k === relationshipField || isSameStoredValue(v, row[k]), + ); +} + +/** + * The child rows' baseline after an edit batch COMMITTED (objectui#10564). + * + * A form that stays mounted after a save must not diff its next save against + * the rows as FIRST read. Against that stale baseline a row the save created + * is still id-less and is created again; a row the save deleted is deleted + * again; and a cell changed back to its first-read value compares clean and is + * dropped while the server keeps the saved value. + * + * Two facts come from the batch, and nothing is inferred beyond them: + * + * - **Ids.** `batchTransaction` answers `results` index-aligned with the + * operations, a create echoing the record it wrote (`DataSource`'s contract; + * the server's `/batch` and `emulateBatchTransaction` both do). The edit + * builder puts the parent at index 0 and then, collection by collection in + * the order it was handed them, one `create` per non-blank row without an + * id, in row order. Each such row is paired with the next create operation, + * and the pair is checked ({@link isBuiltFrom}) before it is trusted. A pair + * that does not check out stops the pairing: the rows left unpaired stay + * creates, which is the behaviour before this advance, never a row updating + * another row's record. A create echo without an id leaves its row a create + * too. + * - **Written values.** What the batch sent, laid over the row as read — the + * same rule `advanceLoadedRecord` applies to the parent. A row the batch + * deleted leaves the baseline; a row it did not touch keeps its snapshot. + * + * Pure, and it does not throw: it runs between a committed batch and the + * parent's own advance, where a throw would report a save that landed as a + * failure. + */ +function childRowsAfterSave( + ops: BatchTransactionOperation[], + results: unknown, + details: SavedDetailInput[], +): SavedChildRows[] { + const echoes = Array.isArray(results) ? results : []; + const creates: Array<{ op: BatchTransactionOperation; echo: unknown }> = []; + ops.forEach((op, k) => { + if (k > 0 && op.action === 'create') creates.push({ op, echo: echoes[k] }); + }); + let nextCreate = 0; + let pairingLost = false; + + const saved = details.map(({ entryId, input }): SavedChildRows => { + const { childObject, relationshipField } = input; + const rows = input.rows || []; + const original = input.original || []; + const createdIds = new Map, unknown>(); + const written = new Map>(); + + for (const row of rows) { + if (pairingLost) break; + if (idOf(row) != null || isBlankRow(row, relationshipField)) continue; + const pair = creates[nextCreate]; + if (!pair || pair.op.object !== childObject || !isBuiltFrom(pair.op.data, row, relationshipField)) { + pairingLost = true; + break; + } + nextCreate += 1; + const id = idOf(pair.echo); + if (id == null) continue; + createdIds.set(row, id); + written.set(id, pair.op.data ?? {}); + } + for (let k = 1; k < ops.length; k++) { + const op = ops[k]; + if (op.action === 'update' && op.object === childObject && op.id != null) { + written.set(op.id, op.data ?? {}); + } + } + + const before = new Map>(); + for (const r of original) { + const id = idOf(r); + if (id != null) before.set(id, r); + } + const next: Record[] = []; + for (const row of rows) { + const id = idOf(row) ?? createdIds.get(row); + if (id == null) continue; + const was = before.get(id); + const wrote = written.get(id); + if (!was && !wrote) continue; + const entry: Record = { ...(was ?? {}), ...(wrote ?? {}) }; + if (idOf(entry) == null) entry.id = id; + next.push(entry); + } + return { entryId, diffedAgainst: input.original, createdIds, original: next }; + }); + + if (pairingLost || nextCreate !== creates.length) { + console.warn( + '[MasterDetailForm] could not pair every row created by this save with its create operation; an unpaired row keeps no id, and the next save creates it again.', + ); + } + return saved; +} + /** * What happened to one detail entry's schema resolution (objectui#6372). * @@ -880,6 +1032,8 @@ export const MasterDetailForm: React.FC = ({ id: outcomeToastId, }); } + // An edit keeps its rows: `submitViaBatch` has already advanced their + // baseline to what this save wrote (objectui#10564). if (!isEdit) { // Every collection back to empty for the next entry. Dropping the whole // record is the same statement the per-detail rebuild made, without @@ -943,25 +1097,33 @@ export const MasterDetailForm: React.FC = ({ parentData[d.totalField] = sumRows(rowStateRef.current[e.id]?.rows ?? [], d.amountField || 'amount'); } }); - const ops = isEdit - ? buildMasterDetailEditBatch( - schema.objectName, - String(schema.recordId), - parentData, - // ⚠️ Read by ENTRY ID. This was - // `.filter(…).map((d, i) => stateRef.current[i])`, where `i` indexes - // the FILTERED array while the row state was indexed against the - // FULL one — so a declined or unresolved entry sitting above a real - // collection shifted every read below it by one and that - // collection's rows were silently dropped from the transaction. - // Data loss on save, not a display defect (objectui#6371). - entries.filter((e) => e.config.relationshipField).map((e) => ({ + // ⚠️ Read by ENTRY ID. This was + // `.filter(…).map((d, i) => stateRef.current[i])`, where `i` indexes + // the FILTERED array while the row state was indexed against the + // FULL one — so a declined or unresolved entry sitting above a real + // collection shifted every read below it by one and that + // collection's rows were silently dropped from the transaction. + // Data loss on save, not a display defect (objectui#6371). + // Kept beside its entry id: the same inputs advance the baseline once + // the batch has committed (objectui#10564). + const editDetails: SavedDetailInput[] | null = isEdit + ? entries.filter((e) => e.config.relationshipField).map((e) => ({ + entryId: e.id, + input: { childObject: e.config.childObject, relationshipField: e.config.relationshipField!, rows: rowStateRef.current[e.id]?.rows ?? [], original: rowStateRef.current[e.id]?.original ?? [], childSchema: childSchemasRef.current[e.config.childObject], - })), + }, + })) + : null; + const ops = editDetails + ? buildMasterDetailEditBatch( + schema.objectName, + String(schema.recordId), + parentData, + editDetails.map((d) => d.input), ) : buildMasterDetailBatch( schema.objectName, @@ -975,6 +1137,36 @@ export const MasterDetailForm: React.FC = ({ })), ); const res = await runBatchTransaction(dataSource, ops); + // The batch COMMITTED: advance the child rows' baseline from it, so a + // form that stays mounted diffs its next save against what this one + // wrote (objectui#10564). The parent's baseline advances in the header + // `` once this handler resolves, from the same save. A batch + // that rejects skips both, so the retry still carries every operation; + // `childRowsAfterSave` does not throw, so a committed batch never reads + // as a failed save with one baseline moved. Create mode has no baseline: + // `handleSaved` empties the rows for the next entry. + if (editDetails) { + const saved = childRowsAfterSave(ops, res?.results, editDetails); + setRowState((prev) => { + let next = prev; + for (const s of saved) { + const cur = prev[s.entryId]; + if (!cur || cur.original !== s.diffedAgainst) continue; + // A created row takes its id by identity with the row the batch + // was built from. A row edited while the save was in flight is a + // new object and keeps none: the next save then deletes the created + // record and creates the row as it now stands. + const rows = s.createdIds.size === 0 + ? cur.rows + : cur.rows.map((r) => { + const id = s.createdIds.get(r); + return id === undefined ? r : { ...r, id }; + }); + next = { ...next, [s.entryId]: { rows, original: s.original } }; + } + return next; + }); + } // create → parent is op 0; edit → echo the parent values back. return res?.results?.[0] ?? { ...parentData, id: schema.recordId }; }, From 354bd97cb46b2a824deb02d096e18b898dc4dbb9 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 25 Sep 2026 12:24:40 +0000 Subject: [PATCH 2/3] chore(changeset): declare the master-detail child-row baseline fix Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_014mXUNuFomfj24w7s1pZzhN --- .../10564-master-detail-child-baseline.md | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 .changeset/10564-master-detail-child-baseline.md diff --git a/.changeset/10564-master-detail-child-baseline.md b/.changeset/10564-master-detail-child-baseline.md new file mode 100644 index 0000000000..4451a8b5b8 --- /dev/null +++ b/.changeset/10564-master-detail-child-baseline.md @@ -0,0 +1,20 @@ +--- +'@object-ui/plugin-form': patch +--- + +A master-detail edit form that stays open after a save no longer re-creates the line items it just created, re-deletes the ones it just deleted, or drops a line cell changed back to its first-read value (objectui#10564). + +**Clause-②: no.** No exported symbol, type or prop changes. The change is in what the form's next save sends. + +**Before.** The form diffed every save against the child rows it read when it opened. A host that keeps the form mounted after a save (an `onSuccess` that does not navigate away) therefore got: + +- a second copy of a line the first save created, because that row never learned its id and was created again on the next save; +- a second `delete` of a line the first save had already deleted; +- a lost edit when a line cell was changed, saved, and changed back to its first-read value: the second save sent no line operation and still reported success, and the server kept the first save's value. + +**What changed, in observable terms.** + +- After a successful edit save, the rows that save created take the ids the batch returned for them, and the collection's baseline takes on what the save wrote. The next save compares with the lines as they now stand. This is the same rule the parent record already followed after a save (objectui#10156). +- A save that fails advances nothing, neither the parent nor the lines, so a retry still sends every operation. +- A line created by a save and edited while that save was still in flight is not duplicated. The next save deletes the created record and creates the line as it now stands. +- Create mode is unchanged: a successful create still clears the lines and the header for the next entry. From f036ff3a77c2374db87a44cca16dc0aae56d4d11 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 25 Sep 2026 12:29:28 +0000 Subject: [PATCH 3/3] refactor(plugin-form): type the child-row advance without `any` Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_014mXUNuFomfj24w7s1pZzhN --- packages/plugin-form/src/MasterDetailForm.tsx | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/plugin-form/src/MasterDetailForm.tsx b/packages/plugin-form/src/MasterDetailForm.tsx index 9c4d859641..c3676368a3 100644 --- a/packages/plugin-form/src/MasterDetailForm.tsx +++ b/packages/plugin-form/src/MasterDetailForm.tsx @@ -161,11 +161,11 @@ interface SavedChildRows { * is still the collection's baseline: a reload that replaced the row state * while the save was in flight has set a newer one. */ - diffedAgainst: Record[]; + diffedAgainst: Record[]; /** Each row the batch CREATED (the row object it was built from) → the id the server gave it. */ - createdIds: Map, unknown>; + createdIds: Map; /** The baseline after the save: every persisted row, laid over with what this batch wrote to it. */ - original: Record[]; + original: Record[]; } /** @@ -174,8 +174,8 @@ interface SavedChildRows { * The builder only drops keys from a row (sanitize) and never rewrites one. */ function isBuiltFrom( - data: Record | undefined, - row: Record, + data: Record | undefined, + row: Record, relationshipField: string, ): boolean { return Object.entries(data ?? {}).every( @@ -230,8 +230,8 @@ function childRowsAfterSave( const { childObject, relationshipField } = input; const rows = input.rows || []; const original = input.original || []; - const createdIds = new Map, unknown>(); - const written = new Map>(); + const createdIds = new Map(); + const written = new Map>(); for (const row of rows) { if (pairingLost) break; @@ -254,19 +254,19 @@ function childRowsAfterSave( } } - const before = new Map>(); + const before = new Map>(); for (const r of original) { const id = idOf(r); if (id != null) before.set(id, r); } - const next: Record[] = []; + const next: Record[] = []; for (const row of rows) { const id = idOf(row) ?? createdIds.get(row); if (id == null) continue; const was = before.get(id); const wrote = written.get(id); if (!was && !wrote) continue; - const entry: Record = { ...(was ?? {}), ...(wrote ?? {}) }; + const entry: Record = { ...(was ?? {}), ...(wrote ?? {}) }; if (idOf(entry) == null) entry.id = id; next.push(entry); }