Skip to content

Commit f4845cc

Browse files
j15zclaude
j15z
and
claude
authored
fix(tables): rework lock settings as Table Security and gate locked actions (#7853)
* fix(tables): block schema-locked column edits and rework lock settings as Table Security * fix(tables): read-only cell editors and add-row form for update-locked tables - Update-locked tables open cell editors read-only; the expanded editor disables Save with the reason on hover, and empty cells that can't be edited open nothing - New row opens the Add Row form when only updates are locked - Row form reads and writes values by column id and uses one date-and-time picker - ChipDatePicker follows InsideModalContext so its calendar is clickable in modals, and gains showTime/timeLabel in single mode * fix(tables): open the add-row form for required columns and gate Save on required fields - New row, Shift+Enter, and Insert row open the Add Row form at their position when updates are locked or any column is required - Add Row and Update Row stay disabled until every required field has a value - Add mode accepts an insert position; Shift+Enter anchors to the neighbor row id * fix(tables): explain denied actions where the user meets them Clicking a column header opened the full column editor on a schema-locked table: every field was editable and Save only failed once the server refused it. The header click is the primary way into that panel, so it now opens read-only — values stay readable and selectable, a disabled `<fieldset>` makes the controls inert, and Save carries the lock reason. Clicking a checkbox cell on an update-locked table did nothing at all, while the keyboard paths explained themselves; it now raises the same notice. The column menu disabled only "Edit column" while "Insert column left/right" and "Delete column" stayed live and explained the lock after the click. All four are disabled now, each with a tooltip. A disabled `DropdownMenuItem` sets `pointer-events: none`, so the tooltip wraps the row rather than the item. "Hide column" is untouched: hiding a workflow output is a metadata change no lock covers. Notices now speak the modal's Allow/Deny vocabulary and name the row that denies the action, and the tooltip strings live in `lock-copy` instead of being written out at each call site. Drops the "This table is append-only" copy, which no path could reach once New row and Shift+Enter started opening the add-row form. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(tables): write only what the row form changed, and report failures once The add/edit row form sent every column on every save. An untouched empty column was written as `null`, so a no-op edit still bumped the row, and an insert filled in nulls for columns the user never opened. It now sends only the fields the user touched, and in edit mode only those whose value actually differs — a save with nothing changed closes without a write. Checkboxes stay the exception on insert: they always carry a concrete boolean, so a required one the user never clicked still reaches the server as `false`. A rejected write also arrived twice: the modal rendered the message inline and the mutation toasted the same sentence. Row mutations take an opt-in `suppressErrorToast` so the form owns its own failure; the cache self-heal on a 423 still runs, only its toast is dropped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(tables): drop the Table Security switch and its device-local store "Enable Table Security" had no server representation: enabled-with-everything- allowed and never-configured both save four `false` flags, so the difference lived only in the browser that set it. Anyone else — another device, another admin — saw the table as unconfigured, and the per-action choices behind the switch were remembered per device too. The modal now always shows the four Allow/Deny rows, mapped one-to-one onto the server flags, so an unconfigured table opens on four `Allow`s and every viewer sees the same state. That removes the reason for the preference store, which is deleted along with its helpers and test. Stale `table-security-preferences` keys are left where they are; nothing reads them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(tables): stage only the Table Security rows an admin moves The modal reset its draft only when it opened, so a lock another admin changed while it sat open went stale behind it: the controls kept rendering the old values and Save submitted all four flags, overwriting the newer state. Only the rows this admin moves are staged now. Every other row keeps rendering the authoritative value, so a concurrent change shows up in the open modal instead of hiding behind it, and Save sends just that patch — the route already takes a partial — so an untouched row can't carry a stale flag over someone else's change. A row both admins moved is the one real conflict, and there the explicit choice wins. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 21291d7 commit f4845cc

23 files changed

Lines changed: 1481 additions & 439 deletions

File tree

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

Lines changed: 142 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,17 @@
11
'use client'
22

33
import { useState } from 'react'
4-
import { Button, ChipCombobox, ChipInput, cn, FieldDivider, Label, Switch, toast } from '@sim/emcn'
4+
import {
5+
Button,
6+
ChipCombobox,
7+
ChipInput,
8+
cn,
9+
FieldDivider,
10+
Label,
11+
Switch,
12+
Tooltip,
13+
toast,
14+
} from '@sim/emcn'
515
import { X } from '@sim/emcn/icons'
616
import { toError } from '@sim/utils/errors'
717
import { findValidationIssue, isValidationError } from '@/lib/api/client/errors'
@@ -59,6 +69,15 @@ interface ColumnConfigSidebarProps {
5969
/** Notify parent of a rename so it can rewrite local `columnOrder` /
6070
* `columnWidths` keys that reference the old name. */
6171
onColumnRename?: (oldName: string, newName: string) => void
72+
/**
73+
* Opens the panel for reading only — every field is inert and Save is
74+
* disabled behind {@link readOnlyReason}. The header click that opens this
75+
* sidebar is a primary affordance, so a schema-locked (or read-only) table
76+
* shows the column's settings rather than swallowing the click.
77+
*/
78+
readOnly?: boolean
79+
/** Why saving is unavailable; surfaced on the disabled Save button. */
80+
readOnlyReason?: string
6281
}
6382

6483
/**
@@ -109,6 +128,8 @@ function ColumnConfigBody({
109128
workspaceId,
110129
tableId,
111130
onColumnRename,
131+
readOnly,
132+
readOnlyReason,
112133
}: ColumnConfigBodyProps) {
113134
const updateColumn = useUpdateColumn({ workspaceId, tableId })
114135
const addColumn = useAddTableColumn({ workspaceId, tableId })
@@ -154,6 +175,8 @@ function ColumnConfigBody({
154175
}
155176

156177
async function handleSave() {
178+
// Belt and braces: the button is disabled, and the server refuses too.
179+
if (readOnly) return
157180
if (!trimmedName) {
158181
setShowValidation(true)
159182
return
@@ -254,118 +277,136 @@ function ColumnConfigBody({
254277
</div>
255278

256279
<div className='flex-1 overflow-y-auto overflow-x-hidden px-2 pt-3 pb-2 [overflow-anchor:none]'>
257-
<div className='flex flex-col gap-[9.5px]'>
258-
<RequiredLabel htmlFor='column-sidebar-name'>Column name</RequiredLabel>
259-
<ChipInput
260-
id='column-sidebar-name'
261-
value={nameInput}
262-
onChange={(e) => {
263-
setNameInput(e.target.value)
264-
if (nameError) setNameError(null)
265-
}}
266-
spellCheck={false}
267-
autoComplete='off'
268-
error={Boolean((showValidation && !trimmedName) || nameError)}
269-
aria-invalid={(showValidation && !trimmedName) || nameError ? true : undefined}
270-
/>
271-
{showValidation && !trimmedName && <FieldError message='Column name is required' />}
272-
{nameError && !(showValidation && !trimmedName) && <FieldError message={nameError} />}
273-
</div>
274-
275-
{config.mode === 'edit' && (
276-
<>
277-
<FieldDivider />
278-
<div className='flex flex-col gap-[9.5px]'>
279-
<RequiredLabel>Type</RequiredLabel>
280-
<ChipCombobox
281-
options={columnTypeOptionsForTable(allColumns, existingColumn, {
282-
tableRowTtlEnabled,
283-
})
284-
.filter((option) => option.type !== 'workflow')
285-
.map((option) => ({
286-
label: option.label,
287-
value: option.type,
288-
icon: option.icon,
289-
disabled: option.disabledReason !== undefined,
290-
}))}
291-
value={typeInput}
292-
onChange={(v) => setTypeInput(v as ColumnDefinition['type'])}
293-
placeholder='Select type'
294-
maxHeight={300}
295-
/>
296-
</div>
297-
</>
298-
)}
280+
{/* `disabled` on the fieldset reaches every native control inside,
281+
including the comboboxes' trigger buttons; `contents` keeps the
282+
existing layout. Values stay readable and selectable. */}
283+
<fieldset disabled={readOnly} className='contents'>
284+
<div className='flex flex-col gap-[9.5px]'>
285+
<RequiredLabel htmlFor='column-sidebar-name'>Column name</RequiredLabel>
286+
<ChipInput
287+
id='column-sidebar-name'
288+
value={nameInput}
289+
onChange={(e) => {
290+
setNameInput(e.target.value)
291+
if (nameError) setNameError(null)
292+
}}
293+
spellCheck={false}
294+
autoComplete='off'
295+
error={Boolean((showValidation && !trimmedName) || nameError)}
296+
aria-invalid={(showValidation && !trimmedName) || nameError ? true : undefined}
297+
/>
298+
{showValidation && !trimmedName && <FieldError message='Column name is required' />}
299+
{nameError && !(showValidation && !trimmedName) && <FieldError message={nameError} />}
300+
</div>
299301

