mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-22 16:03:43 +00:00
fix(drafts): stop the landed-delete fact from outliving the server state it described
The fact is sticky by design, so a delete that landed earlier for a key kept answering after the server moved out from under it: a discard that conflicted left it standing, and the conflict itself is tracked apart from the state the verdict reads, so both said the draft was gone. It is dropped now wherever the server stops being ours to describe — on a conflict, and on a remote resync. Also corrects what the partial-save comments claim. Following the committed rename re-keys every workspace's handle, so a workspace whose write never happened does lose sight of its draft in this editor; the draft is still on the server under the path that workspace holds the item at, which is where it stays reachable. Saying it stayed reachable here was wrong. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KcyBRxE8j3ujCYkum9KrC4
This commit is contained in:
co-authored by
Claude Opus 5
parent
7a7e30de87
commit
9d7b06ecc4
@@ -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
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -313,6 +313,7 @@ async function postSave(opts: UserDraftDbSyncerSaveOpts): Promise<void> {
|
||||
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
|
||||
} {
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user