Skip to content

Commit f0efedb

Browse files
j15zclaude
andcommitted
refactor(tables): drop the Table Security switch and its device-local store
"Enable Table Security" had no server representation: enabled-with-everything- allowed and never-configured both save four `false` flags, so the difference lived only in the browser that set it. Anyone else — another device, another admin — saw the table as unconfigured, and the per-action choices behind the switch were remembered per device too. The modal now always shows the four Allow/Deny rows, mapped one-to-one onto the server flags, so an unconfigured table opens on four `Allow`s and every viewer sees the same state. That removes the reason for the preference store, which is deleted along with its helpers and test. Stale `table-security-preferences` keys are left where they are; nothing reads them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent a555ae0 commit f0efedb

4 files changed

Lines changed: 86 additions & 310 deletions

File tree

‎apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/lock-settings-modal/lock-settings-modal.test.tsx‎

Lines changed: 34 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import { createRoot, type Root } from 'react-dom/client'
66
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
77
import { type TableLocks, UNLOCKED_TABLE_LOCKS } from '@/lib/table/types'
88
import { LockSettingsModal } from '@/app/workspace/[workspaceId]/tables/[tableId]/components/lock-settings-modal/lock-settings-modal'
9-
import { useTableSecurityStore } from '@/stores/table/security/store'
109

