diff --git a/frontend/src/lib/components/ResourceEditorDrawer.svelte b/frontend/src/lib/components/ResourceEditorDrawer.svelte index f3079fb960..f2f888915a 100644 --- a/frontend/src/lib/components/ResourceEditorDrawer.svelte +++ b/frontend/src/lib/components/ResourceEditorDrawer.svelte @@ -249,8 +249,10 @@ // The editor adopts its own new baseline, so a same-path save needs nothing // here — remounting it would re-read over an edit made during the write. // Only while this is still the resource it saved: re-pointed mid-write, - // `path` and the mounted editor are another one's. - if (path === from && submitted) path = submitted + // `path` and the mounted editor are another one's. And only a move: pointing a + // create at the path it just made re-keys the blank form's cell onto it, and + // the mirror then posts that blank state as the new item's draft. + if (path && path === from && submitted) path = submitted // One report per workspace written, each carrying its own — a linked // workspace's rename moves the tabs acting on it and no others. Reported // even when this drawer has moved on: the write is a fact about the item. diff --git a/frontend/src/lib/components/VariableEditor.svelte b/frontend/src/lib/components/VariableEditor.svelte index a630ce3f28..20e11feeca 100644 --- a/frontend/src/lib/components/VariableEditor.svelte +++ b/frontend/src/lib/components/VariableEditor.svelte @@ -375,8 +375,10 @@ // `editPath` 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. - if (actingPath && actingPath !== editPath) editPath = actingPath + // stays mounted, so follow the new path here. Only a move: pointing a create + // at the path it just made re-keys the blank form's cell onto it, and the + // mirror then posts that blank state as the new item's draft. + if (editPath && actingPath && actingPath !== editPath) editPath = actingPath drawer?.closeDrawer() } } catch (err) { diff --git a/frontend/src/lib/userDraft.svelte.ts b/frontend/src/lib/userDraft.svelte.ts index e9db2f1696..401b770023 100644 --- a/frontend/src/lib/userDraft.svelte.ts +++ b/frontend/src/lib/userDraft.svelte.ts @@ -2,7 +2,7 @@ import { get } from 'svelte/store' import { onDestroy, untrack } from 'svelte' import { deepEqual } from 'fast-equals' import { workspaceStore } from './stores' -import { readFieldsRecursively } from './utils' +import { readFieldsRecursively, sendUserToast } from './utils' import { UserDraftDbSyncer } from './userDraftDbSyncer.svelte' import type { UserDraftItemKind } from './gen' @@ -317,6 +317,10 @@ export async function settleDraftAfterWrite( savedPath: string, opts?: UserDraftOptions ): Promise { + // A create has nothing to settle: the blank form's cell is the detached handle + // `useMany` hands an empty path, wired to no key, so every request made for one + // is addressed to `/drafts/update//` and 404s. + if (!fromPath) return // A released cell reads as no cell at all, and its draft is then precisely what // nothing is watching any more — except when the release itself is what took // the handle away mid-write, in which case what it held still speaks for the @@ -324,20 +328,30 @@ export async function settleDraftAfterWrite( const held = live !== undefined ? live : takeReleasedValue(itemKind, fromPath, opts) const diverged = held !== undefined && !draftValuesEqual(held, written) if (savedPath === fromPath && diverged) return - // Sent before this resolves, not left on the keystroke debounce: callers report - // the write and remount on it, and both read a cell this has to have finished - // resolving — including through a frame with a draft store of its own. - UserDraft.discard(itemKind, fromPath, written, opts) - if (await flushDraftDelete(itemKind, fromPath, opts)) return - // Only a moved item forces the point. The form stays editable across that - // request, so an edit made then parks a write behind it: under the path the - // item has left that write would strand a draft no editor is on, but under an - // unchanged path it is the user's, and is meant to stand and read dirty. - if (savedPath === fromPath) return - for (let attempt = 0; attempt < 2; attempt++) { + // The form stays editable across the delete's own request, so a keystroke made + // then queues a write behind it. Under an unchanged path that write is the + // user's and is meant to stand; under a path the item has left it would put the + // draft back where no editor is, so the old key stops accepting writes for the + // length of the delete — which is what makes one attempt final. The edit stays + // in the form, and could not have followed the rename in either case. + const moved = savedPath !== fromPath + if (moved) UserDraft.stopSync(itemKind, fromPath, opts) + try { + // Sent before this resolves, not left on the keystroke debounce: callers report + // the write and remount on it, and both read a cell this has to have finished + // resolving — including through a frame with a draft store of its own. UserDraft.discard(itemKind, fromPath, written, opts) if (await flushDraftDelete(itemKind, fromPath, opts)) return + } finally { + if (moved) UserDraft.restartSync(itemKind, fromPath, opts) } + if (!moved) return + // A rename whose delete could not land — a conflict, or a failed request; no + // number of retries changes either. What is left is a draft-only item at a path + // nothing is editing. Callers still follow the rename, since the item really is + // at `savedPath` and leaving them behind would strand the editor too, so this is + // the only thing that says the leftover is there to be discarded. + sendUserToast(`Saved, but the draft left at ${fromPath} could not be cleared`, true) } /** diff --git a/frontend/src/lib/userDraftSettleAfterWrite.test.ts b/frontend/src/lib/userDraftSettleAfterWrite.test.ts index cc58f25e79..d2be6ad0dc 100644 --- a/frontend/src/lib/userDraftSettleAfterWrite.test.ts +++ b/frontend/src/lib/userDraftSettleAfterWrite.test.ts @@ -1,6 +1,8 @@ import { describe, it, expect, beforeEach, vi } from 'vitest' const discard = vi.fn() +const { toast } = vi.hoisted(() => ({ toast: vi.fn() })) +vi.mock('./utils', async (orig) => ({ ...((await orig()) as object), sendUserToast: toast })) // Whether the key ends up settled on the delete after a flush — false stands for a // form that keeps queueing writes behind it. let settles = true @@ -20,6 +22,7 @@ import { settleDraftAfterWrite, UserDraft } from './userDraft.svelte' beforeEach(() => { settles = true + toast.mockClear() discard.mockClear() vi.spyOn(UserDraft, 'discard').mockImplementation(discard as any) }) @@ -65,14 +68,21 @@ describe('settleDraftAfterWrite', () => { expect(discard).toHaveBeenCalledWith('variable', 'u/me/a', sent, OPTS) }) - // The form stays editable across the delete's own request, so an edit made then - // parks a write that would put the draft back — under a path the item has left, - // once the callers re-key. The delete is re-sent while that is the case, and - // bounded, because a form being typed into can always add one more. - it('re-sends the delete while the key will not settle on it, and gives up bounded', async () => { + // A rename suspends the old key first, so nothing can queue behind the delete and + // re-sending it could only repeat a request the server already refused. What is + // left is a draft under a path the item has left, which has to be reported. + it('sends the delete once on a rename and reports one that will not land', async () => { settles = false await settleDraftAfterWrite('variable', sent, { ...sent }, 'u/me/a', 'u/me/b', OPTS) - expect(discard).toHaveBeenCalledTimes(3) + expect(discard).toHaveBeenCalledTimes(1) + expect(toast).toHaveBeenCalledTimes(1) + }) + + // A create's form cell is the detached handle an empty path gets, wired to no + // key: every request made for one is addressed to `/drafts/update//`. + it('settles nothing for a create', async () => { + await settleDraftAfterWrite('variable', sent, undefined, '', 'u/me/a', OPTS) + expect(discard).not.toHaveBeenCalled() }) // A released handle reads as no cell at all. That is not somebody's newer edit — @@ -82,13 +92,14 @@ describe('settleDraftAfterWrite', () => { expect(discard).toHaveBeenCalledWith('variable', 'u/me/a', sent, OPTS) }) - // Retrying resets the cell to what was written, so it may only chase a path the - // item has left. On an unchanged path an edit made during the delete is the - // user's, and re-sending would revert it. - it('does not retry over an edit made on an unchanged path', async () => { + // On an unchanged path an edit made during the delete is the user's: the key is + // left accepting writes, and a draft standing there is the intended outcome + // rather than something to report. + it('leaves an edit made during the delete on an unchanged path', async () => { settles = false await settleDraftAfterWrite('variable', sent, { ...sent }, 'u/me/a', 'u/me/a', OPTS) expect(discard).toHaveBeenCalledTimes(1) + expect(toast).not.toHaveBeenCalled() }) it('sends it once when the key settles', async () => {