Skip to content

Commit 4e99e4c

Browse files
j15zclaude
andcommitted
fix(tables): stage only the Table Security rows an admin moves
The modal reset its draft only when it opened, so a lock another admin changed while it sat open went stale behind it: the controls kept rendering the old values and Save submitted all four flags, overwriting the newer state. Only the rows this admin moves are staged now. Every other row keeps rendering the authoritative value, so a concurrent change shows up in the open modal instead of hiding behind it, and Save sends just that patch — the route already takes a partial — so an untouched row can't carry a stale flag over someone else's change. A row both admins moved is the one real conflict, and there the explicit choice wins. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent f0efedb commit 4e99e4c

2 files changed

Lines changed: 58 additions & 14 deletions

File tree

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

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,19 +91,46 @@ describe('Table Security', () => {
9191
)
9292
})
9393

94-
it('saves the denied actions as locks', () => {
94+
it('saves only the rows the admin moved', () => {
9595
render()
9696
selectPermission('Inserting Rows', 'Deny')
9797
selectPermission('Changing Table Schema', 'Deny')
9898
expect(getSave().disabled).toBe(false)
9999
save()
100100

101+
// A partial patch: the untouched rows are absent, so a concurrent change to
102+
// one of them survives this save.
101103
expect(mutateAsync.mock.calls[0][0]).toEqual({
102104
tableId: 'table-1',
103-
locks: { insertLocked: true, updateLocked: false, deleteLocked: false, schemaLocked: true },
105+
locks: { insertLocked: true, schemaLocked: true },
104106
})
105107
})
106108

109+
it('follows a lock changed elsewhere while open without staging it', () => {
110+
render()
111+
selectPermission('Inserting Rows', 'Deny')
112+
113+
// Another admin denies updates while this modal is open; the realtime
114+
// refetch lands as a new `locks` prop.
115+
render({ ...UNLOCKED_TABLE_LOCKS, updateLocked: true })
116+
expect(getPermission('Updating Rows', 'Deny').getAttribute('aria-checked')).toBe('true')
117+
expect(getPermission('Inserting Rows', 'Deny').getAttribute('aria-checked')).toBe('true')
118+
119+
save()
120+
expect(mutateAsync.mock.calls[0][0]).toEqual({
121+
tableId: 'table-1',
122+
locks: { insertLocked: true },
123+
})
124+
})
125+
126+
it('treats a row already matching the server as nothing to save', () => {
127+
render({ ...UNLOCKED_TABLE_LOCKS, deleteLocked: true })
128+
selectPermission('Deleting Rows', 'Allow')
129+
expect(getSave().disabled).toBe(false)
130+
selectPermission('Deleting Rows', 'Deny')
131+
expect(getSave().disabled).toBe(true)
132+
})
133+
107134
it('keeps the modal open when the save fails and discards the draft on reopen', async () => {
108135
mutateAsync.mockRejectedValueOnce(new Error('Admin access required to change table locks'))
109136
render()

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

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,17 @@ import type { TableLocks } from '@/lib/table/types'
1616
import { LOCK_FIELDS } from '@/app/workspace/[workspaceId]/tables/[tableId]/lock-copy'
1717
import { useUpdateTableLocks } from '@/hooks/queries/tables'
1818

19-
function locksEqual(a: TableLocks, b: TableLocks): boolean {
20-
return LOCK_FIELDS.every((field) => a[field.key] === b[field.key])
19+
/**
20+
* The rows the admin actually moved, relative to the locks the server holds
21+
* right now. Everything absent from this patch is left alone by the save.
22+
*/
23+
function changedLocks(overrides: Partial<TableLocks>, locks: TableLocks): Partial<TableLocks> {
24+
const changed: Partial<TableLocks> = {}
25+
for (const field of LOCK_FIELDS) {
26+
const next = overrides[field.key]
27+
if (next !== undefined && next !== locks[field.key]) changed[field.key] = next
28+
}
29+
return changed
2130
}
2231

2332
interface LockSettingsModalProps {
@@ -32,9 +41,16 @@ interface LockSettingsModalProps {
3241
* Admin-only panel that sets a table's four mutation locks, one Allow/Deny row
3342
* each. The rows mirror the server flags exactly — `Deny` is a set lock — so a
3443
* 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`.
44+
* same state.
45+
*
46+
* Only the rows this admin moved are staged; every other row keeps rendering
47+
* the authoritative value, so a lock another admin changes while this modal is
48+
* open shows up here instead of going stale behind it. Save sends just that
49+
* patch (the route takes a partial), so it can't carry a stale flag over
50+
* someone else's newer change — a row both admins moved is the only real
51+
* conflict, and there this admin's explicit choice wins. The server re-checks
52+
* admin and rejects a `write`-only caller with a 403 surfaced as a toast.
53+
* Gated at the call site on `canAdmin`.
3854
*/
3955
export function LockSettingsModal({
4056
isOpen,
@@ -45,23 +61,24 @@ export function LockSettingsModal({
4561
}: LockSettingsModalProps) {
4662
const updateLocks = useUpdateTableLocks(workspaceId)
4763

48-
// Stage edits locally; reset to the server value each time the modal opens.
49-
const [draft, setDraft] = useState<TableLocks>(locks)
64+
// Stage only the rows this admin moved; clear them each time the modal opens.
65+
const [overrides, setOverrides] = useState<Partial<TableLocks>>({})
5066
const [prevOpen, setPrevOpen] = useState(isOpen)
5167
if (prevOpen !== isOpen) {
5268
setPrevOpen(isOpen)
53-
if (isOpen) setDraft(locks)
69+
if (isOpen) setOverrides({})
5470
}
5571

56-
const dirty = !locksEqual(draft, locks)
72+
const changed = changedLocks(overrides, locks)
73+
const dirty = Object.keys(changed).length > 0
5774

5875
const handleSave = async () => {
5976
if (!dirty) {
6077
onClose()
6178
return
6279
}
6380
try {
64-
await updateLocks.mutateAsync({ tableId, locks: draft })
81+
await updateLocks.mutateAsync({ tableId, locks: changed })
6582
} catch {
6683
return
6784
}
@@ -102,10 +119,10 @@ export function LockSettingsModal({
102119
<ChipButtonGroup
103120
aria-label={field.label}
104121
className='shrink-0'
105-
value={draft[field.key] ? 'deny' : 'allow'}
122+
value={(overrides[field.key] ?? locks[field.key]) ? 'deny' : 'allow'}
106123
disabled={updateLocks.isPending}
107124
onValueChange={(value) =>
108-
setDraft((prev) => ({ ...prev, [field.key]: value === 'deny' }))
125+
setOverrides((prev) => ({ ...prev, [field.key]: value === 'deny' }))
109126
}
110127
>
111128
<ChipButtonGroupItem value='deny'>Deny</ChipButtonGroupItem>

0 commit comments

Comments
 (0)