300-
{wantsCurrency && (
301-
<>
302-
<FieldDivider />
303-
<div className='flex flex-col gap-[9.5px]'>
304-
<RequiredLabel>Currency</RequiredLabel>
305-
<ChipCombobox
306-
options={CURRENCY_COMBOBOX_OPTIONS}
307-
value={currencyInput}
308-
onChange={setCurrencyInput}
309-
placeholder='Select currency'
310-
searchable
311-
searchPlaceholder='Search currencies'
312-
maxHeight={260}
313-
/>
314-
</div>
315-
</>
316-
)}
302+
{config.mode === 'edit' && (
303+
<>
304+
<FieldDivider />
305+
<div className='flex flex-col gap-[9.5px]'>
306+
<RequiredLabel>Type</RequiredLabel>
307+
<ChipCombobox
308+
options={columnTypeOptionsForTable(allColumns, existingColumn, {
309+
tableRowTtlEnabled,
310+
})
311+
.filter((option) => option.type !== 'workflow')
312+
.map((option) => ({
313+
label: option.label,
314+
value: option.type,
315+
icon: option.icon,
316+
disabled: option.disabledReason !== undefined,
317+
}))}
318+
value={typeInput}
319+
onChange={(v) => setTypeInput(v as ColumnDefinition['type'])}
320+
placeholder='Select type'
321+
maxHeight={300}
322+
/>
323+
</div>
324+
</>
325+
)}
317326

