Skip to content

Commit 07597cc

Browse files
TheodoreSpeaksTheodoreSpeaks
committed
fix(tables): id-correct column undo (rename/create/delete) with id reuse on re-add
1 parent c38d598 commit 07597cc

5 files changed

Lines changed: 63 additions & 29 deletions

File tree

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

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -716,8 +716,11 @@ export function TableGrid({
716716
: null
717717

718718
const columnRename = useInlineRename({
719+
// `columnName` is the column id; record the prior display name + id so undo
720+
// restores the label (not the id) and targets the right column.
719721
onSave: (columnName, newName) => {
720-
pushUndoRef.current({ type: 'rename-column', oldName: columnName, newName })
722+
const oldName = columnsRef.current.find((c) => c.key === columnName)?.name ?? columnName
723+
pushUndoRef.current({ type: 'rename-column', oldName, newName, columnId: columnName })
721724
handleColumnRename(columnName, newName)
722725
updateColumnMutation.mutate({ columnName, updates: { name: newName } })
723726
},
@@ -2651,8 +2654,13 @@ export function TableGrid({
26512654
{ name, type: 'string', position: index },
26522655
{
26532656
onSuccess: (result) => {
2654-
pushUndoRef.current({ type: 'create-column', columnName: name, position: index })
26552657
const newId = result.data.columns.find((c) => c.name === name)?.id ?? name
2658+
pushUndoRef.current({
2659+
type: 'create-column',
2660+
columnName: name,
2661+
columnId: newId,
2662+
position: index,
2663+
})
26562664
insertColumnInOrder(columnId, newId, 'left')
26572665
},
26582666
}
@@ -2671,8 +2679,13 @@ export function TableGrid({
26712679
{ name, type: 'string', position },
26722680
{
26732681
onSuccess: (result) => {
2674-
pushUndoRef.current({ type: 'create-column', columnName: name, position })
26752682
const newId = result.data.columns.find((c) => c.name === name)?.id ?? name
2683+
pushUndoRef.current({
2684+
type: 'create-column',
2685+
columnName: name,
2686+
columnId: newId,
2687+
position,
2688+
})
26762689
insertColumnInOrder(columnId, newId, 'right')
26772690
},
26782691
}
@@ -2838,7 +2851,9 @@ export function TableGrid({
28382851
deletedOriginalPositions.push(entry.position)
28392852
pushUndoRef.current({
28402853
type: 'delete-column',
2841-
columnName: columnToDelete,
2854+
// `columnToDelete` is the stable id; record the display name for re-create.
2855+
columnName: entry.def?.name ?? columnToDelete,
2856+
columnId: columnToDelete,
28422857
columnType: entry.def?.type ?? 'string',
28432858
columnPosition: adjustedPosition >= 0 ? adjustedPosition : cols.length,
28442859
columnUnique: entry.def?.unique ?? false,

‎apps/sim/hooks/use-table-undo.ts‎

Lines changed: 31 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -222,19 +222,22 @@ export function useTableUndo({
222222
}
223223

224224
case 'create-column': {
225+
// Identity (delete lookup + id-keyed metadata) uses the stable id;
226+
// re-create uses the display name.
227+
const colKey = action.columnId ?? action.columnName
225228
if (direction === 'undo') {
226-
deleteColumnMutation.mutate(action.columnName, {
229+
deleteColumnMutation.mutate(colKey, {
227230
onSuccess: () => {
228231
const metadata: Record<string, unknown> = {}
229232
const currentWidths = getColumnWidthsRef.current?.() ?? {}
230-
if (action.columnName in currentWidths) {
231-
const { [action.columnName]: _, ...rest } = currentWidths
233+
if (colKey in currentWidths) {
234+
const { [colKey]: _, ...rest } = currentWidths
232235
onColumnWidthsChangeRef.current?.(rest)
233236
metadata.columnWidths = rest
234237
}
235238
const currentPinned = getPinnedColumnsRef.current?.() ?? []
236-
if (currentPinned.includes(action.columnName)) {
237-
const newPinned = currentPinned.filter((n) => n !== action.columnName)
239+
if (currentPinned.includes(colKey)) {
240+
const newPinned = currentPinned.filter((n) => n !== colKey)
238241
onPinnedColumnsChangeRef.current?.(newPinned)
239242
metadata.pinnedColumns = newPinned
240243
}
@@ -245,6 +248,7 @@ export function useTableUndo({
245248
})
246249
} else {
247250
addColumnMutation.mutate({
251+
...(action.columnId ? { id: action.columnId } : {}),
248252
name: action.columnName,
249253
type: 'string',
250254
position: action.position,
@@ -254,9 +258,15 @@ export function useTableUndo({
254258
}
255259

256260
case 'delete-column': {
261+
// Identity (cell-data keys, id-keyed metadata, delete lookup) uses the
262+
// stable id; re-create uses the display name.
263+
const colKey = action.columnId ?? action.columnName
257264
if (direction === 'undo') {
258265
addColumnMutation.mutate(
259266
{
267+
// Reuse the original id so the saved (id-keyed) cell data below
268+
// lands on the restored column.
269+
...(action.columnId ? { id: action.columnId } : {}),
260270
name: action.columnName,
261271
type: action.columnType,
262272
required: action.columnRequired,
@@ -268,7 +278,7 @@ export function useTableUndo({
268278
if (action.cellData.length > 0) {
269279
const updates = action.cellData.map((c) => ({
270280
rowId: c.rowId,
271-
data: { [action.columnName]: c.value },
281+
data: { [colKey]: c.value },
272282
}))
273283
void (async () => {
274284
try {
@@ -297,26 +307,22 @@ export function useTableUndo({
297307
if (action.previousWidth !== null) {
298308
const merged = {
299309
...(getColumnWidthsRef.current?.() ?? {}),
300-
[action.columnName]: action.previousWidth,
310+
[colKey]: action.previousWidth,
301311
}
302312
metadata.columnWidths = merged
303313
onColumnWidthsChangeRef.current?.(merged)
304314
}
305315
if (action.previousPinnedColumns !== null) {
306-
const wasColumnPinned = action.previousPinnedColumns.includes(
307-
action.columnName
308-
)
316+
const wasColumnPinned = action.previousPinnedColumns.includes(colKey)
309317
if (wasColumnPinned) {
310318
const currentPinned = getPinnedColumnsRef.current?.() ?? []
311-
if (!currentPinned.includes(action.columnName)) {
312-
const insertIndex = action.previousPinnedColumns.indexOf(
313-
action.columnName
314-
)
319+
if (!currentPinned.includes(colKey)) {
320+
const insertIndex = action.previousPinnedColumns.indexOf(colKey)
315321
const restoredPinned = [...currentPinned]
316322
restoredPinned.splice(
317323
Math.min(insertIndex, restoredPinned.length),
318324
0,
319-
action.columnName
325+
colKey
320326
)
321327
onPinnedColumnsChangeRef.current?.(restoredPinned)
322328
metadata.pinnedColumns = restoredPinned
@@ -330,24 +336,24 @@ export function useTableUndo({
330336
}
331337
)
332338
} else {
333-
deleteColumnMutation.mutate(action.columnName, {
339+
deleteColumnMutation.mutate(colKey, {
334340
onSuccess: () => {
335341
const metadata: Record<string, unknown> = {}
336342
if (action.previousOrder) {
337-
const newOrder = action.previousOrder.filter((n) => n !== action.columnName)
343+
const newOrder = action.previousOrder.filter((n) => n !== colKey)
338344
onColumnOrderChangeRef.current?.(newOrder)
339345
metadata.columnOrder = newOrder
340346
}
341347
if (action.previousWidth !== null) {
342348
const currentWidths = getColumnWidthsRef.current?.() ?? {}
343-
const { [action.columnName]: _, ...rest } = currentWidths
349+
const { [colKey]: _, ...rest } = currentWidths
344350
metadata.columnWidths = rest
345351
onColumnWidthsChangeRef.current?.(rest)
346352
}
347353
if (action.previousPinnedColumns !== null) {
348354
const currentPinned = getPinnedColumnsRef.current?.() ?? []
349-
if (currentPinned.includes(action.columnName)) {
350-
const newPinned = currentPinned.filter((n) => n !== action.columnName)
355+
if (currentPinned.includes(colKey)) {
356+
const newPinned = currentPinned.filter((n) => n !== colKey)
351357
onPinnedColumnsChangeRef.current?.(newPinned)
352358
metadata.pinnedColumns = newPinned
353359
}
@@ -364,11 +370,14 @@ export function useTableUndo({
364370
case 'rename-column': {
365371
const fromName = direction === 'undo' ? action.newName : action.oldName
366372
const toName = direction === 'undo' ? action.oldName : action.newName
373+
// Look up by the stable id (falls back to the current name) so undo
374+
// never renames the column to its internal id.
375+
const colKey = action.columnId ?? fromName
367376
updateColumnMutation.mutate({
368-
columnName: fromName,
377+
columnName: colKey,
369378
updates: { name: toName },
370379
})
371-
onColumnRenameRef.current?.(fromName, toName)
380+
onColumnRenameRef.current?.(colKey, toName)
372381
break
373382
}
374383

‎apps/sim/lib/api/contracts/tables.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,9 @@ export const renameTableBodySchema = z.object({
112112
export const createTableColumnBodySchema = z.object({
113113
workspaceId: z.string().min(1, 'Workspace ID is required'),
114114
column: z.object({
115+
// Optional stable id — first-party undo of a delete re-creates the column
116+
// with its original id so saved (id-keyed) cell data restores correctly.
117+
id: z.string().optional(),
115118
name: columnNameSchema,
116119
type: columnTypeSchema,
117120
required: z.boolean().optional(),

‎apps/sim/lib/table/service.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -543,6 +543,7 @@ export async function createTable(
543543
export async function addTableColumn(
544544
tableId: string,
545545
column: {
546+
id?: string
546547
name: string
547548
type: string
548549
required?: boolean
@@ -582,7 +583,9 @@ export async function addTableColumn(
582583
}
583584

584585
const newColumn: TableSchema['columns'][number] = {
585-
id: generateColumnId(collectColumnIds(schema)),
586+
// Honor a caller-provided id (undo of a delete reuses the original id);
587+
// otherwise mint a fresh one.
588+
id: column.id ?? generateColumnId(collectColumnIds(schema)),
586589
name: column.name,
587590
type: column.type as TableSchema['columns'][number]['type'],
588591
required: column.required ?? false,

‎apps/sim/stores/table/types.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,13 @@ export type TableUndoAction =
4646
}>
4747
}
4848
| { type: 'delete-rows'; rows: DeletedRowSnapshot[] }
49-
| { type: 'create-column'; columnName: string; position: number }
49+
// `columnName` is the display name (for re-create); `columnId` is the stable
50+
// storage key used for the delete/update lookup and id-keyed metadata cleanup.
51+
| { type: 'create-column'; columnName: string; columnId?: string; position: number }
5052
| {
5153
type: 'delete-column'
5254
columnName: string
55+
columnId?: string
5356
columnType: ColumnDefinition['type']
5457
columnPosition: number
5558
columnUnique: boolean
@@ -59,7 +62,8 @@ export type TableUndoAction =
5962
previousWidth: number | null
6063
previousPinnedColumns: string[] | null
6164
}
62-
| { type: 'rename-column'; oldName: string; newName: string }
65+
// `oldName`/`newName` are display names; `columnId` is the stable lookup key.
66+
| { type: 'rename-column'; oldName: string; newName: string; columnId?: string }
6367
| {
6468
type: 'update-column-type'
6569
columnName: string

0 commit comments

Comments
 (0)