Skip to content

Commit a555ae0

Browse files
j15zclaude
andcommitted
fix(tables): write only what the row form changed, and report failures once
The add/edit row form sent every column on every save. An untouched empty column was written as `null`, so a no-op edit still bumped the row, and an insert filled in nulls for columns the user never opened. It now sends only the fields the user touched, and in edit mode only those whose value actually differs — a save with nothing changed closes without a write. Checkboxes stay the exception on insert: they always carry a concrete boolean, so a required one the user never clicked still reaches the server as `false`. A rejected write also arrived twice: the modal rendered the message inline and the mutation toasted the same sentence. Row mutations take an opt-in `suppressErrorToast` so the form owns its own failure; the cache self-heal on a 423 still runs, only its toast is dropped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 9e80171 commit a555ae0

3 files changed

Lines changed: 167 additions & 25 deletions

File tree

‎apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/row-modal/row-modal.test.tsx‎

Lines changed: 78 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -389,7 +389,7 @@ describe('RowModal expiration editing', () => {
389389
container.remove()
390390
})
391391

392-
it('keeps unrelated fields editable and omits blocked date values from the update', async () => {
392+
it('sends only the edited field and omits blocked date values from the update', async () => {
393393
mockUseTimezoneState.mockReturnValue({
394394
timezone: 'America/Los_Angeles',
395395
savedTimezone: 'Mars/Olympus',
@@ -437,14 +437,87 @@ describe('RowModal expiration editing', () => {
437437
act(() => changeInput(nameInput as HTMLInputElement, 'Grace'))
438438
await act(async () => submit?.click())
439439

440-
expect(mockUpdateRow).toHaveBeenCalledWith({
441-
rowId: 'row-1',
442-
data: { name: 'Grace', expires_at: row.data.expires_at },
443-
})
440+
// Only the edited field is sent: the untouched TTL would otherwise be
441+
// rewritten with the same value (and re-stamped through the picker), and the
442+
// timezone-blocked date is dropped entirely.
443+
expect(mockUpdateRow).toHaveBeenCalledWith({ rowId: 'row-1', data: { name: 'Grace' } })
444444
expect(props.onSuccess).toHaveBeenCalledTimes(1)
445445
expect(mockToastError).not.toHaveBeenCalled()
446446

447447
act(() => root.unmount())
448448
container.remove()
449449
})
450450
})
451+
452+
describe('RowModal payload', () => {
453+
beforeEach(() => {
454+
vi.clearAllMocks()
455+
mockCreateRow.mockResolvedValue(undefined)
456+
mockUpdateRow.mockResolvedValue(undefined)
457+
mockUseTimezoneState.mockReturnValue({ timezone: 'America/Los_Angeles', status: 'ready' })
458+
})
459+
460+
it('closes without a write when the edit changes nothing', async () => {
461+
const container = document.createElement('div')
462+
document.body.appendChild(container)
463+
const root = createRoot(container)
464+
const props = {
465+
mode: 'edit' as const,
466+
isOpen: true,
467+
onClose: vi.fn(),
468+
table: {
469+
id: 'table-5',
470+
name: 'People',
471+
schema: { columns: [{ id: 'col_name', name: 'Name', type: 'string' as const }] },
472+
},
473+
row: { ...row, data: { col_name: 'Ada' } },
474+
onSuccess: vi.fn(),
475+
}
476+
477+
act(() => root.render(createElement(RowModal, props)))
478+
const submit = container.querySelector<HTMLButtonElement>('[data-testid="submit"]')
479+
await act(async () => submit?.click())
480+
481+
expect(mockUpdateRow).not.toHaveBeenCalled()
482+
expect(props.onSuccess).toHaveBeenCalledTimes(1)
483+
484+
act(() => root.unmount())
485+
container.remove()
486+
})
487+
488+
it('omits untouched columns on insert but still sends toggles', async () => {
489+
const container = document.createElement('div')
490+
document.body.appendChild(container)
491+
const root = createRoot(container)
492+
const props = {
493+
mode: 'add' as const,
494+
isOpen: true,
495+
onClose: vi.fn(),
496+
table: {
497+
id: 'table-6',
498+
name: 'People',
499+
schema: {
500+
columns: [
501+
{ id: 'col_name', name: 'Name', type: 'string' as const },
502+
{ id: 'col_notes', name: 'Notes', type: 'string' as const },
503+
{ id: 'col_done', name: 'Done', type: 'boolean' as const },
504+
],
505+
},
506+
},
507+
onSuccess: vi.fn(),
508+
}
509+
510+
act(() => root.render(createElement(RowModal, props)))
511+
const nameInput = container.querySelector<HTMLInputElement>('[data-testid="modal-input"]')
512+
act(() => changeInput(nameInput as HTMLInputElement, 'Ada'))
513+
const submit = container.querySelector<HTMLButtonElement>('[data-testid="submit"]')
514+
await act(async () => submit?.click())
515+
516+
// `col_notes` was never touched, so it stays absent instead of being written
517+
// as null; a checkbox always carries a concrete boolean.
518+
expect(mockCreateRow).toHaveBeenCalledWith({ data: { col_name: 'Ada', col_done: false } })
519+
520+
act(() => root.unmount())
521+
container.remove()
522+
})
523+
})

‎apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/row-modal/row-modal.tsx‎

Lines changed: 47 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -57,25 +57,53 @@ export interface RowModalProps {
5757
onSuccess: () => void
5858
}
5959

60+
/** Structural equality for a cleaned cell value vs what the row already holds. */
61+
function cellValueUnchanged(next: unknown, previous: unknown): boolean {
62+
if (next === previous) return true
63+
const nextEmpty = next === null || next === undefined
64+
const previousEmpty = previous === null || previous === undefined
65+
if (nextEmpty || previousEmpty) return nextEmpty && previousEmpty
66+
if (typeof next === 'object' || typeof previous === 'object') {
67+
return JSON.stringify(next) === JSON.stringify(previous)
68+
}
69+
return false
70+
}
71+
72+
/**
73+
* Builds the write payload. Only fields the user actually touched are sent, so
74+
* an untouched empty column is left absent instead of being written as `null` —
75+
* and in edit mode a field whose value is unchanged is dropped too, leaving a
76+
* no-op save with nothing to write. Toggles are the exception on insert: they
77+
* always carry a concrete boolean, so a required checkbox the user never
78+
* clicked still has to reach the server as `false`.
79+
*/
6080
function cleanRowData(
6181
columns: ColumnDefinition[],
6282
rowData: Record<string, unknown>,
6383
timeZone: string,
64-
dateEditorsReady: boolean
84+
dateEditorsReady: boolean,
85+
options: { mode: 'add' | 'edit'; baseline?: Record<string, unknown> }
6586
): Record<string, unknown> {
6687
const cleanData: Record<string, unknown> = {}
6788

6889
columns.forEach((col) => {
6990
const columnId = getColumnId(col)
70-
const value = rowData[columnId]
71-
if (columnTypeOf(col).editor === 'date' && !dateEditorsReady) {
91+
const definition = columnTypeOf(col)
92+
if (definition.editor === 'date' && !dateEditorsReady) {
7293
return
7394
}
95+
const touched = columnId in rowData
96+
const alwaysSend = options.mode === 'add' && definition.editor === 'toggle'
97+
if (!touched && !alwaysSend) return
98+
const value = rowData[columnId]
99+
let cleaned: unknown
74100
try {
75-
cleanData[columnId] = cleanCellValue(value, col, timeZone)
101+
cleaned = cleanCellValue(value, col, timeZone)
76102
} catch {
77103
throw new Error(`Invalid JSON for field: ${col.name}`)
78104
}
105+
if (options.baseline && cellValueUnchanged(cleaned, options.baseline[columnId])) return
106+
cleanData[columnId] = cleaned
79107
})
80108

81109
return cleanData
@@ -119,10 +147,13 @@ export function RowModal({
119147
mode === 'edit' && row ? row.data : {}
120148
)
121149
const [error, setError] = useState<string | null>(null)
122-
const createRowMutation = useCreateTableRow({ workspaceId, tableId })
123-
const updateRowMutation = useUpdateTableRow({ workspaceId, tableId })
124-
const deleteRowMutation = useDeleteTableRow({ workspaceId, tableId })
125-
const deleteRowsMutation = useDeleteTableRows({ workspaceId, tableId })
150+
// This modal renders its own failure in `<ChipModalError>`; without the flag
151+
// every rejection would also arrive as a toast saying the same sentence.
152+
const rowMutationContext = { workspaceId, tableId, suppressErrorToast: true }
153+
const createRowMutation = useCreateTableRow(rowMutationContext)
154+
const updateRowMutation = useUpdateTableRow(rowMutationContext)
155+
const deleteRowMutation = useDeleteTableRow(rowMutationContext)
156+
const deleteRowsMutation = useDeleteTableRows(rowMutationContext)
126157
const isSubmitting =
127158
createRowMutation.isPending ||
128159
updateRowMutation.isPending ||
@@ -149,12 +180,18 @@ export function RowModal({
149180
if (!canSubmit) return
150181

151182
try {
152-
const cleanData = cleanRowData(columns, rowData, timeZone, dateEditorsReady)
183+
const cleanData = cleanRowData(columns, rowData, timeZone, dateEditorsReady, {
184+
mode: isAddMode ? 'add' : 'edit',
185+
baseline: isAddMode ? undefined : row?.data,
186+
})
153187

154188
if (isAddMode) {
155189
await createRowMutation.mutateAsync({ data: cleanData, ...insertAt })
156190
} else if (row) {
157-
await updateRowMutation.mutateAsync({ rowId: row.id, data: cleanData })
191+
// Nothing changed — close instead of writing an empty patch.
192+
if (Object.keys(cleanData).length > 0) {
193+
await updateRowMutation.mutateAsync({ rowId: row.id, data: cleanData })
194+
}
158195
}
159196

160197
onSuccess()

‎apps/sim/hooks/queries/tables.ts‎

Lines changed: 42 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,12 @@ export type TableRowsResponse = Pick<
159159
interface RowMutationContext {
160160
workspaceId: string
161161
tableId: string
162+
/**
163+
* Suppresses the error toast for callers that render the failure themselves —
164+
* the row modal shows it inline, and two copies of the same sentence read as
165+
* two separate failures. The cache self-heal on a 423 still runs.
166+
*/
167+
suppressErrorToast?: boolean
162168
}
163169

164170
type UpdateTableRowParams = Pick<TableRowParamsInput, 'rowId'> &
@@ -809,16 +815,22 @@ function notifyRowWriteError(error: Error, onUpgrade: () => void): void {
809815
function handleTableLockRejection(
810816
error: unknown,
811817
queryClient: ReturnType<typeof useQueryClient>,
812-
tableId: string
818+
tableId: string,
819+
options?: { silent?: boolean }
813820
): boolean {
814821
if (!isApiClientError(error) || error.status !== 423) return false
815822
void queryClient.invalidateQueries({ queryKey: tableKeys.detail(tableId), exact: true })
816823
void queryClient.invalidateQueries({ queryKey: tableKeys.lists() })
817-
toast.error(error.message, { duration: 5000 })
824+
// `silent` only drops the toast; the refetches above are what un-stale the grid.
825+
if (!options?.silent) toast.error(error.message, { duration: 5000 })
818826
return true
819827
}
820828

821-
export function useCreateTableRow({ workspaceId, tableId }: RowMutationContext) {
829+
export function useCreateTableRow({
830+
workspaceId,
831+
tableId,
832+
suppressErrorToast,
833+
}: RowMutationContext) {
822834
const queryClient = useQueryClient()
823835
const router = useRouter()
824836

@@ -866,7 +878,9 @@ export function useCreateTableRow({ workspaceId, tableId }: RowMutationContext)
866878
})
867879
},
868880
onError: (error) => {
869-
if (handleTableLockRejection(error, queryClient, tableId)) return
881+
if (handleTableLockRejection(error, queryClient, tableId, { silent: suppressErrorToast }))
882+
return
883+
if (suppressErrorToast) return
870884
notifyRowWriteError(error, () => router.push(buildUpgradeHref(workspaceId, 'tables')))
871885
},
872886
onSettled: () => {
@@ -1065,7 +1079,11 @@ export function useBatchCreateTableRows({ workspaceId, tableId }: RowMutationCon
10651079
* Update a single row in a table.
10661080
* Uses optimistic updates for instant UI feedback on inline cell edits.
10671081
*/
1068-
export function useUpdateTableRow({ workspaceId, tableId }: RowMutationContext) {
1082+
export function useUpdateTableRow({
1083+
workspaceId,
1084+
tableId,
1085+
suppressErrorToast,
1086+
}: RowMutationContext) {
10691087
const queryClient = useQueryClient()
10701088

10711089
return useMutation({
@@ -1150,8 +1168,10 @@ export function useUpdateTableRow({ workspaceId, tableId }: RowMutationContext)
11501168
if (context?.didBumpRunState) {
11511169
queryClient.setQueryData(tableKeys.activeDispatches(tableId), context.runStateSnapshot)
11521170
}
1153-
if (handleTableLockRejection(error, queryClient, tableId)) return
1171+
if (handleTableLockRejection(error, queryClient, tableId, { silent: suppressErrorToast }))
1172+
return
11541173
if (isValidationError(error)) return
1174+
if (suppressErrorToast) return
11551175
toast.error(error.message, { duration: 5000 })
11561176
},
11571177
})
@@ -1234,7 +1254,11 @@ export function useBatchUpdateTableRows({ workspaceId, tableId }: RowMutationCon
12341254
/**
12351255
* Delete a single row from a table.
12361256
*/
1237-
export function useDeleteTableRow({ workspaceId, tableId }: RowMutationContext) {
1257+
export function useDeleteTableRow({
1258+
workspaceId,
1259+
tableId,
1260+
suppressErrorToast,
1261+
}: RowMutationContext) {
12381262
const queryClient = useQueryClient()
12391263

12401264
return useMutation({
@@ -1245,8 +1269,10 @@ export function useDeleteTableRow({ workspaceId, tableId }: RowMutationContext)
12451269
})
12461270
},
12471271
onError: (error) => {
1248-
if (handleTableLockRejection(error, queryClient, tableId)) return
1272+
if (handleTableLockRejection(error, queryClient, tableId, { silent: suppressErrorToast }))
1273+
return
12491274
if (isValidationError(error)) return
1275+
if (suppressErrorToast) return
12501276
toast.error(error.message, { duration: 5000 })
12511277
},
12521278
onSettled: () => {
@@ -1259,7 +1285,11 @@ export function useDeleteTableRow({ workspaceId, tableId }: RowMutationContext)
12591285
* Delete multiple rows from a table.
12601286
* Returns both deleted ids and failure details for partial-failure UI.
12611287
*/
1262-
export function useDeleteTableRows({ workspaceId, tableId }: RowMutationContext) {
1288+
export function useDeleteTableRows({
1289+
workspaceId,
1290+
tableId,
1291+
suppressErrorToast,
1292+
}: RowMutationContext) {
12631293
const queryClient = useQueryClient()
12641294

12651295
return useMutation({
@@ -1294,8 +1324,10 @@ export function useDeleteTableRows({ workspaceId, tableId }: RowMutationContext)
12941324
return { deletedRowIds }
12951325
},
12961326
onError: (error) => {
1297-
if (handleTableLockRejection(error, queryClient, tableId)) return
1327+
if (handleTableLockRejection(error, queryClient, tableId, { silent: suppressErrorToast }))
1328+
return
12981329
if (isValidationError(error)) return
1330+
if (suppressErrorToast) return
12991331
toast.error(error.message, { duration: 5000 })
13001332
},
13011333
onSettled: () => {

0 commit comments

Comments
 (0)