Fix self-inflicted edit conflicts: dialogs no longer pin a stale node version
Properties, Raw JSON and AI proposals sent the version captured when the dialog opened. Any other edit to the node meanwhile (style chip, drag handle, inline text, child insert/move) bumped it, so the save 409'd against the user's own work. Save through ctrl.saveData instead: compare only the keys being written against the current node (a real conflict is a key someone else changed) and send no version. 409 toasts now show the server's reason. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
3fdf415847
commit
d3fb060016
5 files changed
+55
-8
No files matched your search
+1
-1
@@ -201,5 +201,5 @@ export async function applyProposal(ctrl: Controller, p: Proposal): Promise<bool
|
||||
if (p.kind === 'remove') return (await ctrl.remove(p.nodeId)) !== null
|
||||
const node = await ctrl.node(p.nodeId, true)
|
||||
const patch = patchForPath(node.data ?? {}, parsePath(p.path), p.after)
|
||||
return (await ctrl.setData(node.id, patch, { version: node.version })) !== null
|
||||
return (await ctrl.saveData(node.id, node.data ?? {}, patch)) !== null
|
||||
}
|
||||
+26
-1
@@ -615,6 +615,31 @@ export class Controller {
|
||||
return this.apply([op], { coalesce: opts.coalesce })
|
||||
}
|
||||
|
||||
/**
|
||||
* Save a dialog's patch. `base` is the data the dialog started from, `patch` the keys it changed.
|
||||
*
|
||||
* Do NOT send the dialog's `node.version`: every other edit to the node (style chips, drag handles,
|
||||
* inline text, moving/inserting a child) bumps it while the dialog is open, so the save would 409
|
||||
* against the user's own earlier edits. Instead compare per key against the *current* node: only a
|
||||
* key someone changed since the dialog opened is a real conflict. `set` merges top-level keys, so
|
||||
* the untouched keys are safe to leave alone.
|
||||
*/
|
||||
async saveData(id: string, base: Record<string, unknown>, patch: Record<string, unknown>) {
|
||||
let fresh: CmsNode
|
||||
try {
|
||||
fresh = await this.node(id, true)
|
||||
} catch (e) {
|
||||
this.fail(e)
|
||||
return null
|
||||
}
|
||||
const clash = Object.keys(patch).filter((k) => JSON.stringify(fresh.data?.[k] ?? null) !== JSON.stringify(base[k] ?? null))
|
||||
if (clash.length) {
|
||||
this.toast('error', `Someone else changed ${clash.join(', ')} - reopen it and retry`)
|
||||
return null
|
||||
}
|
||||
return this.setData(id, patch)
|
||||
}
|
||||
|
||||
async insert(parent: string, type: string, opts: { index?: number; handle?: string; data?: Record<string, unknown> } = {}) {
|
||||
const res = await this.apply([{ op: 'insert', parent, type, ...opts }])
|
||||
const created = res?.results[0]?.node
|
||||
@@ -1111,7 +1136,7 @@ export class Controller {
|
||||
const msg =
|
||||
e instanceof EditError || e instanceof ApiError
|
||||
? e.status === 409
|
||||
? 'Someone else changed this node - reopen it and retry'
|
||||
? `Conflict: ${e.message} - reopen it and retry`
|
||||
: e.status === 401 || e.status === 403
|
||||
? `Not allowed (${e.status}): ${e.message}`
|
||||
: e.message
|
||||
|
||||
@@ -27,7 +27,7 @@ function Properties({ node, schema, ctrl, close }: EditorProps) {
|
||||
const changed = Object.keys(patch).length > 0
|
||||
const save = async () => {
|
||||
setSaving(true)
|
||||
const res = await ctrl.setData(node.id, patch, { version: node.version })
|
||||
const res = await ctrl.saveData(node.id, node.data ?? {}, patch)
|
||||
setSaving(false)
|
||||
if (res) close()
|
||||
}
|
||||
@@ -70,7 +70,7 @@ function RawJson({ node, ctrl, close }: EditorProps) {
|
||||
<JsonArea value={node.data ?? {}} onChange={setDraft} />
|
||||
{Object.keys(patch).length > 0 && (
|
||||
<footer className="cms-actions">
|
||||
<button className="cms-btn cms-btn--primary" onClick={async () => (await ctrl.setData(node.id, patch, { version: node.version })) && close()}>
|
||||
<button className="cms-btn cms-btn--primary" onClick={async () => (await ctrl.saveData(node.id, node.data ?? {}, patch)) && close()}>
|
||||
<Icon name="save" size={16} /> Save
|
||||
</button>
|
||||
</footer>
|
||||
|
||||
@@ -8,8 +8,10 @@ const node = { id: 'n1', dataType: 'hero', version: 3, data: { title: 'Hello wro
|
||||
|
||||
function fakeCtrl() {
|
||||
const setData = vi.fn(async () => ({}))
|
||||
const saveData = vi.fn(async () => ({}))
|
||||
return {
|
||||
setData,
|
||||
saveData,
|
||||
ctrl: {
|
||||
store: { get: () => ({ selected: 'n1', blocks: [{ id: 'n1', type: 'hero', depth: 0 }] }) },
|
||||
block: (id: string) => (id === 'n1' ? { id } : undefined),
|
||||
@@ -19,6 +21,7 @@ function fakeCtrl() {
|
||||
schema: async () => schema,
|
||||
labelFor: () => 'Hero',
|
||||
setData,
|
||||
saveData,
|
||||
} as any,
|
||||
}
|
||||
}
|
||||
@@ -80,10 +83,10 @@ describe('runAgent + tools', () => {
|
||||
})
|
||||
|
||||
describe('applyProposal', () => {
|
||||
it('rebuilds the nested patch from the fresh node and saves with its version', async () => {
|
||||
const { ctrl, setData } = fakeCtrl()
|
||||
it('rebuilds the nested patch from the fresh node and saves it through the key-level conflict check', async () => {
|
||||
const { ctrl, saveData } = fakeCtrl()
|
||||
await applyProposal(ctrl, { id: 1, kind: 'set', nodeId: 'n1', label: 'Hero', path: 'links.0.text', before: 'a', after: 'b' })
|
||||
expect(setData).toHaveBeenCalledWith('n1', { links: [{ text: 'b' }] }, { version: 3 })
|
||||
expect(saveData).toHaveBeenCalledWith('n1', node.data, { links: [{ text: 'b' }] })
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
+20
-1
@@ -100,7 +100,7 @@ describe('on-page editor against the mock CMS', () => {
|
||||
expect(h.cms.graph.node('s1').data).toMatchObject({ title: 'Hello', tone: 'dark', padded: true })
|
||||
expect(byId('s1').querySelector('.sec__title')!.textContent).toBe('Hello')
|
||||
const edit = h.editor.ctrl.store.get().log.find((l) => l.url === '/api/edit')!
|
||||
expect((edit.request as any).ops[0]).toMatchObject({ op: 'set', id: 's1', data: { title: 'Hello' }, version: 1 })
|
||||
expect((edit.request as any).ops[0]).toMatchObject({ op: 'set', id: 's1', data: { title: 'Hello' } })
|
||||
})
|
||||
|
||||
it('array editors save through the dialog as a minimal diff, and invalid data blocks Save', async () => {
|
||||
@@ -126,6 +126,25 @@ describe('on-page editor against the mock CMS', () => {
|
||||
expect(Object.keys(op.ops[0].data)).toEqual(['links']) // companyName, usps, social... untouched
|
||||
})
|
||||
|
||||
it('edits to OTHER fields of the node while the dialog is open do not conflict (the dialog does not pin a version)', async () => {
|
||||
h = await setup()
|
||||
await selectBlock('s1')
|
||||
await h.press('Properties')
|
||||
// style chip / drag handle / inline edit on the same node bump its version under the open dialog
|
||||
await h.editor.ctrl.setData('s1', { tone: 'accent' })
|
||||
await h.editor.ctrl.setData('s1', { padded: false })
|
||||
const input = h.shadow.querySelector<HTMLInputElement>('[data-path="title"] input')!
|
||||
const set = Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, 'value')!.set!
|
||||
await act(async () => {
|
||||
set.call(input, 'Mine')
|
||||
input.dispatchEvent(new Event('input', { bubbles: true }))
|
||||
})
|
||||
await act(async () => void h.shadow.querySelector<HTMLElement>('.cms-btn--primary')!.click())
|
||||
await h.settle()
|
||||
expect(h.cms.graph.node('s1').data).toMatchObject({ title: 'Mine', tone: 'accent', padded: false })
|
||||
expect(h.editor.ctrl.store.get().toast?.kind).not.toBe('error')
|
||||
})
|
||||
|
||||
it('a stale version shows a conflict instead of overwriting', async () => {
|
||||
h = await setup()
|
||||
await selectBlock('s1')
|
||||
|
||||
Reference in new issue
Block a user