Skip to content

Commit bab5ffc

Browse files
committed
fix(tables): harden reference column metadata
1 parent def0976 commit bab5ffc

15 files changed

Lines changed: 223 additions & 42 deletions

File tree

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/column-config-sidebar/column-config-sidebar.test.tsx

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,26 @@ describe('ColumnConfigSidebar', () => {
162162
})
163163
})
164164

165+
it('keeps Reference creation open until a target table is selected', async () => {
166+
await act(async () => {
167+
root.render(
168+
<ColumnConfigSidebar
169+
config={{ mode: 'create', proposedName: 'Related row', type: 'reference' }}
170+
onClose={vi.fn()}
171+
existingColumn={null}
172+
workspaceId='workspace-1'
173+
tableId='table-current'
174+
/>
175+
)
176+
})
177+
178+
await act(async () => findButton('Save')?.click())
179+
180+
expect(container).toHaveTextContent('Select a table')
181+
expect(mockAddColumn).not.toHaveBeenCalled()
182+
expect(mockUpdateColumn).not.toHaveBeenCalled()
183+
})
184+
165185
it('edits Reference configuration without exposing column renaming', async () => {
166186
await act(async () => {
167187
root.render(

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

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import type {
2424
WorkflowGroup,
2525
} from '@/lib/table'
2626
import { getColumnId } from '@/lib/table/column-keys'
27-
import { columnTypeOf } from '@/lib/table/column-types'
27+
import { columnTypeOf, typeMetadataOf } from '@/lib/table/column-types'
2828
import { TABLE_LIMITS } from '@/lib/table/constants'
2929
import { cellValueFilterConditions } from '@/lib/table/query-builder/cell-filter'
3030
import { SEARCH_DEBOUNCE_MS } from '@/lib/url-state'
@@ -4045,11 +4045,7 @@ export function TableGrid({
40454045
columnPosition: adjustedPosition >= 0 ? adjustedPosition : cols.length,
40464046
columnUnique: entry.def?.unique ?? false,
40474047
columnRequired: entry.def?.required ?? false,
4048-
// Without these a deleted select column can't be re-created — it is
4049-
// invalid with no options, and the saved cell data is option ids.
4050-
...(entry.def?.options ? { columnOptions: entry.def.options } : {}),
4051-
...(entry.def?.multiple ? { columnMultiple: true } : {}),
4052-
...(entry.def?.currencyCode ? { columnCurrencyCode: entry.def.currencyCode } : {}),
4048+
columnTypeMetadata: entry.def ? typeMetadataOf(entry.def) : {},
40534049
cellData,
40544050
previousOrder: orderSnapshot,
40554051
previousWidth,

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

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,7 @@ describe('useTableUndo – delete-column undo cell restore chunking', () => {
195195
columnPosition: 0,
196196
columnUnique: false,
197197
columnRequired: false,
198+
columnTypeMetadata: {},
198199
cellData: [],
199200
previousOrder: null,
200201
previousWidth: null,
@@ -248,8 +249,10 @@ describe('useTableUndo – restoring a deleted select column', () => {
248249
columnPosition: 0,
249250
columnUnique: false,
250251
columnRequired: false,
251-
columnOptions: [{ id: 'opt_open', name: 'Open' }],
252-
columnMultiple: true,
252+
columnTypeMetadata: {
253+
options: [{ id: 'opt_open', name: 'Open' }],
254+
multiple: true,
255+
},
253256
cellData: [],
254257
previousOrder: null,
255258
previousWidth: null,
@@ -272,3 +275,38 @@ describe('useTableUndo – restoring a deleted select column', () => {
272275
expect(payload.id).toBe('col_status')
273276
})
274277
})
278+
279+
describe('useTableUndo – restoring a deleted reference column', () => {
280+
it('re-creates the column with its target table', async () => {
281+
mockPopUndo.mockReturnValueOnce(
282+
makeEntry({
283+
type: 'delete-column',
284+
columnName: 'owner',
285+
columnId: 'col_owner',
286+
columnType: 'reference',
287+
columnPosition: 0,
288+
columnUnique: false,
289+
columnRequired: false,
290+
columnTypeMetadata: { referenceTableId: 'tbl_people' },
291+
cellData: [],
292+
previousOrder: null,
293+
previousWidth: null,
294+
previousPinnedColumns: null,
295+
})
296+
)
297+
298+
const { undo } = TestHook()
299+
;(undo as () => void)()
300+
await flush()
301+
302+
expect(mockMutate).toHaveBeenCalledWith(
303+
expect.objectContaining({
304+
id: 'col_owner',
305+
name: 'owner',
306+
type: 'reference',
307+
referenceTableId: 'tbl_people',
308+
}),
309+
expect.any(Object)
310+
)
311+
})
312+
})

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

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -386,11 +386,7 @@ export function useTableUndo({
386386
type: action.columnType,
387387
required: action.columnRequired,
388388
unique: action.columnUnique,
389-
// A select column is rejected without its options, and the
390-
// cell data restored below is keyed by those option ids.
391-
...(action.columnOptions ? { options: action.columnOptions } : {}),
392-
...(action.columnMultiple ? { multiple: true } : {}),
393-
...(action.columnCurrencyCode ? { currencyCode: action.columnCurrencyCode } : {}),
389+
...action.columnTypeMetadata,
394390
position: action.columnPosition,
395391
},
396392
{

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
tableRowsQuerySchema,
1010
updateTableColumnBodySchema,
1111
} from '@/lib/api/contracts/tables'
12+
import { MAX_REFERENCE_TABLE_ID_LENGTH } from '@/lib/table/constants'
1213

1314
describe('reference column metadata', () => {
1415
const referenceColumn = {
@@ -44,6 +45,45 @@ describe('reference column metadata', () => {
4445
it('rejects reference metadata on another column type', () => {
4546
expect(tableColumnSchema.safeParse({ ...referenceColumn, type: 'string' }).success).toBe(false)
4647
})
48+
49+
it('bounds reference table IDs at the standard identifier length', () => {
50+
const maximumId = 't'.repeat(MAX_REFERENCE_TABLE_ID_LENGTH)
51+
const oversizedId = 't'.repeat(MAX_REFERENCE_TABLE_ID_LENGTH + 1)
52+
53+
expect(
54+
tableColumnSchema.safeParse({ ...referenceColumn, referenceTableId: maximumId }).success
55+
).toBe(true)
56+
expect(
57+
createTableColumnBodySchema.safeParse({
58+
workspaceId: 'ws-1',
59+
column: { ...referenceColumn, referenceTableId: maximumId },
60+
}).success
61+
).toBe(true)
62+
expect(
63+
updateTableColumnBodySchema.safeParse({
64+
workspaceId: 'ws-1',
65+
columnName: 'account',
66+
updates: { referenceTableId: maximumId },
67+
}).success
68+
).toBe(true)
69+
70+
expect(
71+
tableColumnSchema.safeParse({ ...referenceColumn, referenceTableId: oversizedId }).success
72+
).toBe(false)
73+
expect(
74+
createTableColumnBodySchema.safeParse({
75+
workspaceId: 'ws-1',
76+
column: { ...referenceColumn, referenceTableId: oversizedId },
77+
}).success
78+
).toBe(false)
79+
expect(
80+
updateTableColumnBodySchema.safeParse({
81+
workspaceId: 'ws-1',
82+
columnName: 'account',
83+
updates: { referenceTableId: oversizedId },
84+
}).success
85+
).toBe(false)
86+
})
4787
})
4888

4989
/**

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

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import type {
3131
import {
3232
COLUMN_TYPES,
3333
FILTER_OPS,
34+
MAX_REFERENCE_TABLE_ID_LENGTH,
3435
MAX_RUN_TARGET_ROW_IDS,
3536
MAX_SELECT_OPTIONS,
3637
MAX_TABLE_BATCH_ITEMS,
@@ -83,15 +84,18 @@ export const currencyCodeSchema = z
8384
.regex(/^[A-Za-z]{3}$/, 'Must be a 3-letter ISO 4217 currency code, e.g. USD')
8485
.overwrite((code) => code.toUpperCase())
8586

86-
export const referenceTableIdSchema = requiredFieldSchema('Reference table ID is required')
87+
export const referenceTableIdSchema = requiredFieldSchema('Reference table ID is required').max(
88+
MAX_REFERENCE_TABLE_ID_LENGTH,
89+
`Reference table ID must be ${MAX_REFERENCE_TABLE_ID_LENGTH} characters or less`
90+
)
8791

8892
/**
8993
* Cross-field rules for type-owned metadata. A `select` column must declare a
9094
* non-empty option set, a `reference` column must declare its target table,
9195
* and type-specific fields are rejected on every type that does not own them.
9296
* Skipped when `type` is absent (a metadata-only update on an existing column).
9397
*/
94-
export function refineColumnOptions(
98+
export function refineColumnTypeMetadata(
9599
data: {
96100
type?: (typeof COLUMN_TYPES)[number]
97101
options?: z.infer<typeof selectOptionsSchema>
@@ -241,7 +245,7 @@ export const tableColumnSchema = z
241245
.optional()
242246
.describe('Target table whose row IDs are stored by a reference column.'),
243247
})
244-
.superRefine(refineColumnOptions)
248+
.superRefine(refineColumnTypeMetadata)
245249
.describe('A typed column in a table schema.')
246250

247251
export const createTableBodySchema = z.object({
@@ -326,7 +330,7 @@ export const createTableColumnBodySchema = z.object({
326330
.optional()
327331
.describe('Target table for a reference column.'),
328332
})
329-
.superRefine(refineColumnOptions)
333+
.superRefine(refineColumnTypeMetadata)
330334
.describe('Typed column definition to add.'),
331335
})
332336

@@ -346,7 +350,7 @@ export const updateTableColumnBodySchema = z.object({
346350
.optional()
347351
.describe('New target table for a reference column.'),
348352
})
349-
.superRefine(refineColumnOptions)
353+
.superRefine(refineColumnTypeMetadata)
350354
.describe('Column fields to update.'),
351355
})
352356

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import {
1919
predicateSchema,
2020
referenceTableIdSchema,
2121
refineCancelTableRunsScope,
22-
refineColumnOptions,
22+
refineColumnTypeMetadata,
2323
rowAnchorMutexRefine,
2424
runColumnBodyBaseSchema,
2525
runColumnExcludeMutexRefine,
@@ -493,7 +493,7 @@ const v2TableColumnInputShape = {
493493
export const v2TableColumnInputSchema = z
494494
.object(v2TableColumnInputShape)
495495
.strict()
496-
.superRefine(refineColumnOptions)
496+
.superRefine(refineColumnTypeMetadata)
497497

498498
/**
499499
* Initial columns take the same shape as every other v2 column input.
@@ -744,7 +744,7 @@ export const v2CreateTableColumnBodySchema = z
744744
.describe('Zero-based insertion position for the column.'),
745745
})
746746
.strict()
747-
.superRefine(refineColumnOptions)
747+
.superRefine(refineColumnTypeMetadata)
748748
.describe('Column definition to add.'),
749749
})
750750
.strict()
@@ -779,7 +779,7 @@ export const v2UpdateTableColumnBodySchema = z
779779
.describe('Replacement target table for a reference column.'),
780780
})
781781
.strict()
782-
.superRefine(refineColumnOptions)
782+
.superRefine(refineColumnTypeMetadata)
783783
.describe('Mutable column fields.'),
784784
})
785785
.strict()

apps/sim/lib/table/column-types/reference.ts

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import { Table as TableIcon } from '@sim/emcn/icons'
2+
import { stringColumnType } from '@/lib/table/column-types/string'
23
import type { ColumnTypeDefinition } from '@/lib/table/column-types/types'
4+
import { MAX_REFERENCE_TABLE_ID_LENGTH } from '@/lib/table/constants'
35

46
export const referenceColumnType: ColumnTypeDefinition = {
57
id: 'reference',
@@ -16,13 +18,7 @@ export const referenceColumnType: ColumnTypeDefinition = {
1618
editor: 'text',
1719
expandable: false,
1820

19-
coerce(value) {
20-
if (typeof value === 'string') return { ok: true, value }
21-
if (typeof value === 'number' || typeof value === 'boolean') {
22-
return { ok: true, value: String(value) }
23-
}
24-
return { ok: false }
25-
},
21+
coerce: stringColumnType.coerce,
2622

2723
validateCell(value, column) {
2824
return typeof value === 'string' ? null : `${column.name} must be a row ID string`
@@ -32,6 +28,11 @@ export const referenceColumnType: ColumnTypeDefinition = {
3228
if (typeof column.referenceTableId !== 'string' || column.referenceTableId.length === 0) {
3329
return [`Column "${column.name}" must define a reference table ID`]
3430
}
31+
if (column.referenceTableId.length > MAX_REFERENCE_TABLE_ID_LENGTH) {
32+
return [
33+
`Column "${column.name}" reference table ID must be ${MAX_REFERENCE_TABLE_ID_LENGTH} characters or less`,
34+
]
35+
}
3536
return []
3637
},
3738

@@ -41,8 +42,5 @@ export const referenceColumnType: ColumnTypeDefinition = {
4142
return typeof value === 'object' ? JSON.stringify(value) : String(value)
4243
},
4344

44-
formatForInput(value) {
45-
if (typeof value === 'object') return JSON.stringify(value)
46-
return String(value)
47-
},
45+
formatForInput: stringColumnType.formatForInput,
4846
}

apps/sim/lib/table/columns/reference-metadata.test.ts

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
*/
44

55
import { beforeEach, describe, expect, it, vi } from 'vitest'
6+
import { MAX_REFERENCE_TABLE_ID_LENGTH } from '@/lib/table/constants'
67
import type { TableDefinition } from '@/lib/table/types'
78

89
const mocks = vi.hoisted(() => ({
@@ -159,4 +160,59 @@ describe('reference column metadata persistence', () => {
159160
expect(updated).toBe(table)
160161
expect(trx.update).not.toHaveBeenCalled()
161162
})
163+
164+
it('does not rewrite the schema when the target and supplied constraints are unchanged', async () => {
165+
const table = tableWithReference()
166+
table.schema.columns[0] = { ...table.schema.columns[0], required: true, unique: true }
167+
const trx = useTable(table)
168+
169+
const updated = await updateColumnReference(
170+
{
171+
tableId: 'tbl_people',
172+
columnName: 'col_account',
173+
referenceTableId: 'tbl_accounts',
174+
required: true,
175+
unique: true,
176+
},
177+
'req_1'
178+
)
179+
180+
expect(updated).toBe(table)
181+
expect(trx.update).not.toHaveBeenCalled()
182+
})
183+
184+
it('accepts a reference table ID at the standard identifier length', async () => {
185+
const maximumId = 't'.repeat(MAX_REFERENCE_TABLE_ID_LENGTH)
186+
useTable(tableWithReference())
187+
188+
const updated = await updateColumnReference(
189+
{
190+
tableId: 'tbl_people',
191+
columnName: 'col_account',
192+
referenceTableId: maximumId,
193+
},
194+
'req_1'
195+
)
196+
197+
expect(updated.schema.columns[0]).toMatchObject({ referenceTableId: maximumId })
198+
expect(mocks.set).toHaveBeenCalledOnce()
199+
})
200+
201+
it('rejects a reference table ID longer than the standard identifier length', async () => {
202+
const oversizedId = 't'.repeat(MAX_REFERENCE_TABLE_ID_LENGTH + 1)
203+
const trx = useTable(tableWithReference())
204+
205+
await expect(
206+
updateColumnReference(
207+
{
208+
tableId: 'tbl_people',
209+
columnName: 'col_account',
210+
referenceTableId: oversizedId,
211+
},
212+
'req_1'
213+
)
214+
).rejects.toMatchObject({ code: 'validation' })
215+
216+
expect(trx.update).not.toHaveBeenCalled()
217+
})
162218
})

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1516,7 +1516,8 @@ export async function updateColumnReference(
15161516
)
15171517
const renamePending = data.newName !== undefined && data.newName !== column.name
15181518
if (
1519-
constrained === updatedColumn &&
1519+
constrained.required === column.required &&
1520+
constrained.unique === column.unique &&
15201521
updatedColumn.referenceTableId === column.referenceTableId &&
15211522
!renamePending
15221523
) {

0 commit comments

Comments
 (0)