Skip to content

Commit fca76b0

Browse files
fix(tables): revalidate sync imports under the advisory lock, explain every blocked key
- The sync append/replace paths asserted only the request-start snapshot, so a lock committed while the CSV was parsed still let the write through. Both now re-read under the schema advisory lock at the top of their own transaction, taken before acquireRowOrderLock so the order stays advisory -> rows_pos -> definitions - Delete/Backspace, Cmd+D, typeahead and cut raised no notice on an update-locked table; they now explain the lock, and stay silent for users without write access - The multipart import fetch threw a plain Error, dropping the 423 status, so the lock self-heal never ran and the stale detail cache survived. It throws ApiClientError now and both import-into-table hooks call handleTableLockRejection - Force append at submit when the delete lock landed while the dialog was open with Replace already selected
1 parent 1aa9424 commit fca76b0

4 files changed

Lines changed: 76 additions & 16 deletions

File tree

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2269,7 +2269,12 @@ export function TableGrid({
22692269
!rowSelectionIsEmpty(rowSelectionRef.current)
22702270
) {
22712271
if (editingCellRef.current) return
2272-
if (!canEditCellRef.current) return
2272+
if (!canEditCellRef.current) {
2273+
// Explain the lock rather than no-op, but only for users who
2274+
// could otherwise edit — for the rest it is a permission limit.
2275+
if (canEditRef.current) onBlockedActionRef.current('edit-cell')
2276+
return
2277+
}
22732278
e.preventDefault()
22742279
const rowSel = rowSelectionRef.current
22752280
void (async () => {
@@ -2479,7 +2484,12 @@ export function TableGrid({
24792484

24802485
if ((e.metaKey || e.ctrlKey) && e.key === 'd') {
24812486
e.preventDefault()
2482-
if (!canEditCellRef.current) return
2487+
if (!canEditCellRef.current) {
2488+
// Explain the lock rather than no-op, but only for users who
2489+
// could otherwise edit — for the rest it is a permission limit.
2490+
if (canEditRef.current) onBlockedActionRef.current('edit-cell')
2491+
return
2492+
}
24832493
const sel = computeNormalizedSelection(anchor, selectionFocusRef.current)
24842494
if (!sel || sel.startRow === sel.endRow) return
24852495
const sourceRow = currentRows[sel.startRow]
@@ -2513,7 +2523,12 @@ export function TableGrid({
25132523
}
25142524

25152525
if (e.key === 'Delete' || e.key === 'Backspace') {
2516-
if (!canEditCellRef.current) return
2526+
if (!canEditCellRef.current) {
2527+
// Explain the lock rather than no-op, but only for users who
2528+
// could otherwise edit — for the rest it is a permission limit.
2529+
if (canEditRef.current) onBlockedActionRef.current('edit-cell')
2530+
return
2531+
}
25172532
e.preventDefault()
25182533
const sel = computeNormalizedSelection(anchor, selectionFocusRef.current)
25192534
if (!sel) return
@@ -2574,7 +2589,12 @@ export function TableGrid({
25742589
}
25752590

25762591
if (e.key.length === 1 && !e.metaKey && !e.ctrlKey && !e.altKey) {
2577-
if (!canEditCellRef.current) return
2592+
if (!canEditCellRef.current) {
2593+
// Explain the lock rather than no-op, but only for users who
2594+
// could otherwise edit — for the rest it is a permission limit.
2595+
if (canEditRef.current) onBlockedActionRef.current('edit-cell')
2596+
return
2597+
}
25782598
const col = cols[anchor.colIndex]
25792599
// Workflow-output cells are editable: the user can override the
25802600
// workflow's value if they want. Booleans toggle on space/click —
@@ -2777,7 +2797,12 @@ export function TableGrid({
27772797
const tag = (e.target as HTMLElement).tagName
27782798
if (tag === 'INPUT' || tag === 'TEXTAREA') return
27792799
if (editingCellRef.current) return
2780-
if (!canEditCellRef.current) return
2800+
if (!canEditCellRef.current) {
2801+
// Explain the lock rather than no-op, but only for users who
2802+
// could otherwise edit — for the rest it is a permission limit.
2803+
if (canEditRef.current) onBlockedActionRef.current('edit-cell')
2804+
return
2805+
}
27812806

27822807
const rowSel = rowSelectionRef.current
27832808
const cols = columnsRef.current

apps/sim/app/workspace/[workspaceId]/tables/components/import-csv-dialog/import-csv-dialog.tsx

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,9 @@ export function ImportCsvDialog({
314314
async function handleSubmit() {
315315
if (!parsed || !canSubmit) return
316316
setSubmitError(null)
317+
// Hiding the Replace control doesn't clear an already-selected `mode`, so a
318+
// delete lock landing while the dialog is open would still submit replace.
319+
const effectiveMode: CsvImportMode = canReplace ? mode : 'append'
317320
const createColumns =
318321
canCreateColumns && createHeaders.size > 0 ? [...createHeaders] : undefined
319322

@@ -334,7 +337,7 @@ export function ImportCsvDialog({
334337
workspaceId,
335338
tableId: table.id,
336339
file: parsed.file,
337-
mode,
340+
mode: effectiveMode,
338341
mapping,
339342
createColumns,
340343
onProgress: (percent) => {
@@ -365,12 +368,12 @@ export function ImportCsvDialog({
365368
workspaceId,
366369
tableId: table.id,
367370
file: parsed.file,
368-
mode,
371+
mode: effectiveMode,
369372
mapping,
370373
createColumns,
371374
})
372375
const data = result.data
373-
if (mode === 'append') {
376+
if (effectiveMode === 'append') {
374377
toast.success(`Imported ${data?.insertedCount ?? 0} rows into "${table.name}"`)
375378
} else {
376379
toast.success(

apps/sim/hooks/queries/tables.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
} from '@tanstack/react-query'
1818
import { useRouter } from 'next/navigation'
1919
import {
20+
ApiClientError,
2021
extractValidationIssues,
2122
isApiClientError,
2223
isValidationError,
@@ -1578,7 +1579,13 @@ export function useUploadCsvToTable() {
15781579

15791580
if (!response.ok) {
15801581
const data = await response.json().catch(() => ({}))
1581-
throw new Error(data.error || 'CSV import failed')
1582+
// Carry the status: a plain Error drops it, and the 423 self-heal below
1583+
// keys off `error.status`.
1584+
throw new ApiClientError({
1585+
status: response.status,
1586+
body: data,
1587+
message: data.error || 'CSV import failed',
1588+
})
15821589
}
15831590

15841591
return response.json()
@@ -1714,7 +1721,8 @@ export function useImportCsvIntoTableAsync() {
17141721
})
17151722
return response.data
17161723
},
1717-
onError: (error) => {
1724+
onError: (error, variables) => {
1725+
if (handleTableLockRejection(error, queryClient, variables.tableId)) return
17181726
logger.error('Failed to start async CSV import:', error)
17191727
toast.error(error.message, { duration: 5000 })
17201728
},
@@ -1793,7 +1801,8 @@ export function useImportCsvIntoTable() {
17931801

17941802
return response.json()
17951803
},
1796-
onError: (error) => {
1804+
onError: (error, variables) => {
1805+
if (handleTableLockRejection(error, queryClient, variables.tableId)) return
17971806
logger.error('Failed to import CSV into table:', error)
17981807
toast.error(error.message, { duration: 5000 })
17991808
},

apps/sim/lib/table/import-data.ts

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,14 @@ import { assertRowCapacity, notifyTableRowUsage } from '@/lib/table/billing'
1313
import { CSV_MAX_BATCH_SIZE } from '@/lib/table/import'
1414
import { assertRowDelete, assertRowInsert, assertSchemaMutable } from '@/lib/table/mutation-locks'
1515
import { nKeysBetween } from '@/lib/table/order-key'
16+
import type { DbTransaction } from '@/lib/table/planner'
1617
import {
1718
acquireRowOrderLock,
1819
guardBatch,
1920
type MutationRevalidator,
2021
} from '@/lib/table/rows/ordering'
2122
import { batchInsertRowsWithTx, replaceTableRowsWithTx } from '@/lib/table/rows/service'
22-
import { addTableColumnsWithTx, auditTableColumnsAdded } from '@/lib/table/service'
23+
import { addTableColumnsWithTx, auditTableColumnsAdded, getTableById } from '@/lib/table/service'
2324
import type {
2425
ReplaceRowsResult,
2526
RowData,
@@ -187,6 +188,28 @@ export async function setTableSchemaForImport(
187188
})
188189
}
189190

191+
/**
192+
* Re-reads the table under its schema advisory lock inside the caller's
193+
* transaction. The sync import paths own their transaction rather than taking a
194+
* revalidator, so they refresh here: a lock committed while the CSV was being
195+
* parsed must be visible to the asserts in `addTableColumnsWithTx` /
196+
* `batchInsertRowsWithTx` / `replaceTableRowsWithTx`, which all read the
197+
* definition they are handed.
198+
*
199+
* Taken before `acquireRowOrderLock` so the order stays advisory → rows_pos →
200+
* definitions, matching every other advisory-lock holder.
201+
*/
202+
async function refreshUnderLock(
203+
trx: DbTransaction,
204+
table: TableDefinition
205+
): Promise<TableDefinition> {
206+
const fresh = await guardBatch(trx, table.id, async (tx) => {
207+
const latest = await getTableById(table.id, { tx, includeArchived: true })
208+
return latest ?? undefined
209+
})
210+
return fresh ?? table
211+
}
212+
190213
/**
191214
* Owns the append-import transaction so the API route never holds a `trx`:
192215
* optionally creates the new columns, then inserts every row in CSV-sized
@@ -206,15 +229,15 @@ export async function importAppendRows(
206229
addedRows: rows.length,
207230
})
208231
const result = await db.transaction(async (trx) => {
209-
let working = table
232+
let working = await refreshUnderLock(trx, table)
210233
if (additions.length > 0) {
211234
// Take the row-order lock before creating columns so this path uses the
212235
// same rows_pos → user_table_definitions order as plain inserts. Creating
213236
// columns first would lock the definition row before rows_pos, inverting
214237
// the order and deadlocking concurrent inserts on this table. The lock is
215238
// re-entrant, so the per-batch acquire below is a no-op.
216239
await acquireRowOrderLock(trx, table.id)
217-
working = await addTableColumnsWithTx(trx, table, additions, ctx.requestId)
240+
working = await addTableColumnsWithTx(trx, working, additions, ctx.requestId)
218241
}
219242
const inserted: TableRow[] = []
220243
for (let i = 0; i < rows.length; i += CSV_MAX_BATCH_SIZE) {
@@ -264,10 +287,10 @@ export async function importReplaceRows(
264287
addedRows: data.rows.length,
265288
})
266289
const result = await db.transaction(async (trx) => {
267-
let working = table
290+
let working = await refreshUnderLock(trx, table)
268291
if (additions.length > 0) {
269292
await acquireRowOrderLock(trx, table.id)
270-
working = await addTableColumnsWithTx(trx, table, additions, requestId)
293+
working = await addTableColumnsWithTx(trx, working, additions, requestId)
271294
}
272295
return replaceTableRowsWithTx(
273296
trx,

0 commit comments

Comments
 (0)