From c8776848920b04a51cb82051bed19f5ae1e3d352 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 04:16:50 +0000 Subject: [PATCH 1/7] fix(plugin-grid,plugin-list,components,types): the grid's inline-edit saves send each row's If-Match and report a batch per row (objectui#12292) WIP: implementation before build and pins. Claude-Session: https://claude.ai/code/session_01CsyMRsFJBKNEMa4oPY1b5z Co-authored-by: Claude --- .../src/renderers/complex/data-table.tsx | 54 +++- packages/plugin-grid/src/ObjectGrid.tsx | 293 +++++++++++++++++- packages/plugin-list/src/ListView.tsx | 18 +- packages/types/src/data-display.ts | 15 +- 4 files changed, 354 insertions(+), 26 deletions(-) diff --git a/packages/components/src/renderers/complex/data-table.tsx b/packages/components/src/renderers/complex/data-table.tsx index 41cddf6648..2457893293 100644 --- a/packages/components/src/renderers/complex/data-table.tsx +++ b/packages/components/src/renderers/complex/data-table.tsx @@ -281,6 +281,15 @@ function extractSaveErrorMessage(error: unknown): string { */ type RowActionDef = NonNullable[number]; +/** + * The per-row outcome `onBatchSave` may resolve with (objectui#12292), derived + * from the slot's declared return rather than restated here. + */ +type BatchSaveOutcome = Exclude< + Awaited>>, + void +>; + /** * The authoring shape of the built-in row Edit/Delete predicates — derived from * `DataTableSchema`, never restated by hand (objectui#4354). Everything below @@ -2163,30 +2172,57 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => { setIsSaving(true); try { + // The pending key each handed-over `rowIndex` came from, so a per-row + // outcome can name its row back (objectui#12292). + const pendingKeyOf = new Map(); const changesToSave = Array.from(pendingChanges.entries()).map(([rowIndex, changes]) => { const globalIndex = (effectivePage - 1) * pageSize + rowIndex; const row = sortedData[manualPagination ? rowIndex : globalIndex]; + pendingKeyOf.set(globalIndex, rowIndex); return { rowIndex: globalIndex, changes, row }; }); - - if (schema.onBatchSave) { - await schema.onBatchSave(changesToSave); + + // `void` at runtime is `undefined`: the handler persisted every row. + const outcome = (schema.onBatchSave + ? await schema.onBatchSave(changesToSave) + : undefined) as BatchSaveOutcome | undefined; + const failed = outcome?.failed ?? []; + const failedKeys = new Set(); + for (const entry of failed) { + const key = pendingKeyOf.get(entry.rowIndex); + // A failure naming a row this batch never handed over cannot be + // placed, so no row can be called persisted: fall back to the + // all-or-nothing reading below rather than clear an unsaved edit. + if (key === undefined) throw entry.error; + failedKeys.add(key); } - - // Clear all pending changes - setPendingChanges(new Map()); + // Any staged editor left open (e.g. a lookup picker that keeps its widget - // open on pick) must exit edit mode now that every row is persisted — + // open on pick) must exit edit mode now that the batch has settled — // otherwise the edited cell stays stuck showing the editor after 全部保存. editingCellRef.current = null; setEditingCell(null); setEditValue(''); + if (failedKeys.size > 0) { + // Per-row outcome: the rows that persisted are cleared, and only the + // rows that did not are kept and flagged. The server already accepted + // the others, so keeping them pending would report a landed write as + // a failed one, and a retry would send it again. + console.error('Failed to save rows:', failed.map((entry) => entry.error)); + setPendingChanges(new Map(Array.from(pendingChanges).filter(([key]) => failedKeys.has(key)))); + setErroredRows(failedKeys); + setSaveError(extractSaveErrorMessage(failed[0].error)); + return; + } + // Clear all pending changes + setPendingChanges(new Map()); // Saved — clear any prior errors. setErroredRows(new Set()); setSaveError(null); } catch (error) { - // Batch is all-or-nothing here: keep every pending row, flag them all, - // and surface the reason instead of failing silently. + // A rejection says nothing about which rows landed, so it is read as + // all-or-nothing: keep every pending row, flag them all, and surface the + // reason instead of failing silently. console.error('Failed to save batch:', error); setSaveError(extractSaveErrorMessage(error)); setErroredRows(new Set(pendingChanges.keys())); diff --git a/packages/plugin-grid/src/ObjectGrid.tsx b/packages/plugin-grid/src/ObjectGrid.tsx index 4e241b4f7e..1fe4a23306 100644 --- a/packages/plugin-grid/src/ObjectGrid.tsx +++ b/packages/plugin-grid/src/ObjectGrid.tsx @@ -26,7 +26,7 @@ import type { ObjectGridSchema, DataSource, ListColumn, TableColumn, ViewData, T import { isSystemManagedField, normalizeTableColumnType } from '@object-ui/types'; import type { I18nLabel } from '@objectstack/spec/ui'; import { parseFilterAST, resolveFieldScale, type FilterCondition } from '@objectstack/spec/data'; -import { SchemaRenderer, useDataScope, useNavigationOverlay, useAction, useSafeFieldLabel, usePredicateScope, useRelatedRecordActions, RelatedRecordActionsProvider, useDataInvalidation, useFilterScope, useMetadata } from '@object-ui/react'; +import { SchemaRenderer, useDataScope, useNavigationOverlay, useAction, useSafeFieldLabel, usePredicateScope, useRelatedRecordActions, RelatedRecordActionsProvider, useDataInvalidation, notifyDataChanged, useFilterScope, useMetadata } from '@object-ui/react'; import { createSafeTranslation } from '@object-ui/i18n'; // objectui#8920 — the grid reaches a cell renderer through THIS module and // nowhere else. `getCellRenderer` / `resolveCellRendererType` are deliberately @@ -63,6 +63,10 @@ import { usePermissions } from '@object-ui/permissions'; import { RECORD_OVERLAY_DEFAULT_WIDTH, RecordDetailPanel, + ConcurrentUpdateDialog, + isConcurrentUpdateError, + useDetailTranslation, + type ConcurrentUpdateConflict, } from '@object-ui/plugin-detail'; import { ChevronRight, ChevronLeft, ChevronsLeft, ChevronsRight, Download, Rows2, Rows3, Rows4, AlignJustify, Type, Hash, Calendar, CheckSquare, User, Tag, Clock, Loader2 } from 'lucide-react'; import { useRowColor } from './useRowColor'; @@ -1495,6 +1499,63 @@ function describeNonPositivePageSize( */ const NO_GROUPING_REFUSAL = JSON.stringify({ refused: [], warned: [] }); +/** + * The row key that carries a record's optimistic-concurrency token + * (objectui#12292): `updated_at`, the key the record surfaces send as + * `If-Match`. + */ +const OCC_TOKEN_FIELD = 'updated_at'; + +/** + * The concurrency token a row carries, read exactly as the record surfaces read + * it (`occVersionOf` in `plugin-form`, the save in `plugin-detail`'s + * `InlineEditSaveBar`): a non-empty string `updated_at`. Anything else (a + * backend without timestamps, a row read without the column) answers + * `undefined`, and the write goes out unguarded, as it did before. + */ +function rowVersionOf(row: unknown): string | undefined { + const v = (row as Record | null | undefined)?.[OCC_TOKEN_FIELD]; + return typeof v === 'string' && v.length > 0 ? v : undefined; +} + +/** What this grid remembers about its last landed write to one record. */ +interface WrittenRowVersion { + /** The token the row carried when that write was made: the read it superseded. */ + readVersion: string; + /** The token of the record the server answered that write with. */ + version: string; +} + +/** + * The `ifMatch` a row save sends: the rule `plugin-form`'s `versionToSend` + * applies to a form that stays open (objectui#10565), applied to a row. + * + * A saved row keeps the token it was READ with until the grid re-reads it, and + * that token stopped being current the moment this grid's own write landed. So + * while the row still carries the read a landed write superseded, the version + * that write answered with is sent. A row carrying any other token is a newer + * read, and its token wins. A row with no token stays unguarded. + */ +function rowVersionToSend( + written: WrittenRowVersion | undefined, + readVersion: string | undefined, +): string | undefined { + if (!readVersion) return undefined; + return written && written.readVersion === readVersion ? written.version : readVersion; +} + +/** The three ways out of `ConcurrentUpdateDialog`, by its own button names. */ +type RowConflictChoice = 'reload' | 'overwrite' | 'cancel'; + +/** + * The per-row outcome the table's `onBatchSave` slot reads, derived from the + * slot's declared return rather than restated here. + */ +type BatchSaveOutcome = Exclude< + Awaited>>, + void +>; + export const ObjectGrid: React.FC = ({ schema, dataSource, @@ -1547,6 +1608,18 @@ export const ObjectGrid: React.FC = ({ const [objectSchema, setObjectSchema] = useState(null); const [useCardView, setUseCardView] = useState(false); const [refreshKey, setRefreshKey] = useState(0); + // Optimistic concurrency for the default inline-edit saves (objectui#12292). + // Per record, the version this grid's last landed write returned — see + // `rowVersionToSend`. It lives as long as this grid does. + const writtenVersionsRef = React.useRef(new Map()); + // The conflict a 409 opened, on the record surfaces' own dialog, and the + // resolver of the save that is awaiting the user's choice. + const { t: tDetail } = useDetailTranslation(); + const [rowConflict, setRowConflict] = useState(null); + const rowConflictDecisionRef = React.useRef<((choice: RowConflictChoice) => void) | null>(null); + // A grid torn down mid-conflict settles it as Cancel, so the awaiting save + // never hangs. + useEffect(() => () => rowConflictDecisionRef.current?.('cancel'), []); const [showExport, setShowExport] = useState(false); const [exportBusy, setExportBusy] = useState(false); const [exportError, setExportError] = useState(null); @@ -1980,6 +2053,11 @@ export const ObjectGrid: React.FC = ({ const schemaFields = schema.fields; const schemaColumns = schema.columns; + // [objectui#12292] Whether this grid's rows must carry their concurrency + // token: the AUTHORED key, not `inlineEditable`, so the principal's write + // verdict landing does not cost a second fetch. A read-only principal is + // sent one column it does not use. + const carriesConcurrencyToken = schema.editable === true; // [objectui#7179] The fetch effect's dep on the grouping block, as a CONTENT // key over the field NAMES alone. // @@ -2588,9 +2666,20 @@ export const ObjectGrid: React.FC = ({ .filter((f) => isProjectableField(f, declared as Record)) .filter((f) => passesProjectionGate(f)) : []; + // [objectui#12292] The row's concurrency token. An inline-editable + // grid's default save sends it as `If-Match` (`rowVersionOf`), so + // every row has to carry it. A projection built from the columns + // left it off each row no column showed it on, and the save went + // out with no token at all. The same two gates as the harvests + // above: a platform column, and one this principal may read. + const concurrencyFields = carriesConcurrencyToken && declared + ? [OCC_TOKEN_FIELD] + .filter((f) => isProjectableField(f, declared as Record)) + .filter((f) => passesProjectionGate(f)) + : []; // A UNION, never an append: a grouping field that IS also a column // must not produce a duplicate `$select` entry. - const extraFields = [...predicateFields, ...groupingFields]; + const extraFields = [...predicateFields, ...groupingFields, ...concurrencyFields]; const withHarvestedFields = (list: any[]): any[] => { if (extraFields.length === 0) return list; const names = new Set(list.map((f: any) => columnIdentity(f))); @@ -2897,7 +2986,9 @@ export const ObjectGrid: React.FC = ({ // `predicateProjectionKey` and `searchFieldsKey` (objectui#10689): the // harvested predicate operands and the `$searchFields` this query sends, each // a string compared by value — see their declarations above. - }, [objectName, schemaFields, schemaColumns, schemaFilter, schemaFilterRefusal, schemaDefaultFilters, schemaSort, headerSort, querySearchTerm, schemaPagination, schemaPageSize, serverPage, fetchWindow, dataSource, hasInlineData, dataConfig, refreshKey, perms.isLoaded, groupingProjectionKey, predicateProjectionKey, searchFieldsKey, invalidationNonce, serverGroupedFetch, groupingNeedsHeaderQuery]); + // `carriesConcurrencyToken` (objectui#12292): a boolean, so turning inline + // edit on re-reads the rows WITH their token — see its declaration above. + }, [objectName, schemaFields, schemaColumns, schemaFilter, schemaFilterRefusal, schemaDefaultFilters, schemaSort, headerSort, querySearchTerm, schemaPagination, schemaPageSize, serverPage, fetchWindow, dataSource, hasInlineData, dataConfig, refreshKey, perms.isLoaded, groupingProjectionKey, predicateProjectionKey, searchFieldsKey, invalidationNonce, serverGroupedFetch, groupingNeedsHeaderQuery, carriesConcurrencyToken]); // The same reset, for the path the loader above never runs on (objectui#4501 // clause 2). "All N matching are selected" is a claim about ONE query, so it @@ -5325,9 +5416,10 @@ export const ObjectGrid: React.FC = ({ // But a declaratively-configured `editable: true` view has no host wiring — so // "Save All" would otherwise just clear pending changes without writing to the // backend. Supply a default that persists through the grid's `dataSource`, then - // refresh so the grid reflects persisted values. Throwing on failure is - // important: DataTable's saveRow/saveBatch keep pending changes when the save - // promise rejects, so a failed write doesn't silently lose the user's edits. + // refresh so the grid reflects persisted values. Reporting failure is + // important: DataTable's saveRow keeps a row's pending changes when the save + // promise rejects, and saveBatch keeps exactly the rows the batch hands back + // as `failed`, so a failed write doesn't silently lose the user's edits. // The one place a row's primary key is read for a write. `string`, as // `@objectstack/spec` declares every record door — the union this used to // annotate was a claim about `any`-typed row data, not a measurement of it @@ -5335,6 +5427,126 @@ export const ObjectGrid: React.FC = ({ const resolveRecordId = (row: any): string | undefined => row?._id ?? row?.id; + // ── Optimistic concurrency on the default saves (objectui#12292) ────────── + // + // Every save below sends the row's token as `If-Match` (`rowVersionOf`, + // `rowVersionToSend`), so a write made from a stale row is refused `409` + // instead of landing over a concurrent one with a 200. A `409` goes to the + // record surfaces' own conflict dialog, `ConcurrentUpdateDialog` from + // `@object-ui/plugin-detail`, the one `InlineEditSaveBar` opens. There is no + // grid-only dialog, and nothing is retried without the user's choice. + + /** Open the conflict dialog and wait for the user's choice. */ + const askRowConflict = (conflict: ConcurrentUpdateConflict): Promise => + new Promise((resolve) => { + rowConflictDecisionRef.current = resolve; + setRowConflict(conflict); + }); + const settleRowConflict = (choice: RowConflictChoice) => { + const resolve = rowConflictDecisionRef.current; + rowConflictDecisionRef.current = null; + setRowConflict(null); + resolve?.(choice); + }; + + /** The header the user reads over a column, for naming the field in the dialog. */ + const columnHeaderOf = (field: string): string | undefined => { + const column = (orderedColumns as Array<{ accessorKey?: unknown; header?: unknown }>) + .find((c) => c?.accessorKey === field); + return typeof column?.header === 'string' && column.header.length > 0 ? column.header : undefined; + }; + + /** + * The dialog's payload for one row's 409, in the shape `InlineEditSaveBar` + * builds for a record's draft: one edited field shows its own before and + * after, several show the record-level summary. + */ + const buildRowConflict = ( + changes: Record, + err: { currentVersion?: string; currentRecord?: Record | null }, + ): ConcurrentUpdateConflict => { + const keys = Object.keys(changes); + const current = err.currentRecord ?? null; + if (keys.length === 1) { + const field = keys[0]; + return { + field, + label: columnHeaderOf(field), + pendingValue: changes[field], + currentValue: current ? current[field] : undefined, + currentVersion: err.currentVersion, + currentRecord: current, + }; + } + return { + field: keys.join(', '), + label: tDetail('detail.concurrentUpdateRecordLabel', { defaultValue: 'this record' }), + pendingValue: changes, + currentValue: current ? Object.fromEntries(keys.map((k) => [k, current[k]])) : undefined, + currentVersion: err.currentVersion, + currentRecord: current, + }; + }; + + /** + * Remember what a landed write answered with, for the next save of the same + * record. A result with no token clears the entry, so that save falls back + * to the row's own token rather than to an older write's. + */ + const rememberRowWrite = (id: string, readVersion: string | undefined, result: unknown) => { + const version = rowVersionOf(result); + if (readVersion && version) writtenVersionsRef.current.set(id, { readVersion, version }); + else writtenVersionsRef.current.delete(id); + }; + + /** One row's guarded write. Rejects with whatever the data source threw. */ + const writeRowGuarded = async ( + ds: DataSource, + resource: string, + id: string, + changes: Record, + row: any, + ): Promise => { + const readVersion = rowVersionOf(row); + const ifMatch = rowVersionToSend(writtenVersionsRef.current.get(id), readVersion); + const result = await ds.update(resource, id, changes, ifMatch ? { ifMatch } : undefined); + rememberRowWrite(id, readVersion, result); + }; + + /** + * After a row's 409, the user's choice on the dialog: + * - Overwrite: the same changes, re-keyed to the version the 409 reported + * ("I have seen the newer record, apply mine on top"). Resolves `saved`. + * - Reload latest: the edit is dropped and the record is declared changed on + * the data-invalidation bus, so whoever holds the rows reads them again. + * Resolves `reloaded`; nothing was written. + * - Cancel: rejects with the 409, so the table keeps the edit and flags the + * row as not saved. + */ + const resolveRowConflict = async ( + ds: DataSource, + resource: string, + id: string, + changes: Record, + row: any, + err: { currentVersion?: string; currentRecord?: Record | null }, + ): Promise<'saved' | 'reloaded'> => { + const choice = await askRowConflict(buildRowConflict(changes, err)); + if (choice === 'cancel') throw err; + if (choice === 'reload') { + notifyDataChanged({ objectName: resource, recordId: String(id) }); + return 'reloaded'; + } + const currentVersion = typeof err.currentVersion === 'string' && err.currentVersion.length > 0 + ? err.currentVersion + : undefined; + const result = await ds.update( + resource, id, changes, currentVersion ? { ifMatch: currentVersion } : undefined, + ); + rememberRowWrite(id, rowVersionOf(row), result); + return 'saved'; + }; + const defaultRowSave = async ( _rowIndex: number, changes: Record, @@ -5347,30 +5559,69 @@ export const ObjectGrid: React.FC = ({ if (id === undefined || id === null) { throw new Error('Cannot persist inline edit: row has no id/_id.'); } - await dataSource.update(objectName, id, changes); + try { + await writeRowGuarded(dataSource, objectName, id, changes, row); + } catch (err) { + if (!isConcurrentUpdateError(err)) throw err; + // A reload re-reads through the bus; there is nothing of ours to show. + if ((await resolveRowConflict(dataSource, objectName, id, changes, row, err)) === 'reloaded') return; + } // Refresh so the grid shows the persisted values. setRefreshKey(k => k + 1); }; const defaultBatchSave = async ( changes: Array<{ rowIndex: number; changes: Record; row: any }>, - ): Promise => { + ): Promise => { if (!dataSource || !objectName) { throw new Error('Cannot persist inline edits: no dataSource/objectName configured on the grid.'); } // Update each modified row. The DataSource `bulk`/`bulkUpdate` primitives // apply a single uniform patch across many ids, which does NOT fit per-row // edits (each row has its own field changes), so issue one update per row. - await Promise.all( - changes.map(({ changes: rowChanges, row }) => { - const id = resolveRecordId(row); + // + // Each row is its own write and its own outcome (objectui#12292). All are + // attempted together and every one is waited for, so a refused row never + // hides what happened to the others; a conflict is then put to the user + // one row at a time. Nothing the server accepted is taken back: the rows + // that landed are reported as saved, and only the rows that did not are + // handed back to the table as `failed`. + const ids = changes.map(({ row }) => resolveRecordId(row)); + const attempts = await Promise.allSettled( + changes.map(({ changes: rowChanges, row }, i) => { + const id = ids[i]; if (id === undefined || id === null) { - throw new Error('Cannot persist inline edit: row has no id/_id.'); + return Promise.reject(new Error('Cannot persist inline edit: row has no id/_id.')); } - return dataSource.update(objectName, id, rowChanges); + return writeRowGuarded(dataSource, objectName, id, rowChanges, row); }), ); - setRefreshKey(k => k + 1); + const failed: BatchSaveOutcome['failed'] = []; + let landed = false; + for (let i = 0; i < changes.length; i++) { + const attempt = attempts[i]; + if (attempt.status === 'fulfilled') { + landed = true; + continue; + } + const { rowIndex, changes: rowChanges, row } = changes[i]; + const id = ids[i]; + const err: unknown = attempt.reason; + if (!isConcurrentUpdateError(err) || id === undefined || id === null) { + failed.push({ rowIndex, error: err }); + continue; + } + try { + if ((await resolveRowConflict(dataSource, objectName, id, rowChanges, row, err)) === 'saved') { + landed = true; + } + } catch (resolutionError) { + failed.push({ rowIndex, error: resolutionError }); + } + } + // Refresh so the grid shows the persisted values. + if (landed) setRefreshKey(k => k + 1); + return failed.length > 0 ? { failed } : undefined; }; // Determine pagination settings (support both new and legacy formats) @@ -6699,6 +6950,18 @@ export const ObjectGrid: React.FC = ({ /> ); + // A row save's 409, on the record surfaces' own dialog (objectui#12292). + // Shared across both render branches, like the bulk dialog above. + const rowConflictDialog = ( + settleRowConflict('reload')} + onOverwrite={() => settleRowConflict('overwrite')} + onCancel={() => settleRowConflict('cancel')} + /> + ); + // For split mode, wrap the grid in the ResizablePanelGroup. // // ⭐ objectui#9299, MEASURED WHILE REPAIRING THIS CARD and repaired here. @@ -6746,6 +7009,7 @@ export const ObjectGrid: React.FC = ({ {(record) => renderRecordDetail(record)} {bulkDialog} + {rowConflictDialog} ); } @@ -6789,6 +7053,7 @@ export const ObjectGrid: React.FC = ({ )} {bulkDialog} + {rowConflictDialog} ); }; diff --git a/packages/plugin-list/src/ListView.tsx b/packages/plugin-list/src/ListView.tsx index ef3d7fe4ea..5129b2288f 100644 --- a/packages/plugin-list/src/ListView.tsx +++ b/packages/plugin-list/src/ListView.tsx @@ -2054,6 +2054,15 @@ export const ListView = React.forwardRef(({ return resolveAffordance('listInlineEdit', { objectSchema: objectDef as SchemaLike, objectName: schema.objectName, perms: affordancePerms }).allowed; }, [schema.userActions, schema.objectName, objectDef, affordancePerms]); + /** + * Whether the rows this list fetches must carry their concurrency token + * (objectui#12292): exactly when the grid it hands them to is editable in + * place — the same expression as that grid's `editable` below. The grid's + * save sends each row's `updated_at` as `If-Match`, so the fetch projects it; + * turning the mode on re-reads the rows with it. + */ + const rowsCarryConcurrencyToken = inlineEdit && inlineEditOffered; + // Normalize exportOptions: support both ObjectUI object format and spec string[] format const resolvedExportOptions = React.useMemo(() => { if (!schema.exportOptions) return undefined; @@ -2996,6 +3005,13 @@ export const ListView = React.forwardRef(({ userActions: (objectDef as any)?.userActions, }))) addPredicateField(f); + // [objectui#12292] The rows' concurrency token, which the grid's + // inline-edit save sends as `If-Match`. A projection built from the + // columns left it off every row no column showed it on, and that + // save went out with no token. It is a platform column, admitted by + // the same rule as a predicate operand. + if (rowsCarryConcurrencyToken) addPredicateField('updated_at'); + return Array.from(required); })(); @@ -3231,7 +3247,7 @@ export const ListView = React.forwardRef(({ // silently un-suppresses nothing, because the finding it was suppressing // simply moves elsewhere. Add prose ABOVE this point, never below it. // eslint-disable-next-line react-hooks/exhaustive-deps - }, [schema.objectName, schema.data, dataSource, authoredFilter, effectivePageSize, currentSort, appliedFilters, appliedUserFilterConditions, refreshKey, searchTerm, schema.searchableFields, schema.columns, (schema as any).kanban, (schema as any).calendar, (schema as any).gallery, (schema as any).timeline, (schema as any).gantt, schema.map, (schema as any).options, objectDef?.fields, objectDefLoaded, schema.refreshTrigger, perms, fetchSkip, groupingConfig, predicateProjectionKey, ganttOwnsData, invalidationNonce, groupingNeedsHeaderQuery, calendarWindowKey]); // Re-fetch on filter/sort/search/refreshTrigger/perms/window change + }, [schema.objectName, schema.data, dataSource, authoredFilter, effectivePageSize, currentSort, appliedFilters, appliedUserFilterConditions, refreshKey, searchTerm, schema.searchableFields, schema.columns, (schema as any).kanban, (schema as any).calendar, (schema as any).gallery, (schema as any).timeline, (schema as any).gantt, schema.map, (schema as any).options, objectDef?.fields, objectDefLoaded, schema.refreshTrigger, perms, fetchSkip, groupingConfig, predicateProjectionKey, ganttOwnsData, invalidationNonce, groupingNeedsHeaderQuery, calendarWindowKey, rowsCarryConcurrencyToken]); // Re-fetch on filter/sort/search/refreshTrigger/perms/window change; `rowsCarryConcurrencyToken` (objectui#12292) re-reads the rows with their token when inline edit turns on // Any change to the result-defining inputs (object, filters, sort, search, // grouping, page size) invalidates the current page number — snap back to diff --git a/packages/types/src/data-display.ts b/packages/types/src/data-display.ts index 8b50b87311..03e57cb0d5 100644 --- a/packages/types/src/data-display.ts +++ b/packages/types/src/data-display.ts @@ -1574,8 +1574,19 @@ export interface DataTableSchema extends BaseSchema { * (`await schema.onBatchSave(changesToSave)`). Supplied by `ObjectGrid` as * `onBatchSave ?? defaultBatchSave`, the same two-limb shape as * {@link onRowSave}. - */ - onBatchSave?: (changes: Array<{ rowIndex: number; changes: Record; row: any }>) => void | Promise; + * + * The outcome is read PER ROW (objectui#12292). A batch is several + * independent writes, so some can land while others are refused, and the + * server does not take the landed ones back. A handler that knows which rows + * did not persist resolves with `{ failed }`, each entry naming a row by the + * `rowIndex` it was handed: the table keeps and flags exactly those rows and + * clears the rest. Resolving with nothing means every row persisted. + * Rejecting means the handler cannot say which rows landed, so the table + * keeps and flags every row, as it always has. + */ + onBatchSave?: ( + changes: Array<{ rowIndex: number; changes: Record; row: any }>, + ) => void | Promise }>; /** * Row click handler * Called when a row is clicked From 989c00016f3aaa0e3419872940b7a77ebaaa1063 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 04:22:37 +0000 Subject: [PATCH 2/7] test(plugin-grid,plugin-list,components): pin the grid's If-Match saves, the token projection and the per-row batch outcome (objectui#12292) Claude-Session: https://claude.ai/code/session_01CsyMRsFJBKNEMa4oPY1b5z Co-authored-by: Claude --- .changeset/12292-grid-save-if-match.md | 14 + .../data-table-batch-per-row-12292.test.tsx | 106 ++++++ .../inlineEditConcurrency-12292.test.tsx | 333 ++++++++++++++++++ ....concurrencyTokenProjection-12292.test.tsx | 91 +++++ 4 files changed, 544 insertions(+) create mode 100644 .changeset/12292-grid-save-if-match.md create mode 100644 packages/components/src/__tests__/data-table-batch-per-row-12292.test.tsx create mode 100644 packages/plugin-grid/src/__tests__/inlineEditConcurrency-12292.test.tsx create mode 100644 packages/plugin-list/src/__tests__/ListView.concurrencyTokenProjection-12292.test.tsx diff --git a/.changeset/12292-grid-save-if-match.md b/.changeset/12292-grid-save-if-match.md new file mode 100644 index 0000000000..76c2a318e7 --- /dev/null +++ b/.changeset/12292-grid-save-if-match.md @@ -0,0 +1,14 @@ +--- +'@object-ui/plugin-grid': patch +'@object-ui/plugin-list': patch +'@object-ui/components': patch +'@object-ui/types': patch +--- + +A grid's inline-edit save no longer overwrites a concurrent write without a word (objectui#12292). The grid's default row save and "Save All" now send each row's `updated_at` as `If-Match`, the way the record forms and the record page's inline edit already do. A stale grid edit is refused with `409` instead of landing with a `200`. The refusal opens the same conflict dialog the record page opens (`ConcurrentUpdateDialog`, offering Reload latest, Overwrite anyway or Cancel). After a save, the row's next save sends the token of the record the server answered with, so saving the same row twice does not conflict with your own first save. + +The rows now carry that token. An inline-editable grid, and a list view in inline-edit mode, add `updated_at` to the query's `$select` even when no column shows it. Before, the token was missing from every row unless a column displayed it, so there was nothing to send. + +"Save All" reports each row on its own. A row that is refused (a conflict you cancel, a validation error) stays pending and flagged, and the rows that saved are cleared, not reported as failed and not sent again. Nothing the server accepted is rolled back. + +**Type change, additive.** `DataTableSchema.onBatchSave` may now resolve with `{ failed: Array<{ rowIndex, error }> }`, naming the rows that did not persist by the `rowIndex` they were handed. The table keeps and flags exactly those rows. A handler that resolves with nothing or rejects is read exactly as before, so existing handlers need no change. diff --git a/packages/components/src/__tests__/data-table-batch-per-row-12292.test.tsx b/packages/components/src/__tests__/data-table-batch-per-row-12292.test.tsx new file mode 100644 index 0000000000..1f0f42d649 --- /dev/null +++ b/packages/components/src/__tests__/data-table-batch-per-row-12292.test.tsx @@ -0,0 +1,106 @@ +/** + * 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#12292 — "Save All" reads the batch's outcome PER ROW. + * + * A batch is several independent writes: some can land while others are + * refused, and the server keeps the ones it accepted. The table used to read + * every batch as all-or-nothing, so one refused row kept every row pending and + * flagged, rows that had landed included, and a retry sent them again. An + * `onBatchSave` that knows which rows did not persist now resolves with + * `{ failed }`, and the table keeps exactly those rows. A rejection still reads + * as all-or-nothing, because it says nothing about which rows landed; the + * existing pins in `data-table-inline-edit.test.tsx` hold that half. + */ +import { describe, it, expect, vi } from 'vitest'; +import { fireEvent, waitFor } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import React from 'react'; +import { renderComponent } from './test-utils'; +// Registers the renderers at module scope, NOT inside a `beforeAll`; see +// object-ui/no-dynamic-import-in-test-hook (objectui#3010/#3021). +import '../renderers'; + +const schema = { + type: 'data-table' as const, + editable: true, + singleClickEdit: true, + columns: [ + { header: 'Task', accessorKey: 'name', editable: false }, + { header: 'Estimate', accessorKey: 'qty', type: 'number' }, + ], + data: [ + { id: '1', name: 'Ingest pipeline', qty: 60 }, + { id: '2', name: 'Warehouse schema', qty: 33 }, + ], +} as any; + +type Entry = { rowIndex: number; changes: Record; row: any }; + +/** Stage a new value in the Estimate cell of the given body row. */ +function stage(container: HTMLElement, rowNumber: number, value: string) { + const row = container.querySelectorAll('tbody tr')[rowNumber] as HTMLElement; + const cell = row.querySelectorAll('td')[1] as HTMLElement; + fireEvent.click(cell); + const input = cell.querySelector('input') as HTMLInputElement; + fireEvent.change(input, { target: { value } }); + fireEvent.blur(input); +} + +describe('data-table — Save All reads a per-row batch outcome (objectui#12292)', () => { + it('keeps and flags only the rows the batch names as failed', async () => { + const onBatchSave = vi.fn(async (entries: Entry[]) => ({ + failed: entries + .filter((e) => e.row.id === '2') + .map((e) => ({ rowIndex: e.rowIndex, error: new Error('Warehouse schema was refused') })), + })); + const { container, getByText, getByRole } = renderComponent({ ...schema, onBatchSave }); + + stage(container, 0, '61'); + stage(container, 1, '31'); + expect(getByText(/2 rows modified/)).toBeInTheDocument(); + fireEvent.click(getByText(/Save All/)); + + await waitFor(() => expect(getByText(/1 row modified/)).toBeInTheDocument()); + expect(onBatchSave).toHaveBeenCalledTimes(1); + expect(getByRole('button', { name: /Save All \(1\)/ })).toBeInTheDocument(); + expect(container.textContent).toContain('Warehouse schema was refused'); + // The refused row keeps its edit; the row that persisted holds none. + const rows = container.querySelectorAll('tbody tr'); + expect(rows[1].textContent).toContain('31'); + expect(rows[0].textContent).not.toContain('61'); + }); + + it('clears every row when the outcome names no failure', async () => { + const onBatchSave = vi.fn(async () => ({ failed: [] })); + const { container, getByText, queryByText } = renderComponent({ ...schema, onBatchSave }); + + stage(container, 0, '61'); + stage(container, 1, '31'); + fireEvent.click(getByText(/Save All/)); + + await waitFor(() => expect(onBatchSave).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(queryByText(/modified/)).toBeNull()); + expect(container.textContent).not.toContain('Save failed'); + }); + + it('keeps every row when a failure names a row the batch never handed over', async () => { + const onBatchSave = vi.fn(async () => ({ + failed: [{ rowIndex: 999, error: new Error('Unplaceable refusal') }], + })); + const { container, getByText } = renderComponent({ ...schema, onBatchSave }); + + stage(container, 0, '61'); + stage(container, 1, '31'); + fireEvent.click(getByText(/Save All/)); + + await waitFor(() => expect(container.textContent).toContain('Unplaceable refusal')); + expect(getByText(/2 rows modified/)).toBeInTheDocument(); + }); +}); diff --git a/packages/plugin-grid/src/__tests__/inlineEditConcurrency-12292.test.tsx b/packages/plugin-grid/src/__tests__/inlineEditConcurrency-12292.test.tsx new file mode 100644 index 0000000000..5a7eaf4677 --- /dev/null +++ b/packages/plugin-grid/src/__tests__/inlineEditConcurrency-12292.test.tsx @@ -0,0 +1,333 @@ +/** + * 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#12292 — the grid's inline-edit saves carry each row's concurrency + * token, so a stale grid edit is refused instead of silently overwriting a + * concurrent write. + * + * ## What is real here + * + * The real `ObjectGrid` renders the real `data-table`, and every WRITE goes + * through the real `@object-ui/data-objectstack` adapter and the real + * `@objectstack/client`. Only `fetch` is stubbed, by a small server holding a + * store with an `updated_at` per record. It answers a stale `If-Match` with the + * envelope `@objectstack/types` 17.7.0 maps a `ConcurrentUpdateError` to + * (`409`, `code: 'CONCURRENT_UPDATE'`, `currentVersion`, `currentRecord`), so + * the error the grid handles is the one the console's adapter builds from that + * answer. + * + * READS are stubbed on the data source, and they honour `$select` the way the + * server does: a row carries only the keys the query asked for. That is + * deliberate. A projection built from the columns alone leaves `updated_at` + * off every row, and then a save has no token to send. So these pins fail + * on that projection too, not only on a save that drops the token. + */ +import { describe, it, expect, vi, beforeAll, beforeEach, afterEach } from 'vitest'; +import { render, screen, waitFor, fireEvent, cleanup, within } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import React from 'react'; + +import { ObjectStackAdapter } from '@object-ui/data-objectstack'; +import { registerAllFields } from '@object-ui/fields'; +import { ActionProvider, SchemaRendererProvider } from '@object-ui/react'; +import { ObjectGrid } from '../ObjectGrid'; +import { __clearRecordCrudVerdictCache } from '../hooks/useRecordCrudVerdicts'; +import { installExplainDouble } from './explainDouble'; + +registerAllFields(); + +beforeAll(() => { + if (!Element.prototype.scrollIntoView) { + Element.prototype.scrollIntoView = vi.fn() as any; + } +}); + +// The row menu asks the record-level verdict endpoint; serve it from a double. +beforeEach(() => { + __clearRecordCrudVerdictCache(); + installExplainDouble(); +}); +afterEach(() => { + cleanup(); + vi.unstubAllGlobals(); +}); + +const OBJECT = 'task_12292'; + +const json = (status: number, body: unknown) => + new Response(JSON.stringify(body), { status, headers: { 'Content-Type': 'application/json' } }); + +interface Patch { + id: string; + ifMatch: string | undefined; + body: Record; + status: number; +} + +/** + * The fake server. `writeAsOtherUser` is the concurrent write the card + * reproduces with a second client; `freezeReads` makes every later read answer + * the rows as they are now, which is how a re-read that has not landed yet + * looks to the grid. + */ +function makeServer(seed: Array & { id: string }>) { + let clock = 0; + const stamp = () => `2026-10-13 00:00:${String(++clock).padStart(2, '0')}.000`; + const store: Record> = {}; + for (const row of seed) store[row.id] = { ...row, updated_at: stamp() }; + const patches: Patch[] = []; + let frozen: Record[] | null = null; + + const fetch = vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + const url = String(input); + const method = (init?.method ?? 'GET').toUpperCase(); + if (url.endsWith('/api/v1/discovery')) { + return json(200, { success: true, data: { capabilities: {}, routes: {} } }); + } + const match = url.match(/\/api\/v1\/data\/([^/?]+)\/([^/?]+)$/); + if (!match || method !== 'PATCH') throw new Error(`unexpected request ${method} ${url}`); + const [, object, rawId] = match; + const id = decodeURIComponent(rawId); + const headers = (init?.headers ?? {}) as Record; + const ifMatch = headers['If-Match']; + const body = JSON.parse(String(init?.body ?? '{}')) as Record; + const current = store[id]; + if (ifMatch !== undefined && ifMatch !== current.updated_at) { + patches.push({ id, ifMatch, body, status: 409 }); + return json(409, { + error: 'Record was modified by another user', + code: 'CONCURRENT_UPDATE', + currentVersion: current.updated_at, + currentRecord: { ...current }, + object, + }); + } + store[id] = { ...current, ...body, updated_at: stamp() }; + patches.push({ id, ifMatch, body, status: 200 }); + return json(200, { success: true, data: { object, id, record: { ...store[id] } } }); + }); + + const adapter = new ObjectStackAdapter({ baseUrl: 'http://test.local', fetch, autoReconnect: false }); + + const project = (row: Record, select: unknown) => + Array.isArray(select) + ? Object.fromEntries(select.filter((k) => k in row).map((k) => [k, row[k]])) + : { ...row }; + + const dataSource = { + find: vi.fn(async (_object: string, params?: { $select?: unknown }) => { + const rows = (frozen ?? Object.values(store)).map((r) => project(r, params?.$select)); + return { data: rows, total: rows.length, hasMore: false, pageSize: 50 }; + }), + findOne: vi.fn(async (_object: string, id: string) => ({ ...store[id] })), + update: vi.fn((object: string, id: string, data: any, opts?: { ifMatch?: string }) => + adapter.update(object, id, data, opts), + ), + getObjectSchema: vi.fn(async (name: string) => ({ + name, + fields: { + id: { type: 'text' }, + title: { type: 'text', label: 'Title' }, + estimate_hours: { type: 'number', label: 'Estimate' }, + }, + })), + } as any; + + return { + store, + patches, + dataSource, + writeAsOtherUser(id: string, changes: Record) { + store[id] = { ...store[id], ...changes, updated_at: stamp() }; + }, + freezeReads() { + frozen = Object.values(store).map((r) => ({ ...r })); + }, + }; +} + +function renderGrid(server: ReturnType) { + const schema: any = { + type: 'object-grid', + objectName: OBJECT, + editable: true, + singleClickEdit: true, + columns: [ + { field: 'title', label: 'Title', editable: false }, + { field: 'estimate_hours', label: 'Estimate', type: 'number' }, + ], + pagination: { pageSize: 50 }, + }; + return render( + + + {/* Row Edit/Delete handlers: the per-row save button exists. */} + {}} onDelete={() => {}} /> + + , + ); +} + +const rowOf = (title: string) => screen.getByText(title).closest('tr') as HTMLElement; + +/** Stage a new Estimate on the row titled `title`, the way a user types it. */ +async function editEstimate(title: string, from: string, to: string) { + const td = await waitFor(() => { + const cell = Array.from(rowOf(title).querySelectorAll('td')).find( + (c) => c.textContent?.trim() === from, + ); + if (!cell) throw new Error(`no ${from} cell on ${title}`); + return cell as HTMLElement; + }); + fireEvent.click(td); + const input = await waitFor(() => { + const el = td.querySelector('input') as HTMLInputElement | null; + if (!el) throw new Error('editor not open'); + return el; + }); + fireEvent.change(input, { target: { value: to } }); + fireEvent.keyDown(input, { key: 'Enter' }); + await waitFor(() => expect(within(rowOf(title)).getByText(to)).toBeInTheDocument()); +} + +const conflictDialog = () => screen.findByRole('alertdialog'); +const clickSaveAll = () => + fireEvent.click(screen.getByRole('button', { name: /Save All/ })); + +async function loaded(server: ReturnType) { + renderGrid(server); + await waitFor(() => expect(screen.getByText('Ingest pipeline')).toBeInTheDocument()); +} + +describe('ObjectGrid inline edit carries the row token (objectui#12292)', () => { + it('a stale row save sends If-Match and surfaces the 409 on the conflict dialog', async () => { + const server = makeServer([{ id: 't1', title: 'Ingest pipeline', estimate_hours: 60 }]); + await loaded(server); + const readVersion = server.store.t1.updated_at; + + server.writeAsOtherUser('t1', { estimate_hours: 77 }); + await editEstimate('Ingest pipeline', '60', '61'); + fireEvent.click(within(rowOf('Ingest pipeline')).getByTitle('Save row')); + + const dialog = await conflictDialog(); + expect(server.patches).toEqual([ + { id: 't1', ifMatch: readVersion, body: { estimate_hours: 61 }, status: 409 }, + ]); + // The record surfaces' dialog, naming the column and both values. + expect(within(dialog).getByText('This record was modified by someone else')).toBeInTheDocument(); + expect(within(dialog).getByText('Estimate')).toBeInTheDocument(); + expect(within(dialog).getByText('61')).toBeInTheDocument(); + expect(within(dialog).getByText('77')).toBeInTheDocument(); + + // Cancel: nothing is written, the edit is kept and the row reads unsaved. + fireEvent.click(within(dialog).getByRole('button', { name: 'Cancel' })); + await waitFor(() => + expect(screen.getByRole('alert')).toHaveTextContent('Record was modified by another user'), + ); + expect(server.store.t1.estimate_hours).toBe(77); + expect(screen.getByText(/1 row modified/)).toBeInTheDocument(); + expect(server.patches).toHaveLength(1); + }); + + it('Overwrite re-sends the edit keyed to the version the 409 reported', async () => { + const server = makeServer([{ id: 't1', title: 'Ingest pipeline', estimate_hours: 60 }]); + await loaded(server); + + server.writeAsOtherUser('t1', { estimate_hours: 77 }); + const theirVersion = server.store.t1.updated_at; + await editEstimate('Ingest pipeline', '60', '61'); + clickSaveAll(); + + fireEvent.click(within(await conflictDialog()).getByRole('button', { name: 'Overwrite anyway' })); + await waitFor(() => expect(server.store.t1.estimate_hours).toBe(61)); + expect(server.patches.map((p) => [p.ifMatch, p.status])).toEqual([ + [expect.any(String), 409], + [theirVersion, 200], + ]); + await waitFor(() => expect(screen.queryByText(/row modified/)).toBeNull()); + }); + + it('Reload latest writes nothing and re-reads the record', async () => { + const server = makeServer([{ id: 't1', title: 'Ingest pipeline', estimate_hours: 60 }]); + await loaded(server); + + server.writeAsOtherUser('t1', { estimate_hours: 77 }); + await editEstimate('Ingest pipeline', '60', '61'); + clickSaveAll(); + + fireEvent.click(within(await conflictDialog()).getByRole('button', { name: 'Reload latest' })); + await waitFor(() => expect(within(rowOf('Ingest pipeline')).getByText('77')).toBeInTheDocument()); + expect(server.store.t1.estimate_hours).toBe(77); + expect(server.patches).toHaveLength(1); + expect(screen.queryByText(/row modified/)).toBeNull(); + }); + + it('a fresh save succeeds, and the next save sends the token the server answered with', async () => { + const server = makeServer([{ id: 't1', title: 'Ingest pipeline', estimate_hours: 60 }]); + await loaded(server); + const readVersion = server.store.t1.updated_at; + // The grid's re-read after the save has not landed: reads keep answering + // the row as it was loaded, token and all. + server.freezeReads(); + + await editEstimate('Ingest pipeline', '60', '61'); + clickSaveAll(); + await waitFor(() => expect(server.store.t1.estimate_hours).toBe(61)); + const answeredVersion = server.store.t1.updated_at; + expect(server.patches[0]).toEqual({ id: 't1', ifMatch: readVersion, body: { estimate_hours: 61 }, status: 200 }); + await waitFor(() => expect(screen.queryByText(/row modified/)).toBeNull()); + + // The row still shows the stale read; the second save must not 409 over + // the user's own first save. + await editEstimate('Ingest pipeline', '60', '62'); + clickSaveAll(); + await waitFor(() => expect(server.store.t1.estimate_hours).toBe(62)); + expect(server.patches[1]).toEqual({ id: 't1', ifMatch: answeredVersion, body: { estimate_hours: 62 }, status: 200 }); + expect(screen.queryByRole('alertdialog')).toBeNull(); + }); + + it('a batch with one stale row reports that row and keeps the row that saved', async () => { + const server = makeServer([ + { id: 't1', title: 'Ingest pipeline', estimate_hours: 60 }, + { id: 't2', title: 'Warehouse schema', estimate_hours: 33 }, + ]); + await loaded(server); + + server.writeAsOtherUser('t1', { estimate_hours: 77 }); + await editEstimate('Ingest pipeline', '60', '61'); + await editEstimate('Warehouse schema', '33', '31'); + expect(screen.getByRole('button', { name: /Save All \(2\)/ })).toBeInTheDocument(); + clickSaveAll(); + + const dialog = await conflictDialog(); + // Both rows were sent with their own token; only the stale one was refused. + expect(server.patches.map((p) => [p.id, p.status]).sort()).toEqual([['t1', 409], ['t2', 200]]); + expect(server.patches.every((p) => typeof p.ifMatch === 'string')).toBe(true); + fireEvent.click(within(dialog).getByRole('button', { name: 'Cancel' })); + + // The stale row alone stays pending and flagged; the saved row is not + // rolled back and is not reported as failed. + await waitFor(() => expect(screen.getByText(/1 row modified/)).toBeInTheDocument()); + expect(screen.getByRole('button', { name: /Save All \(1\)/ })).toBeInTheDocument(); + expect(screen.getByRole('alert')).toHaveTextContent('Record was modified by another user'); + expect(within(rowOf('Ingest pipeline')).getByText('61')).toBeInTheDocument(); + await waitFor(() => expect(within(rowOf('Warehouse schema')).getByText('31')).toBeInTheDocument()); + expect(server.store.t2.estimate_hours).toBe(31); + expect(server.store.t1.estimate_hours).toBe(77); + // No silent retry. + expect(server.patches).toHaveLength(2); + }); + + it('asks the server for the token even though no column shows it', async () => { + const server = makeServer([{ id: 't1', title: 'Ingest pipeline', estimate_hours: 60 }]); + await loaded(server); + const select = server.dataSource.find.mock.calls.at(-1)?.[1]?.$select as string[]; + expect(select).toEqual(expect.arrayContaining(['id', 'title', 'estimate_hours', 'updated_at'])); + }); +}); diff --git a/packages/plugin-list/src/__tests__/ListView.concurrencyTokenProjection-12292.test.tsx b/packages/plugin-list/src/__tests__/ListView.concurrencyTokenProjection-12292.test.tsx new file mode 100644 index 0000000000..93415eeab3 --- /dev/null +++ b/packages/plugin-list/src/__tests__/ListView.concurrencyTokenProjection-12292.test.tsx @@ -0,0 +1,91 @@ +/** + * 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#12292 — the SECOND projection build site, for the rows' concurrency + * token. + * + * The grid's inline-edit save sends each row's `updated_at` as `If-Match`. In + * the console the rows do not come from the grid's own query: `ListView` + * fetches them and hands them down, and its `$select` was built from the + * columns. A list that shows no `updated_at` column therefore handed the grid + * rows with no token, and every save from it went out unguarded, exactly as + * the card reproduced on the showcase task grid. `plugin-grid`'s + * `inlineEditConcurrency-12292.test.tsx` pins the grid's own build site; this + * pins this one. + */ +import React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, waitFor, cleanup } from '@testing-library/react'; +import { ListView } from '../ListView'; +import { SchemaRendererProvider } from '@object-ui/react'; + +const OBJECT = 'task_12292'; + +const objectDef = { + name: OBJECT, + label: 'Task', + fields: { + id: { name: 'id', type: 'text' }, + title: { name: 'title', type: 'text', label: 'Title' }, + estimate_hours: { name: 'estimate_hours', type: 'number', label: 'Estimate' }, + }, +}; + +const makeDataSource = () => + ({ + find: vi.fn(async () => ({ data: [], total: 0 })), + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + getObjectSchema: vi.fn(async () => objectDef), + }) as any; + +/** Mount a list view; `lastSelect` reads the `$select` of its latest query. */ +async function mount(schemaExtra: Record) { + const dataSource = makeDataSource(); + const schema: any = { + type: 'list-view', + objectName: OBJECT, + viewType: 'grid', + columns: ['title', 'estimate_hours'], + ...schemaExtra, + }; + render( + + + , + ); + const lastSelect = () => (dataSource.find.mock.calls.at(-1)?.[1]?.$select ?? []) as string[]; + // The projection reads the object's definition, so wait for a query that + // was built with it: its `$select` names a declared column. + await waitFor(() => expect(lastSelect()).toContain('estimate_hours')); + return { dataSource, lastSelect }; +} + +beforeEach(() => vi.clearAllMocks()); +afterEach(() => cleanup()); + +describe('ListView — an inline-editable grid\'s rows carry their concurrency token (objectui#12292)', () => { + it('asks for `updated_at` when the grid opens in inline-edit mode, though no column shows it', async () => { + const { lastSelect } = await mount({ inlineEdit: true }); + await waitFor(() => expect(lastSelect()).toContain('updated_at')); + expect(lastSelect()).toEqual(expect.arrayContaining(['id', 'title', 'estimate_hours'])); + }); + + it('does not ask for it while the grid is not in inline-edit mode', async () => { + const { dataSource, lastSelect } = await mount({}); + await waitFor(() => expect(dataSource.getObjectSchema).toHaveBeenCalled()); + expect(dataSource.find.mock.calls.length).toBeGreaterThan(0); + for (const call of dataSource.find.mock.calls) { + expect(call[1]?.$select ?? []).not.toContain('updated_at'); + } + expect(lastSelect()).toEqual(expect.arrayContaining(['id', 'title', 'estimate_hours'])); + }); +}); From aee4247cacb82ceded261eab65d704c5dc980e04 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 04:40:53 +0000 Subject: [PATCH 3/7] docs(plugin-grid,components): the grid's saves refuse a concurrent write, and Save All reads a per-row outcome (objectui#12292) Claude-Session: https://claude.ai/code/session_01CsyMRsFJBKNEMa4oPY1b5z Co-authored-by: Claude --- content/docs/components/complex/data-table.mdx | 7 +++++++ content/docs/plugins/plugin-grid.mdx | 6 ++++++ packages/plugin-grid/README.md | 6 ++++++ 3 files changed, 19 insertions(+) diff --git a/content/docs/components/complex/data-table.mdx b/content/docs/components/complex/data-table.mdx index e3dbc4af18..049feeb907 100644 --- a/content/docs/components/complex/data-table.mdx +++ b/content/docs/components/complex/data-table.mdx @@ -116,6 +116,13 @@ When a host has switched editing on, a cell enters edit mode on double-click (or a single click) and the table renders one of its built-in editors — text, number, date — chosen from the column's `type`. +"Save All" hands every pending row to `onBatchSave` at once, and reads the outcome +per row. A batch is several writes, and some can land while others are refused. A +host that knows which rows did not persist resolves with `{ failed }`, naming each +by the `rowIndex` it was handed: the table keeps and flags exactly those rows, and +clears the rest. Resolving with nothing means every row persisted. A rejection +says nothing about which rows landed, so the table keeps and flags every row. + The host can also supply `renderCellEditor`, a function the table calls first for every cell it is about to edit; it returns a node to use, or `null` to fall through to the built-in editor for that column. This is how `object-grid` gives a `select` diff --git a/content/docs/plugins/plugin-grid.mdx b/content/docs/plugins/plugin-grid.mdx index df0c1d2a3f..1ff516b961 100644 --- a/content/docs/plugins/plugin-grid.mdx +++ b/content/docs/plugins/plugin-grid.mdx @@ -824,6 +824,12 @@ const persistence = ( keys: `onRowSave` for a single row, `onBatchSave` for many, `onCellChange` for each committed cell edit, including one that leaves the value as loaded (only a real change is staged) +- **A concurrent write is refused, not overwritten**: the default saves send each + row's `updated_at` as `If-Match`, and an editable grid asks for that column even + when no column shows it. A row someone else saved after it was read answers + `409`, which opens the record page's conflict dialog (Reload latest, Overwrite + anyway, Cancel). "Save All" reports each row on its own: a refused row stays + pending and flagged, and the rows that saved are cleared. ## TypeScript Support diff --git a/packages/plugin-grid/README.md b/packages/plugin-grid/README.md index f883886c2c..5d5100e767 100644 --- a/packages/plugin-grid/README.md +++ b/packages/plugin-grid/README.md @@ -988,6 +988,12 @@ const persistence = ( - `onRowSave`: called when saving a single row - `onBatchSave`: called when saving multiple rows at once - `onCellChange`: called for each staged cell edit +- **A concurrent write is refused, not overwritten**: the default saves send each + row's `updated_at` as `If-Match`, and an editable grid asks for that column even + when no column shows it. A row someone else saved after it was read answers + `409`, which opens the record page's conflict dialog (Reload latest, Overwrite + anyway, Cancel). "Save All" reports each row on its own: a refused row stays + pending and flagged, and the rows that saved are cleared. **Example Workflow:** 1. User edits multiple cells across different rows From 5a050db61fa9df9782be64dfcdcf7f1d67eec0b3 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 04:45:41 +0000 Subject: [PATCH 4/7] fix(plugin-grid): a second conflict dialog settles the first as Cancel; drop an unused import from the per-row pin (objectui#12292) Claude-Session: https://claude.ai/code/session_01CsyMRsFJBKNEMa4oPY1b5z Co-authored-by: Claude --- .../src/__tests__/data-table-batch-per-row-12292.test.tsx | 1 - packages/plugin-grid/src/ObjectGrid.tsx | 7 ++++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/components/src/__tests__/data-table-batch-per-row-12292.test.tsx b/packages/components/src/__tests__/data-table-batch-per-row-12292.test.tsx index 1f0f42d649..1b63fb5acc 100644 --- a/packages/components/src/__tests__/data-table-batch-per-row-12292.test.tsx +++ b/packages/components/src/__tests__/data-table-batch-per-row-12292.test.tsx @@ -21,7 +21,6 @@ import { describe, it, expect, vi } from 'vitest'; import { fireEvent, waitFor } from '@testing-library/react'; import '@testing-library/jest-dom'; -import React from 'react'; import { renderComponent } from './test-utils'; // Registers the renderers at module scope, NOT inside a `beforeAll`; see // object-ui/no-dynamic-import-in-test-hook (objectui#3010/#3021). diff --git a/packages/plugin-grid/src/ObjectGrid.tsx b/packages/plugin-grid/src/ObjectGrid.tsx index 1fe4a23306..0d90a76021 100644 --- a/packages/plugin-grid/src/ObjectGrid.tsx +++ b/packages/plugin-grid/src/ObjectGrid.tsx @@ -5436,9 +5436,14 @@ export const ObjectGrid: React.FC = ({ // `@object-ui/plugin-detail`, the one `InlineEditSaveBar` opens. There is no // grid-only dialog, and nothing is retried without the user's choice. - /** Open the conflict dialog and wait for the user's choice. */ + /** + * Open the conflict dialog and wait for the user's choice. A save still + * waiting on an earlier dialog is settled as Cancel first, so no save is + * left waiting on a dialog that is no longer shown. + */ const askRowConflict = (conflict: ConcurrentUpdateConflict): Promise => new Promise((resolve) => { + rowConflictDecisionRef.current?.('cancel'); rowConflictDecisionRef.current = resolve; setRowConflict(conflict); }); From d061c98741e17fd74b792a79dbe74218c9693939 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 05:12:01 +0000 Subject: [PATCH 5/7] test(plugin-grid): two save pins state the write-options argument the default saves now pass (objectui#12292) Claude-Session: https://claude.ai/code/session_01CsyMRsFJBKNEMa4oPY1b5z Co-authored-by: Claude --- .../plugin-grid/src/__tests__/inlineEditExitMode.test.tsx | 4 +++- .../plugin-grid/src/__tests__/inlineEditPersistence.test.tsx | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/plugin-grid/src/__tests__/inlineEditExitMode.test.tsx b/packages/plugin-grid/src/__tests__/inlineEditExitMode.test.tsx index af6711e17e..ef762c4f60 100644 --- a/packages/plugin-grid/src/__tests__/inlineEditExitMode.test.tsx +++ b/packages/plugin-grid/src/__tests__/inlineEditExitMode.test.tsx @@ -211,7 +211,9 @@ describe('ObjectGrid — injected text/number editor exits edit mode', () => { }); fireEvent.click(saveAll); - await waitFor(() => expect(ds.update).toHaveBeenCalledWith(OBJECT, 'r1', { name: 'Row One SAVED' })); + // The fourth argument is the save's write options: this fixture's rows carry + // no `updated_at`, so no `ifMatch` is sent (objectui#12292). + await waitFor(() => expect(ds.update).toHaveBeenCalledWith(OBJECT, 'r1', { name: 'Row One SAVED' }, undefined)); expect(ds.store.r1.name).toBe('Row One SAVED'); }); }); diff --git a/packages/plugin-grid/src/__tests__/inlineEditPersistence.test.tsx b/packages/plugin-grid/src/__tests__/inlineEditPersistence.test.tsx index 51b5b2c477..55368c1126 100644 --- a/packages/plugin-grid/src/__tests__/inlineEditPersistence.test.tsx +++ b/packages/plugin-grid/src/__tests__/inlineEditPersistence.test.tsx @@ -118,7 +118,9 @@ describe('ObjectGrid — inline edit persists through save + refresh', () => { fireEvent.click(saveAll); await waitFor(() => expect(ds.update).toHaveBeenCalled()); - expect(ds.update).toHaveBeenCalledWith(OBJECT, 'r1', { actual_start: '2026-06-29' }); + // The fourth argument is the save's write options: this fixture's rows carry + // no `updated_at`, so no `ifMatch` is sent (objectui#12292). + expect(ds.update).toHaveBeenCalledWith(OBJECT, 'r1', { actual_start: '2026-06-29' }, undefined); expect(ds.store['r1'].actual_start).toBe('2026-06-29'); }); }); From 0af4fccbcb1333aefaaac63dff83664fe988d6ed Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 05:39:11 +0000 Subject: [PATCH 6/7] chore(changeset): types and components take minor for the widened onBatchSave outcome (objectui#12292) Claude-Session: https://claude.ai/code/session_01CsyMRsFJBKNEMa4oPY1b5z Co-authored-by: Claude --- .changeset/12292-grid-save-if-match.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.changeset/12292-grid-save-if-match.md b/.changeset/12292-grid-save-if-match.md index 76c2a318e7..5be2e51627 100644 --- a/.changeset/12292-grid-save-if-match.md +++ b/.changeset/12292-grid-save-if-match.md @@ -1,8 +1,8 @@ --- '@object-ui/plugin-grid': patch '@object-ui/plugin-list': patch -'@object-ui/components': patch -'@object-ui/types': patch +'@object-ui/components': minor +'@object-ui/types': minor --- A grid's inline-edit save no longer overwrites a concurrent write without a word (objectui#12292). The grid's default row save and "Save All" now send each row's `updated_at` as `If-Match`, the way the record forms and the record page's inline edit already do. A stale grid edit is refused with `409` instead of landing with a `200`. The refusal opens the same conflict dialog the record page opens (`ConcurrentUpdateDialog`, offering Reload latest, Overwrite anyway or Cancel). After a save, the row's next save sends the token of the record the server answered with, so saving the same row twice does not conflict with your own first save. From b87fdb4b865e08e1398c3f9ec3de5463d0d394de Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 05:51:41 +0000 Subject: [PATCH 7/7] test(plugin-form): the write census follows the grid's inline-edit saves to writeRowGuarded and resolveRowConflict (objectui#12292) Claude-Session: https://claude.ai/code/session_01CsyMRsFJBKNEMa4oPY1b5z Co-authored-by: Claude --- .../plugin-form/src/affordanceGrantMap-12082.test.tsx | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/plugin-form/src/affordanceGrantMap-12082.test.tsx b/packages/plugin-form/src/affordanceGrantMap-12082.test.tsx index c1f34b3059..d40778af37 100644 --- a/packages/plugin-form/src/affordanceGrantMap-12082.test.tsx +++ b/packages/plugin-form/src/affordanceGrantMap-12082.test.tsx @@ -805,8 +805,12 @@ const WRITE_SITES: Record = { ...mapped(['listImport'], CONSOLE_LIST, 'packages/app-shell/src/views/importTargetFields.ts', EXCEL_IMPORT_BAR), note: "one site, two mounts: the console list's Import and the AI build panel's spreadsheet import (`ExcelImportBar`) each read the row where they offer Import; HOST_GATED below holds every mount of the wizard to this list", } as WriteSite, - 'packages/plugin-grid/src/ObjectGrid.tsx :: ObjectGrid > defaultRowSave :: update': mapped(['listInlineEdit'], OBJECT_GRID), - 'packages/plugin-grid/src/ObjectGrid.tsx :: ObjectGrid > defaultBatchSave :: update': mapped(['listInlineEdit'], OBJECT_GRID), + // The inline-edit saves (objectui#12292): `defaultRowSave` and + // `defaultBatchSave` both write through `writeRowGuarded`, the `If-Match` + // write; `resolveRowConflict` is the same save's Overwrite after a 409, re-keyed + // to the server's version. One affordance, the grid's `listInlineEdit` row. + 'packages/plugin-grid/src/ObjectGrid.tsx :: ObjectGrid > writeRowGuarded :: update': mapped(['listInlineEdit'], OBJECT_GRID), + 'packages/plugin-grid/src/ObjectGrid.tsx :: ObjectGrid > resolveRowConflict :: update': mapped(['listInlineEdit'], OBJECT_GRID), 'packages/plugin-grid/src/hooks/useBulkExecutor.ts :: useBulkExecutor > run > perRow :: delete': mapped(['rowDelete'], OBJECT_GRID), 'packages/plugin-grid/src/hooks/useBulkExecutor.ts :: useBulkExecutor > run > perRow :: update': mapped(['rowEdit'], OBJECT_GRID),