Skip to content

Commit 0913728

Browse files
committed
fix(tables): migrate scalar cells when converting a column to select
Found by Cursor Bugbot. `resolveSelectOptionId` stringifies a number or boolean before matching, so a `number` column whose values equal option NAMES passes the compatibility gate — but `migrateCellsToSelectIds` only rewrote JSONB `string` and `array` cells. Those cells stayed raw numbers inside a select column, where they render as nothing and fail option membership on the next write. `data->>key` yields the text form for every scalar, so the existing lookup already worked; the predicate was simply too narrow. Widened to cover `number` and `boolean`. The outbound migration is unchanged — cells leaving a select column are option ids, always strings or arrays. Pre-existing on staging (both the resolver's scalar handling and the migration SQL predate this branch), but it lives in a file this PR creates. Tests pin the resolver behavior the predicate depends on, so narrowing either one without the other now fails.
1 parent 3d3ea8c commit 0913728

2 files changed

Lines changed: 37 additions & 2 deletions

File tree

apps/sim/lib/table/__tests__/column-conversion.test.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
isValueCompatibleWithType,
1111
selectValueForConversion,
1212
} from '@/lib/table/columns/service'
13+
import { resolveSelectOptionId } from '@/lib/table/select-options'
1314
import type { ColumnDefinition, SelectOption } from '@/lib/table/types'
1415

1516
const OPTIONS: SelectOption[] = [
@@ -194,3 +195,30 @@ describe('rename folded into another write', () => {
194195
expect(() => applyPendingRename(columns, 0, 'a'.repeat(200))).toThrow(/maximum length/)
195196
})
196197
})
198+
199+
describe('select accepts scalar cells', () => {
200+
// The resolver stringifies a scalar before matching, so a `number` or
201+
// `boolean` column whose values equal option NAMES converts. The migration's
202+
// JSONB predicate has to cover those types too — matching only `'string'`
203+
// left the cells as raw numbers inside a select column, where they render as
204+
// nothing and fail option membership on the next write.
205+
const NUMERIC_OPTIONS: SelectOption[] = [
206+
{ id: 'opt_1', name: '123' },
207+
{ id: 'opt_t', name: 'true' },
208+
]
209+
210+
it('resolves a numeric or boolean cell to its option id', () => {
211+
expect(resolveSelectOptionId(123, NUMERIC_OPTIONS)).toBe('opt_1')
212+
expect(resolveSelectOptionId(true, NUMERIC_OPTIONS)).toBe('opt_t')
213+
})
214+
215+
it('reports those cells as convertible, which is what obliges the migration', () => {
216+
expect(isValueCompatibleWithType(123, 'select', NUMERIC_OPTIONS)).toBe(true)
217+
expect(isValueCompatibleWithType(true, 'select', NUMERIC_OPTIONS)).toBe(true)
218+
expect(isValueCompatibleWithType(999, 'select', NUMERIC_OPTIONS)).toBe(false)
219+
})
220+
221+
it('leaves structured values unresolvable', () => {
222+
expect(resolveSelectOptionId({ a: 1 } as never, NUMERIC_OPTIONS)).toBeNull()
223+
})
224+
})

apps/sim/lib/table/column-types/registry.server.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,13 @@ export async function writeBackCoercedCells(
111111
* scalar id while multi filters compile to array containment, which a scalar
112112
* never matches. Either way the cell silently drops out until it is re-edited.
113113
*
114+
* Scalar cells count, not just strings: `resolveSelectOptionId` stringifies a
115+
* number or boolean before matching, so a `number` column whose values equal
116+
* option NAMES passes the compatibility gate. Matching only `jsonb_typeof =
117+
* 'string'` left those cells as raw numbers inside a select column, where they
118+
* render as nothing and fail option membership on the next write. `data->>key`
119+
* yields the text form for every scalar, so one widened predicate covers them.
120+
*
114121
* The map keys ids, names, and lower-cased names — ids so re-running is a no-op,
115122
* lower-cased names because `resolveSelectOptionId` accepts a case-mismatched
116123
* name and a cell that passed that check must actually migrate. Duplicate option
@@ -162,7 +169,7 @@ async function migrateCellsToSelectIds(
162169
) d), '[]'::jsonb)
163170
END)
164171
WHERE table_id = ${tableId}
165-
AND jsonb_typeof(data->${columnKey}::text) = 'string'`
172+
AND jsonb_typeof(data->${columnKey}::text) IN ('string', 'number', 'boolean')`
166173
)
167174
await trx.execute(
168175
sql`UPDATE ${userTableRows}
@@ -186,7 +193,7 @@ async function migrateCellsToSelectIds(
186193
ELSE COALESCE(${idByRef}::jsonb -> (data->>${columnKey}::text), ${idByRef}::jsonb -> lower(data->>${columnKey}::text), data->${columnKey}::text)
187194
END)
188195
WHERE table_id = ${tableId}
189-
AND jsonb_typeof(data->${columnKey}::text) = 'string'`
196+
AND jsonb_typeof(data->${columnKey}::text) IN ('string', 'number', 'boolean')`
190197
)
191198
// Compatibility already rejected multi-valued cells for a single target, so
192199
// any array here holds at most one option.

0 commit comments

Comments
 (0)