Skip to content

Commit b416776

Browse files
j15zclaude
andcommitted
refactor(tables): migrate row parse/validation to the column-type registry
coerceValueToColumnType and validateRowAgainstSchema's switches are replaced by delegates to the registry's parse/isValidValue, so a new column type's coercion and shape-check rules only need to be declared once. The now-dead optionIds helper is removed, and resolveSelectOptionId/splitMultiSelectInput move to select-values.ts (their tests move with them) now that the registry depends on them living there instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent e26a5a6 commit b416776

3 files changed

Lines changed: 26 additions & 178 deletions

File tree

apps/sim/lib/table/select-values.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,21 @@ import type {
3131
* gate (`columns/service.ts`) share one resolution rule.
3232
*/
3333
export function resolveSelectOptionId(value: JsonValue, options: SelectOption[]): string | null {
34-
if (typeof value !== 'string') return null
35-
const byId = options.find((o) => o.id === value)
34+
// The block builder serializes without schema access, so an option NAME that
35+
// looks numeric or boolean ("123", "true") arrives scalar-coerced. Stringify
36+
// scalars so the name still resolves; arrays/objects stay unresolvable.
37+
const text =
38+
typeof value === 'string'
39+
? value
40+
: typeof value === 'number' || typeof value === 'boolean'
41+
? String(value)
42+
: null
43+
if (text === null) return null
44+
const byId = options.find((o) => o.id === text)
3645
if (byId) return byId.id
3746
const byName =
38-
options.find((o) => o.name === value) ??
39-
options.find((o) => o.name.toLowerCase() === value.toLowerCase())
47+
options.find((o) => o.name === text) ??
48+
options.find((o) => o.name.toLowerCase() === text.toLowerCase())
4049
return byName ? byName.id : null
4150
}
4251

apps/sim/lib/table/validation.test.ts

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import { describe, expect, it } from 'vitest'
55
import type { ColumnDefinition, RowData, TableSchema } from '@/lib/table/types'
66
import {
77
coerceRowToSchema,
8-
resolveSelectOptionId,
98
validateColumnDefinition,
109
validateRowAgainstSchema,
1110
} from '@/lib/table/validation'
@@ -123,22 +122,6 @@ describe('coerceRowToSchema — multiselect', () => {
123122
})
124123
})
125124