318-
{wantsOptions && (
319-
<>
320-
<FieldDivider />
321-
<div className='flex flex-col gap-[9.5px]'>
322-
<RequiredLabel>Options</RequiredLabel>
323-
<SelectOptionsEditor
324-
options={optionsInput}
325-
onChange={(next) => {
326-
setOptionsInput(next)
327-
if (optionsError) setOptionsError(null)
328-
}}
329-
/>
330-
{optionsError && <FieldError message={optionsError} />}
331-
</div>
332-
<FieldDivider />
333-
<div className='flex items-center justify-between pl-0.5'>
334-
<Label htmlFor='column-sidebar-multiple'>Multiselect</Label>
335-
<Switch
336-
id='column-sidebar-multiple'
337-
checked={multipleInput}
338-
onCheckedChange={(v) => setMultipleInput(!!v)}
339-
/>
340-
</div>
341-
</>
342-
)}
327+
{wantsCurrency && (
328+
<>
329+
<FieldDivider />
330+
<div className='flex flex-col gap-[9.5px]'>
331+
<RequiredLabel>Currency</RequiredLabel>
332+
<ChipCombobox
333+
options={CURRENCY_COMBOBOX_OPTIONS}
334+
value={currencyInput}
335+
onChange={setCurrencyInput}
336+
placeholder='Select currency'
337+
searchable
338+
searchPlaceholder='Search currencies'
339+
maxHeight={260}
340+
/>
341+
</div>
342+
</>
343+
)}
343344

