Skip to content

Commit 1536278

Browse files
committed
fix(table): resolve strict id-keyed columns through getColumnId
assertKnownColumnIds read column.id directly, but a column id is optional — pre-backfill columns have none and are stored under their name, which is why getColumnId exists and is what every other consumer of the schema uses. Such columns still exist, so a strict id-keyed write naming one would have been refused as unknown. Latent today: no surface yet combines dataKeying 'ids' with strictWrite. Fixed before one does. Verified to fail: reading column.id turns the covering test red.
1 parent bca66ea commit 1536278

2 files changed

Lines changed: 32 additions & 2 deletions

File tree

apps/sim/lib/table/application/rows.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1255,6 +1255,33 @@ describe('row data keying', () => {
12551255
expect(mockUpdateRow).not.toHaveBeenCalled()
12561256
})
12571257

1258+
it('accepts a legacy column that has no id and is stored under its name', async () => {
1259+
// Two production tables still carry pre-backfill columns with no `id`.
1260+
// Their storage key is the name, so a strict id-keyed write naming one must
1261+
// be accepted, not refused as unknown.
1262+
mockResolveContext.mockResolvedValue({
1263+
tableId: TABLE.id,
1264+
table: { ...TABLE, schema: { columns: [{ name: 'legacy', type: 'string' }] } },
1265+
workspaceId: TABLE.workspaceId,
1266+
workspaceOrganizationId: 'organization-1',
1267+
allowPersonalApiKeys: true,
1268+
billedAccountUserId: 'billing-owner-1',
1269+
})
1270+
1271+
await expect(
1272+
updateTableRow.execute({
1273+
principal: PRINCIPAL,
1274+
input: {
1275+
tableId: TABLE.id,
1276+
rowId: 'row-1',
1277+
data: { legacy: 'x' },
1278+
strictWrite: true,
1279+
dataKeying: 'ids',
1280+
},
1281+
})
1282+
).resolves.toBeDefined()
1283+
})
1284+
12581285
it('names every unknown id at once, as the name wire does', async () => {
12591286
await expect(
12601287
createTableRows.execute({

apps/sim/lib/table/application/rows.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ import { defineAuthorizedTableUseCase } from '@/lib/table/application/authorized
4343
import { resolveActiveTableContext } from '@/lib/table/application/context'
4444
import { tableOperations } from '@/lib/table/application/operations'
4545
import { assertRowCapacity, notifyTableRowUsage } from '@/lib/table/billing'
46-
import { buildIdByName, unknownColumnNames } from '@/lib/table/column-keys'
46+
import { buildIdByName, getColumnId, unknownColumnNames } from '@/lib/table/column-keys'
4747
import { columnTypeOf } from '@/lib/table/column-types'
4848
import { TableQueryValidationError } from '@/lib/table/errors'
4949
import { signalTableRowsChanged, signalTableRowsChangedByActor } from '@/lib/table/events'
@@ -179,7 +179,10 @@ export type TableRowDataKeying = 'names' | 'ids'
179179
* `strictWrite` condition, so strictness means the same thing on either wire.
180180
*/
181181
function assertKnownColumnIds(data: RowData, table: TableDefinition, rowLabel?: string): void {
182-
const ids = new Set(table.schema.columns.map((column) => column.id))
182+
// `getColumnId`, not `column.id`: a legacy pre-backfill column has no id and is
183+
// stored under its name, so reading the raw field would reject a write that
184+
// every other consumer of the schema accepts.
185+
const ids = new Set(table.schema.columns.map((column) => getColumnId(column)))
183186
const unknown = Object.keys(data).filter((key) => !ids.has(key))
184187
if (unknown.length === 0) return
185188
const where = rowLabel ? `${rowLabel}: ` : ''

0 commit comments

Comments
 (0)