diff --git a/frontend/src/lib/components/ResourceEditor.svelte b/frontend/src/lib/components/ResourceEditor.svelte index 15154f1573..14cc726873 100644 --- a/frontend/src/lib/components/ResourceEditor.svelte +++ b/frontend/src/lib/components/ResourceEditor.svelte @@ -354,6 +354,11 @@ // the form at a linked workspace, and a rename made there is that workspace's // alone — reporting it would move a host that is looking at this one. const savedPath = payloads.find((pl) => pl.ws === effectiveWorkspace)?.s.path ?? from + // Set once the acting workspace's own write has committed. The workspaces are + // written in sequence and a later one throwing aborts the rest, but what this + // one wrote is already deployed — a caller told nothing would stay pointed at + // a path it has moved off. + let committed: string | undefined = undefined try { for (const { ws, s, ini, existed } of payloads) { if (existed) { @@ -389,6 +394,7 @@ // `remove`. See VariableEditor for the full rationale. initialStates[ws] = s existedInitially[ws] = true + if (ws === effectiveWorkspace) committed = s.path // `s.path`, not the reported one: each workspace settles against the path // its own write used. settleDraftAfterWrite('resource', s, states[ws]?.draft, from, s.path, { workspace: ws }) @@ -405,7 +411,7 @@ return savedPath } catch (err) { sendUserToast(`Could not save resource: ${err.body ?? err.message}`, true) - return undefined + return committed } } diff --git a/frontend/src/lib/components/VariableEditor.svelte b/frontend/src/lib/components/VariableEditor.svelte index f1c3c267f1..24c8548d45 100644 --- a/frontend/src/lib/components/VariableEditor.svelte +++ b/frontend/src/lib/components/VariableEditor.svelte @@ -299,6 +299,21 @@ // the form at a linked workspace, and a rename made there is that workspace's // alone — reporting it would move a host that is looking at this one. const savedPath = payloads.find((pl) => pl.ws === fromWs)?.s.path ?? from + // Set once the acting workspace's own write has committed. The workspaces are + // written in sequence and a later one throwing aborts the rest, but what this + // one wrote is already deployed — a host told nothing would stay pointed at a + // path it has moved off. + let committed: string | undefined = undefined + // Follow the rename locally and tell the host, for whatever committed. Guarded + // on this editor still being the one that was saved: re-pointed, `editPath` is + // the variable it moved to. + const reportSaved = (saved: string | undefined) => { + if (editPath === from) { + if (saved && saved !== editPath) editPath = saved + drawer?.closeDrawer() + } + onSaved?.(saved, from, fromWs) + } try { for (const { ws, s, ini, existed } of payloads) { if (existed) { @@ -342,6 +357,7 @@ // resets the handle to it via `discard` (not `remove` — blanking the cell // to `undefined` reads as dirty), and keeps an edit made mid-request. Each // workspace settles against the path its own write used, not the reported one. + if (ws === fromWs) committed = s.path settleDraftAfterWrite('variable', s, states[ws]?.draft, from ?? '', s.path, { workspace: ws }) @@ -353,17 +369,10 @@ edit ? `Updated variable in ${payloads.length} workspace(s)` : `Created variable` ) dispatch('create') - // Only while this editor is still the one that was saved: re-pointed, the - // path below is the variable it moved to. - if (editPath === from) { - // A rename moved the item; the drawer host closes over it, but an inline one - // stays mounted, so follow the new path here and tell the host about it. - if (savedPath && savedPath !== editPath) editPath = savedPath - drawer?.closeDrawer() - } - onSaved?.(savedPath, from, fromWs) + reportSaved(savedPath) } catch (err) { sendUserToast(`Could not save variable: ${err.body}`, true) + if (committed) reportSaved(committed) } } diff --git a/frontend/src/lib/userDraft.svelte.ts b/frontend/src/lib/userDraft.svelte.ts index d90d3bb434..0e2354a96a 100644 --- a/frontend/src/lib/userDraft.svelte.ts +++ b/frontend/src/lib/userDraft.svelte.ts @@ -4,6 +4,7 @@ import { deepEqual } from 'fast-equals' import { workspaceStore } from './stores' import { readFieldsRecursively } from './utils' import { UserDraftDbSyncer } from './userDraftDbSyncer.svelte' +import { getLocalDraftHint } from './localDraftHints.svelte' import type { UserDraftItemKind } from './gen' export type { UserDraftItemKind } @@ -291,22 +292,21 @@ export function settleDraftAfterWrite( } /** - * Wait for a queued draft delete to actually land, reporting whether it did. The - * delete rides the autosave debouncer, so a caller that acts on it — a host that - * leaves an editor whose item the discard removed — would otherwise navigate onto - * a row that is still there, or away from a delete that failed. + * Wait for a queued draft delete to actually land, reporting whether the draft is + * gone. A caller acts on this — a host leaves an editor whose item the discard + * removed — so it has to be the outcome, not the pipeline's state: the delete + * shares its queue with the edits, and the form stays editable after Discard, so + * it can be displaced by a newer upsert as easily as it can fail. The hint is + * written by whichever POST landed, which is what tells the two apart. */ export async function flushDraftDelete( itemKind: UserDraftItemKind, path: string, opts?: UserDraftOptions ): Promise { - const query = { workspace: resolveWorkspace(opts), itemKind, path } - await UserDraftDbSyncer.flush(query) - return ( - UserDraftDbSyncer.getState(query).state !== 'failed' && - UserDraftDbSyncer.getConflict(query).conflict === undefined - ) + const workspace = resolveWorkspace(opts) + await UserDraftDbSyncer.flush({ workspace, itemKind, path }) + return getLocalDraftHint(workspace, itemKind, path) === false } export type UserDraftHandle = { diff --git a/frontend/src/lib/userDraftFlushDelete.test.ts b/frontend/src/lib/userDraftFlushDelete.test.ts new file mode 100644 index 0000000000..d80c4b0ebb --- /dev/null +++ b/frontend/src/lib/userDraftFlushDelete.test.ts @@ -0,0 +1,48 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest' + +// The flush stands in for the POST it drives: whatever write actually lands is +// what publishes the hint, which is the only thing that tells a delete apart +// from an edit that displaced it. +let landed: 'delete' | 'upsert' | 'nothing' = 'delete' +vi.mock('./userDraftDbSyncer.svelte', () => ({ + UserDraftDbSyncer: { + flush: vi.fn(async ({ workspace, itemKind, path }: any) => { + if (landed === 'nothing') return + const { setLocalDraftHint } = await import('./localDraftHints.svelte') + setLocalDraftHint(workspace, itemKind, path, landed === 'upsert') + }), + save: vi.fn() + } +})) +vi.mock('./gen', () => ({ DraftService: { updateDraft: vi.fn() } })) +vi.mock('./gen/core/OpenAPI', () => ({ OpenAPI: { BASE: '' } })) + +import { flushDraftDelete } from './userDraft.svelte' + +let n = 0 +let path = '' +beforeEach(() => { + // A fresh key per case: hints persist by design, so a reused one would carry + // the previous case's answer. + path = `u/me/v${n++}` +}) + +describe('flushDraftDelete', () => { + it('confirms a delete that landed', async () => { + landed = 'delete' + expect(await flushDraftDelete('variable', path, { workspace: 'ws' })).toBe(true) + }) + + // Discard leaves the form editable, so typing after it can replace the queued + // `value: null` with an upsert. The pipeline settles either way; only the item + // still being there tells the caller not to leave the editor. + it('rejects a delete displaced by a later edit', async () => { + landed = 'upsert' + expect(await flushDraftDelete('variable', path, { workspace: 'ws' })).toBe(false) + }) + + it('rejects a delete that never landed', async () => { + landed = 'nothing' + expect(await flushDraftDelete('variable', path, { workspace: 'ws' })).toBe(false) + }) +})