Skip to content

Commit caaf511

Browse files
committed
fix(tables): address audit findings across the column-type change
Range filters were the live bug. `buildComparisonClause` fell through to `?? 'numeric'` for every type whose `jsonbCast` is null but whose id is not literally 'string', so `>=` on an email/phone/url column emitted `(data->>'email')::numeric` — a Postgres error on the first non-numeric row, 500ing the whole rows query. Types now declare `orderable`, and a null cast means text comparison for every type rather than only for `string`. Date truncation was unsafe twice over. `.slice(0, 10)` assumed a four-digit year, but the canonical form does not pad it: `0001-01-01T00:00:00Z` (.NET's DateTime.MinValue, routine in exported CSVs) normalized to `1-01-01T00:00:00Z` and sliced to `1-01-01T00`, which coerce accepted and validateCell then rejected. Years are padded at the source and truncation is anchored on a `YYYY-MM-DD` match. The set-based migration gained the same guard: legacy columns can hold unparseable strings like `March 5, 2024`, and `left(…, 10)` would have rewritten them to `March 5, 2` irreversibly. Precision no longer defaults. Stamping 0 rounded every new percent column in the grid AND in CSV export (12.5 exporting as 13%), and rode back onto a number column through a number → percent → number round-trip. Absent keeps meaning "render as stored" for both owners. Also: url read `example.com:8080`'s host as a scheme and nulled the cell; duration rewrote 90.7 to 91 when an editor merely opened and closed; phone turned a negative number positive; the color picker put raw buttons inside a role=menu, so it never closed on select and had no keyboard navigation; `updateColumnMetadata` could let defaultMetadata introduce a key the caller never sent, wiping times off a legacy date column. Consolidation: the v2 predicate grammar now asks the registry for its operator allowlist (it had a private copy, so a restricted type was gated on one wire shape and not the other); sort-by-label, filter value coercion, filter pruning, the filter UI, and the grid's dirty check read `storesOpaqueIds` / `storesMultipleValues` instead of naming `select`; the expanded popover formats every type through `formatForDisplay` rather than only date and currency; `formatValueForInput` takes the column instead of synthesizing one and dropping its metadata.
1 parent d08eebc commit caaf511

37 files changed

Lines changed: 621 additions & 201 deletions

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

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,15 @@ function ColumnConfigBody({
131131
? resolveCurrencyCode(existingColumn?.currencyCode)
132132
: DEFAULT_CURRENCY_CODE
133133
)
134-
const [precisionInput, setPrecisionInput] = useState<number>(() =>
135-
clampPrecision(existingColumn?.precision)
134+
// The RAW string, not a number. A numeric state round-tripped through
135+
// `Number()` on every keystroke cannot be cleared (`'' → 0`) and clamps
136+
// mid-typing (`1`, then `2` → `10`, never `12`). Parsing happens on commit,
137+
// matching `usage-limit-field`. Empty means "no precision declared", which is
138+
// what keeps a column rendering its values as stored.
139+
const [precisionInput, setPrecisionInput] = useState<string>(() =>
140+
config.mode === 'edit' && existingColumn?.precision !== undefined
141+
? String(existingColumn.precision)
142+
: ''
136143
)
137144
const [includeTimeInput, setIncludeTimeInput] = useState<boolean>(() =>
138145
// Absent means a column created before the key existed, and those hold
@@ -151,6 +158,10 @@ function ColumnConfigBody({
151158
// type that loses it cannot leave a stale control behind.
152159
const wantsCurrency = typeOwnsMetadataKey(typeInput, 'currencyCode')
153160
const wantsPrecision = typeOwnsMetadataKey(typeInput, 'precision')
161+
// `undefined` (blank field) is meaningful: it leaves the column rendering
162+
// values exactly as stored rather than forcing a decimal count.
163+
const parsedPrecision =
164+
precisionInput.trim() === '' ? undefined : clampPrecision(Number(precisionInput))
154165
const wantsIncludeTime = typeOwnsMetadataKey(typeInput, 'includeTime')
155166
const trimmedOptions = optionsInput.map((o) => ({ ...o, name: o.name.trim() }))
156167

@@ -186,7 +197,9 @@ function ColumnConfigBody({
186197
...(wantsOptions ? { options: trimmedOptions } : {}),
187198
...(wantsOptions && multipleInput ? { multiple: true } : {}),
188199
...(wantsCurrency ? { currencyCode: currencyInput } : {}),
189-
...(wantsPrecision ? { precision: precisionInput } : {}),
200+
...(wantsPrecision && parsedPrecision !== undefined
201+
? { precision: parsedPrecision }
202+
: {}),
190203
...(wantsIncludeTime ? { includeTime: includeTimeInput } : {}),
191204
})
192205
toast.success(`Added "${trimmedName}"`)
@@ -208,8 +221,7 @@ function ColumnConfigBody({
208221
const multipleChanged = wantsOptions && !!existingColumn?.multiple !== multipleInput
209222
const currencyChanged =
210223
wantsCurrency && resolveCurrencyCode(existingColumn?.currencyCode) !== currencyInput
211-
const precisionChanged =
212-
wantsPrecision && clampPrecision(existingColumn?.precision) !== precisionInput
224+
const precisionChanged = wantsPrecision && existingColumn?.precision !== parsedPrecision
213225
const includeTimeChanged =
214226
wantsIncludeTime && (existingColumn?.includeTime !== false) !== includeTimeInput
215227

@@ -232,8 +244,8 @@ function ColumnConfigBody({
232244
...(wantsCurrency && (typeChanged || currencyChanged)
233245
? { currencyCode: currencyInput }
234246
: {}),
235-
...(wantsPrecision && (typeChanged || precisionChanged)
236-
? { precision: precisionInput }
247+
...(wantsPrecision && (typeChanged || precisionChanged) && parsedPrecision !== undefined
248+
? { precision: parsedPrecision }
237249
: {}),
238250
...(wantsIncludeTime && (typeChanged || includeTimeChanged)
239251
? { includeTime: includeTimeInput }
@@ -339,14 +351,17 @@ function ColumnConfigBody({
339351
<>
340352
<FieldDivider />
341353
<div className='flex flex-col gap-[9.5px]'>
342-
<RequiredLabel>Decimal places</RequiredLabel>
354+
<RequiredLabel htmlFor='column-sidebar-precision'>Decimal places</RequiredLabel>
343355
<ChipInput
356+
id='column-sidebar-precision'
344357
type='number'
345358
inputMode='numeric'
346359
min={DEFAULT_PRECISION.min}
347360
max={DEFAULT_PRECISION.max}
348-
value={String(precisionInput)}
349-
onChange={(e) => setPrecisionInput(clampPrecision(Number(e.target.value)))}
361+
value={precisionInput}
362+
onChange={(e) => setPrecisionInput(e.target.value)}
363+
placeholder='As stored'
364+
inputClassName='[appearance:textfield] [&::-webkit-inner-spin-button]:appearance-none [&::-webkit-outer-spin-button]:appearance-none'
350365
/>
351366
</div>
352367
</>

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/row-modal/row-modal.tsx

Lines changed: 31 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,7 @@ import { createLogger } from '@sim/logger'
1818
import { getErrorMessage } from '@sim/utils/errors'
1919
import { useParams } from 'next/navigation'
2020
import type { ColumnDefinition, TableInfo, TableRow } from '@/lib/table'
21-
import { columnTypeOf } from '@/lib/table/column-types'
22-
import { resolveCurrencyCode } from '@/lib/table/currency'
21+
import { columnTypeOf, describeColumnType } from '@/lib/table/column-types'
2322
import { useTimezone } from '@/hooks/queries/general-settings'
2423
import { useDeleteTableRow, useDeleteTableRows, useUpdateTableRow } from '@/hooks/queries/tables'
2524
import {
@@ -211,13 +210,11 @@ function ColumnField({ column, value, onChange }: ColumnFieldProps) {
211210
)}
212211
</>
213212
)
214-
// Currency names its code — the modal edits the bare amount, so without it
215-
// there is nothing on screen saying which currency the number is in.
216-
const typeLabel =
217-
column.type === 'currency'
218-
? `currency (${resolveCurrencyCode(column.currencyCode)})`
219-
: column.type
220-
const hint = `Type: ${typeLabel}${column.required ? '' : ' (optional)'}`
213+
// The type's own description, which folds in the configuration that changes
214+
// what a cell means — a currency's code, a date's time, a number's decimals.
215+
// This also reads the registry's LABEL rather than the raw id, so the hint
216+
// says "Text" and not "string".
217+
const hint = `Type: ${describeColumnType(column)}${column.required ? '' : ' (optional)'}`
221218
const definition = columnTypeOf(column)
222219

223220
if (definition.editor === 'toggle') {
@@ -251,7 +248,7 @@ function ColumnField({ column, value, onChange }: ColumnFieldProps) {
251248
required={column.required}
252249
hint={hint}
253250
mono
254-
value={formatValueForInput(value, column.type)}
251+
value={formatValueForInput(value, column)}
255252
onChange={onChange}
256253
placeholder='{"key": "value"}'
257254
rows={4}
@@ -260,7 +257,12 @@ function ColumnField({ column, value, onChange }: ColumnFieldProps) {
260257
}
261258

262259
if (definition.editor === 'date') {
263-
const parts = dateValueToLocalParts(formatValueForInput(value, 'date'))
260+
const parts = dateValueToLocalParts(formatValueForInput(value, column))
261+
// A date-only column must not offer a time the write path is going to
262+
// truncate. `includeTime` absent means a column predating the key, which
263+
// still stores instants — so only an explicit false drops the picker,
264+
// matching `date.coerce`.
265+
const withTime = column.includeTime !== false
264266
return (
265267
<ChipModalField type='custom' title={title} required={column.required} hint={hint}>
266268
<div className='flex items-center gap-2'>
@@ -272,17 +274,23 @@ function ColumnField({ column, value, onChange }: ColumnFieldProps) {
272274
flush
273275
className='flex-1'
274276
/>
275-
<ChipTimePicker
276-
value={parts.time?.slice(0, 5)}
277-
onChange={(time) =>
278-
onChange(
279-
localPartsToDateValue(parts.day ?? todayLocalCalendarDate(timeZone), time, timeZone)
280-
)
281-
}
282-
placeholder='Add time'
283-
flush
284-
className='w-[110px]'
285-
/>
277+
{withTime && (
278+
<ChipTimePicker
279+
value={parts.time?.slice(0, 5)}
280+
onChange={(time) =>
281+
onChange(
282+
localPartsToDateValue(
283+
parts.day ?? todayLocalCalendarDate(timeZone),
284+
time,
285+
timeZone
286+
)
287+
)
288+
}
289+
placeholder='Add time'
290+
flush
291+
className='w-[110px]'
292+
/>
293+
)}
286294
</div>
287295
</ChipModalField>
288296
)
@@ -308,7 +316,7 @@ function ColumnField({ column, value, onChange }: ColumnFieldProps) {
308316
inputType={
309317
definition.inputMode === 'decimal' && !definition.acceptsFormattedInput ? 'number' : 'text'
310318
}
311-
value={formatValueForInput(value, column.type)}
319+
value={formatValueForInput(value, column)}
312320
onChange={onChange}
313321
placeholder={`Enter ${column.name}`}
314322
/>

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/select-field/select-color-picker.tsx

Lines changed: 42 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
11
'use client'
22

3-
import { Badge, DropdownMenu, DropdownMenuContent, DropdownMenuTrigger } from '@sim/emcn'
3+
import {
4+
Badge,
5+
DropdownMenu,
6+
DropdownMenuContent,
7+
DropdownMenuRadioGroup,
8+
DropdownMenuRadioItem,
9+
DropdownMenuTrigger,
10+
} from '@sim/emcn'
411
import { SELECT_OPTION_COLORS, type SelectOptionColor } from '@/lib/table'
512

613
interface SelectColorPickerProps {
@@ -10,38 +17,50 @@ interface SelectColorPickerProps {
1017
optionName: string
1118
}
1219

20+
/** Sentence-cased for display; the stored value stays the lowercase token. */
21+
function labelFor(color: SelectOptionColor): string {
22+
return color.charAt(0).toUpperCase() + color.slice(1)
23+
}
24+
1325
/**
14-
* Swatch dropdown for one option's pill color.
26+
* Colour picker for one option's pill.
27+
*
28+
* A named list rather than a bare swatch grid, for two reasons. Colour is then
29+
* not the only signal — the name is readable when the swatches are not
30+
* distinguishable to the viewer — and it lets the menu use the real
31+
* `DropdownMenuRadioItem` primitive: mutually exclusive `menuitemradio`
32+
* semantics, roving focus and typeahead, a visible selected indicator, and
33+
* close-on-select. Raw `<button>`s inside a `role="menu"` get none of that, and
34+
* notably leave the menu open after a pick.
1535
*
16-
* Each swatch is a real `Badge` in the variant it selects, so the menu shows the
17-
* exact chrome the pill will have in both themes rather than an approximation
18-
* of it — the badge stays the single owner of its colors.
36+
* Each swatch is a real `Badge` in the variant it selects, so the menu shows
37+
* the exact chrome the pill will have in both themes — the badge stays the one
38+
* owner of its colours.
1939
*/
2040
export function SelectColorPicker({ color, onChange, optionName }: SelectColorPickerProps) {
2141
const current = color ?? 'gray'
2242
return (
2343
<DropdownMenu>
2444
<DropdownMenuTrigger
25-
className='size-7 shrink-0 rounded-md p-1'
26-
aria-label={`Color for ${optionName || 'option'}`}
45+
className='flex size-7 shrink-0 items-center justify-center'
46+
aria-label={`Color for ${optionName || 'option'}: ${labelFor(current)}`}
2747
>
28-
<Badge variant={current} size='sm' className='size-full justify-center p-0'>
29-
<span className='sr-only'>{current}</span>
30-
</Badge>
48+
<Badge variant={current} size='swatch' />
3149
</DropdownMenuTrigger>
32-
<DropdownMenuContent align='end' className='grid grid-cols-5 gap-1 p-1.5'>
33-
{SELECT_OPTION_COLORS.map((swatch) => (
34-
<button
35-
key={swatch}
36-
type='button'
37-
onClick={() => onChange(swatch)}
38-
aria-label={swatch}
39-
aria-pressed={swatch === current}
40-
className='rounded-md p-0.5 transition-colors hover-hover:bg-[var(--surface-5)]'
41-
>
42-
<Badge variant={swatch} size='sm' className='size-5 justify-center p-0' />
43-
</button>
44-
))}
50+
<DropdownMenuContent align='start' className='min-w-[160px]'>
51+
<DropdownMenuRadioGroup
52+
value={current}
53+
onValueChange={(next) => onChange(next as SelectOptionColor)}
54+
>
55+
{SELECT_OPTION_COLORS.map((swatch) => (
56+
<DropdownMenuRadioItem key={swatch} value={swatch}>
57+
<span className='flex items-center gap-2'>
58+
<Badge variant={swatch} size='swatch' />
59+
{labelFor(swatch)}
60+
</span>
61+
</DropdownMenuRadioItem>
62+
))}
63+
</DropdownMenuRadioGroup>
4564
</DropdownMenuContent>
4665
</DropdownMenu>
4766
)

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/select-field/select-options-editor.tsx

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,18 @@ interface SelectOptionsEditorProps {
1919
* option and focus jumps into it so typing flows straight through.
2020
*/
2121
export function SelectOptionsEditor({ options, onChange }: SelectOptionsEditorProps) {
22-
const inputRefs = useRef<Map<string, HTMLInputElement>>(new Map())
22+
// Lazy-init: `useRef(new Map())` allocates a Map on every render and throws
23+
// all but the first away.
24+
const inputRefs = useRef<Map<string, HTMLInputElement> | null>(null)
25+
inputRefs.current ??= new Map()
2326
const trailingRef = useRef<HTMLInputElement>(null)
2427
const [pendingFocusId, setPendingFocusId] = useState<string | null>(null)
2528

2629
// The new row and `pendingFocusId` land in the same commit, so the ref is
2730
// registered by the time this effect runs.
2831
useEffect(() => {
2932
if (!pendingFocusId) return
30-
const el = inputRefs.current.get(pendingFocusId)
33+
const el = inputRefs.current?.get(pendingFocusId)
3134
if (el) {
3235
el.focus()
3336
const end = el.value.length
@@ -41,7 +44,7 @@ export function SelectOptionsEditor({ options, onChange }: SelectOptionsEditorPr
4144
}
4245

4346
const remove = (id: string) => {
44-
inputRefs.current.delete(id)
47+
inputRefs.current?.delete(id)
4548
onChange(options.filter((o) => o.id !== id))
4649
}
4750

@@ -70,8 +73,8 @@ export function SelectOptionsEditor({ options, onChange }: SelectOptionsEditorPr
7073
/>
7174
<ChipInput
7275
ref={(el) => {
73-
if (el) inputRefs.current.set(option.id, el)
74-
else inputRefs.current.delete(option.id)
76+
if (el) inputRefs.current?.set(option.id, el)
77+
else inputRefs.current?.delete(option.id)
7578
}}
7679
value={option.name}
7780
onChange={(e) => update(option.id, { name: e.target.value })}

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

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { Plus, X } from '@sim/emcn/icons'
66
import { generateShortId } from '@sim/utils/id'
77
import type { ColumnDefinition, FilterRule, TablePredicate } from '@/lib/table'
88
import { getColumnId } from '@/lib/table/column-keys'
9+
import { columnTypeOf, storesMultipleValues } from '@/lib/table/column-types'
910
import {
1011
COMPARISON_OPERATORS,
1112
MULTI_SELECT_FILTER_OPERATORS,
@@ -25,7 +26,19 @@ const MULTI_SELECT_COMPARISON_OPERATORS = COMPARISON_OPERATORS.filter((o) =>
2526
)
2627

2728
function selectFilterOperators(column: ColumnDefinition | undefined): Set<string> {
28-
return column?.multiple ? MULTI_SELECT_FILTER_OPERATORS : SINGLE_SELECT_FILTER_OPERATORS
29+
return column && storesMultipleValues(column)
30+
? MULTI_SELECT_FILTER_OPERATORS
31+
: SINGLE_SELECT_FILTER_OPERATORS
32+
}
33+
34+
/**
35+
* Whether a column's filter value is picked from a declared option set rather
36+
* than typed. Asks the registry, so a future opaque-id type gets the dropdown
37+
* and the restricted operator list instead of a free-text box and the full
38+
* operator menu that the server would then reject.
39+
*/
40+
function picksFromOptions(column: ColumnDefinition | undefined): boolean {
41+
return column ? columnTypeOf(column).storesOpaqueIds : false
2942
}
3043

3144
interface TableFilterProps {
@@ -88,13 +101,13 @@ export function TableFilter({ columns, filter, onApply, onClose }: TableFilterPr
88101
if (r.id !== id) return r
89102
const previous = columnById.get(r.column)
90103
const next = columnById.get(columnId)
91-
const wasSelect = previous?.type === 'select'
92-
const isSelect = next?.type === 'select'
104+
const wasSelect = picksFromOptions(previous)
105+
const isSelect = picksFromOptions(next)
93106
if (!wasSelect && !isSelect) return { ...r, column: columnId }
94107
// Single- and multi-select take different operators, so a switch
95108
// between them has to fall back too, not just select ↔ non-select.
96109
const allowed = selectFilterOperators(next)
97-
const fallback = next?.multiple ? 'contains' : 'eq'
110+
const fallback = next && storesMultipleValues(next) ? 'contains' : 'eq'
98111
const operator = isSelect && !allowed.has(r.operator) ? fallback : r.operator
99112
return { ...r, column: columnId, operator, value: '' }
100113
})
@@ -204,17 +217,17 @@ const FilterRuleRow = memo(function FilterRuleRow({
204217
: columns
205218

206219
const selectedColumn = columnById.get(rule.column)
207-
const isSelect = selectedColumn?.type === 'select'
220+
const isSelect = picksFromOptions(selectedColumn)
208221
const operatorOptions = !isSelect
209222
? COMPARISON_OPERATORS
210-
: selectedColumn?.multiple
223+
: selectedColumn && storesMultipleValues(selectedColumn)
211224
? MULTI_SELECT_COMPARISON_OPERATORS
212225
: SINGLE_SELECT_COMPARISON_OPERATORS
213226

214227
// A stale id (option since deleted) stays selectable so the rule still shows.
215228
const selectValueOptions = isSelect
216229
? (() => {
217-
const opts = (selectedColumn.options ?? []).map((o) => ({ value: o.id, label: o.name }))
230+
const opts = (selectedColumn?.options ?? []).map((o) => ({ value: o.id, label: o.name }))
218231
return rule.value && !opts.some((o) => o.value === rule.value)
219232
? [...opts, { value: rule.value, label: rule.value }]
220233
: opts
@@ -297,8 +310,9 @@ function createRule(columns: ColumnDefinition[]): FilterRule {
297310
id: generateShortId(),
298311
logicalOperator: 'and',
299312
column: first ? getColumnId(first) : '',
300-
// A multi-select can't be compared for equality — default it to membership.
301-
operator: first?.type === 'select' && first.multiple ? 'contains' : 'eq',
313+
// A multi-valued column can't be compared for equality — default it to
314+
// membership.
315+
operator: first && storesMultipleValues(first) ? 'contains' : 'eq',
302316
value: '',
303317
}
304318
}

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,12 @@ export function resolveCellRender({
150150
case 'text':
151151
return { kind: 'text', text: cell.text }
152152
default: {
153+
// Compile-time exhaustiveness gate. At RUNTIME an unrecognised kind must
154+
// degrade to a blank cell — returning the value itself would hand a
155+
// `CellRenderKind` switch a shape it has no arm for.
153156
const _exhaustive: never = cell
154-
return _exhaustive
157+
void _exhaustive
158+
return { kind: 'empty' }
155159
}
156160
}
157161
}

0 commit comments

Comments
 (0)