diff --git a/frontend/src/lib/components/ResourceEditor.svelte b/frontend/src/lib/components/ResourceEditor.svelte index 123e78591c..b077cc412a 100644 --- a/frontend/src/lib/components/ResourceEditor.svelte +++ b/frontend/src/lib/components/ResourceEditor.svelte @@ -411,9 +411,10 @@ return savedPath } catch (err) { sendUserToast(`Could not save resource: ${err.body ?? err.message}`, true) - // The workspaces that did not get this far keep their drafts under the path - // they still hold the item at; re-keying them onto this rename would move a - // path they never wrote. + // Following the rename remounts on the new path, so a workspace whose write + // never happened loses sight of its draft here — it stays on the server under + // the path that workspace still holds the item at, listed and editable from + // there. Re-keying it instead would move a path it never wrote. return committed } } diff --git a/frontend/src/lib/components/VariableEditor.svelte b/frontend/src/lib/components/VariableEditor.svelte index edb0e4e8cc..ca313ea632 100644 --- a/frontend/src/lib/components/VariableEditor.svelte +++ b/frontend/src/lib/components/VariableEditor.svelte @@ -373,9 +373,10 @@ reportSaved(savedPath, true) } catch (err) { sendUserToast(`Could not save variable: ${err.body}`, true) - // The workspaces that did not get this far keep their drafts under the path - // they still hold the item at; re-keying them onto this rename would move a - // path they never wrote. They stay listed, and retryable from here. + // Following the rename re-keys every workspace's handle, so a workspace whose + // write never happened loses sight of its draft here — it stays on the server + // under the path that workspace still holds the item at, listed and editable + // from there. Re-keying it instead would move a path it never wrote. if (committed) reportSaved(committed, false) } } diff --git a/frontend/src/lib/userDraftDbSyncer.svelte.ts b/frontend/src/lib/userDraftDbSyncer.svelte.ts index 1718700f07..2a7dd6db50 100644 --- a/frontend/src/lib/userDraftDbSyncer.svelte.ts +++ b/frontend/src/lib/userDraftDbSyncer.svelte.ts @@ -313,6 +313,7 @@ async function postSave(opts: UserDraftDbSyncerSaveOpts): Promise { serverTimestamp: resp.current_timestamp, localLastSync: lastSync ?? null }) + lastLanded.delete(key) return } // resp.status === 'saved' — advance lastSync (or drop on delete). @@ -546,9 +547,11 @@ export const UserDraftDbSyncer = { } else { clearLastSync(query.workspace, query.itemKind, query.path) } - // Back in sync with the server: clear any conflict / failure. + // Back in sync with the server: clear any conflict / failure. `lastLanded` + // goes with them — whatever we last wrote no longer describes what is there. conflicts.delete(key) failures.delete(key) + lastLanded.delete(key) }, /** @@ -604,16 +607,18 @@ export const UserDraftDbSyncer = { } }, - /** Reactive conflict snapshot (if any) for a draft. */ /** * Whether the last save that landed for this key was the draft's deletion. - * False while none has landed at all — a failed or conflicted delete never - * reaches the response handler, so it never claims to have landed. + * False while none has landed — a delete that failed or conflicted never + * reaches the response handler, so it never claims to have landed, and one + * that landed earlier is dropped as soon as the server's state moves outside + * our writes. */ lastLandedWasDelete(query: UserDraftLastSyncQuery): boolean { return lastLanded.get(draftKey(query.workspace, query.itemKind, query.path)) === 'delete' }, + /** Reactive conflict snapshot (if any) for a draft. */ getConflict(query: UserDraftLastSyncQuery): { readonly conflict: DraftConflictInfo | undefined } { diff --git a/frontend/src/lib/userDraftLastLanded.test.ts b/frontend/src/lib/userDraftLastLanded.test.ts new file mode 100644 index 0000000000..fd030d79b0 --- /dev/null +++ b/frontend/src/lib/userDraftLastLanded.test.ts @@ -0,0 +1,37 @@ +import { describe, it, expect, vi } from 'vitest' + +let response: { status: 'saved' | 'conflict'; current_timestamp: string } = { + status: 'saved', + current_timestamp: '2020-01-01T00:00:00Z' +} +const updateDraft = vi.fn(async (..._args: any[]) => response) +vi.mock('./gen', () => ({ + DraftService: { updateDraft: (...a: unknown[]) => updateDraft(...(a as [])) } +})) +vi.mock('./gen/core/OpenAPI', () => ({ OpenAPI: { BASE: '' } })) +vi.mock('./localDraftHints.svelte', () => ({ setLocalDraftHint: vi.fn() })) + +import { UserDraftDbSyncer } from './userDraftDbSyncer.svelte' + +const query = { workspace: 'ws', itemKind: 'variable' as const, path: 'u/me/v' } +const send = (value: unknown) => UserDraftDbSyncer.save({ ...query, value, immediate: true }) + +/** + * `flushDraftDelete` leaves an editor on this fact, and it is deliberately + * sticky — so it has to stop describing the server the moment the server stops + * being ours to describe. + */ +describe('lastLandedWasDelete', () => { + it('follows what the response handler saw land, and drops it on a conflict', async () => { + await send({ path: 'u/me/v' }) + expect(UserDraftDbSyncer.lastLandedWasDelete(query)).toBe(false) + + await send(null) + expect(UserDraftDbSyncer.lastLandedWasDelete(query)).toBe(true) + + // The server moved under us: the earlier delete no longer answers for this one. + response = { status: 'conflict', current_timestamp: '2020-01-02T00:00:00Z' } + await send(null) + expect(UserDraftDbSyncer.lastLandedWasDelete(query)).toBe(false) + }) +})