126-
describe('resolveSelectOptionId', () => {
127-
const options = selectColumn.options ?? []
128-
129-
it('resolves a stable id', () => {
130-
expect(resolveSelectOptionId('opt_open', options)).toBe('opt_open')
131-
})
132-
133-
it('resolves a display name (case-insensitively)', () => {
134-
expect(resolveSelectOptionId('closed', options)).toBe('opt_closed')
135-
})
136-
137-
it('returns null for an unknown value (drives the type-conversion compatibility gate)', () => {
138-
expect(resolveSelectOptionId('nope', options)).toBeNull()
139-
})
140-
})
141-
142125
describe('validateColumnDefinition — select options', () => {
143126
it('accepts a well-formed select column', () => {
144127
expect(validateColumnDefinition(selectColumn).valid).toBe(true)

apps/sim/lib/table/validation.ts

Lines changed: 13 additions & 157 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { userTableRows } from '@sim/db/schema'
77
import { and, eq, or, type SQL, sql } from 'drizzle-orm'
88
import { NextResponse } from 'next/server'
99
import { getColumnId } from '@/lib/table/column-keys'
10+
import { isValidColumnValue, parseColumnValue } from '@/lib/table/column-types'
1011
import {
1112
COLUMN_TYPES,
1213
getMaxRowSizeBytes,
@@ -15,14 +16,12 @@ import {
1516
TABLE_LIMITS,
1617
USER_TABLE_ROWS_SQL_NAME,
1718
} from '@/lib/table/constants'
18-
import { normalizeDateCellValue } from '@/lib/table/dates'
1919
import { withSeqscanOff } from '@/lib/table/planner'
2020
import { fieldPredicate } from '@/lib/table/sql'
2121
import type {
2222
ColumnDefinition,
2323
JsonValue,
2424
RowData,
25-
SelectOption,
2625
TableSchema,
2726
ValidationResult,
2827
} from '@/lib/table/types'
@@ -220,7 +219,12 @@ export function validateTableSchema(schema: TableSchema): ValidationResult {
220219
return { valid: errors.length === 0, errors }
221220
}
222221

223-
/** Validates row data matches schema column types and required fields. */
222+
/**
223+
* Validates row data matches schema column types and required fields.
224+
* Delegates each column's shape check to the column-type registry
225+
* (`column-types.ts`) so a new column type's validation rule only needs to be
226+
* added in one place — see `ColumnTypeDefinition.isValidValue`.
227+
*/
224228
export function validateRowAgainstSchema(data: RowData, schema: TableSchema): ValidationResult {
225229
const errors: string[] = []
226230

@@ -234,174 +238,26 @@ export function validateRowAgainstSchema(data: RowData, schema: TableSchema): Va
234238

235239
if (value === null || value === undefined) continue
236240

237-
switch (column.type) {
238-
case 'string':
239-
if (typeof value !== 'string') {
240-
errors.push(`${column.name} must be string, got ${typeof value}`)
241-
}
242-
break
243-
case 'number':
244-
if (typeof value !== 'number' || Number.isNaN(value)) {
245-
errors.push(`${column.name} must be number`)
246-
}
247-
break
248-
case 'boolean':
249-
if (typeof value !== 'boolean') {
250-
errors.push(`${column.name} must be boolean`)
251-
}
252-
break
253-
case 'date':
254-
if (
255-
!(value instanceof Date) &&
256-
(typeof value !== 'string' || Number.isNaN(Date.parse(value)))
257-
) {
258-
errors.push(`${column.name} must be valid date`)
259-
}
260-
break
261-
case 'json':
262-
try {
263-
JSON.stringify(value)
264-
} catch {
265-
errors.push(`${column.name} must be valid JSON`)
266-
}
267-
break
268-
case 'select': {
269-
const ids = optionIds(column)
270-
if (column.multiple) {
271-
if (!Array.isArray(value)) {
272-
errors.push(`${column.name} must be a list of options`)
273-
} else if (!value.every((v) => typeof v === 'string' && ids.has(v))) {
274-
errors.push(`${column.name} must only contain defined options`)
275-
} else if (column.required && value.length === 0) {
276-
errors.push(`Missing required field: ${column.name}`)
277-
}
278-
} else if (typeof value !== 'string' || !ids.has(value)) {
279-
errors.push(`${column.name} must be one of the defined options`)
280-
}
281-
break
282-
}
283-
}
241+
const error = isValidColumnValue(value, column)
242+
if (error) errors.push(error)
284243
}
285244

286245
return { valid: errors.length === 0, errors }
287246
}
288247

289-
/** Set of valid option ids for a `select`/`multiselect` column. */
290-
function optionIds(column: ColumnDefinition): Set<string> {
291-
return new Set((column.options ?? []).map((o) => o.id))
292-
}
293-
294-
/**
295-
* Resolves a raw cell value to a declared option id, accepting either the
296-
* stable id or (tolerant for tool/import writes) the option's display name.
297-
* Returns null when no option matches. Exported so the column-type-conversion
298-
* path can gate a `select`/`multiselect` change on whether existing values
299-
* actually fit the target option set.
300-
*/
301-
export function resolveSelectOptionId(value: JsonValue, options: SelectOption[]): string | null {
302-
// The block builder serializes without schema access, so an option NAME that
303-
// looks numeric or boolean ("123", "true") arrives scalar-coerced. Stringify
304-
// scalars so the name still resolves; arrays/objects stay unresolvable.
305-
const text =
306-
typeof value === 'string'
307-
? value
308-
: typeof value === 'number' || typeof value === 'boolean'
309-
? String(value)
310-
: null
311-
if (text === null) return null
312-
const byId = options.find((o) => o.id === text)
313-
if (byId) return byId.id
314-
const byName =
315-
options.find((o) => o.name === text) ??
316-
options.find((o) => o.name.toLowerCase() === text.toLowerCase())
317-
return byName ? byName.id : null
318-
}
319-
320-
/**
321-
* Splits a raw value into the parts a multi-select cell should resolve. A cell
322-
* may arrive as an array (canonical) or as a single comma-delimited string —
323-
* the shape a multi cell exports, copies, and converts to text as — so both the
324-
* write-path coercion and the column-conversion compatibility check read it
325-
* through here rather than each deciding for itself. Option names that
326-
* themselves contain commas are an accepted ambiguity.
327-
*/
328-
export function splitMultiSelectInput(value: JsonValue): JsonValue[] {
329-
if (Array.isArray(value)) return value
330-
if (typeof value !== 'string') return [value]
331-
return value
332-
.split(',')
333-
.map((part) => part.trim())
334-
.filter((part) => part !== '')
335-
}
336-
337248
/**
338249
* Attempts to coerce a non-null value to a column's declared type. Returns the
339250
* coerced value when the value already matches or can be converted without
340251
* ambiguity (e.g. the string `"1999"` to the number `1999`), and `ok: false`
341-
* when no safe conversion exists.
252+
* when no safe conversion exists. Delegates to the column-type registry
253+
* (`column-types.ts`) so a new column type's parse rule only needs to be
254+
* added in one place — see `ColumnTypeDefinition.parse`.
342255
*/
343256
function coerceValueToColumnType(
344257
value: JsonValue,
345258
column: ColumnDefinition
346259
): { ok: true; value: JsonValue } | { ok: false } {
347-
switch (column.type) {
348-
case 'string':
349-
if (typeof value === 'string') return { ok: true, value }
350-
if (typeof value === 'number' || typeof value === 'boolean') {
351-
return { ok: true, value: String(value) }
352-
}
353-
return { ok: false }
354-
case 'number':
355-
if (typeof value === 'number') {
356-
return Number.isFinite(value) ? { ok: true, value } : { ok: false }
357-
}
358-
if (typeof value === 'string' && value.trim() !== '') {
359-
const parsed = Number(value)
360-
return Number.isFinite(parsed) ? { ok: true, value: parsed } : { ok: false }
361-
}
362-
return { ok: false }
363-
case 'boolean':
364-
if (typeof value === 'boolean') return { ok: true, value }
365-
if (typeof value === 'string') {
366-
const normalized = value.trim().toLowerCase()
367-
if (normalized === 'true') return { ok: true, value: true }
368-
if (normalized === 'false') return { ok: true, value: false }
369-
}
370-
return { ok: false }
371-
case 'date': {
372-
if (typeof value === 'string') {
373-
const normalized = normalizeDateCellValue(value)
374-
return normalized === null ? { ok: false } : { ok: true, value: normalized }
375-
}
376-
// Date instances and epoch numbers may still be out of the representable
377-
// range (>±8.64e15ms) — guard `toISOString()`, which throws RangeError on
378-
// an Invalid Date, so an over-range value degrades to `{ ok: false }`
379-
// rather than crashing the write.
380-
const date =
381-
value instanceof Date ? value : typeof value === 'number' ? new Date(value) : null
382-
if (date && !Number.isNaN(date.getTime())) return { ok: true, value: date.toISOString() }
383-
return { ok: false }
384-
}
385-
case 'select': {
386-
const options = column.options ?? []
387-
if (column.multiple) {
388-
const raw = splitMultiSelectInput(value)
389-
const ids: string[] = []
390-
for (const entry of raw) {
391-
const id = resolveSelectOptionId(entry, options)
392-
if (id !== null && !ids.includes(id)) ids.push(id)
393-
}
394-
return { ok: true, value: ids }
395-
}
396-
// Single: tolerate an array (e.g. right after a multiple→single toggle) by
397-
// resolving its first element so the value isn't dropped wholesale.
398-
const single = Array.isArray(value) ? value[0] : value
399-
const id = single === undefined ? null : resolveSelectOptionId(single, options)
400-
return id !== null ? { ok: true, value: id } : { ok: false }
401-
}
402-
default:
403-
return { ok: true, value }
404-
}
260+
return parseColumnValue(value, column)
405261
}
406262

407263
/**

0 commit comments

Comments
 (0)