Skip to content

Commit 13ab9ea

Browse files
test(tables): bind the column-update tests to the orchestration function
The base's route tests assert which column service each payload reaches — the behavior that now lives in performUpdateTableColumn. They mocked the `@/lib/table` barrel; the orchestration module imports the service directly, so they mock that too and keep asserting the same thing through the extracted implementation. The orchestration tests move onto the base's semantics: writes address the stable column id, a rename rides inside the write it accompanies rather than running first, and the currency guards replace the non-select options guard the service now owns. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent e263f59 commit 13ab9ea

3 files changed

Lines changed: 46 additions & 11 deletions

File tree

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,13 @@ vi.mock('@/lib/table', () => ({
4242
updateColumnOptions: mockUpdateColumnOptions,
4343
updateColumnType: mockUpdateColumnType,
4444
}))
45+
vi.mock('@/lib/table/columns/service', () => ({
46+
renameColumn: mockRenameColumn,
47+
updateColumnConstraints: mockUpdateColumnConstraints,
48+
updateColumnCurrency: mockUpdateColumnCurrency,
49+
updateColumnOptions: mockUpdateColumnOptions,
50+
updateColumnType: mockUpdateColumnType,
51+
}))
4552
vi.mock('@/app/api/table/utils', () => ({
4653
accessError: () => new Response('denied', { status: 403 }),
4754
checkAccess: mockCheckAccess,

apps/sim/lib/table/orchestration/columns.test.ts

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,14 @@ const {
1212
mockUpdateColumnType,
1313
mockUpdateColumnOptions,
1414
mockUpdateColumnConstraints,
15+
mockUpdateColumnCurrency,
1516
mockRecordAudit,
1617
} = vi.hoisted(() => ({
1718
mockRenameColumn: vi.fn(),
1819
mockUpdateColumnType: vi.fn(),
1920
mockUpdateColumnOptions: vi.fn(),
2021
mockUpdateColumnConstraints: vi.fn(),
22+
mockUpdateColumnCurrency: vi.fn(),
2123
mockRecordAudit: vi.fn(),
2224
}))
2325

@@ -30,6 +32,7 @@ vi.mock('@sim/audit', () => ({
3032
vi.mock('@/lib/table/columns/service', () => ({
3133
renameColumn: mockRenameColumn,
3234
updateColumnConstraints: mockUpdateColumnConstraints,
35+
updateColumnCurrency: mockUpdateColumnCurrency,
3336
updateColumnOptions: mockUpdateColumnOptions,
3437
updateColumnType: mockUpdateColumnType,
3538
}))
@@ -71,6 +74,7 @@ describe('performUpdateTableColumn', () => {
7174
mockUpdateColumnType.mockResolvedValue(UPDATED)
7275
mockUpdateColumnOptions.mockResolvedValue(UPDATED)
7376
mockUpdateColumnConstraints.mockResolvedValue(UPDATED)
77+
mockUpdateColumnCurrency.mockResolvedValue(UPDATED)
7478
})
7579

7680
it('refuses to make a select column unique before writing anything', async () => {
@@ -94,8 +98,9 @@ describe('performUpdateTableColumn', () => {
9498
await run({ type: 'select', options: ['Open', 'Closed'] })
9599

96100
expect(mockUpdateColumnType).not.toHaveBeenCalled()
101+
// Addressed by stable id so a rename folded into the write can't break it.
97102
expect(mockUpdateColumnOptions).toHaveBeenCalledWith(
98-
expect.objectContaining({ columnName: 'Status' }),
103+
expect.objectContaining({ columnName: 'col-1' }),
99104
'req-1'
100105
)
101106
})
@@ -118,24 +123,41 @@ describe('performUpdateTableColumn', () => {
118123
)
119124
})
120125

121-
it('applies a rename before the later writes and targets the new name', async () => {
126+
it('folds a rename into the write it rides on rather than running it separately', async () => {
127+
// A rename is metadata-only, so folding it into the last write's transaction
128+
// is what stops a combined request committing one half and failing the other.
122129
await run({ name: 'State', required: true })
123130

124-
expect(mockRenameColumn).toHaveBeenCalledWith(
125-
{ tableId: 'table-1', oldName: 'Status', newName: 'State' },
131+
expect(mockRenameColumn).not.toHaveBeenCalled()
132+
expect(mockUpdateColumnConstraints).toHaveBeenCalledWith(
133+
expect.objectContaining({ columnName: 'col-1', newName: 'State' }),
126134
'req-1'
127135
)
128-
expect(mockUpdateColumnConstraints).toHaveBeenCalledWith(
129-
expect.objectContaining({ columnName: 'State' }),
136+
})
137+
138+
it('runs a rename standalone when there is no write to ride on', async () => {
139+
mockRenameColumn.mockResolvedValue(UPDATED)
140+
141+
await run({ name: 'State' })
142+
143+
expect(mockRenameColumn).toHaveBeenCalledWith(
144+
{ tableId: 'table-1', oldName: 'col-1', newName: 'State' },
130145
'req-1'
131146
)
132147
})
133148

134-
it('rejects an options edit on a column that is not a select', async () => {
135-
const result = await run({ multiple: true }, 'Priority')
149+
it('rejects setting a currency code on a non-currency column', async () => {
150+
const result = await run({ currencyCode: 'USD' })
151+
152+
expect(result).toMatchObject({ success: false, errorCode: 'validation' })
153+
expect(mockUpdateColumnCurrency).not.toHaveBeenCalled()
154+
})
155+
156+
it('rejects an unsupported currency code before any write', async () => {
157+
const result = await run({ type: 'currency', currencyCode: 'XX' }, 'Priority')
136158

137159
expect(result.errorCode).toBe('validation')
138-
expect(mockUpdateColumnOptions).not.toHaveBeenCalled()
160+
expect(mockUpdateColumnType).not.toHaveBeenCalled()
139161
})
140162

141163
it('reports an empty payload as a validation failure', async () => {

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,10 @@ export async function performUpdateTableColumn(
160160
) {
161161
return fail(`Column "${updates.name}" already exists`, 'validation')
162162
}
163-
if (currentColumn.workflowGroupId && (updates.required !== undefined || updates.unique !== undefined)) {
163+
if (
164+
currentColumn.workflowGroupId &&
165+
(updates.required !== undefined || updates.unique !== undefined)
166+
) {
164167
return fail(
165168
`Cannot change constraints on workflow-output column "${currentColumn.name}". Constraints aren't applicable to columns whose values come from workflow execution.`,
166169
'validation'
@@ -241,7 +244,10 @@ export async function performUpdateTableColumn(
241244
// stops a combined request from committing one half and then failing. Only
242245
// a rename with nothing to ride on runs standalone.
243246
if (updates.name && !updated) {
244-
updated = await renameColumn({ tableId, oldName: columnRef, newName: updates.name }, requestId)
247+
updated = await renameColumn(
248+
{ tableId, oldName: columnRef, newName: updates.name },
249+
requestId
250+
)
245251
}
246252
} catch (error) {
247253
logger.error(`[${requestId}] Failed to update column "${columnName}" on table ${tableId}`, {

0 commit comments

Comments
 (0)