1110
const { mutateAsync } = vi.hoisted(() => ({ mutateAsync: vi.fn() }))
1211
vi.mock('@/hooks/queries/tables', () => ({
@@ -32,18 +31,6 @@ function render(locks: TableLocks = UNLOCKED_TABLE_LOCKS, isOpen = true) {
3231
})
3332
}
3433

35-
function getSwitch(label: string): HTMLButtonElement {
36-
const element = document.querySelector<HTMLButtonElement>(
37-
`button[role="switch"][aria-label="${label}"]`
38-
)
39-
if (!element) throw new Error(`Missing switch: ${label}`)
40-
return element
41-
}
42-
43-
function clickSwitch(label: string) {
44-
act(() => getSwitch(label).click())
45-
}
46-
4734
function getPermission(label: string, choice: 'Deny' | 'Allow'): HTMLButtonElement {
4835
const group = document.querySelector(`[role="radiogroup"][aria-label="${label}"]`)
4936
const button = [
@@ -57,19 +44,22 @@ function selectPermission(label: string, choice: 'Deny' | 'Allow') {
5744
act(() => getPermission(label, choice).click())
5845
}
5946

60-
function save() {
47+
function getSave(): HTMLButtonElement {
6148
const button = [...document.querySelectorAll<HTMLButtonElement>('button')].find(
6249
(element) => element.textContent === 'Save'
6350
)
6451
if (!button) throw new Error('Missing Save button')
65-
act(() => button.click())
52+
return button
53+
}
54+
55+
function save() {
56+
act(() => getSave().click())
6657
}
6758

6859
beforeEach(() => {
6960
globalThis.IS_REACT_ACT_ENVIRONMENT = true
7061
vi.clearAllMocks()
7162
mutateAsync.mockReturnValue(new Promise(() => {}))
72-
useTableSecurityStore.getState().reset()
7363
container = document.createElement('div')
7464
document.body.appendChild(container)
7565
root = createRoot(container)
@@ -81,75 +71,51 @@ afterEach(() => {
8171
})
8272

8373
describe('Table Security', () => {
84-
it('hides permissions while disabled and enables all four backend locks by default', () => {
74+
it('always shows the four rows and starts an unconfigured table on Allow', () => {
8575
render()
86-
expect(getSwitch('Enable Table Security').getAttribute('aria-checked')).toBe('false')
87-
expect(document.querySelector('[role="radiogroup"]')).toBeNull()
88-
89-
clickSwitch('Enable Table Security')
9076
for (const label of LABELS) {
91-
expect(getPermission(label, 'Deny').disabled).toBe(false)
92-
expect(getPermission(label, 'Allow').disabled).toBe(false)
93-
expect(getPermission(label, 'Deny').getAttribute('aria-checked')).toBe('true')
94-
expect(getPermission(label, 'Allow').getAttribute('aria-checked')).toBe('false')
77+
expect(getPermission(label, 'Allow').getAttribute('aria-checked')).toBe('true')
78+
expect(getPermission(label, 'Deny').getAttribute('aria-checked')).toBe('false')
9579
}
96-
save()
97-
98-
expect(mutateAsync.mock.calls[0][0]).toEqual({
99-
tableId: 'table-1',
100-
locks: { insertLocked: true, updateLocked: true, deleteLocked: true, schemaLocked: true },
101-
})
80+
// Nothing staged yet, so there is nothing to save.
81+
expect(getSave().disabled).toBe(true)
10282
})
10383

104-
it('inverts existing locks and remembers permissions after disabling, saving, and reopening', async () => {
105-
render({ insertLocked: true, updateLocked: true, deleteLocked: false, schemaLocked: true })
106-
expect(getSwitch('Enable Table Security').getAttribute('aria-checked')).toBe('true')
107-
expect(getPermission('Deleting Rows', 'Allow').getAttribute('aria-checked')).toBe('true')
108-
expect(getPermission('Updating Rows', 'Deny').getAttribute('aria-checked')).toBe('true')
109-
110-
selectPermission('Inserting Rows', 'Allow')
111-
clickSwitch('Enable Table Security')
112-
expect(document.querySelector('[role="radiogroup"]')).toBeNull()
113-
let resolveSave!: () => void
114-
mutateAsync.mockReturnValueOnce(
115-
new Promise<void>((resolve) => {
116-
resolveSave = resolve
117-
})
84+
it('mirrors the server locks, with Deny meaning a set lock', () => {
85+
render({ insertLocked: true, updateLocked: false, deleteLocked: true, schemaLocked: false })
86+
expect(getPermission('Inserting Rows', 'Deny').getAttribute('aria-checked')).toBe('true')
87+
expect(getPermission('Deleting Rows', 'Deny').getAttribute('aria-checked')).toBe('true')
88+
expect(getPermission('Updating Rows', 'Allow').getAttribute('aria-checked')).toBe('true')
89+
expect(getPermission('Changing Table Schema', 'Allow').getAttribute('aria-checked')).toBe(
90+
'true'
11891
)
119-
save()
120-
expect(mutateAsync.mock.calls[0][0]).toEqual({
121-
tableId: 'table-1',
122-
locks: UNLOCKED_TABLE_LOCKS,
123-
})
124-
await act(async () => resolveSave())
92+
})
12593

126-
render(UNLOCKED_TABLE_LOCKS, false)
94+
it('saves the denied actions as locks', () => {
12795
render()
128-
expect(getSwitch('Enable Table Security').getAttribute('aria-checked')).toBe('false')
129-
expect(document.querySelector('[role="radiogroup"]')).toBeNull()
130-
clickSwitch('Enable Table Security')
131-
expect(getPermission('Inserting Rows', 'Allow').getAttribute('aria-checked')).toBe('true')
132-
expect(getPermission('Updating Rows', 'Deny').getAttribute('aria-checked')).toBe('true')
96+
selectPermission('Inserting Rows', 'Deny')
97+
selectPermission('Changing Table Schema', 'Deny')
98+
expect(getSave().disabled).toBe(false)
13399
save()
134-
expect(mutateAsync.mock.calls[1][0]).toEqual({
100+
101+
expect(mutateAsync.mock.calls[0][0]).toEqual({
135102
tableId: 'table-1',
136-
locks: { insertLocked: false, updateLocked: true, deleteLocked: false, schemaLocked: true },
103+
locks: { insertLocked: true, updateLocked: false, deleteLocked: false, schemaLocked: true },
137104
})
138105
})
139106

140-
it('does not remember unsuccessful changes and discards them on reopen', () => {
107+
it('keeps the modal open when the save fails and discards the draft on reopen', async () => {
108+
mutateAsync.mockRejectedValueOnce(new Error('Admin access required to change table locks'))
141109
render()
142-
clickSwitch('Enable Table Security')
143-
selectPermission('Inserting Rows', 'Allow')
144-
save()
145-
expect(useTableSecurityStore.getState().preferences['table-1']).toBeUndefined()
110+
selectPermission('Updating Rows', 'Deny')
111+
await act(async () => {
112+
getSave().click()
113+
})
146114
expect(onClose).not.toHaveBeenCalled()
147115

148116
render(UNLOCKED_TABLE_LOCKS, false)
149117
render()
150-
expect(getSwitch('Enable Table Security').getAttribute('aria-checked')).toBe('false')
151-
expect(document.querySelector('[role="radiogroup"]')).toBeNull()
152-
clickSwitch('Enable Table Security')
153-
expect(getPermission('Inserting Rows', 'Deny').getAttribute('aria-checked')).toBe('true')
118+
expect(getPermission('Updating Rows', 'Allow').getAttribute('aria-checked')).toBe('true')
119+
expect(getSave().disabled).toBe(true)
154120
})
155121
})

‎apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/lock-settings-modal/lock-settings-modal.tsx‎

Lines changed: 52 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -9,19 +9,16 @@ import {
99
ChipModalField,
1010
ChipModalFooter,
1111
ChipModalHeader,
12-
Switch,
1312
Tooltip,
1413
} from '@sim/emcn'
1514
import { CircleInfo, Lock } from '@sim/emcn/icons'
1615
import type { TableLocks } from '@/lib/table/types'
1716
import { LOCK_FIELDS } from '@/app/workspace/[workspaceId]/tables/[tableId]/lock-copy'
1817
import { useUpdateTableLocks } from '@/hooks/queries/tables'
19-
import {
20-
getTableSecurityLocks,
21-
getTableSecuritySettings,
22-
tableSecuritySettingsEqual,
23-
useTableSecurityStore,
24-
} from '@/stores/table/security/store'
18+
19+
function locksEqual(a: TableLocks, b: TableLocks): boolean {
20+
return LOCK_FIELDS.every((field) => a[field.key] === b[field.key])
21+
}
2522

2623
interface LockSettingsModalProps {
2724
isOpen: boolean
@@ -32,10 +29,12 @@ interface LockSettingsModalProps {
3229
}
3330

3431
/**
35-
* Admin-only panel that sets a table's four mutation locks. Changes are staged
36-
* locally and applied on Save (one request); the server re-checks admin and
37-
* rejects a `write`-only caller with a 403 surfaced as a toast. Gated at the
38-
* call site on `canAdmin`.
32+
* Admin-only panel that sets a table's four mutation locks, one Allow/Deny row
33+
* each. The rows mirror the server flags exactly — `Deny` is a set lock — so a
34+
* table nobody has configured opens on four `Allow`s and every viewer sees the
35+
* same state. Changes are staged locally and applied on Save (one request); the
36+
* server re-checks admin and rejects a `write`-only caller with a 403 surfaced
37+
* as a toast. Gated at the call site on `canAdmin`.
3938
*/
4039
export function LockSettingsModal({
4140
isOpen,
@@ -45,30 +44,27 @@ export function LockSettingsModal({
4544
locks,
4645
}: LockSettingsModalProps) {
4746
const updateLocks = useUpdateTableLocks(workspaceId)
48-
const preference = useTableSecurityStore((state) => state.preferences[tableId])
49-
const setPreference = useTableSecurityStore((state) => state.setPreference)
50-
const settings = getTableSecuritySettings(locks, preference)
5147

52-
const [draft, setDraft] = useState(settings)
48+
// Stage edits locally; reset to the server value each time the modal opens.
49+
const [draft, setDraft] = useState<TableLocks>(locks)
5350
const [prevOpen, setPrevOpen] = useState(isOpen)
5451
if (prevOpen !== isOpen) {
5552
setPrevOpen(isOpen)
56-
if (isOpen) setDraft(settings)
53+
if (isOpen) setDraft(locks)
5754
}
5855

59-
const dirty = !tableSecuritySettingsEqual(draft, settings)
56+
const dirty = !locksEqual(draft, locks)
6057

6158
const handleSave = async () => {
6259
if (!dirty) {
6360
onClose()
6461
return
6562
}
6663
try {
67-
await updateLocks.mutateAsync({ tableId, locks: getTableSecurityLocks(draft) })
64+
await updateLocks.mutateAsync({ tableId, locks: draft })
6865
} catch {
6966
return
7067
}
71-
setPreference(tableId, draft)
7268
onClose()
7369
}
7470

@@ -78,59 +74,45 @@ export function LockSettingsModal({
7874
Table Security
7975
</ChipModalHeader>
8076
<ChipModalBody>
81-
<ChipModalField
82-
type='custom'
83-
title='Enable Table Security'
84-
className='flex-row items-center justify-between'
85-
>
86-
<Switch
87-
aria-label='Enable Table Security'
88-
checked={draft.enabled}
89-
disabled={updateLocks.isPending}
90-
onCheckedChange={(enabled) => setDraft((prev) => ({ ...prev, enabled }))}
91-
/>
92-
</ChipModalField>
93-
{draft.enabled &&
94-
LOCK_FIELDS.map((field) => (
95-
<ChipModalField
96-
key={field.key}
97-
type='custom'
98-
className='flex-row items-center justify-between'
99-
title={
100-
<span className='inline-flex items-center gap-1.5'>
101-
{field.label}
102-
<Tooltip.Root>
103-
<Tooltip.Trigger
104-
type='button'
105-
aria-label={`About ${field.label.toLowerCase()}`}
106-
className='inline-flex cursor-help'
107-
>
108-
<CircleInfo className='size-[14px] text-[var(--text-icon)]' />
109-
</Tooltip.Trigger>
110-
<Tooltip.Content>
111-
<p>{field.hint}</p>
112-
</Tooltip.Content>
113-
</Tooltip.Root>
114-
</span>
77+
{LOCK_FIELDS.map((field) => (
78+
<ChipModalField
79+
key={field.key}
80+
type='custom'
81+
className='flex-row items-center justify-between'
82+
title={
83+
<span className='inline-flex items-center gap-1.5'>
84+
{field.label}
85+
<Tooltip.Root>
86+
{/* Not `asChild`: the hint is each row's only explanation, so
87+
the trigger must be a focusable button for keyboard users. */}
88+
<Tooltip.Trigger
89+
type='button'
90+
aria-label={`About ${field.label.toLowerCase()}`}
91+
className='inline-flex cursor-help'
92+
>
93+
<CircleInfo className='size-[14px] text-[var(--text-icon)]' />
94+
</Tooltip.Trigger>
95+
<Tooltip.Content>
96+
<p>{field.hint}</p>
97+
</Tooltip.Content>
98+
</Tooltip.Root>
99+
</span>
100+
}
101+
>
102+
<ChipButtonGroup
103+
aria-label={field.label}
104+
className='shrink-0'
105+
value={draft[field.key] ? 'deny' : 'allow'}
106+
disabled={updateLocks.isPending}
107+
onValueChange={(value) =>
108+
setDraft((prev) => ({ ...prev, [field.key]: value === 'deny' }))
115109
}
116110
>
117-
<ChipButtonGroup
118-
aria-label={field.label}
119-
className='shrink-0'
120-
value={draft.allowedActions[field.kind] ? 'allow' : 'deny'}
121-
disabled={updateLocks.isPending}
122-
onValueChange={(value) =>
123-
setDraft((prev) => ({
124-
...prev,
125-
allowedActions: { ...prev.allowedActions, [field.kind]: value === 'allow' },
126-
}))
127-
}
128-
>
129-
<ChipButtonGroupItem value='deny'>Deny</ChipButtonGroupItem>
130-
<ChipButtonGroupItem value='allow'>Allow</ChipButtonGroupItem>
131-
</ChipButtonGroup>
132-
</ChipModalField>
133-
))}
111+
<ChipButtonGroupItem value='deny'>Deny</ChipButtonGroupItem>
112+
<ChipButtonGroupItem value='allow'>Allow</ChipButtonGroupItem>
113+
</ChipButtonGroup>
114+
</ChipModalField>
115+
))}
134116
</ChipModalBody>
135117
<ChipModalFooter
136118
onCancel={onClose}

‎apps/sim/stores/table/security/store.test.ts‎

Lines changed: 0 additions & 64 deletions
This file was deleted.

0 commit comments

Comments
 (0)