mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix(frontend): keep a draft-only rename consistent with its storage key
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wy24UHSVRZdDaPWiBay9MG
This commit is contained in:
co-authored by
Claude Opus 5
parent
eb6e12f0d2
commit
1b95662f49
@@ -237,10 +237,14 @@
|
||||
!draftValuesEqual({ ...current, path: '' }, { ...openedWith[selected], path: '' }),
|
||||
value: () => (current ? ($state.snapshot(current) as ResourceState) : undefined),
|
||||
pathIsFree: (p) => (selected ? resourcePathIsFree(selected, p) : Promise.resolve(false)),
|
||||
keyed: (v, p) => ({ ...v, path: p }),
|
||||
onAbandonKey: (ws, p) => {
|
||||
// The handle is pinned to the path this editor opened; once the draft
|
||||
// has moved off it, its next write would recreate the row it left.
|
||||
if (p === initialPath) UserDraft.stopSync('resource', p, { workspace: ws })
|
||||
},
|
||||
onResumeKey: (ws, p) => {
|
||||
if (p === initialPath) UserDraft.restartSync('resource', p, { workspace: ws })
|
||||
}
|
||||
})
|
||||
|
||||
|
||||
@@ -149,10 +149,14 @@
|
||||
!draftValuesEqual({ ...current, path: '' }, { ...openedWith[selected], path: '' }),
|
||||
value: () => (current ? ($state.snapshot(current) as VariableState) : undefined),
|
||||
pathIsFree: (p) => (selected ? variablePathIsFree(selected, p) : Promise.resolve(false)),
|
||||
keyed: (v, p) => ({ ...v, path: p }),
|
||||
onAbandonKey: (ws, p) => {
|
||||
// The handle is pinned to the path this editor opened; once the draft
|
||||
// has moved off it, its next write would recreate the row it left.
|
||||
if (p === editPath) UserDraft.stopSync('variable', p, { workspace: ws })
|
||||
},
|
||||
onResumeKey: (ws, p) => {
|
||||
if (p === editPath) UserDraft.restartSync('variable', p, { workspace: ws })
|
||||
}
|
||||
})
|
||||
|
||||
|
||||
@@ -4,12 +4,14 @@ import { flushSync } from 'svelte'
|
||||
const save = vi.fn()
|
||||
const remove = vi.fn()
|
||||
const discard = vi.fn()
|
||||
const forcePersist = vi.fn(async () => {})
|
||||
const flush = vi.fn(async () => {})
|
||||
vi.mock('$lib/userDraft.svelte', () => ({
|
||||
UserDraft: {
|
||||
save: (...a: unknown[]) => save(...a),
|
||||
remove: (...a: unknown[]) => remove(...a),
|
||||
discard: (...a: unknown[]) => discard(...a)
|
||||
discard: (...a: unknown[]) => discard(...a),
|
||||
forcePersist: (...a: unknown[]) => forcePersist(...a)
|
||||
}
|
||||
}))
|
||||
vi.mock('$lib/userDraftDbSyncer.svelte', () => ({
|
||||
@@ -220,6 +222,84 @@ describe('useNewItemDraftSync', () => {
|
||||
cleanup()
|
||||
})
|
||||
|
||||
/** The list synthesizes a draft-only row from the path INSIDE the draft while
|
||||
* get and delete address the key, so a stored draft has to describe its own
|
||||
* key. A rename the draft can't follow yet must not smuggle the new path
|
||||
* into the row it is still living in. */
|
||||
it('stores a draft describing the key it lives under, not an unusable path', async () => {
|
||||
const form = $state({ path: 'u/me/home', pathError: '', n: 1 })
|
||||
let sync: ReturnType<typeof useNewItemDraftSync> | undefined
|
||||
const cleanup = $effect.root(() => {
|
||||
sync = useNewItemDraftSync({
|
||||
itemKind: 'resource',
|
||||
enabled: () => true,
|
||||
workspace: () => 'w',
|
||||
path: () => form.path,
|
||||
pathError: () => form.pathError,
|
||||
contentTouched: () => false,
|
||||
value: () => ({ path: form.path, n: form.n }),
|
||||
keyed: (v, p) => ({ ...v, path: p })
|
||||
})
|
||||
sync.adopt('w', 'u/me/home', { path: 'u/me/home', n: 1 })
|
||||
})
|
||||
flushSync()
|
||||
|
||||
// Renamed to a path the draft cannot move to, then edited.
|
||||
form.path = 'u/me/taken'
|
||||
form.pathError = 'path already used'
|
||||
form.n = 2
|
||||
flushSync()
|
||||
vi.advanceTimersByTime(2000)
|
||||
flushSync()
|
||||
await sync!.flush()
|
||||
expect(save).toHaveBeenLastCalledWith(
|
||||
'resource',
|
||||
'u/me/home',
|
||||
{ path: 'u/me/home', n: 2 },
|
||||
{ workspace: 'w' }
|
||||
)
|
||||
})
|
||||
|
||||
/** Renaming away suspends the handle pinned to the original key; renaming
|
||||
* back has to resume it, or the draft is deleted at both keys and written
|
||||
* to neither. */
|
||||
it('resumes a key it returns to after abandoning it', async () => {
|
||||
const form = $state({ path: 'u/me/there', n: 1 })
|
||||
const events: string[] = []
|
||||
let sync: ReturnType<typeof useNewItemDraftSync> | undefined
|
||||
const cleanup = $effect.root(() => {
|
||||
sync = useNewItemDraftSync({
|
||||
itemKind: 'resource',
|
||||
enabled: () => true,
|
||||
workspace: () => 'w',
|
||||
path: () => form.path,
|
||||
pathError: () => '',
|
||||
contentTouched: () => false,
|
||||
value: () => ({ n: form.n }),
|
||||
onAbandonKey: (_ws, p) => events.push(`abandon:${p}`),
|
||||
onResumeKey: (_ws, p) => events.push(`resume:${p}`)
|
||||
})
|
||||
sync.adopt('w', 'u/me/there', { n: 1 })
|
||||
})
|
||||
flushSync()
|
||||
|
||||
form.path = 'u/me/away'
|
||||
flushSync()
|
||||
vi.advanceTimersByTime(1000)
|
||||
flushSync()
|
||||
|
||||
form.path = 'u/me/there'
|
||||
flushSync()
|
||||
vi.advanceTimersByTime(1000)
|
||||
flushSync()
|
||||
await sync!.flush()
|
||||
expect(events).toEqual(['abandon:u/me/there', 'abandon:u/me/away', 'resume:u/me/there'])
|
||||
expect(save).toHaveBeenLastCalledWith('resource', 'u/me/there', { n: 1 }, { workspace: 'w' })
|
||||
// The resumed handle's own change detection can no-op this write away.
|
||||
expect(forcePersist).toHaveBeenCalledWith('resource', 'u/me/there', { workspace: 'w' })
|
||||
cleanup()
|
||||
})
|
||||
|
||||
/** An editor's own autosave handle stays pinned to the path it opened, so
|
||||
* once the draft moves the helper has to hand that key back for suspension —
|
||||
* otherwise the handle's next write recreates the row just deleted. */
|
||||
|
||||
@@ -30,6 +30,15 @@ export interface NewItemDraftSyncOptions<V> {
|
||||
* deleted. An editor whose own autosave handle is pinned to that key MUST
|
||||
* suspend it here, or the handle's next write would recreate the row. */
|
||||
onAbandonKey?: (workspace: string, path: string) => void
|
||||
/** Called before writing to a key previously passed to `onAbandonKey`, so
|
||||
* the editor can resume the handle it suspended there. */
|
||||
onResumeKey?: (workspace: string, path: string) => void
|
||||
/** Return `value` with its own path set to `path`. A stored draft has to
|
||||
* describe the key it lives under: the list synthesizes a draft-only row
|
||||
* from the path INSIDE the draft, while get and delete address the key, so
|
||||
* letting the two diverge makes the row unreachable. Divergence is normal
|
||||
* while the form holds a path the draft cannot move to yet. */
|
||||
keyed?: (value: V, path: string) => V
|
||||
}
|
||||
|
||||
export interface NewItemDraftSync<V> {
|
||||
@@ -90,6 +99,8 @@ export function useNewItemDraftSync<V>(opts: NewItemDraftSyncOptions<V>): NewIte
|
||||
// edits anything, and an invalid path leaves it where it is rather than
|
||||
// deleting it. Only a brand-new item's draft is gated on being touched.
|
||||
let adopted = false
|
||||
// Keys handed to `onAbandonKey`, so a return to one can resume it.
|
||||
const abandoned = new Set<string>()
|
||||
|
||||
function markUnsettled(workspace: string, path: string): void {
|
||||
if (!unsettled.some((k) => k.workspace === workspace && k.path === path)) {
|
||||
@@ -110,14 +121,28 @@ export function useNewItemDraftSync<V>(opts: NewItemDraftSyncOptions<V>): NewIte
|
||||
// mid-rename. The fallback leaves the cell holding what the form holds.
|
||||
UserDraft.discard(opts.itemKind, written.path, value, { workspace: written.workspace })
|
||||
opts.onAbandonKey?.(written.workspace, written.path)
|
||||
abandoned.add(`${written.workspace}/${written.path}`)
|
||||
markUnsettled(written.workspace, written.path)
|
||||
written = undefined
|
||||
writtenValue = undefined
|
||||
}
|
||||
if (!workspace || !path || value === undefined) return
|
||||
const serialized = JSON.stringify(value)
|
||||
// Stored describing its own key: while the form holds a path the draft
|
||||
// cannot move to yet, the row must still name where it actually lives.
|
||||
const stored = opts.keyed ? opts.keyed(value, path) : value
|
||||
const serialized = JSON.stringify(stored)
|
||||
if (written && serialized === writtenValue) return
|
||||
UserDraft.save(opts.itemKind, path, value, { workspace })
|
||||
const resumed = abandoned.delete(`${workspace}/${path}`)
|
||||
if (resumed) opts.onResumeKey?.(workspace, path)
|
||||
UserDraft.save(opts.itemKind, path, stored, { workspace })
|
||||
if (resumed) {
|
||||
// A live handle at this key mirrors CHANGES, and its baseline advanced
|
||||
// while it was suspended — the value we just restored can equal it, so
|
||||
// nothing would be sent and the row we deleted on the way out would
|
||||
// never come back. Safe here: only a never-deployed draft is keyed this
|
||||
// way, so there is no baseline this could overwrite.
|
||||
void UserDraft.forcePersist(opts.itemKind, path, { workspace })
|
||||
}
|
||||
markUnsettled(workspace, path)
|
||||
written = { workspace, path }
|
||||
writtenValue = serialized
|
||||
@@ -230,6 +255,7 @@ export function useNewItemDraftSync<V>(opts: NewItemDraftSyncOptions<V>): NewIte
|
||||
reset() {
|
||||
finished = false
|
||||
adopted = false
|
||||
abandoned.clear()
|
||||
dropPending()
|
||||
written = undefined
|
||||
writtenValue = undefined
|
||||
|
||||
Reference in New Issue
Block a user