From f911b7d0059fa35a7b1f8951143d51f7cf9bd9fe Mon Sep 17 00:00:00 2001 From: Justin Blumencranz <96924014+j15z@users.noreply.github.com> Date: Tue, 15 Sep 2026 10:12:56 -0700 Subject: [PATCH 1/7] fix(tables): block schema-locked column edits and rework lock settings as Table Security --- .../column-dropdown/column-dropdown.test.tsx | 31 +++- .../column-dropdown/column-dropdown.tsx | 43 ++--- .../lock-settings-modal.test.tsx | 155 ++++++++++++++++++ .../lock-settings-modal.tsx | 136 ++++++++------- .../table-grid/headers/column-header-menu.tsx | 3 + .../headers/workflow-group-meta-cell.tsx | 7 +- .../components/table-grid/table-grid.tsx | 18 +- .../table-grid/table-primitives.tsx | 49 ++++-- .../tables/[tableId]/lock-copy.ts | 45 ++--- .../[workspaceId]/tables/[tableId]/table.tsx | 8 +- apps/sim/stores/table/security/store.test.ts | 64 ++++++++ apps/sim/stores/table/security/store.ts | 108 ++++++++++++ 12 files changed, 536 insertions(+), 131 deletions(-) create mode 100644 apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/lock-settings-modal/lock-settings-modal.test.tsx create mode 100644 apps/sim/stores/table/security/store.test.ts create mode 100644 apps/sim/stores/table/security/store.ts diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/column-dropdown/column-dropdown.test.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/column-dropdown/column-dropdown.test.tsx index e4321a7eb59..c2e5bcb3886 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/column-dropdown/column-dropdown.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/column-dropdown/column-dropdown.test.tsx @@ -23,6 +23,36 @@ afterEach(() => { }) describe('ColumnDropdown', () => { + it('keeps a schema-locked trigger focusable for its explanation without opening a menu', () => { + const onPickType = vi.fn() + act(() => { + root.render( + + ) + }) + const trigger = container.querySelector('button')! + expect(trigger.getAttribute('aria-disabled')).toBe('true') + expect(trigger.disabled).toBe(false) + act(() => { + trigger.focus() + trigger.click() + }) + expect(document.querySelector('[role="tooltip"]')?.textContent).toContain( + 'Changing the table schema is disabled in Table Security.' + ) + expect(document.querySelector('[role="menu"]')).toBeNull() + expect(onPickType).not.toHaveBeenCalled() + }) + it('lists Enrichments as a regular entry after the column options', () => { const onPickEnrichment = vi.fn() @@ -37,7 +67,6 @@ describe('ColumnDropdown', () => { onPickWorkflow={vi.fn()} onPickEnrichment={onPickEnrichment} blocked={false} - onBlocked={vi.fn()} /> ) }) diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/column-dropdown/column-dropdown.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/column-dropdown/column-dropdown.tsx index 1f4ab32cef4..2481852bfff 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/column-dropdown/column-dropdown.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/column-dropdown/column-dropdown.tsx @@ -10,12 +10,14 @@ import { DropdownMenuContent, DropdownMenuItem, DropdownMenuTrigger, - Plus, Tooltip, } from '@sim/emcn' -import { Sparkles } from '@sim/emcn/icons' +import { Lock, Plus, Sparkles } from '@sim/emcn/icons' import type { ColumnDefinition } from '@/lib/table' -import { type ColumnTypeOption, columnTypeOptionsForTable } from '../column-config-sidebar' +import { + type ColumnTypeOption, + columnTypeOptionsForTable, +} from '@/app/workspace/[workspaceId]/tables/[tableId]/components/column-config-sidebar' const CELL_HEADER = 'border-[var(--border)] border-r border-b bg-[var(--bg)] px-2 py-[7px] text-left align-middle' @@ -30,14 +32,8 @@ interface ColumnDropdownProps { onPickType: (type: ColumnDefinition['type']) => void onPickWorkflow: () => void onPickEnrichment: () => void - /** - * When true, the trigger stays visible and clickable but opens nothing — it - * calls {@link onBlocked} instead. Used when the table is schema-locked: - * hiding the control leaves the user guessing, so it stays and explains. - * Paired required so `blocked` can never be set without a handler. - */ + /** A schema lock disables the action and explains why on hover or focus. */ blocked: boolean - onBlocked: () => void } interface ColumnTypeMenuItemProps { @@ -88,37 +84,46 @@ export function ColumnDropdown({ onPickWorkflow, onPickEnrichment, blocked, - onBlocked, }: ColumnDropdownProps) { + const Icon = blocked ? Lock : Plus const triggerButton = trigger === 'header' ? ( ) : ( ) if (blocked) { + const lockedTrigger = ( + + {triggerButton} + Changing the table schema is disabled in Table Security. + + ) return trigger === 'inline-header' ? ( - {triggerButton} + {lockedTrigger} ) : ( - triggerButton + lockedTrigger ) } diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/lock-settings-modal/lock-settings-modal.test.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/lock-settings-modal/lock-settings-modal.test.tsx new file mode 100644 index 00000000000..6389861fb12 --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/lock-settings-modal/lock-settings-modal.test.tsx @@ -0,0 +1,155 @@ +/** + * @vitest-environment jsdom + */ +import { act } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { type TableLocks, UNLOCKED_TABLE_LOCKS } from '@/lib/table/types' +import { LockSettingsModal } from '@/app/workspace/[workspaceId]/tables/[tableId]/components/lock-settings-modal/lock-settings-modal' +import { useTableSecurityStore } from '@/stores/table/security/store' + +const { mutateAsync } = vi.hoisted(() => ({ mutateAsync: vi.fn() })) +vi.mock('@/hooks/queries/tables', () => ({ + useUpdateTableLocks: () => ({ mutateAsync, isPending: false }), +})) + +const LABELS = ['Inserting Rows', 'Updating Rows', 'Deleting Rows', 'Changing Table Schema'] +let container: HTMLDivElement +let root: Root +const onClose = vi.fn() + +function render(locks: TableLocks = UNLOCKED_TABLE_LOCKS, isOpen = true) { + act(() => { + root.render( + + ) + }) +} + +function getSwitch(label: string): HTMLButtonElement { + const element = document.querySelector( + `button[role="switch"][aria-label="${label}"]` + ) + if (!element) throw new Error(`Missing switch: ${label}`) + return element +} + +function clickSwitch(label: string) { + act(() => getSwitch(label).click()) +} + +function getPermission(label: string, choice: 'Deny' | 'Allow'): HTMLButtonElement { + const group = document.querySelector(`[role="radiogroup"][aria-label="${label}"]`) + const button = [ + ...(group?.querySelectorAll('button[role="radio"]') ?? []), + ].find((element) => element.textContent === choice) + if (!button) throw new Error(`Missing permission: ${label} ${choice}`) + return button +} + +function selectPermission(label: string, choice: 'Deny' | 'Allow') { + act(() => getPermission(label, choice).click()) +} + +function save() { + const button = [...document.querySelectorAll('button')].find( + (element) => element.textContent === 'Save' + ) + if (!button) throw new Error('Missing Save button') + act(() => button.click()) +} + +beforeEach(() => { + globalThis.IS_REACT_ACT_ENVIRONMENT = true + vi.clearAllMocks() + mutateAsync.mockReturnValue(new Promise(() => {})) + useTableSecurityStore.getState().reset() + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) +}) + +afterEach(() => { + act(() => root.unmount()) + container.remove() +}) + +describe('Table Security', () => { + it('hides permissions while disabled and enables all four backend locks by default', () => { + render() + expect(getSwitch('Enable Table Security').getAttribute('aria-checked')).toBe('false') + expect(document.querySelector('[role="radiogroup"]')).toBeNull() + + clickSwitch('Enable Table Security') + for (const label of LABELS) { + expect(getPermission(label, 'Deny').disabled).toBe(false) + expect(getPermission(label, 'Allow').disabled).toBe(false) + expect(getPermission(label, 'Deny').getAttribute('aria-checked')).toBe('true') + expect(getPermission(label, 'Allow').getAttribute('aria-checked')).toBe('false') + } + save() + + expect(mutateAsync.mock.calls[0][0]).toEqual({ + tableId: 'table-1', + locks: { insertLocked: true, updateLocked: true, deleteLocked: true, schemaLocked: true }, + }) + }) + + it('inverts existing locks and remembers permissions after disabling, saving, and reopening', async () => { + render({ insertLocked: true, updateLocked: true, deleteLocked: false, schemaLocked: true }) + expect(getSwitch('Enable Table Security').getAttribute('aria-checked')).toBe('true') + expect(getPermission('Deleting Rows', 'Allow').getAttribute('aria-checked')).toBe('true') + expect(getPermission('Updating Rows', 'Deny').getAttribute('aria-checked')).toBe('true') + + selectPermission('Inserting Rows', 'Allow') + clickSwitch('Enable Table Security') + expect(document.querySelector('[role="radiogroup"]')).toBeNull() + let resolveSave!: () => void + mutateAsync.mockReturnValueOnce( + new Promise((resolve) => { + resolveSave = resolve + }) + ) + save() + expect(mutateAsync.mock.calls[0][0]).toEqual({ + tableId: 'table-1', + locks: UNLOCKED_TABLE_LOCKS, + }) + await act(async () => resolveSave()) + + render(UNLOCKED_TABLE_LOCKS, false) + render() + expect(getSwitch('Enable Table Security').getAttribute('aria-checked')).toBe('false') + expect(document.querySelector('[role="radiogroup"]')).toBeNull() + clickSwitch('Enable Table Security') + expect(getPermission('Inserting Rows', 'Allow').getAttribute('aria-checked')).toBe('true') + expect(getPermission('Updating Rows', 'Deny').getAttribute('aria-checked')).toBe('true') + save() + expect(mutateAsync.mock.calls[1][0]).toEqual({ + tableId: 'table-1', + locks: { insertLocked: false, updateLocked: true, deleteLocked: false, schemaLocked: true }, + }) + }) + + it('does not remember unsuccessful changes and discards them on reopen', () => { + render() + clickSwitch('Enable Table Security') + selectPermission('Inserting Rows', 'Allow') + save() + expect(useTableSecurityStore.getState().preferences['table-1']).toBeUndefined() + expect(onClose).not.toHaveBeenCalled() + + render(UNLOCKED_TABLE_LOCKS, false) + render() + expect(getSwitch('Enable Table Security').getAttribute('aria-checked')).toBe('false') + expect(document.querySelector('[role="radiogroup"]')).toBeNull() + clickSwitch('Enable Table Security') + expect(getPermission('Inserting Rows', 'Deny').getAttribute('aria-checked')).toBe('true') + }) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/lock-settings-modal/lock-settings-modal.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/lock-settings-modal/lock-settings-modal.tsx index b30531933ca..6b58f6914bc 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/lock-settings-modal/lock-settings-modal.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/lock-settings-modal/lock-settings-modal.tsx @@ -1,31 +1,27 @@ 'use client' -import { useId, useState } from 'react' +import { useState } from 'react' import { + ChipButtonGroup, + ChipButtonGroupItem, ChipModal, ChipModalBody, + ChipModalField, ChipModalFooter, ChipModalHeader, - Label, Switch, Tooltip, } from '@sim/emcn' import { CircleInfo, Lock } from '@sim/emcn/icons' -import type { TableLocks } from '@/lib/table' -import { - describeLocks, - LOCK_FIELDS, -} from '@/app/workspace/[workspaceId]/tables/[tableId]/lock-copy' +import type { TableLocks } from '@/lib/table/types' +import { LOCK_FIELDS } from '@/app/workspace/[workspaceId]/tables/[tableId]/lock-copy' import { useUpdateTableLocks } from '@/hooks/queries/tables' - -function locksEqual(a: TableLocks, b: TableLocks): boolean { - return ( - a.schemaLocked === b.schemaLocked && - a.insertLocked === b.insertLocked && - a.updateLocked === b.updateLocked && - a.deleteLocked === b.deleteLocked - ) -} +import { + getTableSecurityLocks, + getTableSecuritySettings, + tableSecuritySettingsEqual, + useTableSecurityStore, +} from '@/stores/table/security/store' interface LockSettingsModalProps { isOpen: boolean @@ -36,7 +32,7 @@ interface LockSettingsModalProps { } /** - * Admin-only panel to toggle a table's four mutation locks. Changes are staged + * Admin-only panel that sets a table's four mutation locks. Changes are staged * locally and applied on Save (one request); the server re-checks admin and * rejects a `write`-only caller with a 403 surfaced as a toast. Gated at the * call site on `canAdmin`. @@ -48,65 +44,93 @@ export function LockSettingsModal({ tableId, locks, }: LockSettingsModalProps) { - const idPrefix = useId() const updateLocks = useUpdateTableLocks(workspaceId) + const preference = useTableSecurityStore((state) => state.preferences[tableId]) + const setPreference = useTableSecurityStore((state) => state.setPreference) + const settings = getTableSecuritySettings(locks, preference) - // Stage edits locally; reset to the server value each time the modal opens. - const [draft, setDraft] = useState(locks) + const [draft, setDraft] = useState(settings) const [prevOpen, setPrevOpen] = useState(isOpen) if (prevOpen !== isOpen) { setPrevOpen(isOpen) - if (isOpen) setDraft(locks) + if (isOpen) setDraft(settings) } - const dirty = !locksEqual(draft, locks) - const summary = describeLocks(draft) + const dirty = !tableSecuritySettingsEqual(draft, settings) - const handleSave = () => { + const handleSave = async () => { if (!dirty) { onClose() return } - updateLocks.mutate({ tableId, locks: draft }, { onSuccess: () => onClose() }) + try { + await updateLocks.mutateAsync({ tableId, locks: getTableSecurityLocks(draft) }) + } catch { + return + } + setPreference(tableId, draft) + onClose() } return ( - !open && onClose()} srTitle='Table locks'> + !open && onClose()} srTitle='Table Security'> - Table locks + Table Security -

- {summary.name} — {summary.detail} -

- {LOCK_FIELDS.map((field) => { - const fieldId = `${idPrefix}-${field.kind}` - return ( -
-
- - - {/* Not `asChild`: the hint is each lock's only explanation, so - the trigger must be a focusable button for keyboard users. */} - - - - -

{field.hint}

- - -
- + setDraft((prev) => ({ ...prev, enabled }))} + /> + + {draft.enabled && + LOCK_FIELDS.map((field) => ( + + {field.label} + + + + + +

{field.hint}

+ + + + } + > + - setDraft((prev) => ({ ...prev, [field.key]: checked })) + onValueChange={(value) => + setDraft((prev) => ({ + ...prev, + allowedActions: { ...prev.allowedActions, [field.kind]: value === 'allow' }, + })) } - /> -
- ) - })} + > + Deny + Allow + + + ))} void + schemaLocked?: boolean onInsertLeft: (columnName: string) => void onInsertRight: (columnName: string) => void onDeleteColumn: (columnName: string) => void @@ -122,6 +123,7 @@ export function ColumnOptionsMenu({ column, deleteLabel, onOpenConfig, + schemaLocked, onInsertLeft, onInsertRight, onDeleteColumn, @@ -228,7 +230,7 @@ export function ColumnOptionsMenu({ View workflow )} - onOpenConfig(column.key)}> + onOpenConfig(column.key)}> Edit column @@ -281,6 +283,7 @@ interface WorkflowGroupMetaCellProps { isGroupSelected: boolean onSelectGroup: (startColIndex: number, size: number) => void onOpenConfig: (columnName: string) => void + schemaLocked?: boolean onRunColumn?: (groupId: string, mode?: RunMode, rowIds?: string[], limit?: RunLimit) => void onInsertLeft?: (columnName: string) => void onInsertRight?: (columnName: string) => void @@ -334,6 +337,7 @@ export function WorkflowGroupMetaCell({ isGroupSelected, onSelectGroup, onOpenConfig, + schemaLocked, onRunColumn, onInsertLeft, onInsertRight, @@ -539,6 +543,7 @@ export function WorkflowGroupMetaCell({ position={optionsMenuPosition} column={column} onOpenConfig={onOpenConfig} + schemaLocked={schemaLocked} onInsertLeft={onInsertLeft} onInsertRight={onInsertRight} onDeleteColumn={onDeleteColumn} diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx index f800493d046..2da8f90ce0d 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx @@ -705,8 +705,8 @@ export function TableGrid({ // Manual grid entry is "add an empty row, then type into its cells" — the // typing is an update. So a *useful* manual add needs BOTH insert and update // unlocked; on an append-only table (update locked) it would leave a blank - // row the user can't fill. The control stays visible and explains itself via - // `onBlockedAction`. Full-row inserts still flow through CSV import / API / + // row the user can't fill. The control stays visible and explains itself in + // a tooltip. Full-row inserts still flow through CSV import / API / // blocks / Mothership, which the insert lock alone governs server-side. const canManualAddRow = userPermissions.canEdit && !locks?.insertLocked && !locks?.updateLocked const canEditCellRef = useRef(canEditCell) @@ -4746,6 +4746,7 @@ export function TableGrid({ groupName={workflowGroupById.get(g.groupId)?.name} onSelectGroup={handleGroupSelect} onOpenConfig={() => handleConfigureWorkflowGroup(g.groupId)} + schemaLocked={locks?.schemaLocked} onRunColumn={userPermissions.canEdit ? handleRunColumn : undefined} hasActiveFilter={Boolean(effectiveFilter)} selectedRowIds={selectedRowIds} @@ -4887,6 +4888,7 @@ export function TableGrid({ workflowGroups={tableWorkflowGroups} sourceInfo={columnSourceInfo.get(column.key)} onOpenConfig={handleConfigureColumn} + schemaLocked={locks?.schemaLocked} onViewWorkflow={handleViewWorkflow} onSortColumn={onSortColumn} onClearSort={onClearSort} @@ -4907,7 +4909,6 @@ export function TableGrid({ trigger='inline-header' disabled={addColumnMutation.isPending} blocked={!canMutateSchema} - onBlocked={() => onBlockedAction('add-column')} onPickType={handleAddColumnOfType} onPickWorkflow={handleAddWorkflowColumn} onPickEnrichment={onOpenEnrichments} @@ -5050,7 +5051,16 @@ export function TableGrid({ )} {!isLoadingTable && !isLoadingRows && userPermissions.canEdit && ( - + )} diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-primitives.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-primitives.tsx index f05d7332524..fab52045ad8 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-primitives.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-primitives.tsx @@ -1,8 +1,8 @@ 'use client' import React from 'react' -import { Button, Checkbox, cn } from '@sim/emcn' -import { Plus } from '@sim/emcn/icons' +import { Button, Checkbox, cn, Tooltip } from '@sim/emcn' +import { Lock, Plus } from '@sim/emcn/icons' import { ADD_COL_WIDTH, CELL_HEADER_CHECKBOX, COL_WIDTH } from './constants' import type { DisplayColumn } from './types' @@ -58,19 +58,42 @@ export const SelectAllCheckbox = React.memo(function SelectAllCheckbox({ ) }) -export const AddRowButton = React.memo(function AddRowButton({ onClick }: { onClick: () => void }) { +interface AddRowButtonProps { + onClick: () => void + blockedReason?: string +} + +export const AddRowButton = React.memo(function AddRowButton({ + onClick, + blockedReason, +}: AddRowButtonProps) { + const Icon = blockedReason ? Lock : Plus + const button = ( + + ) return (
- + {blockedReason ? ( + + {button} + {blockedReason} + + ) : ( + button + )}
) }) diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/lock-copy.ts b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/lock-copy.ts index 6f559998503..16aea98fbaf 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/lock-copy.ts +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/lock-copy.ts @@ -8,11 +8,12 @@ import type { TableLockKind, TableLocks } from '@/lib/table/types' export interface LockField { - /** The `TableLocks` flag this row toggles. */ + /** The `TableLocks` flag this row controls. */ key: keyof TableLocks kind: TableLockKind - /** The action being locked, phrased to read after "Lock " and inside a list. */ + /** The action being locked, phrased to read inside a list. */ noun: string + label: string hint: string } @@ -21,25 +22,29 @@ export const LOCK_FIELDS: LockField[] = [ key: 'insertLocked', kind: 'insert', noun: 'adding rows', - hint: 'On: no new rows can be added — by anyone, including CSV import, the API, workflow blocks, and Sim.', + label: 'Inserting Rows', + hint: 'Allow new rows to be added, including through CSV imports, the API, workflows, and Sim. Deny blocks new rows when Table Security is enabled.', }, { key: 'updateLocked', kind: 'update', noun: 'editing rows', - hint: 'On: existing cell values cannot be changed. Workflow and enrichment columns still populate.', + label: 'Updating Rows', + hint: 'Allow existing cell values to be changed. Deny blocks edits when Table Security is enabled. Workflow and enrichment columns still populate.', }, { key: 'deleteLocked', kind: 'delete', noun: 'deleting rows', - hint: 'On: rows cannot be deleted, and the table cannot be archived.', + label: 'Deleting Rows', + hint: 'Allow rows to be deleted and the table to be archived. Deny blocks these actions and destructive column changes when Table Security is enabled.', }, { key: 'schemaLocked', kind: 'schema', noun: 'changing columns', - hint: 'On: columns cannot be added, renamed, retyped, or removed.', + label: 'Changing Table Schema', + hint: 'Allow columns to be added, renamed, retyped, or removed. Deny blocks schema changes when Table Security is enabled. Removing or retyping columns also requires Deleting Rows to be set to Allow.', }, ] @@ -48,32 +53,6 @@ export function lockedNouns(locks: TableLocks): string[] { return LOCK_FIELDS.filter((f) => locks[f.key]).map((f) => f.noun) } -/** - * Plain-language summary of a lock set — the named mode when the combination - * matches one, otherwise a list of what is locked. - */ -export function describeLocks(locks: TableLocks): { name: string; detail: string } { - const locked = lockedNouns(locks) - if (locked.length === 0) { - return { name: 'Unlocked', detail: 'anyone with edit access can change this table.' } - } - if (locked.length === LOCK_FIELDS.length) { - return { name: 'Read-only', detail: 'no one can change this table’s rows or columns.' } - } - // Append-only describes the row semantics — adding is the only thing left. - // A schema lock on top doesn't change that, so it keeps the name and is - // called out in the detail rather than demoted to the generic case. - if (!locks.insertLocked && locks.updateLocked && locks.deleteLocked) { - return { - name: 'Append-only', - detail: locks.schemaLocked - ? 'rows can be added, but not edited or deleted, and columns are locked.' - : 'rows can be added, but not edited or deleted.', - } - } - return { name: 'Locked', detail: `${locked.join(', ')} locked.` } -} - /** * Why a locked-table notice was raised. `'status'` is the informational case * (the announcement shown once when a locked table is opened); the rest are @@ -127,7 +106,7 @@ export function describeBlockedAction( case 'status': { const nouns = lockedNouns(locks) return { - title: 'Table locks', + title: 'Table Security', text: nouns.length > 0 ? `An admin has locked ${nouns.join(', ')} on this table.` diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/table.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/table.tsx index a6fbe933f0c..27b7eb41422 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/table.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/table.tsx @@ -1303,7 +1303,7 @@ export function Table({ ...(userPermissions.canAdmin ? [ { - label: 'Lock settings', + label: 'Table Security', icon: Lock, onClick: () => setShowLockSettings(true), }, @@ -1355,7 +1355,7 @@ export function Table({ description: text, ...(canOpenLockSettings ? { - action: { label: 'Lock settings', onClick: () => setShowLockSettings(true) }, + action: { label: 'Table Security', onClick: () => setShowLockSettings(true) }, // An action would otherwise pin the toast open until dismissed. duration: BLOCKED_TOAST_MS, } @@ -1392,7 +1392,7 @@ export function Table({ ) // A toast's action is captured when it is created, so a viewer who loses - // admin access mid-toast would keep a Lock settings button that opens + // admin access mid-toast would keep a Table Security button that opens // nothing. Dismiss on that transition only — a viewer who never had access // has a legitimate action-less notice that must survive. const couldOpenLockSettingsRef = useRef(canOpenLockSettings) @@ -1434,7 +1434,6 @@ export function Table({ trigger='header' disabled={false} blocked={!canMutateSchema} - onBlocked={() => showBlockedToast('add-column')} onPickType={handleAddColumnOfType} onPickWorkflow={handleAddWorkflowColumn} onPickEnrichment={onOpenEnrichments} @@ -1873,6 +1872,7 @@ export function Table({ )} {tableData && userPermissions.canAdmin && ( setShowLockSettings(false)} workspaceId={workspaceId} diff --git a/apps/sim/stores/table/security/store.test.ts b/apps/sim/stores/table/security/store.test.ts new file mode 100644 index 00000000000..af7fd17504c --- /dev/null +++ b/apps/sim/stores/table/security/store.test.ts @@ -0,0 +1,64 @@ +/** + * @vitest-environment jsdom + */ +import { beforeEach, describe, expect, it } from 'vitest' +import { UNLOCKED_TABLE_LOCKS } from '@/lib/table/types' +import { + getTableSecurityLocks, + getTableSecuritySettings, + type TableSecuritySettings, + useTableSecurityStore, +} from '@/stores/table/security/store' + +const PREFERENCE: TableSecuritySettings = { + enabled: false, + allowedActions: { insert: true, update: false, delete: false, schema: false }, +} + +beforeEach(() => useTableSecurityStore.getState().reset()) + +describe('table security preferences', () => { + it('uses current server locks over remembered local permissions', () => { + const serverLocks = { ...UNLOCKED_TABLE_LOCKS, insertLocked: true } + const settings = getTableSecuritySettings(serverLocks, PREFERENCE) + expect(settings.enabled).toBe(true) + expect(settings.allowedActions).toEqual({ + insert: false, + update: true, + delete: true, + schema: true, + }) + expect(getTableSecurityLocks(settings)).toEqual(serverLocks) + }) + + it('recognizes an external unlock even when the browser previously enabled restrictions', () => { + const settings = getTableSecuritySettings(UNLOCKED_TABLE_LOCKS, { + ...PREFERENCE, + enabled: true, + }) + expect(settings.enabled).toBe(false) + expect(settings.allowedActions).toEqual(PREFERENCE.allowedActions) + }) + + it('keeps security visibly enabled when all actions are allowed', () => { + const settings: TableSecuritySettings = { + enabled: true, + allowedActions: { insert: true, update: true, delete: true, schema: true }, + } + expect(getTableSecurityLocks(settings)).toEqual(UNLOCKED_TABLE_LOCKS) + expect(getTableSecuritySettings(UNLOCKED_TABLE_LOCKS, settings).enabled).toBe(true) + }) + + it('remembers disabled permissions across hydration without sharing them with another table', async () => { + useTableSecurityStore.getState().setPreference('table-1', PREFERENCE) + const saved = localStorage.getItem('table-security-preferences') + useTableSecurityStore.getState().reset() + localStorage.setItem('table-security-preferences', saved!) + await useTableSecurityStore.persist.rehydrate() + expect(useTableSecurityStore.getState().preferences['table-1']).toEqual(PREFERENCE) + expect(useTableSecurityStore.getState().preferences['table-2']).toBeUndefined() + expect(getTableSecurityLocks(useTableSecurityStore.getState().preferences['table-1'])).toEqual( + UNLOCKED_TABLE_LOCKS + ) + }) +}) diff --git a/apps/sim/stores/table/security/store.ts b/apps/sim/stores/table/security/store.ts new file mode 100644 index 00000000000..401eb0d9cb9 --- /dev/null +++ b/apps/sim/stores/table/security/store.ts @@ -0,0 +1,108 @@ +'use client' + +import { create } from 'zustand' +import { devtools, persist } from 'zustand/middleware' +import { BrowserStorage } from '@/lib/core/utils/browser-storage' +import { + TABLE_LOCK_FLAGS, + TABLE_LOCK_KINDS, + type TableLockKind, + type TableLocks, +} from '@/lib/table/types' +import { registerUserDataReset } from '@/stores/user-data-reset-registry' + +export interface TableSecuritySettings { + enabled: boolean + allowedActions: Record +} + +interface TableSecurityState { + preferences: Record + setPreference: (tableId: string, settings: TableSecuritySettings) => void + reset: () => void +} + +const DEFAULT_SETTINGS: TableSecuritySettings = { + enabled: false, + allowedActions: { insert: false, update: false, delete: false, schema: false }, +} + +/** Maps Table Security settings to the backend lock flags. */ +export function getTableSecurityLocks(settings: TableSecuritySettings): TableLocks { + return { + insertLocked: settings.enabled && !settings.allowedActions.insert, + updateLocked: settings.enabled && !settings.allowedActions.update, + deleteLocked: settings.enabled && !settings.allowedActions.delete, + schemaLocked: settings.enabled && !settings.allowedActions.schema, + } +} + +/** + * Server locks are authoritative. When none are set, the browser preference + * supplies the remembered per-action choices and distinguishes + * enabled-with-everything-allowed from disabled, which both use four false + * backend flags. + */ +export function getTableSecuritySettings( + locks: TableLocks, + preference?: TableSecuritySettings +): TableSecuritySettings { + if (TABLE_LOCK_KINDS.some((kind) => locks[TABLE_LOCK_FLAGS[kind]])) { + return { + enabled: true, + allowedActions: { + insert: !locks.insertLocked, + update: !locks.updateLocked, + delete: !locks.deleteLocked, + schema: !locks.schemaLocked, + }, + } + } + + if (!preference) return DEFAULT_SETTINGS + return { + enabled: + preference.enabled && TABLE_LOCK_KINDS.every((kind) => preference.allowedActions[kind]), + allowedActions: preference.allowedActions, + } +} + +export function tableSecuritySettingsEqual( + a: TableSecuritySettings, + b: TableSecuritySettings +): boolean { + return ( + a.enabled === b.enabled && + TABLE_LOCK_KINDS.every((kind) => a.allowedActions[kind] === b.allowedActions[kind]) + ) +} + +/** Device-local presentation preferences; actual locks are owned by React Query. */ +export const useTableSecurityStore = create()( + devtools( + persist( + (set) => ({ + preferences: {}, + setPreference: (tableId, settings) => + set((state) => ({ preferences: { ...state.preferences, [tableId]: settings } })), + reset: () => set({ preferences: {} }), + }), + { + name: 'table-security-preferences', + partialize: (state) => ({ preferences: state.preferences }), + storage: { + getItem: (name) => BrowserStorage.getItem(name, null), + setItem: (name, value) => { + BrowserStorage.setItem(name, value) + }, + removeItem: (name) => { + BrowserStorage.removeItem(name) + }, + }, + } + ), + { name: 'table-security-preferences' } + ) +) + +registerUserDataReset('table-security-preferences', () => useTableSecurityStore.getState().reset()) From 1dbc256dfad85ea1638e1e77aec73d72ab1d6a43 Mon Sep 17 00:00:00 2001 From: Justin Blumencranz <96924014+j15z@users.noreply.github.com> Date: Tue, 15 Sep 2026 12:14:19 -0700 Subject: [PATCH 2/7] 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 --- .../components/row-modal/row-modal.test.tsx | 139 +++++++++++++---- .../components/row-modal/row-modal.tsx | 109 ++++++++----- .../table-grid/cells/cell-content.tsx | 4 + .../cells/expanded-cell-popover.test.tsx | 144 ++++++++++++++++++ .../cells/expanded-cell-popover.tsx | 29 +++- .../table-grid/cells/inline-editors.test.ts | 63 ++++++++ .../table-grid/cells/inline-editors.tsx | 62 +++++--- .../components/table-grid/data-row.tsx | 5 + .../components/table-grid/table-grid.tsx | 82 ++++++---- .../tables/[tableId]/lock-copy.ts | 2 +- .../[workspaceId]/tables/[tableId]/table.tsx | 12 ++ .../chip-date-picker/chip-date-picker.tsx | 22 ++- 12 files changed, 543 insertions(+), 130 deletions(-) create mode 100644 apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/expanded-cell-popover.test.tsx diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/row-modal/row-modal.test.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/row-modal/row-modal.test.tsx index 18573ba0f98..881c7fcbb22 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/row-modal/row-modal.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/row-modal/row-modal.test.tsx @@ -7,14 +7,21 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import type { TableInfo, TableRow } from '@/lib/table' import { RowModal } from '@/app/workspace/[workspaceId]/tables/[tableId]/components/row-modal/row-modal' -const { mockToastError, mockUseTimezoneState, mockUpdateRow, mockDeleteRow, mockDeleteRows } = - vi.hoisted(() => ({ - mockToastError: vi.fn(), - mockUseTimezoneState: vi.fn(), - mockUpdateRow: vi.fn(), - mockDeleteRow: vi.fn(), - mockDeleteRows: vi.fn(), - })) +const { + mockToastError, + mockUseTimezoneState, + mockCreateRow, + mockUpdateRow, + mockDeleteRow, + mockDeleteRows, +} = vi.hoisted(() => ({ + mockToastError: vi.fn(), + mockUseTimezoneState: vi.fn(), + mockCreateRow: vi.fn(), + mockUpdateRow: vi.fn(), + mockDeleteRow: vi.fn(), + mockDeleteRows: vi.fn(), +})) vi.mock('next/navigation', () => ({ useParams: () => ({ workspaceId: 'workspace-1' }), @@ -23,6 +30,7 @@ vi.mock('@/hooks/queries/general-settings', () => ({ useTimezoneState: mockUseTimezoneState, })) vi.mock('@/hooks/queries/tables', () => ({ + useCreateTableRow: () => ({ mutateAsync: mockCreateRow, isPending: false }), useUpdateTableRow: () => ({ mutateAsync: mockUpdateRow, isPending: false }), useDeleteTableRow: () => ({ mutateAsync: mockDeleteRow, isPending: false }), useDeleteTableRows: () => ({ mutateAsync: mockDeleteRows, isPending: false }), @@ -35,11 +43,12 @@ vi.mock('@sim/emcn', () => { createElement('button', { type: 'button', ...props }, children), ChipConfirmModal: passthrough, ChipDatePicker: ({ value, onChange }: { value?: string; onChange: (value: string) => void }) => - createElement( - 'button', - { type: 'button', 'data-testid': 'date', onClick: () => onChange(value ?? '2026-11-01') }, - value - ), + createElement('input', { + 'data-testid': 'date', + value: value ?? '', + onChange: (event: { currentTarget: { value: string } }) => + onChange(event.currentTarget.value), + }), ChipModal: passthrough, ChipModalBody: passthrough, ChipModalError: passthrough, @@ -80,13 +89,6 @@ vi.mock('@sim/emcn', () => { 'Update Row' ), ChipModalHeader: passthrough, - ChipTimePicker: ({ value, onChange }: { value?: string; onChange: (value: string) => void }) => - createElement('input', { - 'data-testid': 'time', - value: value ?? '', - onChange: (event: { currentTarget: { value: string } }) => - onChange(event.currentTarget.value), - }), Label: passthrough, toast: { error: mockToastError }, } @@ -113,6 +115,85 @@ function changeInput(input: HTMLInputElement, value: string) { input.dispatchEvent(new Event('input', { bubbles: true })) } +describe('RowModal add mode', () => { + beforeEach(() => { + vi.clearAllMocks() + mockCreateRow.mockResolvedValue(undefined) + mockUseTimezoneState.mockReturnValue({ timezone: 'America/Los_Angeles', status: 'ready' }) + }) + + it('inserts the complete row under column ids in one request without updating', async () => { + const container = document.createElement('div') + document.body.appendChild(container) + const root = createRoot(container) + const props = { + mode: 'add' as const, + isOpen: true, + onClose: vi.fn(), + table: { + id: 'table-3', + name: 'People', + schema: { columns: [{ id: 'col_name', name: 'Name', type: 'string' as const }] }, + }, + onSuccess: vi.fn(), + } + + ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + act(() => root.render(createElement(RowModal, props))) + + const nameInput = container.querySelector('[data-testid="modal-input"]') + expect(nameInput?.value).toBe('') + act(() => changeInput(nameInput as HTMLInputElement, 'Ada')) + const submit = container.querySelector('[data-testid="submit"]') + await act(async () => submit?.click()) + + expect(mockCreateRow).toHaveBeenCalledWith({ data: { col_name: 'Ada' } }) + expect(mockUpdateRow).not.toHaveBeenCalled() + expect(props.onSuccess).toHaveBeenCalledTimes(1) + + act(() => root.unmount()) + container.remove() + }) +}) + +describe('RowModal column ids', () => { + beforeEach(() => { + vi.clearAllMocks() + mockUpdateRow.mockResolvedValue(undefined) + mockUseTimezoneState.mockReturnValue({ timezone: 'America/Los_Angeles', status: 'ready' }) + }) + + it('shows and saves edit values stored under the column id', async () => { + const container = document.createElement('div') + document.body.appendChild(container) + const root = createRoot(container) + const props = { + mode: 'edit' as const, + isOpen: true, + onClose: vi.fn(), + table: { + id: 'table-4', + name: 'People', + schema: { columns: [{ id: 'col_name', name: 'Name', type: 'string' as const }] }, + }, + row: { ...row, data: { col_name: 'Ada' } }, + onSuccess: vi.fn(), + } + + act(() => root.render(createElement(RowModal, props))) + + const nameInput = container.querySelector('[data-testid="modal-input"]') + expect(nameInput?.value).toBe('Ada') + act(() => changeInput(nameInput as HTMLInputElement, 'Grace')) + const submit = container.querySelector('[data-testid="submit"]') + await act(async () => submit?.click()) + + expect(mockUpdateRow).toHaveBeenCalledWith({ rowId: 'row-1', data: { col_name: 'Grace' } }) + act(() => root.unmount()) + container.remove() + }) +}) + describe('RowModal expiration editing', () => { beforeEach(() => { vi.clearAllMocks() @@ -136,7 +217,9 @@ describe('RowModal expiration editing', () => { ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true act(() => root.render(createElement(RowModal, props))) - expect(container.querySelector('[data-testid="time"]')?.value).toBe('01:00') + expect(container.querySelector('[data-testid="date"]')?.value).toBe( + '2026-11-01T01:00:00' + ) expect(container.querySelector('[data-testid="submit"]')?.disabled).toBe( false ) @@ -153,9 +236,9 @@ describe('RowModal expiration editing', () => { }) act(() => root.render(createElement(RowModal, props))) - const timeInput = container.querySelector('[data-testid="time"]') - expect(timeInput?.value).toBe('01:00') - act(() => changeInput(timeInput as HTMLInputElement, '01:30')) + const dateInput = container.querySelector('[data-testid="date"]') + expect(dateInput?.value).toBe('2026-11-01T01:00:00') + act(() => changeInput(dateInput as HTMLInputElement, '2026-11-01T01:30')) const submit = container.querySelector('[data-testid="submit"]') await act(async () => submit?.click()) @@ -193,7 +276,7 @@ describe('RowModal expiration editing', () => { expect(container.querySelector('[aria-label="Edit starts_at"]')?.textContent).toBe( 'Loading timezone…' ) - expect(container.querySelector('[data-testid="time"]')).toBeNull() + expect(container.querySelector('[data-testid="date"]')).toBeNull() mockUseTimezoneState.mockReturnValue({ timezone: 'America/Los_Angeles', @@ -201,7 +284,7 @@ describe('RowModal expiration editing', () => { }) act(() => root.render(createElement(RowModal, props))) - expect(container.querySelector('[data-testid="time"]')).not.toBeNull() + expect(container.querySelector('[data-testid="date"]')).not.toBeNull() act(() => root.unmount()) container.remove() }) @@ -226,7 +309,9 @@ describe('RowModal expiration editing', () => { act(() => root.render(createElement(RowModal, props))) - expect(container.querySelector('[data-testid="time"]')?.value).toBe('01:00') + expect(container.querySelector('[data-testid="date"]')?.value).toBe( + '2026-11-01T01:00:00' + ) expect(container.querySelector('[data-testid="submit"]')?.disabled).toBe( false ) diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/row-modal/row-modal.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/row-modal/row-modal.tsx index 94939e31e28..d787309a009 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/row-modal/row-modal.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/row-modal/row-modal.tsx @@ -12,7 +12,6 @@ import { ChipModalField, ChipModalFooter, ChipModalHeader, - ChipTimePicker, Label, toast, } from '@sim/emcn' @@ -20,17 +19,24 @@ import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' import { useParams } from 'next/navigation' import type { ColumnDefinition, TableInfo, TableRow } from '@/lib/table' +import { getColumnId } from '@/lib/table/column-keys' import { columnTypeOf } from '@/lib/table/column-types' import { resolveCurrencyCode } from '@/lib/table/currency' import { todayAtTtlOffset, ttlValueFromPicker, ttlValueToPickerParts } from '@/lib/table/ttl-values' import { getTimezoneEditBlockedMessage } from '@/app/workspace/[workspaceId]/tables/[tableId]/components/timezone-editing' import { type TimezoneState, useTimezoneState } from '@/hooks/queries/general-settings' -import { useDeleteTableRow, useDeleteTableRows, useUpdateTableRow } from '@/hooks/queries/tables' +import { + useCreateTableRow, + useDeleteTableRow, + useDeleteTableRows, + useUpdateTableRow, +} from '@/hooks/queries/tables' import { cleanCellValue, dateValueToLocalParts, formatValueForInput, localPartsToDateValue, + storageToDisplay, todayLocalCalendarDate, } from '../../utils' import { SelectValueEditor } from '../select-field' @@ -38,7 +44,7 @@ import { SelectValueEditor } from '../select-field' const logger = createLogger('RowModal') export interface RowModalProps { - mode: 'edit' | 'delete' + mode: 'add' | 'edit' | 'delete' isOpen: boolean onClose: () => void table: TableInfo @@ -56,12 +62,13 @@ function cleanRowData( const cleanData: Record = {} columns.forEach((col) => { - const value = rowData[col.name] + const columnId = getColumnId(col) + const value = rowData[columnId] if (columnTypeOf(col).editor === 'date' && !dateEditorsReady) { return } try { - cleanData[col.name] = cleanCellValue(value, col, timeZone) + cleanData[columnId] = cleanCellValue(value, col, timeZone) } catch { throw new Error(`Invalid JSON for field: ${col.name}`) } @@ -71,10 +78,12 @@ function cleanRowData( } /** - * Modal for editing a row's values or confirming row deletion. + * Modal for adding a complete row, editing a row's values, or confirming row + * deletion. Adding inserts every value in one request, so it works on a table + * whose update lock blocks filling in a blank row from the grid. * - * `rowData` is initialized from the `row` prop at mount time only. Both call-sites - * conditionally mount this component per open, so each open gets fresh state. If a + * `rowData` is initialized from the `row` prop at mount time only. Every call-site + * conditionally mounts this component per open, so each open gets fresh state. If a * call-site ever keeps it mounted across target-row changes, it must supply a `key` * prop (e.g. the row id) so React remounts with the new row's values. */ @@ -97,11 +106,16 @@ export function RowModal({ mode, isOpen, onClose, table, row, rowIds, onSuccess mode === 'edit' && row ? row.data : {} ) const [error, setError] = useState(null) + const createRowMutation = useCreateTableRow({ workspaceId, tableId }) const updateRowMutation = useUpdateTableRow({ workspaceId, tableId }) const deleteRowMutation = useDeleteTableRow({ workspaceId, tableId }) const deleteRowsMutation = useDeleteTableRows({ workspaceId, tableId }) const isSubmitting = - updateRowMutation.isPending || deleteRowMutation.isPending || deleteRowsMutation.isPending + createRowMutation.isPending || + updateRowMutation.isPending || + deleteRowMutation.isPending || + deleteRowsMutation.isPending + const isAddMode = mode === 'add' const timezoneBlockedMessage = getTimezoneEditBlockedMessage(timezoneState) const hasEditableColumn = columns.some( @@ -116,14 +130,17 @@ export function RowModal({ mode, isOpen, onClose, table, row, rowIds, onSuccess try { const cleanData = cleanRowData(columns, rowData, timeZone, dateEditorsReady) - if (row) { + if (isAddMode) { + await createRowMutation.mutateAsync({ data: cleanData }) + } else if (row) { await updateRowMutation.mutateAsync({ rowId: row.id, data: cleanData }) } onSuccess() } catch (err) { - logger.error('Failed to edit row:', err) - setError(getErrorMessage(err, 'Failed to edit row')) + const action = isAddMode ? 'add' : 'edit' + logger.error(`Failed to ${action} row:`, err) + setError(getErrorMessage(err, `Failed to ${action} row`)) } } @@ -182,20 +199,25 @@ export function RowModal({ mode, isOpen, onClose, table, row, rowIds, onSuccess } return ( - - Edit Row + + {isAddMode ? 'Add Row' : 'Edit Row'}

- Update values for {table?.name ?? 'table'} + {isAddMode ? 'Fill in values for' : 'Update values for'} {table?.name ?? 'table'}

- + {saveBlockedReason ? ( + + + + + + + {saveBlockedReason} + + ) : ( + + )} diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/inline-editors.test.ts b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/inline-editors.test.ts index d08d7cb5a9e..66900798c6d 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/inline-editors.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/inline-editors.test.ts @@ -396,3 +396,66 @@ describe('dateEditorRawValue', () => { container.remove() }) }) + +describe('read-only InlineEditor', () => { + beforeEach(() => { + vi.clearAllMocks() + mockUseTimezoneState.mockReturnValue({ + timezone: 'America/Los_Angeles', + status: 'ready', + }) + }) + + it('shows a text value that can be selected but not changed', () => { + const container = document.createElement('div') + document.body.appendChild(container) + const root = createRoot(container) + const onSave = vi.fn() + + act(() => + root.render( + createElement(InlineEditor, { + value: 'Original text', + column: column('string'), + readOnly: true, + onSave, + onCancel: vi.fn(), + }) + ) + ) + + const input = container.querySelector('input') as HTMLInputElement + expect(input.value).toBe('Original text') + expect(input.readOnly).toBe(true) + act(() => input.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true }))) + + expect(onSave).toHaveBeenCalledWith('Original text', 'enter') + act(() => root.unmount()) + container.remove() + }) + + it('opens a date read-only without the calendar picker', () => { + const container = document.createElement('div') + document.body.appendChild(container) + const root = createRoot(container) + + act(() => + root.render( + createElement(InlineEditor, { + value: '2026-06-15T06:00:30-07:00', + column: column('ttl'), + readOnly: true, + onSave: vi.fn(), + onCancel: vi.fn(), + }) + ) + ) + + const input = container.querySelector('input') as HTMLInputElement + expect(input.value).toBe('2026-06-15T06:00:30-07:00') + expect(input.readOnly).toBe(true) + expect(mockCalendar).not.toHaveBeenCalled() + act(() => root.unmount()) + container.remove() + }) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/inline-editors.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/inline-editors.tsx index d6e754beb4a..dff9efd7ba3 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/inline-editors.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/inline-editors.tsx @@ -35,6 +35,8 @@ interface InlineEditorProps { value: unknown column: ColumnDefinition initialCharacter?: string + /** Shows the value without allowing changes; text stays selectable and copyable. */ + readOnly?: boolean onSave: (value: unknown, reason: SaveReason) => void onCancel: () => void } @@ -105,6 +107,7 @@ function ReadyInlineDateEditor({ value, column, initialCharacter, + readOnly, onSave, onCancel, initialTimeZone, @@ -274,33 +277,38 @@ function ReadyInlineDateEditor({ }} onKeyDown={handleKeyDown} onBlur={scheduleBlurSave} + readOnly={readOnly} placeholder={isOffsetDate ? 'YYYY-MM-DDTHH:mm:ss±HH:mm' : 'mm/dd/yyyy'} className={cn( 'w-full min-w-0 select-text border-none bg-transparent p-0 text-[var(--text-primary)] text-small outline-hidden', invalid && 'text-[var(--text-error)]' )} /> - - - - - - + {!readOnly && ( + + + + + + + )} ) } @@ -310,6 +318,7 @@ function InlineTextEditor({ value, column, initialCharacter, + readOnly, onSave, onCancel, }: InlineEditorProps) { @@ -394,6 +403,7 @@ function InlineTextEditor({ onKeyDown={handleKeyDown} onWheel={handleEditorWheel} onBlur={() => doSave('blur')} + readOnly={readOnly} className={cn( 'w-full min-w-0 select-text border-none bg-transparent p-0 text-[var(--text-primary)] text-small outline-hidden', invalid && 'text-[var(--text-error)]' @@ -409,7 +419,7 @@ function InlineTextEditor({ * toggles and commits when the menu closes. Escape discards the draft, matching * the text/date inline editors. */ -function InlineSelectEditor({ value, column, onSave, onCancel }: InlineEditorProps) { +function InlineSelectEditor({ value, column, readOnly, onSave, onCancel }: InlineEditorProps) { const isMulti = !!column.multiple const allOptions = column.options ?? [] const [draft, setDraft] = useState(() => selectedOptionIds(column, value)) @@ -475,13 +485,17 @@ function InlineSelectEditor({ value, column, onSave, onCancel }: InlineEditorPro {!isMulti && !column.required && ( - setDraftAnd([])}> + setDraftAnd([])}> None {draft.length === 0 && } )} {allOptions.map((option) => ( - handleSelectOption(e, option.id)}> + handleSelectOption(e, option.id)} + > {draft.includes(option.id) && } diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/data-row.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/data-row.tsx index 86963aa768b..0c603a6e1dc 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/data-row.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/data-row.tsx @@ -33,6 +33,8 @@ export interface DataRowProps { isFirstRow: boolean editingColumnName: string | null initialCharacter: string | null + /** Opens cell editors read-only, e.g. on an update-locked table. */ + editorsReadOnly: boolean pendingCellValue: Record | null normalizedSelection: NormalizedSelection | null onClick: (rowId: string, columnName: string, options?: { toggleBoolean?: boolean }) => void @@ -121,6 +123,7 @@ function dataRowPropsAreEqual(prev: DataRowProps, next: DataRowProps): boolean { prev.rowIndex !== next.rowIndex || prev.isFirstRow !== next.isFirstRow || prev.editingColumnName !== next.editingColumnName || + prev.editorsReadOnly !== next.editorsReadOnly || prev.pendingCellValue !== next.pendingCellValue || prev.onClick !== next.onClick || prev.onDoubleClick !== next.onDoubleClick || @@ -170,6 +173,7 @@ export const DataRow = React.memo(function DataRow({ isFirstRow, editingColumnName, initialCharacter, + editorsReadOnly, pendingCellValue, normalizedSelection, isRowChecked, @@ -417,6 +421,7 @@ export const DataRow = React.memo(function DataRow({ column={column} isEditing={isEditing} initialCharacter={isEditing ? initialCharacter : undefined} + readOnly={editorsReadOnly} onSave={(value, reason) => onSave(row.id, column.key, value, reason)} onCancel={onCancel} waitingOnLabels={ diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx index 2da8f90ce0d..46a42cb65c1 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx @@ -27,6 +27,7 @@ import type { import { getColumnId } from '@/lib/table/column-keys' import { columnTypeOf } from '@/lib/table/column-types' import { TABLE_LIMITS } from '@/lib/table/constants' +import { isEmptyCellValue } from '@/lib/table/deps' import { cellValueFilterConditions } from '@/lib/table/query-builder/cell-filter' import { SEARCH_DEBOUNCE_MS } from '@/lib/url-state' import { FindBar } from '@/app/workspace/[workspaceId]/components' @@ -208,6 +209,8 @@ interface TableGridProps { onOpenEnrichmentDetails: (rowId: string, groupId: string) => void /** Open the row-edit modal for `row`. Wrapper renders the modal. */ onOpenRowModal: (row: TableRowType) => void + /** Opens the add-row form, which inserts a complete row in one request. */ + onOpenAddRowModal: () => void /** Open the row-delete modal for `snapshots`. Wrapper renders the modal. */ onRequestDeleteRows: (snapshots: DeletedRowSnapshot[]) => void /** @@ -450,6 +453,7 @@ export function TableGrid({ onOpenExecutionDetails, onOpenEnrichmentDetails, onOpenRowModal, + onOpenAddRowModal, onRequestDeleteRows, onRequestDeleteAllByFilter, onRequestDeleteColumns, @@ -705,9 +709,9 @@ export function TableGrid({ // Manual grid entry is "add an empty row, then type into its cells" — the // typing is an update. So a *useful* manual add needs BOTH insert and update // unlocked; on an append-only table (update locked) it would leave a blank - // row the user can't fill. The control stays visible and explains itself in - // a tooltip. Full-row inserts still flow through CSV import / API / - // blocks / Mothership, which the insert lock alone governs server-side. + // row the user can't fill, so New row opens the add-row form instead, which + // inserts the complete row in one request. Full-row inserts (the form, CSV + // import, API, blocks, Mothership) need only the insert lock off server-side. const canManualAddRow = userPermissions.canEdit && !locks?.insertLocked && !locks?.updateLocked const canEditCellRef = useRef(canEditCell) canEditCellRef.current = canEditCell @@ -715,8 +719,8 @@ export function TableGrid({ canManualAddRowRef.current = canManualAddRow const canInsertFullRowRef = useRef(canInsertFullRow) canInsertFullRowRef.current = canInsertFullRow - // Read by the closure-free double-click handler to tell "locked" apart from - // "no write permission" — only the former gets the explanation modal. + // Read by the closure-free save and keyboard handlers to tell "locked" apart + // from "no write permission" — only the former gets the explanation toast. const updateLockedRef = useRef(locks?.updateLocked) updateLockedRef.current = locks?.updateLocked const onBlockedActionRef = useRef(onBlockedAction) @@ -727,6 +731,8 @@ export function TableGrid({ // Refs for callback props read inside effects with stable empty deps. const onOpenRowModalRef = useRef(onOpenRowModal) onOpenRowModalRef.current = onOpenRowModal + const onOpenAddRowModalRef = useRef(onOpenAddRowModal) + onOpenAddRowModalRef.current = onOpenAddRowModal const { contextMenu, @@ -1756,6 +1762,10 @@ export function TableGrid({ // Stable identity so 's React.memo still bails out; lock state // is read from refs instead of being closed over. const handleAddRowClick = useCallback(() => { + if (canInsertFullRowRef.current && updateLockedRef.current) { + onOpenAddRowModalRef.current() + return + } if (!canManualAddRowRef.current) { onBlockedActionRef.current('add-row') return @@ -2741,23 +2751,24 @@ export function TableGrid({ (rowId: string, columnName: string, columnKey: string) => { const column = columnsRef.current.find((c) => c.key === columnKey) if (column && columnTypeOf(column).editor === 'toggle') return - - // Double-click means "edit this cell". On an update-locked table, say so - // rather than opening the expanded viewer — which looks like an editor - // that silently refuses to save. Only for users who could otherwise edit: - // without write access the lock isn't why they can't, and they still get - // the read-only expanded viewer below. - if (canEditRef.current && updateLockedRef.current) { - onBlockedActionRef.current('edit-cell') + // A read-only view of an empty cell has nothing to show or copy. + if ( + !canEditCellRef.current && + isEmptyCellValue(rowsRef.current.find((r) => r.id === rowId)?.data[columnName]) + ) { return } setSelectionFocus(null) setIsColumnSelection(false) - // Types with a bounded value edit in place (calendar picker, numeric - // input); only free-form prose opens the big expanded popover. - if (column && !columnTypeOf(column).expandable && canEditCellRef.current) { + // Editors open for anyone with write access. On an update-locked table + // they open read-only, so the value can still be selected and copied; + // `handleInlineSave` stays as a backstop that refuses any change with the + // lock explanation. Types with a bounded value edit in place (calendar + // picker, numeric input); only free-form prose opens the big expanded + // popover. + if (column && !columnTypeOf(column).expandable && canEditRef.current) { setEditingCell({ rowId, columnName }) setInitialCharacter(null) return @@ -2997,23 +3008,24 @@ export function TableGrid({ if (e.key === 'Enter' || e.key === 'F2') { if (!canEditRef.current) return e.preventDefault() - // The primary keyboard edit path — same lock notice as double-click and - // Space, rather than a keypress that silently does nothing. - if (updateLockedRef.current) { - onBlockedActionRef.current('edit-cell') - return - } - if (!canEditCellRef.current) return const col = cols[anchor.colIndex] if (!col) return const row = currentRows[anchor.rowIndex] if (!row) return + // The keyboard twin of double-click: the editor opens read-only on an + // update-locked table. A toggle writes on the keypress itself, so it + // explains the lock here instead. if (columnTypeOf(col).editor === 'toggle') { + if (updateLockedRef.current) { + onBlockedActionRef.current('edit-cell') + return + } toggleBooleanCellRef.current(row.id, col.key, row.data[col.key]) return } + if (!canEditCellRef.current && isEmptyCellValue(row.data[col.key])) return setEditingCell({ rowId: row.id, columnName: col.key }) setInitialCharacter(null) return @@ -3022,8 +3034,8 @@ export function TableGrid({ if (e.key === ' ' && !e.shiftKey) { if (!canEditRef.current) return e.preventDefault() - // Space opens the same row editor as double-click, so it follows the - // update lock too — otherwise the form fills in and only 423s on save. + // Space opens the whole-row editor, which explains the update lock up + // front — otherwise the form fills in and only 423s on save. if (updateLockedRef.current) { onBlockedActionRef.current('edit-cell') return @@ -3846,6 +3858,14 @@ export function TableGrid({ } const changed = !cellValuesEqual(oldValue, normalizedValue, column) + if (changed && updateLockedRef.current) { + onBlockedActionRef.current('edit-cell') + setEditingCell(null) + setInitialCharacter(null) + scrollRef.current?.focus({ preventScroll: true }) + return + } + if (changed) { pushUndoRef.current({ type: 'update-cell', @@ -4963,6 +4983,7 @@ export function TableGrid({ initialCharacter={ editingCell?.rowId === row.id ? initialCharacter : null } + editorsReadOnly={Boolean(locks?.updateLocked)} pendingCellValue={ pendingUpdate && pendingUpdate.rowId === row.id ? pendingUpdate.data @@ -5054,11 +5075,7 @@ export function TableGrid({ )} @@ -5118,7 +5135,10 @@ export function TableGrid({ rows={rows} columns={displayColumns} onSave={handleInlineSave} - canEdit={canEditCell} + canEdit={userPermissions.canEdit} + saveBlockedReason={ + locks?.updateLocked ? 'Updating rows is disabled in Table Security.' : undefined + } scrollContainer={scrollRef.current} /> diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/lock-copy.ts b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/lock-copy.ts index 16aea98fbaf..a5103edcc56 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/lock-copy.ts +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/lock-copy.ts @@ -79,7 +79,7 @@ export function describeBlockedAction( } return { title: 'This table is append-only', - text: 'Rows can’t be edited once added, so typing one into the grid is unavailable. Import a CSV, or add rows from the API, a workflow, or Sim.', + text: 'Rows can’t be edited once added, so typing one into the grid is unavailable. Use New row to fill in a complete row, import a CSV, or add rows from the API, a workflow, or Sim.', } case 'add-column': return { diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/table.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/table.tsx index 27b7eb41422..835f9e16d5a 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/table.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/table.tsx @@ -226,6 +226,7 @@ export function Table({ const blockedToastIdRef = useRef(null) const [isImportCsvOpen, setIsImportCsvOpen] = useState(false) const [editingRow, setEditingRow] = useState(null) + const [isAddingRow, setIsAddingRow] = useState(false) const [deletingRows, setDeletingRows] = useState([]) const [deletingAll, setDeletingAll] = useState<{ excludeRowIds: string[] @@ -295,6 +296,7 @@ export function Table({ }, []) const onCloseSlideout = () => dispatch({ type: 'CLOSE' }) const onOpenRowModal = (row: TableRowType) => setEditingRow(row) + const onOpenAddRowModal = () => setIsAddingRow(true) // useCallback because is memo-wrapped — these flow into // the breadcrumbs / headerActions memos, whose identity drives that re-render. const onRequestDeleteTable = useCallback(() => setShowDeleteTableConfirm(true), []) @@ -1609,6 +1611,7 @@ export function Table({ onOpenExecutionDetails={onOpenExecutionDetails} onOpenEnrichmentDetails={onOpenEnrichmentDetails} onOpenRowModal={onOpenRowModal} + onOpenAddRowModal={onOpenAddRowModal} onRequestDeleteRows={onRequestDeleteRows} onRequestDeleteAllByFilter={onRequestDeleteAllByFilter} onRequestDeleteColumns={onRequestDeleteColumns} @@ -1753,6 +1756,15 @@ export function Table({ table={tableData} /> )} + {isAddingRow && tableData && ( + setIsAddingRow(false)} + table={tableData} + onSuccess={() => setIsAddingRow(false)} + /> + )} {editingRow && tableData && ( ( className, } = props + /** + * Inside a modal dialog the calendar must be modal too: a non-modal popover + * portaled to `body` inherits the dialog's `pointer-events: none` body lock + * and cannot be clicked. Outside dialogs it stays non-modal. + */ + const insideModal = useContext(InsideModalContext) const [open, setOpen] = useState(false) const triggerText = @@ -98,7 +110,7 @@ const ChipDatePicker = forwardRef( : formatDateLabel(props.value)) return ( - + - + {readOnly ? ( + + + + + + + {readOnlyReason && {readOnlyReason}} + + ) : ( + + )} ) diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/column-dropdown/column-dropdown.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/column-dropdown/column-dropdown.tsx index 2481852bfff..4fc8addc7ee 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/column-dropdown/column-dropdown.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/column-dropdown/column-dropdown.tsx @@ -18,6 +18,7 @@ import { type ColumnTypeOption, columnTypeOptionsForTable, } from '@/app/workspace/[workspaceId]/tables/[tableId]/components/column-config-sidebar' +import { LOCK_TOOLTIPS } from '@/app/workspace/[workspaceId]/tables/[tableId]/lock-copy' const CELL_HEADER = 'border-[var(--border)] border-r border-b bg-[var(--bg)] px-2 py-[7px] text-left align-middle' @@ -117,7 +118,7 @@ export function ColumnDropdown({ const lockedTrigger = ( {triggerButton} - Changing the table schema is disabled in Table Security. + {LOCK_TOOLTIPS.schema} ) return trigger === 'inline-header' ? ( diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/expanded-cell-popover.test.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/expanded-cell-popover.test.tsx index b6f1be07154..1130b892adb 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/expanded-cell-popover.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/expanded-cell-popover.test.tsx @@ -120,6 +120,9 @@ describe('ExpandedCellPopover', () => { expect(getTextarea().readOnly).toBe(true) expect(getSaveButton().disabled).toBe(true) expect(document.body.textContent).not.toContain(BLOCKED_REASON) + // The ↵ half of the shortcut hint would advertise a save that never happens. + expect(document.body.textContent).toContain('esc close') + expect(document.body.textContent).not.toContain('save ·') const trigger = getSaveButton().parentElement if (!trigger) throw new Error('Missing Save tooltip trigger') @@ -135,6 +138,7 @@ describe('ExpandedCellPopover', () => { render() expect(getTextarea().readOnly).toBe(false) expect(getSaveButton().disabled).toBe(false) + expect(document.body.textContent).toContain('save ·') typeDraft('Changed text') pressEnter() diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/expanded-cell-popover.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/expanded-cell-popover.tsx index e50e7e22b9c..bab40b19ddd 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/expanded-cell-popover.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/expanded-cell-popover.tsx @@ -278,6 +278,11 @@ function ExpandedCellEditor({
{parseError ? ( {parseError} + ) : saveBlockedReason ? ( + // Saving is refused, so the ↵ half of the shortcut hint would be a lie. + + esc close + ) : ( ↵ save · esc cancel diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/headers/column-header-menu.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/headers/column-header-menu.tsx index 6fadb12f5c4..6ba6ebfa820 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/headers/column-header-menu.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/headers/column-header-menu.tsx @@ -15,7 +15,10 @@ interface ColumnHeaderMenuProps { column: DisplayColumn colIndex: number readOnly?: boolean - schemaLocked?: boolean + /** Why column changes are unavailable; disables the schema rows and explains them. */ + schemaLockedReason?: string + /** Why deleting is unavailable; disables the destructive column row. */ + deleteLockedReason?: string isRenaming: boolean isColumnSelected: boolean renameValue: string @@ -66,7 +69,8 @@ export const ColumnHeaderMenu = React.memo(function ColumnHeaderMenu({ column, colIndex, readOnly, - schemaLocked, + schemaLockedReason, + deleteLockedReason, isRenaming, isColumnSelected, renameValue, @@ -348,7 +352,8 @@ export const ColumnHeaderMenu = React.memo(function ColumnHeaderMenu({ column={column} deleteLabel={deleteLabel} onOpenConfig={onOpenConfig} - schemaLocked={schemaLocked} + schemaLockedReason={schemaLockedReason} + deleteLockedReason={deleteLockedReason} onInsertLeft={onInsertLeft} onInsertRight={onInsertRight} onDeleteColumn={onDeleteColumn} diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/headers/workflow-group-meta-cell.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/headers/workflow-group-meta-cell.tsx index 7b13b399548..69c159e585b 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/headers/workflow-group-meta-cell.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/headers/workflow-group-meta-cell.tsx @@ -12,6 +12,7 @@ import { DropdownMenuSubContent, DropdownMenuSubTrigger, DropdownMenuTrigger, + Tooltip, } from '@sim/emcn' import { ArrowDown, @@ -70,7 +71,10 @@ interface ColumnOptionsMenuProps { * it leaves the group with siblings). */ deleteLabel?: string onOpenConfig: (columnName: string) => void - schemaLocked?: boolean + /** Why column changes are unavailable; disables the schema rows and explains them. */ + schemaLockedReason?: string + /** Why deleting is unavailable; disables the destructive column row. */ + deleteLockedReason?: string onInsertLeft: (columnName: string) => void onInsertRight: (columnName: string) => void onDeleteColumn: (columnName: string) => void @@ -109,6 +113,24 @@ interface ColumnOptionsMenuProps { onPinToggle?: (columnName: string) => void } +/** + * A menu row a lock disables. A disabled `DropdownMenuItem` sets + * `pointer-events: none`, so it can never receive the hover its own tooltip + * would need — the trigger wraps it instead (same shape as the folder menu). + * Renders the row untouched when nothing blocks it. + */ +function MenuRow({ reason, children }: { reason?: string; children: React.ReactElement }) { + if (!reason) return children + return ( + + +
{children}
+ + {reason} + + ) +} + /** * Shared column-options dropdown rendered next to the column header chevron * AND on right-click of the workflow group meta cell. Anchors to a fixed @@ -123,7 +145,8 @@ export function ColumnOptionsMenu({ column, deleteLabel, onOpenConfig, - schemaLocked, + schemaLockedReason, + deleteLockedReason, onInsertLeft, onInsertRight, onDeleteColumn, @@ -141,6 +164,9 @@ export function ColumnOptionsMenu({ isPinned, onPinToggle, }: ColumnOptionsMenuProps) { + // Hiding a workflow output leaves the data alone, so no lock covers it. + const destructiveReason = + deleteLabel === 'Hide column' ? undefined : (schemaLockedReason ?? deleteLockedReason) const showRunActions = Boolean(onRunColumnAll && onRunColumnIncomplete) const showRunSelected = Boolean(onRunColumnSelected) && selectedRowCount > 0 const runLabels = runMenuLabels(hasActiveFilter) @@ -230,10 +256,15 @@ export function ColumnOptionsMenu({ View workflow )} - onOpenConfig(column.key)}> - - Edit column - + + onOpenConfig(column.key)} + > + + Edit column + + {onPinToggle && ( onPinToggle(column.key)}> {isPinned ? : } @@ -241,23 +272,36 @@ export function ColumnOptionsMenu({ )} {/* Stops acting on this column and starts creating siblings — `Edit column` - above is unconditional, so the rule is always backed. */} + above always renders (disabled or not), so the rule is always backed. */} - onInsertLeft(column.key)}> - - Insert column left - - onInsertRight(column.key)}> - - Insert column right - + + onInsertLeft(column.key)} + > + + Insert column left + + + + onInsertRight(column.key)} + > + + Insert column right + + - (onDeleteGroup ? onDeleteGroup() : onDeleteColumn(column.key))} - > - {deleteLabel === 'Hide column' ? : } - {deleteLabel ?? 'Delete column'} - + + (onDeleteGroup ? onDeleteGroup() : onDeleteColumn(column.key))} + > + {deleteLabel === 'Hide column' ? : } + {deleteLabel ?? 'Delete column'} + + ) @@ -283,7 +327,10 @@ interface WorkflowGroupMetaCellProps { isGroupSelected: boolean onSelectGroup: (startColIndex: number, size: number) => void onOpenConfig: (columnName: string) => void - schemaLocked?: boolean + /** Why column changes are unavailable; disables the schema rows and explains them. */ + schemaLockedReason?: string + /** Why deleting is unavailable; disables the destructive column row. */ + deleteLockedReason?: string onRunColumn?: (groupId: string, mode?: RunMode, rowIds?: string[], limit?: RunLimit) => void onInsertLeft?: (columnName: string) => void onInsertRight?: (columnName: string) => void @@ -337,7 +384,8 @@ export function WorkflowGroupMetaCell({ isGroupSelected, onSelectGroup, onOpenConfig, - schemaLocked, + schemaLockedReason, + deleteLockedReason, onRunColumn, onInsertLeft, onInsertRight, @@ -543,7 +591,8 @@ export function WorkflowGroupMetaCell({ position={optionsMenuPosition} column={column} onOpenConfig={onOpenConfig} - schemaLocked={schemaLocked} + schemaLockedReason={schemaLockedReason} + deleteLockedReason={deleteLockedReason} onInsertLeft={onInsertLeft} onInsertRight={onInsertRight} onDeleteColumn={onDeleteColumn} diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx index adee5573981..f040e922466 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx @@ -35,6 +35,7 @@ import { useUserPermissionsContext } from '@/app/workspace/[workspaceId]/provide import { getTimezoneEditBlockedMessage } from '@/app/workspace/[workspaceId]/tables/[tableId]/components/timezone-editing' import type { RemoteTableSelection } from '@/app/workspace/[workspaceId]/tables/[tableId]/hooks/use-table-room' import type { BlockedTableAction } from '@/app/workspace/[workspaceId]/tables/[tableId]/lock-copy' +import { LOCK_TOOLTIPS } from '@/app/workspace/[workspaceId]/tables/[tableId]/lock-copy' import { useTimezoneState } from '@/hooks/queries/general-settings' import { useAddTableColumn, @@ -2747,7 +2748,16 @@ export function TableGrid({ (rowId: string, columnName: string, options?: { toggleBoolean?: boolean }) => { const column = columnsRef.current.find((c) => c.key === columnName) if (column && columnTypeOf(column).editor === 'toggle') { - if (!options?.toggleBoolean || !canEditCellRef.current) return + if (!options?.toggleBoolean) return + // A toggle writes on the click itself, so there is no read-only editor to + // fall back to — an update-locked table has to explain the refusal here, + // the same way the Enter/Space keyboard paths do. + if (!canEditCellRef.current) { + if (canEditRef.current && updateLockedRef.current) { + onBlockedActionRef.current('edit-cell') + } + return + } const row = rowsRef.current.find((r) => r.id === rowId) if (row) { toggleBooleanCell(rowId, columnName, row.data[columnName]) @@ -4790,7 +4800,12 @@ export function TableGrid({ groupName={workflowGroupById.get(g.groupId)?.name} onSelectGroup={handleGroupSelect} onOpenConfig={() => handleConfigureWorkflowGroup(g.groupId)} - schemaLocked={locks?.schemaLocked} + schemaLockedReason={ + locks?.schemaLocked ? LOCK_TOOLTIPS.schema : undefined + } + deleteLockedReason={ + locks?.deleteLocked ? LOCK_TOOLTIPS.delete : undefined + } onRunColumn={userPermissions.canEdit ? handleRunColumn : undefined} hasActiveFilter={Boolean(effectiveFilter)} selectedRowIds={selectedRowIds} @@ -4932,7 +4947,12 @@ export function TableGrid({ workflowGroups={tableWorkflowGroups} sourceInfo={columnSourceInfo.get(column.key)} onOpenConfig={handleConfigureColumn} - schemaLocked={locks?.schemaLocked} + schemaLockedReason={ + locks?.schemaLocked ? LOCK_TOOLTIPS.schema : undefined + } + deleteLockedReason={ + locks?.deleteLocked ? LOCK_TOOLTIPS.delete : undefined + } onViewWorkflow={handleViewWorkflow} onSortColumn={onSortColumn} onClearSort={onClearSort} @@ -5098,9 +5118,7 @@ export function TableGrid({ {!isLoadingTable && !isLoadingRows && userPermissions.canEdit && ( )}
@@ -5160,9 +5178,7 @@ export function TableGrid({ columns={displayColumns} onSave={handleInlineSave} canEdit={userPermissions.canEdit} - saveBlockedReason={ - locks?.updateLocked ? 'Updating rows is disabled in Table Security.' : undefined - } + saveBlockedReason={locks?.updateLocked ? LOCK_TOOLTIPS.update : undefined} scrollContainer={scrollRef.current} /> diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/lock-copy.ts b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/lock-copy.ts index a5103edcc56..8ff9790506d 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/lock-copy.ts +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/lock-copy.ts @@ -1,8 +1,11 @@ /** - * Single source of truth for lock vocabulary shared by the lock settings modal + * Single source of truth for lock vocabulary shared by the Table Security modal * and the lock toasts (the on-open announcement and blocked actions). Kept out of * `lib/table/mutation-locks.ts` — that module is server-tainted (importing it * from a client component pulls `next/headers` into the browser bundle). + * + * The modal speaks Allow/Deny, so this copy does too: a set lock reads as its + * action being "disabled", never as a separate "locked" state. */ import type { TableLockKind, TableLocks } from '@/lib/table/types' @@ -11,7 +14,7 @@ export interface LockField { /** The `TableLocks` flag this row controls. */ key: keyof TableLocks kind: TableLockKind - /** The action being locked, phrased to read inside a list. */ + /** The action being denied, phrased to read inside a list. */ noun: string label: string hint: string @@ -21,49 +24,61 @@ export const LOCK_FIELDS: LockField[] = [ { key: 'insertLocked', kind: 'insert', - noun: 'adding rows', + noun: 'inserting rows', label: 'Inserting Rows', - hint: 'Allow new rows to be added, including through CSV imports, the API, workflows, and Sim. Deny blocks new rows when Table Security is enabled.', + hint: 'Allow new rows to be added, including through CSV imports, the API, workflows, and Sim. Deny blocks new rows from every surface.', }, { key: 'updateLocked', kind: 'update', - noun: 'editing rows', + noun: 'updating rows', label: 'Updating Rows', - hint: 'Allow existing cell values to be changed. Deny blocks edits when Table Security is enabled. Workflow and enrichment columns still populate.', + hint: 'Allow existing cell values to be changed. Deny blocks edits from every surface. Workflow and enrichment columns still populate.', }, { key: 'deleteLocked', kind: 'delete', noun: 'deleting rows', label: 'Deleting Rows', - hint: 'Allow rows to be deleted and the table to be archived. Deny blocks these actions and destructive column changes when Table Security is enabled.', + hint: 'Allow rows to be deleted and the table to be archived. Deny blocks those actions and destructive column changes.', }, { key: 'schemaLocked', kind: 'schema', - noun: 'changing columns', + noun: 'changing the table schema', label: 'Changing Table Schema', - hint: 'Allow columns to be added, renamed, retyped, or removed. Deny blocks schema changes when Table Security is enabled. Removing or retyping columns also requires Deleting Rows to be set to Allow.', + hint: 'Allow columns to be added, renamed, retyped, or removed. Deny blocks schema changes. Removing or retyping columns also requires Deleting Rows set to Allow.', }, ] -/** The locked verbs' nouns, in display order. Empty when nothing is locked. */ +/** + * Tooltip for a control a denied action disables. One sentence per lock kind so + * the grid chrome (New row, New column, the column menu, the expanded editor's + * Save) all name the same Table Security row. + */ +export const LOCK_TOOLTIPS: Record = { + insert: 'Inserting rows is disabled in Table Security.', + update: 'Updating rows is disabled in Table Security.', + delete: 'Deleting rows is disabled in Table Security.', + schema: 'Changing the table schema is disabled in Table Security.', +} + +/** The denied actions' nouns, in display order. Empty when everything is allowed. */ export function lockedNouns(locks: TableLocks): string[] { return LOCK_FIELDS.filter((f) => locks[f.key]).map((f) => f.noun) } /** * Why a locked-table notice was raised. `'status'` is the informational case - * (the announcement shown once when a locked table is opened); the rest are + * (the announcement shown once when a restricted table is opened); the rest are * actions the user just tried and couldn't do. */ export type BlockedTableAction = 'add-row' | 'add-column' | 'delete-column' | 'edit-cell' | 'status' /** - * Copy for the action the user attempted. Explains what is blocked and — for - * the append-only manual-entry case — what to do instead, since that one is - * blocked by the *update* lock rather than the insert lock. + * Copy for the action the user attempted, in the modal's vocabulary: each + * notice names the Table Security row that denies it, so the reader knows which + * setting an admin has to flip. */ export function describeBlockedAction( action: BlockedTableAction, @@ -71,37 +86,31 @@ export function describeBlockedAction( ): { title: string; text: string } { switch (action) { case 'add-row': - if (locks.insertLocked) { - return { - title: 'Adding rows is locked', - text: 'No new rows can be added until an admin unlocks this table.', - } - } return { - title: 'This table is append-only', - text: 'Rows can’t be edited once added, so typing one into the grid is unavailable. Use New row to fill in a complete row, import a CSV, or add rows from the API, a workflow, or Sim.', + title: 'Inserting rows is disabled', + text: 'An admin has set Inserting Rows to Deny in Table Security.', } case 'add-column': return { - title: 'Changing columns is locked', - text: 'Columns can’t be added, renamed, retyped, or removed until an admin unlocks this table.', + title: 'Changing the table schema is disabled', + text: 'An admin has set Changing Table Schema to Deny in Table Security, so columns can’t be added, renamed, retyped, or removed.', } case 'delete-column': - // Reachable with the schema lock off but the delete lock on — removing a - // column clears its value from every row, so it needs both. + // Reachable with Changing Table Schema on Allow but Deleting Rows on Deny — + // removing a column clears its value from every row, so it needs both. return locks.schemaLocked ? { - title: 'Changing columns is locked', - text: 'Columns can’t be added, renamed, retyped, or removed until an admin unlocks this table.', + title: 'Changing the table schema is disabled', + text: 'An admin has set Changing Table Schema to Deny in Table Security, so columns can’t be added, renamed, retyped, or removed.', } : { - title: 'Deleting columns is locked', - text: 'Removing a column deletes its value from every row, so it’s blocked while deleting is locked.', + title: 'Deleting rows is disabled', + text: 'Removing a column clears its value from every row, so it needs Deleting Rows set to Allow in Table Security.', } case 'edit-cell': return { - title: 'Editing rows is locked', - text: 'Existing cell values can’t be changed until an admin unlocks this table.', + title: 'Updating rows is disabled', + text: 'An admin has set Updating Rows to Deny in Table Security.', } case 'status': { const nouns = lockedNouns(locks) @@ -109,8 +118,8 @@ export function describeBlockedAction( title: 'Table Security', text: nouns.length > 0 - ? `An admin has locked ${nouns.join(', ')} on this table.` - : 'Nothing is locked on this table.', + ? `An admin has set ${nouns.join(', ')} to Deny on this table.` + : 'Every action is allowed on this table.', } } } diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/table.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/table.tsx index cbaffd7e13b..2f3a338ff5b 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/table.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/table.tsx @@ -93,7 +93,12 @@ import { import { COLUMN_SIDEBAR_WIDTH } from './components/table-grid/constants' import { columnTypeIcon } from './components/table-grid/headers' import { useTable, useTableEventStream, useTableRoom } from './hooks' -import { type BlockedTableAction, describeBlockedAction, lockedNouns } from './lock-copy' +import { + type BlockedTableAction, + describeBlockedAction, + LOCK_TOOLTIPS, + lockedNouns, +} from './lock-copy' import { ALL_VIEW_PARAM, DEFAULT_TABLE_DETAIL_SORT_DIRECTION, @@ -1716,6 +1721,12 @@ export function Table({ workspaceId={workspaceId} tableId={tableId} onColumnRename={onColumnRename} + readOnly={!canMutateSchema} + readOnlyReason={ + tableData?.locks.schemaLocked + ? LOCK_TOOLTIPS.schema + : 'You don’t have permission to change columns.' + } /> Date: Tue, 15 Sep 2026 16:24:32 -0700 Subject: [PATCH 5/7] fix(tables): write only what the row form changed, and report failures once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../components/row-modal/row-modal.test.tsx | 83 +++++++++++++++++-- .../components/row-modal/row-modal.tsx | 57 ++++++++++--- apps/sim/hooks/queries/tables.ts | 52 +++++++++--- 3 files changed, 167 insertions(+), 25 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/row-modal/row-modal.test.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/row-modal/row-modal.test.tsx index d311d61f226..34b1872a4ed 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/row-modal/row-modal.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/row-modal/row-modal.test.tsx @@ -389,7 +389,7 @@ describe('RowModal expiration editing', () => { container.remove() }) - it('keeps unrelated fields editable and omits blocked date values from the update', async () => { + it('sends only the edited field and omits blocked date values from the update', async () => { mockUseTimezoneState.mockReturnValue({ timezone: 'America/Los_Angeles', savedTimezone: 'Mars/Olympus', @@ -437,10 +437,10 @@ describe('RowModal expiration editing', () => { act(() => changeInput(nameInput as HTMLInputElement, 'Grace')) await act(async () => submit?.click()) - expect(mockUpdateRow).toHaveBeenCalledWith({ - rowId: 'row-1', - data: { name: 'Grace', expires_at: row.data.expires_at }, - }) + // Only the edited field is sent: the untouched TTL would otherwise be + // rewritten with the same value (and re-stamped through the picker), and the + // timezone-blocked date is dropped entirely. + expect(mockUpdateRow).toHaveBeenCalledWith({ rowId: 'row-1', data: { name: 'Grace' } }) expect(props.onSuccess).toHaveBeenCalledTimes(1) expect(mockToastError).not.toHaveBeenCalled() @@ -448,3 +448,76 @@ describe('RowModal expiration editing', () => { container.remove() }) }) + +describe('RowModal payload', () => { + beforeEach(() => { + vi.clearAllMocks() + mockCreateRow.mockResolvedValue(undefined) + mockUpdateRow.mockResolvedValue(undefined) + mockUseTimezoneState.mockReturnValue({ timezone: 'America/Los_Angeles', status: 'ready' }) + }) + + it('closes without a write when the edit changes nothing', async () => { + const container = document.createElement('div') + document.body.appendChild(container) + const root = createRoot(container) + const props = { + mode: 'edit' as const, + isOpen: true, + onClose: vi.fn(), + table: { + id: 'table-5', + name: 'People', + schema: { columns: [{ id: 'col_name', name: 'Name', type: 'string' as const }] }, + }, + row: { ...row, data: { col_name: 'Ada' } }, + onSuccess: vi.fn(), + } + + act(() => root.render(createElement(RowModal, props))) + const submit = container.querySelector('[data-testid="submit"]') + await act(async () => submit?.click()) + + expect(mockUpdateRow).not.toHaveBeenCalled() + expect(props.onSuccess).toHaveBeenCalledTimes(1) + + act(() => root.unmount()) + container.remove() + }) + + it('omits untouched columns on insert but still sends toggles', async () => { + const container = document.createElement('div') + document.body.appendChild(container) + const root = createRoot(container) + const props = { + mode: 'add' as const, + isOpen: true, + onClose: vi.fn(), + table: { + id: 'table-6', + name: 'People', + schema: { + columns: [ + { id: 'col_name', name: 'Name', type: 'string' as const }, + { id: 'col_notes', name: 'Notes', type: 'string' as const }, + { id: 'col_done', name: 'Done', type: 'boolean' as const }, + ], + }, + }, + onSuccess: vi.fn(), + } + + act(() => root.render(createElement(RowModal, props))) + const nameInput = container.querySelector('[data-testid="modal-input"]') + act(() => changeInput(nameInput as HTMLInputElement, 'Ada')) + const submit = container.querySelector('[data-testid="submit"]') + await act(async () => submit?.click()) + + // `col_notes` was never touched, so it stays absent instead of being written + // as null; a checkbox always carries a concrete boolean. + expect(mockCreateRow).toHaveBeenCalledWith({ data: { col_name: 'Ada', col_done: false } }) + + act(() => root.unmount()) + container.remove() + }) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/row-modal/row-modal.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/row-modal/row-modal.tsx index 28f02ede321..9da75292fae 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/row-modal/row-modal.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/row-modal/row-modal.tsx @@ -57,25 +57,53 @@ export interface RowModalProps { onSuccess: () => void } +/** Structural equality for a cleaned cell value vs what the row already holds. */ +function cellValueUnchanged(next: unknown, previous: unknown): boolean { + if (next === previous) return true + const nextEmpty = next === null || next === undefined + const previousEmpty = previous === null || previous === undefined + if (nextEmpty || previousEmpty) return nextEmpty && previousEmpty + if (typeof next === 'object' || typeof previous === 'object') { + return JSON.stringify(next) === JSON.stringify(previous) + } + return false +} + +/** + * Builds the write payload. Only fields the user actually touched are sent, so + * an untouched empty column is left absent instead of being written as `null` — + * and in edit mode a field whose value is unchanged is dropped too, leaving a + * no-op save with nothing to write. Toggles are the exception on insert: they + * always carry a concrete boolean, so a required checkbox the user never + * clicked still has to reach the server as `false`. + */ function cleanRowData( columns: ColumnDefinition[], rowData: Record, timeZone: string, - dateEditorsReady: boolean + dateEditorsReady: boolean, + options: { mode: 'add' | 'edit'; baseline?: Record } ): Record { const cleanData: Record = {} columns.forEach((col) => { const columnId = getColumnId(col) - const value = rowData[columnId] - if (columnTypeOf(col).editor === 'date' && !dateEditorsReady) { + const definition = columnTypeOf(col) + if (definition.editor === 'date' && !dateEditorsReady) { return } + const touched = columnId in rowData + const alwaysSend = options.mode === 'add' && definition.editor === 'toggle' + if (!touched && !alwaysSend) return + const value = rowData[columnId] + let cleaned: unknown try { - cleanData[columnId] = cleanCellValue(value, col, timeZone) + cleaned = cleanCellValue(value, col, timeZone) } catch { throw new Error(`Invalid JSON for field: ${col.name}`) } + if (options.baseline && cellValueUnchanged(cleaned, options.baseline[columnId])) return + cleanData[columnId] = cleaned }) return cleanData @@ -119,10 +147,13 @@ export function RowModal({ mode === 'edit' && row ? row.data : {} ) const [error, setError] = useState(null) - const createRowMutation = useCreateTableRow({ workspaceId, tableId }) - const updateRowMutation = useUpdateTableRow({ workspaceId, tableId }) - const deleteRowMutation = useDeleteTableRow({ workspaceId, tableId }) - const deleteRowsMutation = useDeleteTableRows({ workspaceId, tableId }) + // This modal renders its own failure in ``; without the flag + // every rejection would also arrive as a toast saying the same sentence. + const rowMutationContext = { workspaceId, tableId, suppressErrorToast: true } + const createRowMutation = useCreateTableRow(rowMutationContext) + const updateRowMutation = useUpdateTableRow(rowMutationContext) + const deleteRowMutation = useDeleteTableRow(rowMutationContext) + const deleteRowsMutation = useDeleteTableRows(rowMutationContext) const isSubmitting = createRowMutation.isPending || updateRowMutation.isPending || @@ -149,12 +180,18 @@ export function RowModal({ if (!canSubmit) return try { - const cleanData = cleanRowData(columns, rowData, timeZone, dateEditorsReady) + const cleanData = cleanRowData(columns, rowData, timeZone, dateEditorsReady, { + mode: isAddMode ? 'add' : 'edit', + baseline: isAddMode ? undefined : row?.data, + }) if (isAddMode) { await createRowMutation.mutateAsync({ data: cleanData, ...insertAt }) } else if (row) { - await updateRowMutation.mutateAsync({ rowId: row.id, data: cleanData }) + // Nothing changed — close instead of writing an empty patch. + if (Object.keys(cleanData).length > 0) { + await updateRowMutation.mutateAsync({ rowId: row.id, data: cleanData }) + } } onSuccess() diff --git a/apps/sim/hooks/queries/tables.ts b/apps/sim/hooks/queries/tables.ts index 22b4359b9f8..25a9fcb56ad 100644 --- a/apps/sim/hooks/queries/tables.ts +++ b/apps/sim/hooks/queries/tables.ts @@ -159,6 +159,12 @@ export type TableRowsResponse = Pick< interface RowMutationContext { workspaceId: string tableId: string + /** + * Suppresses the error toast for callers that render the failure themselves — + * the row modal shows it inline, and two copies of the same sentence read as + * two separate failures. The cache self-heal on a 423 still runs. + */ + suppressErrorToast?: boolean } type UpdateTableRowParams = Pick & @@ -809,16 +815,22 @@ function notifyRowWriteError(error: Error, onUpgrade: () => void): void { function handleTableLockRejection( error: unknown, queryClient: ReturnType, - tableId: string + tableId: string, + options?: { silent?: boolean } ): boolean { if (!isApiClientError(error) || error.status !== 423) return false void queryClient.invalidateQueries({ queryKey: tableKeys.detail(tableId), exact: true }) void queryClient.invalidateQueries({ queryKey: tableKeys.lists() }) - toast.error(error.message, { duration: 5000 }) + // `silent` only drops the toast; the refetches above are what un-stale the grid. + if (!options?.silent) toast.error(error.message, { duration: 5000 }) return true } -export function useCreateTableRow({ workspaceId, tableId }: RowMutationContext) { +export function useCreateTableRow({ + workspaceId, + tableId, + suppressErrorToast, +}: RowMutationContext) { const queryClient = useQueryClient() const router = useRouter() @@ -866,7 +878,9 @@ export function useCreateTableRow({ workspaceId, tableId }: RowMutationContext) }) }, onError: (error) => { - if (handleTableLockRejection(error, queryClient, tableId)) return + if (handleTableLockRejection(error, queryClient, tableId, { silent: suppressErrorToast })) + return + if (suppressErrorToast) return notifyRowWriteError(error, () => router.push(buildUpgradeHref(workspaceId, 'tables'))) }, onSettled: () => { @@ -1065,7 +1079,11 @@ export function useBatchCreateTableRows({ workspaceId, tableId }: RowMutationCon * Update a single row in a table. * Uses optimistic updates for instant UI feedback on inline cell edits. */ -export function useUpdateTableRow({ workspaceId, tableId }: RowMutationContext) { +export function useUpdateTableRow({ + workspaceId, + tableId, + suppressErrorToast, +}: RowMutationContext) { const queryClient = useQueryClient() return useMutation({ @@ -1150,8 +1168,10 @@ export function useUpdateTableRow({ workspaceId, tableId }: RowMutationContext) if (context?.didBumpRunState) { queryClient.setQueryData(tableKeys.activeDispatches(tableId), context.runStateSnapshot) } - if (handleTableLockRejection(error, queryClient, tableId)) return + if (handleTableLockRejection(error, queryClient, tableId, { silent: suppressErrorToast })) + return if (isValidationError(error)) return + if (suppressErrorToast) return toast.error(error.message, { duration: 5000 }) }, }) @@ -1234,7 +1254,11 @@ export function useBatchUpdateTableRows({ workspaceId, tableId }: RowMutationCon /** * Delete a single row from a table. */ -export function useDeleteTableRow({ workspaceId, tableId }: RowMutationContext) { +export function useDeleteTableRow({ + workspaceId, + tableId, + suppressErrorToast, +}: RowMutationContext) { const queryClient = useQueryClient() return useMutation({ @@ -1245,8 +1269,10 @@ export function useDeleteTableRow({ workspaceId, tableId }: RowMutationContext) }) }, onError: (error) => { - if (handleTableLockRejection(error, queryClient, tableId)) return + if (handleTableLockRejection(error, queryClient, tableId, { silent: suppressErrorToast })) + return if (isValidationError(error)) return + if (suppressErrorToast) return toast.error(error.message, { duration: 5000 }) }, onSettled: () => { @@ -1259,7 +1285,11 @@ export function useDeleteTableRow({ workspaceId, tableId }: RowMutationContext) * Delete multiple rows from a table. * Returns both deleted ids and failure details for partial-failure UI. */ -export function useDeleteTableRows({ workspaceId, tableId }: RowMutationContext) { +export function useDeleteTableRows({ + workspaceId, + tableId, + suppressErrorToast, +}: RowMutationContext) { const queryClient = useQueryClient() return useMutation({ @@ -1294,8 +1324,10 @@ export function useDeleteTableRows({ workspaceId, tableId }: RowMutationContext) return { deletedRowIds } }, onError: (error) => { - if (handleTableLockRejection(error, queryClient, tableId)) return + if (handleTableLockRejection(error, queryClient, tableId, { silent: suppressErrorToast })) + return if (isValidationError(error)) return + if (suppressErrorToast) return toast.error(error.message, { duration: 5000 }) }, onSettled: () => { From f0efedb433ac21e5a6912e1a081c220023c07ac5 Mon Sep 17 00:00:00 2001 From: Justin Blumencranz <96924014+j15z@users.noreply.github.com> Date: Tue, 15 Sep 2026 16:24:33 -0700 Subject: [PATCH 6/7] refactor(tables): drop the Table Security switch and its device-local store MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "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 --- .../lock-settings-modal.test.tsx | 102 +++++---------- .../lock-settings-modal.tsx | 122 ++++++++---------- apps/sim/stores/table/security/store.test.ts | 64 --------- apps/sim/stores/table/security/store.ts | 108 ---------------- 4 files changed, 86 insertions(+), 310 deletions(-) delete mode 100644 apps/sim/stores/table/security/store.test.ts delete mode 100644 apps/sim/stores/table/security/store.ts diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/lock-settings-modal/lock-settings-modal.test.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/lock-settings-modal/lock-settings-modal.test.tsx index 6389861fb12..2fa0ef96d9e 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/lock-settings-modal/lock-settings-modal.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/lock-settings-modal/lock-settings-modal.test.tsx @@ -6,7 +6,6 @@ import { createRoot, type Root } from 'react-dom/client' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { type TableLocks, UNLOCKED_TABLE_LOCKS } from '@/lib/table/types' import { LockSettingsModal } from '@/app/workspace/[workspaceId]/tables/[tableId]/components/lock-settings-modal/lock-settings-modal' -import { useTableSecurityStore } from '@/stores/table/security/store' const { mutateAsync } = vi.hoisted(() => ({ mutateAsync: vi.fn() })) vi.mock('@/hooks/queries/tables', () => ({ @@ -32,18 +31,6 @@ function render(locks: TableLocks = UNLOCKED_TABLE_LOCKS, isOpen = true) { }) } -function getSwitch(label: string): HTMLButtonElement { - const element = document.querySelector( - `button[role="switch"][aria-label="${label}"]` - ) - if (!element) throw new Error(`Missing switch: ${label}`) - return element -} - -function clickSwitch(label: string) { - act(() => getSwitch(label).click()) -} - function getPermission(label: string, choice: 'Deny' | 'Allow'): HTMLButtonElement { const group = document.querySelector(`[role="radiogroup"][aria-label="${label}"]`) const button = [ @@ -57,19 +44,22 @@ function selectPermission(label: string, choice: 'Deny' | 'Allow') { act(() => getPermission(label, choice).click()) } -function save() { +function getSave(): HTMLButtonElement { const button = [...document.querySelectorAll('button')].find( (element) => element.textContent === 'Save' ) if (!button) throw new Error('Missing Save button') - act(() => button.click()) + return button +} + +function save() { + act(() => getSave().click()) } beforeEach(() => { globalThis.IS_REACT_ACT_ENVIRONMENT = true vi.clearAllMocks() mutateAsync.mockReturnValue(new Promise(() => {})) - useTableSecurityStore.getState().reset() container = document.createElement('div') document.body.appendChild(container) root = createRoot(container) @@ -81,75 +71,51 @@ afterEach(() => { }) describe('Table Security', () => { - it('hides permissions while disabled and enables all four backend locks by default', () => { + it('always shows the four rows and starts an unconfigured table on Allow', () => { render() - expect(getSwitch('Enable Table Security').getAttribute('aria-checked')).toBe('false') - expect(document.querySelector('[role="radiogroup"]')).toBeNull() - - clickSwitch('Enable Table Security') for (const label of LABELS) { - expect(getPermission(label, 'Deny').disabled).toBe(false) - expect(getPermission(label, 'Allow').disabled).toBe(false) - expect(getPermission(label, 'Deny').getAttribute('aria-checked')).toBe('true') - expect(getPermission(label, 'Allow').getAttribute('aria-checked')).toBe('false') + expect(getPermission(label, 'Allow').getAttribute('aria-checked')).toBe('true') + expect(getPermission(label, 'Deny').getAttribute('aria-checked')).toBe('false') } - save() - - expect(mutateAsync.mock.calls[0][0]).toEqual({ - tableId: 'table-1', - locks: { insertLocked: true, updateLocked: true, deleteLocked: true, schemaLocked: true }, - }) + // Nothing staged yet, so there is nothing to save. + expect(getSave().disabled).toBe(true) }) - it('inverts existing locks and remembers permissions after disabling, saving, and reopening', async () => { - render({ insertLocked: true, updateLocked: true, deleteLocked: false, schemaLocked: true }) - expect(getSwitch('Enable Table Security').getAttribute('aria-checked')).toBe('true') - expect(getPermission('Deleting Rows', 'Allow').getAttribute('aria-checked')).toBe('true') - expect(getPermission('Updating Rows', 'Deny').getAttribute('aria-checked')).toBe('true') - - selectPermission('Inserting Rows', 'Allow') - clickSwitch('Enable Table Security') - expect(document.querySelector('[role="radiogroup"]')).toBeNull() - let resolveSave!: () => void - mutateAsync.mockReturnValueOnce( - new Promise((resolve) => { - resolveSave = resolve - }) + it('mirrors the server locks, with Deny meaning a set lock', () => { + render({ insertLocked: true, updateLocked: false, deleteLocked: true, schemaLocked: false }) + expect(getPermission('Inserting Rows', 'Deny').getAttribute('aria-checked')).toBe('true') + expect(getPermission('Deleting Rows', 'Deny').getAttribute('aria-checked')).toBe('true') + expect(getPermission('Updating Rows', 'Allow').getAttribute('aria-checked')).toBe('true') + expect(getPermission('Changing Table Schema', 'Allow').getAttribute('aria-checked')).toBe( + 'true' ) - save() - expect(mutateAsync.mock.calls[0][0]).toEqual({ - tableId: 'table-1', - locks: UNLOCKED_TABLE_LOCKS, - }) - await act(async () => resolveSave()) + }) - render(UNLOCKED_TABLE_LOCKS, false) + it('saves the denied actions as locks', () => { render() - expect(getSwitch('Enable Table Security').getAttribute('aria-checked')).toBe('false') - expect(document.querySelector('[role="radiogroup"]')).toBeNull() - clickSwitch('Enable Table Security') - expect(getPermission('Inserting Rows', 'Allow').getAttribute('aria-checked')).toBe('true') - expect(getPermission('Updating Rows', 'Deny').getAttribute('aria-checked')).toBe('true') + selectPermission('Inserting Rows', 'Deny') + selectPermission('Changing Table Schema', 'Deny') + expect(getSave().disabled).toBe(false) save() - expect(mutateAsync.mock.calls[1][0]).toEqual({ + + expect(mutateAsync.mock.calls[0][0]).toEqual({ tableId: 'table-1', - locks: { insertLocked: false, updateLocked: true, deleteLocked: false, schemaLocked: true }, + locks: { insertLocked: true, updateLocked: false, deleteLocked: false, schemaLocked: true }, }) }) - it('does not remember unsuccessful changes and discards them on reopen', () => { + it('keeps the modal open when the save fails and discards the draft on reopen', async () => { + mutateAsync.mockRejectedValueOnce(new Error('Admin access required to change table locks')) render() - clickSwitch('Enable Table Security') - selectPermission('Inserting Rows', 'Allow') - save() - expect(useTableSecurityStore.getState().preferences['table-1']).toBeUndefined() + selectPermission('Updating Rows', 'Deny') + await act(async () => { + getSave().click() + }) expect(onClose).not.toHaveBeenCalled() render(UNLOCKED_TABLE_LOCKS, false) render() - expect(getSwitch('Enable Table Security').getAttribute('aria-checked')).toBe('false') - expect(document.querySelector('[role="radiogroup"]')).toBeNull() - clickSwitch('Enable Table Security') - expect(getPermission('Inserting Rows', 'Deny').getAttribute('aria-checked')).toBe('true') + expect(getPermission('Updating Rows', 'Allow').getAttribute('aria-checked')).toBe('true') + expect(getSave().disabled).toBe(true) }) }) diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/lock-settings-modal/lock-settings-modal.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/lock-settings-modal/lock-settings-modal.tsx index 6b58f6914bc..aa9c56741ee 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/lock-settings-modal/lock-settings-modal.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/lock-settings-modal/lock-settings-modal.tsx @@ -9,19 +9,16 @@ import { ChipModalField, ChipModalFooter, ChipModalHeader, - Switch, Tooltip, } from '@sim/emcn' import { CircleInfo, Lock } from '@sim/emcn/icons' import type { TableLocks } from '@/lib/table/types' import { LOCK_FIELDS } from '@/app/workspace/[workspaceId]/tables/[tableId]/lock-copy' import { useUpdateTableLocks } from '@/hooks/queries/tables' -import { - getTableSecurityLocks, - getTableSecuritySettings, - tableSecuritySettingsEqual, - useTableSecurityStore, -} from '@/stores/table/security/store' + +function locksEqual(a: TableLocks, b: TableLocks): boolean { + return LOCK_FIELDS.every((field) => a[field.key] === b[field.key]) +} interface LockSettingsModalProps { isOpen: boolean @@ -32,10 +29,12 @@ interface LockSettingsModalProps { } /** - * Admin-only panel that sets a table's four mutation locks. Changes are staged - * locally and applied on Save (one request); the server re-checks admin and - * rejects a `write`-only caller with a 403 surfaced as a toast. Gated at the - * call site on `canAdmin`. + * Admin-only panel that sets a table's four mutation locks, one Allow/Deny row + * each. The rows mirror the server flags exactly — `Deny` is a set lock — so a + * table nobody has configured opens on four `Allow`s and every viewer sees the + * same state. Changes are staged locally and applied on Save (one request); the + * server re-checks admin and rejects a `write`-only caller with a 403 surfaced + * as a toast. Gated at the call site on `canAdmin`. */ export function LockSettingsModal({ isOpen, @@ -45,18 +44,16 @@ export function LockSettingsModal({ locks, }: LockSettingsModalProps) { const updateLocks = useUpdateTableLocks(workspaceId) - const preference = useTableSecurityStore((state) => state.preferences[tableId]) - const setPreference = useTableSecurityStore((state) => state.setPreference) - const settings = getTableSecuritySettings(locks, preference) - const [draft, setDraft] = useState(settings) + // Stage edits locally; reset to the server value each time the modal opens. + const [draft, setDraft] = useState(locks) const [prevOpen, setPrevOpen] = useState(isOpen) if (prevOpen !== isOpen) { setPrevOpen(isOpen) - if (isOpen) setDraft(settings) + if (isOpen) setDraft(locks) } - const dirty = !tableSecuritySettingsEqual(draft, settings) + const dirty = !locksEqual(draft, locks) const handleSave = async () => { if (!dirty) { @@ -64,11 +61,10 @@ export function LockSettingsModal({ return } try { - await updateLocks.mutateAsync({ tableId, locks: getTableSecurityLocks(draft) }) + await updateLocks.mutateAsync({ tableId, locks: draft }) } catch { return } - setPreference(tableId, draft) onClose() } @@ -78,59 +74,45 @@ export function LockSettingsModal({ Table Security - - setDraft((prev) => ({ ...prev, enabled }))} - /> - - {draft.enabled && - LOCK_FIELDS.map((field) => ( - - {field.label} - - - - - -

{field.hint}

- - - + {LOCK_FIELDS.map((field) => ( + + {field.label} + + {/* Not `asChild`: the hint is each row's only explanation, so + the trigger must be a focusable button for keyboard users. */} + + + + +

{field.hint}

+ + + + } + > + + setDraft((prev) => ({ ...prev, [field.key]: value === 'deny' })) } > - - setDraft((prev) => ({ - ...prev, - allowedActions: { ...prev.allowedActions, [field.kind]: value === 'allow' }, - })) - } - > - Deny - Allow - - - ))} + Deny + Allow + + + ))} useTableSecurityStore.getState().reset()) - -describe('table security preferences', () => { - it('uses current server locks over remembered local permissions', () => { - const serverLocks = { ...UNLOCKED_TABLE_LOCKS, insertLocked: true } - const settings = getTableSecuritySettings(serverLocks, PREFERENCE) - expect(settings.enabled).toBe(true) - expect(settings.allowedActions).toEqual({ - insert: false, - update: true, - delete: true, - schema: true, - }) - expect(getTableSecurityLocks(settings)).toEqual(serverLocks) - }) - - it('recognizes an external unlock even when the browser previously enabled restrictions', () => { - const settings = getTableSecuritySettings(UNLOCKED_TABLE_LOCKS, { - ...PREFERENCE, - enabled: true, - }) - expect(settings.enabled).toBe(false) - expect(settings.allowedActions).toEqual(PREFERENCE.allowedActions) - }) - - it('keeps security visibly enabled when all actions are allowed', () => { - const settings: TableSecuritySettings = { - enabled: true, - allowedActions: { insert: true, update: true, delete: true, schema: true }, - } - expect(getTableSecurityLocks(settings)).toEqual(UNLOCKED_TABLE_LOCKS) - expect(getTableSecuritySettings(UNLOCKED_TABLE_LOCKS, settings).enabled).toBe(true) - }) - - it('remembers disabled permissions across hydration without sharing them with another table', async () => { - useTableSecurityStore.getState().setPreference('table-1', PREFERENCE) - const saved = localStorage.getItem('table-security-preferences') - useTableSecurityStore.getState().reset() - localStorage.setItem('table-security-preferences', saved!) - await useTableSecurityStore.persist.rehydrate() - expect(useTableSecurityStore.getState().preferences['table-1']).toEqual(PREFERENCE) - expect(useTableSecurityStore.getState().preferences['table-2']).toBeUndefined() - expect(getTableSecurityLocks(useTableSecurityStore.getState().preferences['table-1'])).toEqual( - UNLOCKED_TABLE_LOCKS - ) - }) -}) diff --git a/apps/sim/stores/table/security/store.ts b/apps/sim/stores/table/security/store.ts deleted file mode 100644 index 401eb0d9cb9..00000000000 --- a/apps/sim/stores/table/security/store.ts +++ /dev/null @@ -1,108 +0,0 @@ -'use client' - -import { create } from 'zustand' -import { devtools, persist } from 'zustand/middleware' -import { BrowserStorage } from '@/lib/core/utils/browser-storage' -import { - TABLE_LOCK_FLAGS, - TABLE_LOCK_KINDS, - type TableLockKind, - type TableLocks, -} from '@/lib/table/types' -import { registerUserDataReset } from '@/stores/user-data-reset-registry' - -export interface TableSecuritySettings { - enabled: boolean - allowedActions: Record -} - -interface TableSecurityState { - preferences: Record - setPreference: (tableId: string, settings: TableSecuritySettings) => void - reset: () => void -} - -const DEFAULT_SETTINGS: TableSecuritySettings = { - enabled: false, - allowedActions: { insert: false, update: false, delete: false, schema: false }, -} - -/** Maps Table Security settings to the backend lock flags. */ -export function getTableSecurityLocks(settings: TableSecuritySettings): TableLocks { - return { - insertLocked: settings.enabled && !settings.allowedActions.insert, - updateLocked: settings.enabled && !settings.allowedActions.update, - deleteLocked: settings.enabled && !settings.allowedActions.delete, - schemaLocked: settings.enabled && !settings.allowedActions.schema, - } -} - -/** - * Server locks are authoritative. When none are set, the browser preference - * supplies the remembered per-action choices and distinguishes - * enabled-with-everything-allowed from disabled, which both use four false - * backend flags. - */ -export function getTableSecuritySettings( - locks: TableLocks, - preference?: TableSecuritySettings -): TableSecuritySettings { - if (TABLE_LOCK_KINDS.some((kind) => locks[TABLE_LOCK_FLAGS[kind]])) { - return { - enabled: true, - allowedActions: { - insert: !locks.insertLocked, - update: !locks.updateLocked, - delete: !locks.deleteLocked, - schema: !locks.schemaLocked, - }, - } - } - - if (!preference) return DEFAULT_SETTINGS - return { - enabled: - preference.enabled && TABLE_LOCK_KINDS.every((kind) => preference.allowedActions[kind]), - allowedActions: preference.allowedActions, - } -} - -export function tableSecuritySettingsEqual( - a: TableSecuritySettings, - b: TableSecuritySettings -): boolean { - return ( - a.enabled === b.enabled && - TABLE_LOCK_KINDS.every((kind) => a.allowedActions[kind] === b.allowedActions[kind]) - ) -} - -/** Device-local presentation preferences; actual locks are owned by React Query. */ -export const useTableSecurityStore = create()( - devtools( - persist( - (set) => ({ - preferences: {}, - setPreference: (tableId, settings) => - set((state) => ({ preferences: { ...state.preferences, [tableId]: settings } })), - reset: () => set({ preferences: {} }), - }), - { - name: 'table-security-preferences', - partialize: (state) => ({ preferences: state.preferences }), - storage: { - getItem: (name) => BrowserStorage.getItem(name, null), - setItem: (name, value) => { - BrowserStorage.setItem(name, value) - }, - removeItem: (name) => { - BrowserStorage.removeItem(name) - }, - }, - } - ), - { name: 'table-security-preferences' } - ) -) - -registerUserDataReset('table-security-preferences', () => useTableSecurityStore.getState().reset()) From 4e99e4c4f477d77661217c0df683b2601133e583 Mon Sep 17 00:00:00 2001 From: Justin Blumencranz <96924014+j15z@users.noreply.github.com> Date: Tue, 15 Sep 2026 16:50:27 -0700 Subject: [PATCH 7/7] fix(tables): stage only the Table Security rows an admin moves MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../lock-settings-modal.test.tsx | 31 +++++++++++++- .../lock-settings-modal.tsx | 41 +++++++++++++------ 2 files changed, 58 insertions(+), 14 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/lock-settings-modal/lock-settings-modal.test.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/lock-settings-modal/lock-settings-modal.test.tsx index 2fa0ef96d9e..029a34b05af 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/lock-settings-modal/lock-settings-modal.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/lock-settings-modal/lock-settings-modal.test.tsx @@ -91,19 +91,46 @@ describe('Table Security', () => { ) }) - it('saves the denied actions as locks', () => { + it('saves only the rows the admin moved', () => { render() selectPermission('Inserting Rows', 'Deny') selectPermission('Changing Table Schema', 'Deny') expect(getSave().disabled).toBe(false) save() + // A partial patch: the untouched rows are absent, so a concurrent change to + // one of them survives this save. expect(mutateAsync.mock.calls[0][0]).toEqual({ tableId: 'table-1', - locks: { insertLocked: true, updateLocked: false, deleteLocked: false, schemaLocked: true }, + locks: { insertLocked: true, schemaLocked: true }, }) }) + it('follows a lock changed elsewhere while open without staging it', () => { + render() + selectPermission('Inserting Rows', 'Deny') + + // Another admin denies updates while this modal is open; the realtime + // refetch lands as a new `locks` prop. + render({ ...UNLOCKED_TABLE_LOCKS, updateLocked: true }) + expect(getPermission('Updating Rows', 'Deny').getAttribute('aria-checked')).toBe('true') + expect(getPermission('Inserting Rows', 'Deny').getAttribute('aria-checked')).toBe('true') + + save() + expect(mutateAsync.mock.calls[0][0]).toEqual({ + tableId: 'table-1', + locks: { insertLocked: true }, + }) + }) + + it('treats a row already matching the server as nothing to save', () => { + render({ ...UNLOCKED_TABLE_LOCKS, deleteLocked: true }) + selectPermission('Deleting Rows', 'Allow') + expect(getSave().disabled).toBe(false) + selectPermission('Deleting Rows', 'Deny') + expect(getSave().disabled).toBe(true) + }) + it('keeps the modal open when the save fails and discards the draft on reopen', async () => { mutateAsync.mockRejectedValueOnce(new Error('Admin access required to change table locks')) render() diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/lock-settings-modal/lock-settings-modal.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/lock-settings-modal/lock-settings-modal.tsx index aa9c56741ee..e72cdce34ad 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/lock-settings-modal/lock-settings-modal.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/lock-settings-modal/lock-settings-modal.tsx @@ -16,8 +16,17 @@ import type { TableLocks } from '@/lib/table/types' import { LOCK_FIELDS } from '@/app/workspace/[workspaceId]/tables/[tableId]/lock-copy' import { useUpdateTableLocks } from '@/hooks/queries/tables' -function locksEqual(a: TableLocks, b: TableLocks): boolean { - return LOCK_FIELDS.every((field) => a[field.key] === b[field.key]) +/** + * The rows the admin actually moved, relative to the locks the server holds + * right now. Everything absent from this patch is left alone by the save. + */ +function changedLocks(overrides: Partial, locks: TableLocks): Partial { + const changed: Partial = {} + for (const field of LOCK_FIELDS) { + const next = overrides[field.key] + if (next !== undefined && next !== locks[field.key]) changed[field.key] = next + } + return changed } interface LockSettingsModalProps { @@ -32,9 +41,16 @@ interface LockSettingsModalProps { * Admin-only panel that sets a table's four mutation locks, one Allow/Deny row * each. The rows mirror the server flags exactly — `Deny` is a set lock — so a * table nobody has configured opens on four `Allow`s and every viewer sees the - * same state. Changes are staged locally and applied on Save (one request); the - * server re-checks admin and rejects a `write`-only caller with a 403 surfaced - * as a toast. Gated at the call site on `canAdmin`. + * same state. + * + * Only the rows this admin moved are staged; every other row keeps rendering + * the authoritative value, so a lock another admin changes while this modal is + * open shows up here instead of going stale behind it. Save sends just that + * patch (the route takes a partial), so it can't carry a stale flag over + * someone else's newer change — a row both admins moved is the only real + * conflict, and there this admin's explicit choice wins. The server re-checks + * admin and rejects a `write`-only caller with a 403 surfaced as a toast. + * Gated at the call site on `canAdmin`. */ export function LockSettingsModal({ isOpen, @@ -45,15 +61,16 @@ export function LockSettingsModal({ }: LockSettingsModalProps) { const updateLocks = useUpdateTableLocks(workspaceId) - // Stage edits locally; reset to the server value each time the modal opens. - const [draft, setDraft] = useState(locks) + // Stage only the rows this admin moved; clear them each time the modal opens. + const [overrides, setOverrides] = useState>({}) const [prevOpen, setPrevOpen] = useState(isOpen) if (prevOpen !== isOpen) { setPrevOpen(isOpen) - if (isOpen) setDraft(locks) + if (isOpen) setOverrides({}) } - const dirty = !locksEqual(draft, locks) + const changed = changedLocks(overrides, locks) + const dirty = Object.keys(changed).length > 0 const handleSave = async () => { if (!dirty) { @@ -61,7 +78,7 @@ export function LockSettingsModal({ return } try { - await updateLocks.mutateAsync({ tableId, locks: draft }) + await updateLocks.mutateAsync({ tableId, locks: changed }) } catch { return } @@ -102,10 +119,10 @@ export function LockSettingsModal({ - setDraft((prev) => ({ ...prev, [field.key]: value === 'deny' })) + setOverrides((prev) => ({ ...prev, [field.key]: value === 'deny' })) } > Deny