344-
{/* Select columns don't expose a unique constraint. */}
345-
{!wantsOptions && (
346-
<>
347-
<FieldDivider />
348-
<div className='flex flex-col gap-[9.5px]'>
345+
{wantsOptions && (
346+
<>
347+
<FieldDivider />
348+
<div className='flex flex-col gap-[9.5px]'>
349+
<RequiredLabel>Options</RequiredLabel>
350+
<SelectOptionsEditor
351+
options={optionsInput}
352+
onChange={(next) => {
353+
setOptionsInput(next)
354+
if (optionsError) setOptionsError(null)
355+
}}
356+
/>
357+
{optionsError && <FieldError message={optionsError} />}
358+
</div>
359+
<FieldDivider />
349360
<div className='flex items-center justify-between pl-0.5'>
350-
<Label htmlFor='column-sidebar-unique'>Unique</Label>
361+
<Label htmlFor='column-sidebar-multiple'>Multiselect</Label>
351362
<Switch
352-
id='column-sidebar-unique'
353-
checked={uniqueInput}
354-
onCheckedChange={(v) => setUniqueInput(!!v)}
363+
id='column-sidebar-multiple'
364+
checked={multipleInput}
365+
onCheckedChange={(v) => setMultipleInput(!!v)}
355366
/>
356367
</div>
357-
</div>
358-
</>
359-
)}
368+
</>
369+
)}
370+
371+
{/* Select columns don't expose a unique constraint. */}
372+
{!wantsOptions && (
373+
<>
374+
<FieldDivider />
375+
<div className='flex flex-col gap-[9.5px]'>
376+
<div className='flex items-center justify-between pl-0.5'>
377+
<Label htmlFor='column-sidebar-unique'>Unique</Label>
378+
<Switch
379+
id='column-sidebar-unique'
380+
checked={uniqueInput}
381+
onCheckedChange={(v) => setUniqueInput(!!v)}
382+
/>
383+
</div>
384+
</div>
385+
</>
386+
)}
387+
</fieldset>
360388
</div>
361389

362390
<div className='flex items-center justify-end gap-2 border-[var(--border)] border-t px-2 py-3'>
363391
<Button variant='default' size='sm' onClick={onClose}>
364-
Cancel
365-
</Button>
366-
<Button variant='primary' size='sm' onClick={handleSave} disabled={saveDisabled}>
367-
{saveDisabled ? 'Saving…' : 'Save'}
392+
{readOnly ? 'Close' : 'Cancel'}
368393
</Button>
394+
{readOnly ? (
395+
<Tooltip.Root>
396+
<Tooltip.Trigger asChild>
397+
<span className='inline-flex'>
398+
<Button variant='primary' size='sm' disabled>
399+
Save
400+
</Button>
401+
</span>
402+
</Tooltip.Trigger>
403+
{readOnlyReason && <Tooltip.Content>{readOnlyReason}</Tooltip.Content>}
404+
</Tooltip.Root>
405+
) : (
406+
<Button variant='primary' size='sm' onClick={handleSave} disabled={saveDisabled}>
407+
{saveDisabled ? 'Saving…' : 'Save'}
408+
</Button>
409+
)}
369410
</div>
370411
</div>
371412
)

‎apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/column-dropdown/column-dropdown.test.tsx‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,36 @@ afterEach(() => {
2323
})
2424

2525
describe('ColumnDropdown', () => {
26+
it('keeps a schema-locked trigger focusable for its explanation without opening a menu', () => {
27+
const onPickType = vi.fn()
28+
act(() => {
29+
root.render(
30+
<ColumnDropdown
31+
columns={[]}
32+
tableRowTtlEnabled
33+
trigger='header'
34+
disabled={false}
35+
blocked
36+
onPickType={onPickType}
37+
onPickWorkflow={vi.fn()}
38+
onPickEnrichment={vi.fn()}
39+
/>
40+
)
41+
})
42+
const trigger = container.querySelector<HTMLButtonElement>('button')!
43+
expect(trigger.getAttribute('aria-disabled')).toBe('true')
44+
expect(trigger.disabled).toBe(false)
45+
act(() => {
46+
trigger.focus()
47+
trigger.click()
48+
})
49+
expect(document.querySelector('[role="tooltip"]')?.textContent).toContain(
50+
'Changing the table schema is disabled in Table Security.'
51+
)
52+
expect(document.querySelector('[role="menu"]')).toBeNull()
53+
expect(onPickType).not.toHaveBeenCalled()
54+
})
55+
2656
it('lists Enrichments as a regular entry after the column options', () => {
2757
const onPickEnrichment = vi.fn()
2858

@@ -37,7 +67,6 @@ describe('ColumnDropdown', () => {
3767
onPickWorkflow={vi.fn()}
3868
onPickEnrichment={onPickEnrichment}
3969
blocked={false}
40-
onBlocked={vi.fn()}
4170
/>
4271
)
4372
})

0 commit comments

Comments
 (0)