Skip to content

Commit f7811f8

Browse files
feat(tables): stable column ids for metadata-only rename (#4898)
* feat(tables): stable column ids for metadata-only rename * fix(tables): address review — id-key exec clears + waiting labels, name in upsert error, un-gate group output ids * fix(tables): id-correct column undo (rename/create/delete) with id reuse on re-add * refactor(tables): mint column ids via generateId (uuid), drop collision-check plumbing * fix(tables): match column id or name when removing column from optimistic delete cache * fix(tables): resolve column storage id once for delete optimistic strips; biome-format 0228 snapshot * fix(tables): make in-grid find id-native (scan/return JSONB keys as column ids) * fix(tables): translate id→name in CSV/JSON export read path (+regression test)
1 parent efeacb9 commit f7811f8

44 files changed

Lines changed: 17781 additions & 567 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { hybridAuthMockFns } from '@sim/testing'
5+
import { NextRequest } from 'next/server'
6+
import { beforeEach, describe, expect, it, vi } from 'vitest'
7+
import type { TableDefinition } from '@/lib/table'
8+
9+
const { mockCheckAccess, mockQueryRows } = vi.hoisted(() => ({
10+
mockCheckAccess: vi.fn(),
11+
mockQueryRows: vi.fn(),
12+
}))
13+
14+
vi.mock('@/app/api/table/utils', async () => {
15+
const { NextResponse } = await import('next/server')
16+
return {
17+
checkAccess: mockCheckAccess,
18+
accessError: (result: { status: number }) =>
19+
NextResponse.json({ error: 'Access denied' }, { status: result.status }),
20+
}
21+
})
22+
23+
vi.mock('@/lib/table/service', () => ({
24+
queryRows: mockQueryRows,
25+
}))
26+
27+
import { GET } from '@/app/api/table/[tableId]/export/route'
28+
29+
/** Table with an id-native column whose stable id (`col_email`) differs from its display name. */
30+
function buildTable(): TableDefinition {
31+
return {
32+
id: 'tbl_1',
33+
name: 'People',
34+
description: null,
35+
schema: {
36+
columns: [
37+
{ id: 'col_email', name: 'email', type: 'string' },
38+
{ name: 'legacy', type: 'string' }, // legacy: id == name
39+
],
40+
},
41+
metadata: null,
42+
rowCount: 1,
43+
maxRows: 100,
44+
workspaceId: 'workspace-1',
45+
createdBy: 'user-1',
46+
archivedAt: null,
47+
createdAt: new Date('2024-01-01'),
48+
updatedAt: new Date('2024-01-01'),
49+
}
50+
}
51+
52+
function callGet(format: string) {
53+
const req = new NextRequest(`http://localhost:3000/api/table/tbl_1/export?format=${format}`, {
54+
method: 'GET',
55+
})
56+
return GET(req, { params: Promise.resolve({ tableId: 'tbl_1' }) })
57+
}
58+
59+
describe('table export route — id→name translation', () => {
60+
beforeEach(() => {
61+
vi.clearAllMocks()
62+
hybridAuthMockFns.mockCheckSessionOrInternalAuth.mockResolvedValue({
63+
success: true,
64+
userId: 'user-1',
65+
authType: 'session',
66+
})
67+
mockCheckAccess.mockResolvedValue({ ok: true, table: buildTable() })
68+
// Row data is keyed by stable column id (`col_email`), not the display name.
69+
mockQueryRows.mockResolvedValue({
70+
rows: [{ id: 'r1', data: { col_email: 'a@b.c', legacy: 'x' }, executions: {}, position: 0 }],
71+
rowCount: 1,
72+
totalCount: 1,
73+
limit: 1000,
74+
offset: 0,
75+
})
76+
})
77+
78+
it('CSV: header uses display names and cell values resolve from id-keyed data', async () => {
79+
const res = await callGet('csv')
80+
expect(res.status).toBe(200)
81+
const body = await res.text()
82+
const [header, firstRow] = body.trim().split('\n')
83+
expect(header).toBe('email,legacy')
84+
// Without id→name resolution the email cell would be blank.
85+
expect(firstRow).toBe('a@b.c,x')
86+
})
87+
88+
it('JSON: keys are display names, never the stable column id', async () => {
89+
const res = await callGet('json')
90+
expect(res.status).toBe(200)
91+
const parsed = JSON.parse(await res.text())
92+
expect(parsed).toEqual([{ email: 'a@b.c', legacy: 'x' }])
93+
expect(JSON.stringify(parsed)).not.toContain('col_email')
94+
})
95+
})

apps/sim/app/api/table/[tableId]/export/route.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { getValidationErrorMessage } from '@/lib/api/server'
55
import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
66
import { generateRequestId } from '@/lib/core/utils/request'
77
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
8+
import { buildNameById, getColumnId, rowDataIdToName } from '@/lib/table/column-keys'
89
import { queryRows } from '@/lib/table/service'
910
import { accessError, checkAccess } from '@/app/api/table/utils'
1011

@@ -45,6 +46,9 @@ export const GET = withRouteHandler(async (request: NextRequest, { params }: Rou
4546
const { table } = access
4647

4748
const columns = table.schema.columns
49+
// Stored row data is id-keyed; CSV headers and JSON keys are display names, so
50+
// translate id → name on the way out (export is a name-friendly boundary).
51+
const nameById = buildNameById(table.schema)
4852
const safeName = sanitizeFilename(table.name)
4953
const filename = `${safeName}.${format}`
5054

@@ -71,12 +75,14 @@ export const GET = withRouteHandler(async (request: NextRequest, { params }: Rou
7175

7276
for (const row of result.rows) {
7377
if (format === 'csv') {
74-
const values = columns.map((c) => formatCsvValue(row.data[c.name]))
78+
const values = columns.map((c) => formatCsvValue(row.data[getColumnId(c)]))
7579
controller.enqueue(encoder.encode(`${toCsvRow(values)}\n`))
7680
} else {
7781
const prefix = firstJsonRow ? '' : ','
7882
firstJsonRow = false
79-
controller.enqueue(encoder.encode(prefix + JSON.stringify({ ...row.data })))
83+
controller.enqueue(
84+
encoder.encode(prefix + JSON.stringify(rowDataIdToName(row.data, nameById)))
85+
)
8086
}
8187
}
8288

apps/sim/app/api/table/[tableId]/import/route.test.ts

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -393,10 +393,14 @@ describe('POST /api/table/[tableId]/import', () => {
393393
)
394394
expect(response.status).toBe(200)
395395
expect(mockImportAppendRows).toHaveBeenCalledTimes(1)
396-
expect(appendAdditions()).toEqual([{ name: 'email', type: 'string' }])
396+
expect(appendAdditions()).toEqual([
397+
expect.objectContaining({ name: 'email', type: 'string' }),
398+
])
399+
// Existing columns have no id (legacy) → keyed by name; the new `email`
400+
// column was assigned id `col_deadbeefcafef00d` (mocked generateId).
397401
expect(appendRows()).toEqual([
398-
{ name: 'Alice', age: 30, email: 'a@x.io' },
399-
{ name: 'Bob', age: 40, email: 'b@x.io' },
402+
{ name: 'Alice', age: 30, col_deadbeefcafef00d: 'a@x.io' },
403+
{ name: 'Bob', age: 40, col_deadbeefcafef00d: 'b@x.io' },
400404
])
401405
})
402406

@@ -408,7 +412,9 @@ describe('POST /api/table/[tableId]/import', () => {
408412
})
409413
)
410414
expect(response.status).toBe(200)
411-
expect(appendAdditions()).toEqual([{ name: 'score', type: 'number' }])
415+
expect(appendAdditions()).toEqual([
416+
expect.objectContaining({ name: 'score', type: 'number' }),
417+
])
412418
})
413419

414420
it('dedupes when sanitized name collides with an existing column', async () => {
@@ -431,7 +437,9 @@ describe('POST /api/table/[tableId]/import', () => {
431437
})
432438
)
433439
expect(response.status).toBe(200)
434-
expect(appendAdditions()).toEqual([{ name: 'Email_2', type: 'string' }])
440+
expect(appendAdditions()).toEqual([
441+
expect.objectContaining({ name: 'Email_2', type: 'string' }),
442+
])
435443
})
436444

437445
it('returns 400 when createColumns references a header not in the CSV', async () => {
@@ -494,7 +502,9 @@ describe('POST /api/table/[tableId]/import', () => {
494502
})
495503
)
496504
// Route forwarded the column addition into the (now atomic) import op.
497-
expect(appendAdditions()).toEqual([{ name: 'email', type: 'string' }])
505+
expect(appendAdditions()).toEqual([
506+
expect.objectContaining({ name: 'email', type: 'string' }),
507+
])
498508
expect(response.status).toBe(400)
499509
const data = await response.json()
500510
expect(data.success).toBeUndefined()

apps/sim/app/api/table/[tableId]/import/route.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import {
2424
coerceRowsForTable,
2525
createCsvParser,
2626
dispatchAfterBatchInsert,
27+
generateColumnId,
2728
importAppendRows,
2829
importReplaceRows,
2930
inferColumnType,
@@ -176,7 +177,7 @@ export const POST = withRouteHandler(async (request: NextRequest, { params }: Ro
176177

177178
let effectiveMapping = mapping ?? buildAutoMapping(headers, table.schema)
178179
let prospectiveTable: TableDefinition = table
179-
const additions: { name: string; type: string }[] = []
180+
const additions: { id?: string; name: string; type: string }[] = []
180181

181182
if (createColumns && createColumns.length > 0) {
182183
const headerSet = new Set(headers)
@@ -204,8 +205,12 @@ export const POST = withRouteHandler(async (request: NextRequest, { params }: Ro
204205
}
205206
usedNames.add(columnName.toLowerCase())
206207
const inferredType = inferColumnType(rows.map((r) => r[header]))
207-
additions.push({ name: columnName, type: inferredType })
208+
// Pre-assign the id so the prospective schema (used to coerce rows) and
209+
// the persisted column (created in importAppendRows) share the same key.
210+
const id = generateColumnId()
211+
additions.push({ id, name: columnName, type: inferredType })
208212
newColumns.push({
213+
id,
209214
name: columnName,
210215
type: inferredType as TableSchema['columns'][number]['type'],
211216
required: false,

apps/sim/app/api/table/import-csv/route.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,9 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
137137
},
138138
requestId
139139
)
140-
return { table, schema, headerToColumn: inferred.headerToColumn }
140+
// Coerce against the *created* schema so rows key by the ids `createTable`
141+
// assigned (the local `schema` is the id-less inferred one).
142+
return { table, schema: table.schema, headerToColumn: inferred.headerToColumn }
141143
}
142144

143145
let state: ImportState | null = null

apps/sim/app/api/table/utils.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,9 @@ export const DeleteColumnSchema = deleteTableColumnBodySchema
200200

201201
export function normalizeColumn(col: ColumnDefinition): ColumnDefinition {
202202
return {
203+
// Preserve the stable column id — it's the row-data storage key, so dropping
204+
// it makes clients fall back to `name` and miss id-keyed cell values.
205+
...(col.id ? { id: col.id } : {}),
203206
name: col.name,
204207
type: col.type,
205208
required: col.required ?? false,

apps/sim/app/api/v1/tables/[tableId]/rows/[rowId]/route.ts

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,14 @@ import {
1212
import { parseRequest, validationErrorResponseFromError } from '@/lib/api/server'
1313
import { generateRequestId } from '@/lib/core/utils/request'
1414
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
15-
import type { RowData } from '@/lib/table'
16-
import { updateRow } from '@/lib/table'
15+
import type { RowData, TableSchema } from '@/lib/table'
16+
import {
17+
buildIdByName,
18+
buildNameById,
19+
rowDataIdToName,
20+
rowDataNameToId,
21+
updateRow,
22+
} from '@/lib/table'
1723
import { accessError, checkAccess } from '@/app/api/table/utils'
1824
import {
1925
checkRateLimit,
@@ -81,12 +87,13 @@ export const GET = withRouteHandler(async (request: NextRequest, context: RowRou
8187
return NextResponse.json({ error: 'Row not found' }, { status: 404 })
8288
}
8389

90+
const nameById = buildNameById(result.table.schema as TableSchema)
8491
return NextResponse.json({
8592
success: true,
8693
data: {
8794
row: {
8895
id: row.id,
89-
data: row.data,
96+
data: rowDataIdToName(row.data as RowData, nameById),
9097
position: row.position,
9198
createdAt:
9299
row.createdAt instanceof Date ? row.createdAt.toISOString() : String(row.createdAt),
@@ -129,11 +136,13 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: RowR
129136
return NextResponse.json({ error: 'Invalid workspace ID' }, { status: 400 })
130137
}
131138

139+
const idByName = buildIdByName(table.schema as TableSchema)
140+
const nameById = buildNameById(table.schema as TableSchema)
132141
const updatedRow = await updateRow(
133142
{
134143
tableId,
135144
rowId,
136-
data: validated.data as RowData,
145+
data: rowDataNameToId(validated.data as RowData, idByName),
137146
workspaceId: validated.workspaceId,
138147
},
139148
table,
@@ -153,7 +162,7 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: RowR
153162
data: {
154163
row: {
155164
id: updatedRow.id,
156-
data: updatedRow.data,
165+
data: rowDataIdToName(updatedRow.data, nameById),
157166
position: updatedRow.position,
158167
createdAt:
159168
updatedRow.createdAt instanceof Date

0 commit comments

Comments
 (0)