mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-03 16:02:12 +00:00
fix: tell the user when a draft conflict has stopped their edits saving (#11241)
* fix: tell the user when a draft conflict has stopped their edits saving Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: give nothing up until the other version has actually loaded Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: ignore a reload the editor has moved on from, and settle saves before resolving Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: settle the key before a forced resolution, and cover quiesce with tests Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: let a resolution tell it belongs to an editor that is gone Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: end a conflict resolution when the editing session does, not the component Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: show the resource conflict in the fixed banner, and scope busy to its session Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: keep the conflict alert in editors no host wraps Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: free a reopened editor from a resolution the closed one left in flight Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: make switching workspace-specific versions a new conflict session Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: keep the local draft when a conflict resolution is abandoned Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: let a newer session outrank a resolution that outlived its editor Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: end a conflict resolution with the session that started it Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: reopen a conflicted editor on the version the server refused Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: keep a refused draft deletion a deletion Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: ungate the editor when a resolution loads the other draft Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: settle the draft key before reading the version to load Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: settle the key again after reading, for edits made under the read Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
5095456bb6
commit
d8ebbc06f5
@@ -0,0 +1,31 @@
|
||||
<script lang="ts">
|
||||
import { Alert, Button } from '$lib/components/common'
|
||||
|
||||
interface Props {
|
||||
/** Take the draft the server holds, replacing what is on screen. */
|
||||
onReload: () => void
|
||||
/** Write what is on screen over the server's draft. */
|
||||
onOverwrite: () => void
|
||||
/** A resolution is in flight: neither answer is offered again until it settles. */
|
||||
busy?: boolean
|
||||
}
|
||||
|
||||
let { onReload, onOverwrite, busy = false }: Props = $props()
|
||||
</script>
|
||||
|
||||
<Alert type="warning" title="Your draft changed elsewhere">
|
||||
<div class="flex flex-col items-start gap-2">
|
||||
<div>
|
||||
It was saved from another tab or session since this one read it, so your changes here are no
|
||||
longer being saved.
|
||||
</div>
|
||||
<div class="flex flex-row gap-2">
|
||||
<Button unifiedSize="sm" variant="default" disabled={busy} onClick={onReload}>
|
||||
Load the other version
|
||||
</Button>
|
||||
<Button unifiedSize="sm" variant="default" disabled={busy} onClick={onOverwrite}>
|
||||
Keep mine
|
||||
</Button>
|
||||
</div>
|
||||
</div>
|
||||
</Alert>
|
||||
@@ -19,6 +19,8 @@
|
||||
import { useActingUser } from '$lib/actingUser.svelte'
|
||||
import { UserDraft, draftValuesEqual, type UserDraftHandle } from '$lib/userDraft.svelte'
|
||||
import { UserDraftDbSyncer } from '$lib/userDraftDbSyncer.svelte'
|
||||
import { useDraftConflictSession } from '$lib/draftConflictSession.svelte'
|
||||
import DraftConflictAlert from './DraftConflictAlert.svelte'
|
||||
import { setLocalDraftHint } from '$lib/localDraftHints.svelte'
|
||||
import { onUserInput } from '$lib/userDraftEditGate'
|
||||
import { useOperatingWorkspace } from '$lib/components/operatingWorkspace.svelte'
|
||||
@@ -48,6 +50,9 @@
|
||||
* so it can hide the banner's Discard button in read-only mode (matches
|
||||
* the trigger editors' `disabled={!can_write}` wiring). */
|
||||
onCanWriteChange?: (canWrite: boolean) => void
|
||||
/** The drawer renders the conflict alert: inside this scrollable form it can land above the
|
||||
* viewport on a long resource, leaving the fixed banner claiming the edits are saved. */
|
||||
onDraftConflictChange?: (state: { conflicted: boolean; busy: boolean }) => void
|
||||
}
|
||||
|
||||
let {
|
||||
@@ -61,7 +66,8 @@
|
||||
selected: selectedProp = $bindable(),
|
||||
viewJsonSchema = $bindable(),
|
||||
onDraftStateChange,
|
||||
onCanWriteChange
|
||||
onCanWriteChange,
|
||||
onDraftConflictChange
|
||||
}: Props = $props()
|
||||
|
||||
type ResourceState = {
|
||||
@@ -263,6 +269,132 @@
|
||||
)
|
||||
const anyDirty = $derived(dirtyWorkspaces.length > 0)
|
||||
|
||||
/** Scoped by `selected` so switching workspace-specific versions is a new session. Ending is
|
||||
* exported on top of that because the drawer, not this component, knows when its own session
|
||||
* is over: this component outlives it. */
|
||||
const conflictSession = useDraftConflictSession(() => selected)
|
||||
export function endEditingSession(): void {
|
||||
conflictSession.end()
|
||||
}
|
||||
onDestroy(endEditingSession)
|
||||
const resolvingConflict = $derived(conflictSession.busy)
|
||||
/** The server refused this tab's autosave because the row moved under it: another tab, or the
|
||||
* AI chat, which writes these drafts too. Nothing typed here reaches the server until the user
|
||||
* picks a version, and the unsaved-changes banner says the opposite — that the edits are held
|
||||
* as a draft — so without this they are told their work is safe while it is being dropped. */
|
||||
const draftConflict = $derived(
|
||||
selected && initialPath
|
||||
? UserDraftDbSyncer.getConflict({
|
||||
workspace: selected,
|
||||
itemKind: 'resource',
|
||||
path: initialPath
|
||||
}).conflict
|
||||
: undefined
|
||||
)
|
||||
|
||||
async function resolveDraftConflict(keepMine: boolean): Promise<void> {
|
||||
const ws = selected
|
||||
const p = initialPath
|
||||
if (!ws || !p || resolvingConflict) return
|
||||
const query = { workspace: ws, itemKind: 'resource' as const, path: p }
|
||||
const token = conflictSession.start()
|
||||
const stillOurs = () => conflictSession.holds(token) && selected === ws
|
||||
try {
|
||||
if (keepMine) {
|
||||
// Settle the key first: an ordinary autosave still queued would displace the forced
|
||||
// write below, and being conditional it would be refused — so "Keep mine" would
|
||||
// finish without keeping anything and leave the alert standing.
|
||||
await UserDraftDbSyncer.quiesce(query)
|
||||
// A resolution belongs to the session that started it. One that outlives its editor
|
||||
// stops here rather than writing on: whatever replaced it — another session on the
|
||||
// same draft, or its own resolution — owns the key now, and the edit this one was
|
||||
// keeping is still parked for a later flush either way.
|
||||
if (!stillOurs()) return
|
||||
// A parked `null` is this tab's "no draft any more" — a discard, or an edit that
|
||||
// landed back on the deployed value. Keeping that means removing the row, not
|
||||
// writing the baseline back as a draft with no dirty banner to discard it through.
|
||||
const parked = UserDraftDbSyncer.peekPending(query)
|
||||
const mine = parked?.value === null ? null : $state.snapshot(states[ws]?.draft)
|
||||
// Forced, so it goes over the row that refused us, and its response reseeds
|
||||
// `last_sync` so the next ordinary save is conditional again.
|
||||
if (mine !== undefined) await UserDraftDbSyncer.overwrite({ ...query, value: mine })
|
||||
// Say so rather than leave the alert up with no explanation: a write displaced by
|
||||
// something typed meanwhile can still lose the race.
|
||||
if (UserDraftDbSyncer.getConflict(query).conflict) {
|
||||
sendUserToast('Could not keep your version — try again', true)
|
||||
}
|
||||
return
|
||||
}
|
||||
// Settle the key BEFORE reading, so what comes back is the version the server is left
|
||||
// holding: a write this tab started can still be in flight — a forced one it walked
|
||||
// away from included — and a response fetched past it describes a version about to be
|
||||
// replaced, which would then be seeded along with its already-stale `last_sync`.
|
||||
// Nothing is given up by waiting; `quiesce` only stops the pipeline.
|
||||
await UserDraftDbSyncer.quiesce(query)
|
||||
if (!stillOurs()) return
|
||||
// Read BEFORE giving anything up: until the server has answered, the refused payload is
|
||||
// still the only copy of this tab's edit, and the conflict is still true.
|
||||
const r = await ResourceService.getResource({ workspace: ws, path: p, getDraft: true })
|
||||
const deployedState: ResourceState = {
|
||||
path: r.path,
|
||||
args: (r.value ?? {}) as Record<string, any>,
|
||||
description: r.description ?? '',
|
||||
labels: r.labels ?? undefined,
|
||||
wsSpecific: r.ws_specific ?? false
|
||||
}
|
||||
// Everything below writes shared editor state, so first make sure it is still this
|
||||
// resource's: the drawer stays closable while the read is out, and another resource
|
||||
// opened meanwhile would otherwise get this one's baseline — and with it this one's
|
||||
// path as its save target.
|
||||
if (!stillOurs()) return
|
||||
// Again, because the form stayed editable while the read was out: `quiesce` settles what
|
||||
// is running when it is called, not the key for the rest of the resolution, so a
|
||||
// keystroke since can have started a save of its own. Left running, its rejection lands
|
||||
// after the baseline below and raises the conflict this just resolved.
|
||||
await UserDraftDbSyncer.quiesce(query)
|
||||
if (!stillOurs()) return
|
||||
// Now, and not in `quiesce`: the refused payload belongs to the version being replaced,
|
||||
// but until this point it was still the only copy of the edit, and a resolution that
|
||||
// gave up before here has to leave it behind.
|
||||
UserDraftDbSyncer.dropPending(query)
|
||||
UserDraftDbSyncer.clearConflict(query)
|
||||
initialStates[ws] = structuredClone(deployedState)
|
||||
// Everything else the load path takes from this same response. The item can have been
|
||||
// deleted, recreated under another type, or had its permissions changed while the
|
||||
// conflict stood, and the fields below decide create-vs-update, the schema and write
|
||||
// access — so refreshing only what is displayed would leave those deciding on the
|
||||
// version the user just replaced.
|
||||
fetchedResources[ws] = r
|
||||
existedInitially[ws] = !(r as any).no_deployed
|
||||
if (ws === effectiveWorkspace) resource_type = r.resource_type
|
||||
UserDraftDbSyncer.recordRemoteSync(query, (r as any).draft_saved_at)
|
||||
const loadedDraft = (r as any).draft as ResourceState | undefined
|
||||
// Loading a draft makes this workspace one that opened with a draft, whatever it opened
|
||||
// with before — a refused deletion opens gated, and gated the settling absorber would
|
||||
// fold the version just loaded into the deployed baseline, leaving it silently clean
|
||||
// with Save disabled. Ungate here rather than leaving it to the effect, so no write
|
||||
// between the two is absorbed.
|
||||
openedOnDraft[ws] = !!loadedDraft
|
||||
if (loadedDraft) {
|
||||
setGated(ws, false)
|
||||
} else {
|
||||
// Accepting "there is no draft" has to shut the gate, the way discarding one does.
|
||||
// The edit that raised the conflict set `userEdited`, and left open, the form's
|
||||
// settling writes — schema defaults materializing over the loaded value — read as
|
||||
// the user's and recreate the draft just accepted away.
|
||||
userEdited[ws] = false
|
||||
setGated(ws, true)
|
||||
}
|
||||
UserDraft.seed('resource', p, loadedDraft ?? deployedState, { workspace: ws })
|
||||
} catch (e) {
|
||||
// Nothing was given up above, so the conflict stands and the edit is still here to
|
||||
// resolve again — which is the whole point of reading first.
|
||||
sendUserToast(`Could not load the other version: ${e}`, true)
|
||||
} finally {
|
||||
conflictSession.finish(token)
|
||||
}
|
||||
}
|
||||
|
||||
// The syncer owns the list-page `*` hint; the editor only CLEARS it when a
|
||||
// workspace is at the deployed baseline (so a draft discarded elsewhere
|
||||
// vanishes on reopen). Never SET here. See VariableEditor for the full note.
|
||||
@@ -338,15 +470,33 @@
|
||||
labels: r.labels ?? undefined,
|
||||
wsSpecific: r.ws_specific ?? false
|
||||
}
|
||||
// Open with the saved draft if present, else the deployed.
|
||||
const s: ResourceState = savedDraftState ?? deployedState
|
||||
openedOnDraft[ws] = !!savedDraftState
|
||||
// A refused save leaves this tab's own version parked. `.draft` is the
|
||||
// version that refused it, so opening on that would quietly drop the edit
|
||||
// the alert is about and leave "Keep mine" offering to keep the other one.
|
||||
const conflictQuery = {
|
||||
workspace: ws,
|
||||
itemKind: 'resource' as const,
|
||||
path: initialPath
|
||||
}
|
||||
const refused = UserDraftDbSyncer.getConflict(conflictQuery).conflict
|
||||
? UserDraftDbSyncer.peekPending(conflictQuery)
|
||||
: undefined
|
||||
// A parked `null` is this tab's "no draft any more", which on screen is the
|
||||
// deployed value — so only a payload with content counts as a local draft.
|
||||
const refusedDraft = (refused?.value ?? undefined) as ResourceState | undefined
|
||||
const hasLocalDraft = refused ? refused.value !== null : !!savedDraftState
|
||||
// Open with this tab's refused version if there is one, else the saved
|
||||
// draft, else the deployed.
|
||||
const s: ResourceState = refused
|
||||
? (refusedDraft ?? deployedState)
|
||||
: (savedDraftState ?? deployedState)
|
||||
openedOnDraft[ws] = hasLocalDraft
|
||||
// Gate BEFORE the handle is acquired: `stopSync` queues on a
|
||||
// not-yet-live entry, and the form can settle before the effect
|
||||
// above gets a chance to run. Only worth doing when no draft exists
|
||||
// yet — where one does, there is no phantom to prevent and
|
||||
// suspending could only drop a write.
|
||||
if (!savedDraftState) setGated(ws, true)
|
||||
if (!hasLocalDraft) setGated(ws, true)
|
||||
ensureHandle(ws, s)
|
||||
initialStates[ws] = structuredClone(deployedState)
|
||||
// Draft-only paths (`no_deployed`) have no row — saving must
|
||||
@@ -426,6 +576,13 @@
|
||||
$effect(() => {
|
||||
onCanWriteChange?.(can_write === true)
|
||||
})
|
||||
$effect(() => {
|
||||
onDraftConflictChange?.({ conflicted: !!draftConflict, busy: resolvingConflict })
|
||||
})
|
||||
|
||||
export function resolveDraftConflictFromBanner(keepMine: boolean): void {
|
||||
void resolveDraftConflict(keepMine)
|
||||
}
|
||||
|
||||
export function localDraftDeployed(): ResourceState | undefined {
|
||||
return selected ? initialStates[selected] : undefined
|
||||
@@ -539,6 +696,18 @@
|
||||
|
||||
<div>
|
||||
<div class="flex flex-col gap-6 pb-2">
|
||||
<!-- Only when nobody above is showing it. A host that takes `onDraftConflictChange` puts it
|
||||
in its own fixed banner, where a long form cannot scroll it out of view; one that embeds
|
||||
this editor directly — the AI chat's MCP section, and the SDK surface — would otherwise
|
||||
get no warning at all, which is the very thing this alert exists to prevent. -->
|
||||
{#if draftConflict && !onDraftConflictChange}
|
||||
<DraftConflictAlert
|
||||
busy={resolvingConflict}
|
||||
onReload={() => void resolveDraftConflict(false)}
|
||||
onOverwrite={() => void resolveDraftConflict(true)}
|
||||
/>
|
||||
{/if}
|
||||
|
||||
{#if otherDirty.length > 0}
|
||||
<Alert type="warning" title="Editing multiple workspaces">
|
||||
You are going to edit the value in: {otherDirty.join(', ')}
|
||||
|
||||
@@ -8,6 +8,7 @@
|
||||
import { isOwner } from '$lib/utils'
|
||||
import { useActingUser } from '$lib/actingUser.svelte'
|
||||
import LocalDraftBanner from './LocalDraftBanner.svelte'
|
||||
import DraftConflictAlert from './DraftConflictAlert.svelte'
|
||||
import OpenInSessionButton from './sessions/OpenInSessionButton.svelte'
|
||||
import {
|
||||
clearPageDrawerAnchor,
|
||||
@@ -62,9 +63,12 @@
|
||||
localDraftDeployed: () => unknown
|
||||
localDraftCurrent: () => unknown
|
||||
discardLocalDraft: () => void
|
||||
endEditingSession: () => void
|
||||
resolveDraftConflictFromBanner: (keepMine: boolean) => void
|
||||
}
|
||||
| undefined = $state(undefined)
|
||||
let hasLocalDraft = $state(false)
|
||||
let draftConflict = $state({ conflicted: false, busy: false })
|
||||
let canWriteSelected = $state(true)
|
||||
|
||||
let path: string | undefined = $state(undefined)
|
||||
@@ -151,6 +155,9 @@
|
||||
size="50rem"
|
||||
{disableChatOffset}
|
||||
on:close={() => {
|
||||
// The editor outlives this drawer, so tell it the session is over: a conflict resolution
|
||||
// still in flight must not land on whatever the next opening shows.
|
||||
resourceEditor?.endEditingSession?.()
|
||||
if (keepAnchorOnClose) {
|
||||
keepAnchorOnClose = false
|
||||
return
|
||||
@@ -168,7 +175,15 @@
|
||||
bannerReserved={mode == 'edit'}
|
||||
hideClose={inline && !onClose}
|
||||
fullScreen={!inline}
|
||||
on:close={() => (inline ? onClose?.() : drawer?.closeDrawer())}
|
||||
on:close={() => {
|
||||
// Inline has no drawer to emit a close, so the session ends here instead.
|
||||
if (inline) {
|
||||
resourceEditor?.endEditingSession?.()
|
||||
onClose?.()
|
||||
} else {
|
||||
drawer?.closeDrawer()
|
||||
}
|
||||
}}
|
||||
>
|
||||
{#snippet titleExtra()}
|
||||
{#if mode == 'new' && resource_type}
|
||||
@@ -189,10 +204,18 @@
|
||||
bind:selected
|
||||
bind:viewJsonSchema
|
||||
onDraftStateChange={(v) => (hasLocalDraft = v)}
|
||||
onDraftConflictChange={(v) => (draftConflict = v)}
|
||||
onCanWriteChange={(v) => (canWriteSelected = v)}
|
||||
/>
|
||||
{/await}
|
||||
{#snippet banner()}
|
||||
{#if draftConflict.conflicted}
|
||||
<DraftConflictAlert
|
||||
busy={draftConflict.busy}
|
||||
onReload={() => resourceEditor?.resolveDraftConflictFromBanner?.(false)}
|
||||
onOverwrite={() => resourceEditor?.resolveDraftConflictFromBanner?.(true)}
|
||||
/>
|
||||
{/if}
|
||||
<LocalDraftBanner
|
||||
show={hasLocalDraft}
|
||||
reserveSpace={mode == 'edit'}
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
<script lang="ts">
|
||||
import { VariableService, WorkspaceService } from '$lib/gen'
|
||||
import { createEventDispatcher, untrack } from 'svelte'
|
||||
import { createEventDispatcher, onDestroy, untrack } from 'svelte'
|
||||
import { Button } from './common'
|
||||
import Drawer from './common/drawer/Drawer.svelte'
|
||||
import DrawerContent from './common/drawer/DrawerContent.svelte'
|
||||
@@ -23,6 +23,9 @@
|
||||
import { useActingUser } from '$lib/actingUser.svelte'
|
||||
import { UserDraft, draftValuesEqual, type UserDraftHandle } from '$lib/userDraft.svelte'
|
||||
import LocalDraftBanner from './LocalDraftBanner.svelte'
|
||||
import DraftConflictAlert from './DraftConflictAlert.svelte'
|
||||
import { UserDraftDbSyncer } from '$lib/userDraftDbSyncer.svelte'
|
||||
import { useDraftConflictSession } from '$lib/draftConflictSession.svelte'
|
||||
import { isEncryptedDraftValue } from '$lib/encryptedDraft'
|
||||
import { setLocalDraftHint } from '$lib/localDraftHints.svelte'
|
||||
import { useOperatingWorkspace } from '$lib/components/operatingWorkspace.svelte'
|
||||
@@ -141,6 +144,126 @@
|
||||
Object.keys(states).filter((ws) => !draftValuesEqual(states[ws].draft, initialStates[ws]))
|
||||
)
|
||||
|
||||
/** Scoped by `selected` so switching workspace-specific versions is a new session. Ended on top
|
||||
* of that by every entry point and by the teardown, since reopening the same variable reuses
|
||||
* this component. */
|
||||
const conflictSession = useDraftConflictSession(() => selected)
|
||||
function endEditingSession(): void {
|
||||
conflictSession.end()
|
||||
}
|
||||
onDestroy(endEditingSession)
|
||||
const resolvingConflict = $derived(conflictSession.busy)
|
||||
/** The server refused this tab's autosave because the row moved under it: another tab, or the
|
||||
* AI chat, which writes these drafts too. Nothing typed here reaches the server until the user
|
||||
* picks a version, and the unsaved-changes banner says the opposite — that the edits are held
|
||||
* as a draft — so without this they are told their work is safe while it is being dropped. */
|
||||
const draftConflict = $derived(
|
||||
edit && selected && editPath
|
||||
? UserDraftDbSyncer.getConflict({
|
||||
workspace: selected,
|
||||
itemKind: 'variable',
|
||||
path: editPath
|
||||
}).conflict
|
||||
: undefined
|
||||
)
|
||||
|
||||
async function resolveDraftConflict(keepMine: boolean): Promise<void> {
|
||||
const ws = selected
|
||||
const p = editPath
|
||||
if (!ws || !p || resolvingConflict) return
|
||||
const query = { workspace: ws, itemKind: 'variable' as const, path: p }
|
||||
const token = conflictSession.start()
|
||||
const stillOurs = () => conflictSession.holds(token) && selected === ws
|
||||
try {
|
||||
if (keepMine) {
|
||||
// Settle the key first: an ordinary autosave still queued would displace the forced
|
||||
// write below, and being conditional it would be refused — so "Keep mine" would
|
||||
// finish without keeping anything and leave the alert standing.
|
||||
await UserDraftDbSyncer.quiesce(query)
|
||||
// A resolution belongs to the session that started it. One that outlives its editor
|
||||
// stops here rather than writing on: whatever replaced it — another session on the
|
||||
// same draft, or its own resolution — owns the key now, and the edit this one was
|
||||
// keeping is still parked for a later flush either way.
|
||||
if (!stillOurs()) return
|
||||
// A parked `null` is this tab's "no draft any more" — a discard, or an edit that
|
||||
// landed back on the deployed value. Keeping that means removing the row, not
|
||||
// writing the baseline back as a draft with no dirty banner to discard it through.
|
||||
const parked = UserDraftDbSyncer.peekPending(query)
|
||||
const mine = parked?.value === null ? null : $state.snapshot(states[ws]?.draft)
|
||||
// Forced, so it goes over the row that refused us, and its response reseeds
|
||||
// `last_sync` so the next ordinary save is conditional again.
|
||||
if (mine !== undefined) await UserDraftDbSyncer.overwrite({ ...query, value: mine })
|
||||
// Say so rather than leave the alert up with no explanation: a write displaced by
|
||||
// something typed meanwhile can still lose the race.
|
||||
if (UserDraftDbSyncer.getConflict(query).conflict) {
|
||||
sendUserToast('Could not keep your version — try again', true)
|
||||
}
|
||||
return
|
||||
}
|
||||
// Settle the key BEFORE reading, so what comes back is the version the server is left
|
||||
// holding: a write this tab started can still be in flight — a forced one it walked
|
||||
// away from included — and a response fetched past it describes a version about to be
|
||||
// replaced, which would then be seeded along with its already-stale `last_sync`.
|
||||
// Nothing is given up by waiting; `quiesce` only stops the pipeline.
|
||||
await UserDraftDbSyncer.quiesce(query)
|
||||
if (!stillOurs()) return
|
||||
// Read BEFORE giving anything up: until the server has answered, the refused payload is
|
||||
// still the only copy of this tab's edit, and the conflict is still true.
|
||||
const v = await VariableService.getVariable({
|
||||
workspace: ws,
|
||||
path: p,
|
||||
decryptSecret: false,
|
||||
getDraft: true
|
||||
})
|
||||
const deployedState: VariableState = {
|
||||
path: v.path,
|
||||
variable: {
|
||||
value: v.value ?? '',
|
||||
is_secret: v.is_secret,
|
||||
description: v.description ?? ''
|
||||
},
|
||||
labels: v.labels ?? undefined,
|
||||
wsSpecific: v.ws_specific ?? false
|
||||
}
|
||||
// Everything below writes shared editor state, so first make sure it is still this
|
||||
// variable's: the drawer stays closable while the read is out, and another variable
|
||||
// opened meanwhile would otherwise get this one's baseline — and with it this one's
|
||||
// path as its save target.
|
||||
if (!stillOurs()) return
|
||||
// Again, because the form stayed editable while the read was out: `quiesce` settles what
|
||||
// is running when it is called, not the key for the rest of the resolution, so a
|
||||
// keystroke since can have started a save of its own. Left running, its rejection lands
|
||||
// after the baseline below and raises the conflict this just resolved.
|
||||
await UserDraftDbSyncer.quiesce(query)
|
||||
if (!stillOurs()) return
|
||||
// Now, and not in `quiesce`: the refused payload belongs to the version being replaced,
|
||||
// but until this point it was still the only copy of the edit, and a resolution that
|
||||
// gave up before here has to leave it behind.
|
||||
UserDraftDbSyncer.dropPending(query)
|
||||
UserDraftDbSyncer.clearConflict(query)
|
||||
initialStates[ws] = structuredClone(deployedState)
|
||||
// Everything else the load path takes from this same response. The variable can have
|
||||
// been deleted, recreated, or had its permissions changed while the conflict stood,
|
||||
// and these decide create-vs-update and write access — so refreshing only what is
|
||||
// displayed would leave those deciding on the version the user just replaced.
|
||||
existedInitially[ws] = !(v as any).no_deployed
|
||||
extraPerms[ws] = v.extra_perms ?? {}
|
||||
UserDraftDbSyncer.recordRemoteSync(query, (v as any).draft_saved_at)
|
||||
UserDraft.seed(
|
||||
'variable',
|
||||
p,
|
||||
((v as any).draft as VariableState | undefined) ?? deployedState,
|
||||
{ workspace: ws }
|
||||
)
|
||||
} catch (e) {
|
||||
// Nothing was given up above, so the conflict stands and the edit is still here to
|
||||
// resolve again — which is the whole point of reading first.
|
||||
sendUserToast(`Could not load the other version: ${e}`, true)
|
||||
} finally {
|
||||
conflictSession.finish(token)
|
||||
}
|
||||
}
|
||||
|
||||
// The list-page `*` hint is owned by UserDraftDbSyncer (set on save, cleared
|
||||
// on delete). The editor only CLEARS it — a workspace at the deployed
|
||||
// baseline has no draft, so drop any stale hint (this is how a draft
|
||||
@@ -210,8 +333,21 @@
|
||||
labels: v.labels ?? undefined,
|
||||
wsSpecific: v.ws_specific ?? false
|
||||
}
|
||||
// Open with the saved draft if present, else the deployed.
|
||||
const s: VariableState = savedDraftState ?? deployedState
|
||||
// A refused save leaves this tab's own version parked. `.draft` is the version
|
||||
// that refused it, so opening on that would quietly drop the edit the alert is
|
||||
// about and leave "Keep mine" offering to keep the other one.
|
||||
const conflictQuery = { workspace: ws, itemKind: 'variable' as const, path: p }
|
||||
const refused = UserDraftDbSyncer.getConflict(conflictQuery).conflict
|
||||
? UserDraftDbSyncer.peekPending(conflictQuery)
|
||||
: undefined
|
||||
// A parked `null` is this tab's "no draft any more", which on screen is the
|
||||
// deployed value.
|
||||
const refusedDraft = (refused?.value ?? undefined) as VariableState | undefined
|
||||
// Open with this tab's refused version if there is one, else the saved draft,
|
||||
// else the deployed.
|
||||
const s: VariableState = refused
|
||||
? (refusedDraft ?? deployedState)
|
||||
: (savedDraftState ?? deployedState)
|
||||
ensureHandle(ws, s)
|
||||
initialStates[ws] = structuredClone(deployedState)
|
||||
// Draft-only paths (`no_deployed`) have no row — saving must
|
||||
@@ -223,6 +359,8 @@
|
||||
})
|
||||
|
||||
function reset() {
|
||||
// A new session starts here, so anything still running for the last one is spent.
|
||||
endEditingSession()
|
||||
// Clearing workspaceSpecs triggers useMany's reconcile to release
|
||||
// every acquired entry. The $derived `states` then collapses to {}.
|
||||
workspaceSpecs = []
|
||||
@@ -334,7 +472,14 @@
|
||||
{#if inline}
|
||||
{@render content()}
|
||||
{:else}
|
||||
<Drawer bind:this={drawer} size="50rem" on:close={() => clearPageDrawerAnchor(VARIABLES_PATH)}>
|
||||
<Drawer
|
||||
bind:this={drawer}
|
||||
size="50rem"
|
||||
on:close={() => {
|
||||
endEditingSession()
|
||||
clearPageDrawerAnchor(VARIABLES_PATH)
|
||||
}}
|
||||
>
|
||||
{@render content()}
|
||||
</Drawer>
|
||||
{/if}
|
||||
@@ -345,9 +490,24 @@
|
||||
bannerReserved={edit}
|
||||
hideClose={inline && !onClose}
|
||||
fullScreen={!inline}
|
||||
on:close={() => (inline ? onClose?.() : drawer?.closeDrawer())}
|
||||
on:close={() => {
|
||||
// Inline has no drawer to emit a close, so the session ends here instead.
|
||||
if (inline) {
|
||||
endEditingSession()
|
||||
onClose?.()
|
||||
} else {
|
||||
drawer?.closeDrawer()
|
||||
}
|
||||
}}
|
||||
>
|
||||
{#snippet banner()}
|
||||
{#if draftConflict}
|
||||
<DraftConflictAlert
|
||||
busy={resolvingConflict}
|
||||
onReload={() => void resolveDraftConflict(false)}
|
||||
onOverwrite={() => void resolveDraftConflict(true)}
|
||||
/>
|
||||
{/if}
|
||||
<LocalDraftBanner
|
||||
show={edit && selectedDirty}
|
||||
reserveSpace={edit}
|
||||
|
||||
@@ -0,0 +1,65 @@
|
||||
import { untrack } from 'svelte'
|
||||
|
||||
/**
|
||||
* Identity for one editing session's conflict resolution.
|
||||
*
|
||||
* An editor outlives what it shows, so an awaited step cannot tell from the workspace and path
|
||||
* alone whether it still speaks for what is on screen: both come back to the values it started
|
||||
* with when the user returns to where they were. Every resolution takes a token and checks it
|
||||
* before touching shared editor state.
|
||||
*/
|
||||
export function createDraftConflictSession() {
|
||||
let generation = $state(0)
|
||||
/** The token of the resolution in flight, or 0. A token rather than a flag: one left over from
|
||||
* a closed session must not clear the busy state of the session that replaced it. */
|
||||
let inFlight = $state(0)
|
||||
|
||||
return {
|
||||
/** A resolution started in the current session is in flight. Ending a session clears it, so
|
||||
* a reopened editor never shows buttons that only a stale request settling can re-enable. */
|
||||
get busy(): boolean {
|
||||
return inFlight !== 0
|
||||
},
|
||||
/** Claim the session for a resolution about to start. */
|
||||
start(): number {
|
||||
inFlight = ++generation
|
||||
return inFlight
|
||||
},
|
||||
/** Whether `token` still speaks for the editor. */
|
||||
holds(token: number): boolean {
|
||||
return token === generation
|
||||
},
|
||||
/** Release `token`'s claim. A stale token is ignored. */
|
||||
finish(token: number): void {
|
||||
if (inFlight === token) inFlight = 0
|
||||
},
|
||||
/** Nothing outstanding speaks for this editor any more: a different item, a different
|
||||
* session on the same one, or the component going away. */
|
||||
end(): void {
|
||||
generation++
|
||||
inFlight = 0
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A session for an editor that swaps what it shows in place, `scopeOf` naming what that currently
|
||||
* is. Leaving a scope and coming back is a new session, so the change itself ends the old one:
|
||||
* afterwards every value a resolution captured before the switch reads as current again, and no
|
||||
* later comparison could tell it apart from one that never left.
|
||||
*/
|
||||
export function useDraftConflictSession(scopeOf: () => unknown) {
|
||||
const session = createDraftConflictSession()
|
||||
|
||||
let lastScope = untrack(scopeOf)
|
||||
$effect(() => {
|
||||
const next = scopeOf()
|
||||
untrack(() => {
|
||||
if (next === lastScope) return
|
||||
lastScope = next
|
||||
session.end()
|
||||
})
|
||||
})
|
||||
|
||||
return session
|
||||
}
|
||||
@@ -0,0 +1,31 @@
|
||||
import { describe, it, expect } from 'vitest'
|
||||
|
||||
import { createDraftConflictSession } from './draftConflictSession.svelte'
|
||||
|
||||
/**
|
||||
* A resolution the user has walked away from must neither speak for what replaced it nor hold its
|
||||
* buttons disabled until it settles.
|
||||
*/
|
||||
describe('createDraftConflictSession', () => {
|
||||
it('frees the next session from a resolution the last one left in flight', () => {
|
||||
const session = createDraftConflictSession()
|
||||
|
||||
const stale = session.start()
|
||||
expect(session.busy).toBe(true)
|
||||
|
||||
session.end()
|
||||
expect(session.busy).toBe(false)
|
||||
expect(session.holds(stale)).toBe(false)
|
||||
|
||||
const fresh = session.start()
|
||||
expect(session.busy).toBe(true)
|
||||
expect(session.holds(fresh)).toBe(true)
|
||||
|
||||
// The stale request settles last: it must not release the live session's claim.
|
||||
session.finish(stale)
|
||||
expect(session.busy).toBe(true)
|
||||
|
||||
session.finish(fresh)
|
||||
expect(session.busy).toBe(false)
|
||||
})
|
||||
})
|
||||
@@ -723,5 +723,35 @@ export const UserDraftDbSyncer = {
|
||||
const key = draftKey(query.workspace, query.itemKind, query.path)
|
||||
pendingSaveOpts.delete(key)
|
||||
debouncer.cancel(key)
|
||||
},
|
||||
|
||||
/**
|
||||
* The value parked for this key, wrapped so a parked delete (`null`) stays distinguishable
|
||||
* from nothing parked at all. While a conflict stands this is the version the server refused,
|
||||
* which is to say this tab's own: an editor opening on the key takes it over the server's
|
||||
* draft, which is the version that did the refusing.
|
||||
*/
|
||||
peekPending(query: UserDraftLastSyncQuery): { value: unknown } | undefined {
|
||||
const parked = pendingSaveOpts.get(draftKey(query.workspace, query.itemKind, query.path))
|
||||
return parked ? { value: parked.value } : undefined
|
||||
},
|
||||
|
||||
/**
|
||||
* Stop scheduling saves for this key and wait until nothing for it is still in flight.
|
||||
* Cancelling alone cannot stop a POST the runner already started, and such a POST settles
|
||||
* *after* the caller has moved on — a rejected one re-raising the conflict it was told to
|
||||
* resolve. Await this before installing a baseline that would make a stale payload acceptable.
|
||||
*
|
||||
* What is parked is deliberately left alone: a refused save keeps its payload here, and while
|
||||
* the conflict stands that is the only copy of the edit outside the editor's own memory. A
|
||||
* caller that gives up half way must leave it recoverable, so dropping it is the committing
|
||||
* caller's job, via `dropPending`, once it has something to replace it with.
|
||||
*/
|
||||
async quiesce(query: UserDraftLastSyncQuery): Promise<void> {
|
||||
const key = draftKey(query.workspace, query.itemKind, query.path)
|
||||
debouncer.cancel(key)
|
||||
await runner.settled(key)
|
||||
// A save that landed while we waited can have scheduled the next one.
|
||||
debouncer.cancel(key)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,126 @@
|
||||
import { describe, it, expect, afterEach, vi } from 'vitest'
|
||||
|
||||
// Mocked so a test can hold a POST in flight: that window is the whole point of `quiesce`,
|
||||
// which exists to wait one out rather than return while it is still going.
|
||||
const updateDraft = vi.fn(async (..._args: any[]) => ({
|
||||
status: 'saved' as const,
|
||||
current_timestamp: '2020-01-01T00:00:00Z'
|
||||
}))
|
||||
|
||||
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'
|
||||
|
||||
function deferred<T = void>() {
|
||||
let resolve!: (v: T) => void
|
||||
const promise = new Promise<T>((res) => (resolve = res))
|
||||
return { promise, resolve }
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
vi.clearAllMocks()
|
||||
updateDraft.mockResolvedValue({ status: 'saved', current_timestamp: '2020-01-01T00:00:00Z' })
|
||||
})
|
||||
|
||||
/**
|
||||
* Resolving a draft conflict installs a fresh baseline. Anything of the old version still in the
|
||||
* air when that lands would be made acceptable by it, and a rejected one re-raises the conflict
|
||||
* just resolved — so the resolution waits for the key to go quiet first. What it must NOT do is
|
||||
* discard the payload while waiting: a refused save keeps it here, and until the resolution
|
||||
* commits it is the only copy of the edit outside the editor's own memory.
|
||||
*/
|
||||
describe('UserDraftDbSyncer.quiesce', () => {
|
||||
it('waits for a save already in flight', async () => {
|
||||
const q = { workspace: 'w', itemKind: 'variable' as const, path: 'u/me/quiesce_a' }
|
||||
const inFlight = deferred()
|
||||
let settledBeforeQuiesceReturned = false
|
||||
updateDraft.mockImplementationOnce(async () => {
|
||||
await inFlight.promise
|
||||
settledBeforeQuiesceReturned = true
|
||||
return { status: 'saved', current_timestamp: '2020-01-01T00:00:00Z' }
|
||||
})
|
||||
|
||||
// In flight, not awaited: the POST is held open by the mock above.
|
||||
const saving = UserDraftDbSyncer.save({ ...q, value: { v: 'in flight' }, immediate: true })
|
||||
|
||||
let quiesced = false
|
||||
const quiescing = UserDraftDbSyncer.quiesce(q).then(() => (quiesced = true))
|
||||
// A full macrotask, not a microtask: anything that merely yields would have resolved by
|
||||
// now, so this is what tells "waited for the POST" apart from "waited for nothing".
|
||||
await new Promise((r) => setTimeout(r, 0))
|
||||
expect(quiesced).toBe(false)
|
||||
|
||||
inFlight.resolve()
|
||||
await quiescing
|
||||
await saving
|
||||
expect(settledBeforeQuiesceReturned).toBe(true)
|
||||
})
|
||||
|
||||
it('keeps a refused payload recoverable, until the resolution that replaces it says otherwise', async () => {
|
||||
const q = { workspace: 'w', itemKind: 'variable' as const, path: 'u/me/quiesce_b' }
|
||||
// Refused, so the payload stays parked — the state a conflict resolution starts from.
|
||||
updateDraft.mockResolvedValueOnce({
|
||||
status: 'conflict',
|
||||
current_timestamp: '2020-01-02T00:00:00Z'
|
||||
})
|
||||
await UserDraftDbSyncer.save({ ...q, value: { v: 'refused' }, immediate: true })
|
||||
expect(UserDraftDbSyncer.getConflict(q).conflict).toBeTruthy()
|
||||
|
||||
await UserDraftDbSyncer.quiesce(q)
|
||||
|
||||
// A resolution abandoned here (the editor closed while quiescing) must leave the edit
|
||||
// somewhere it can still be sent. Flushing is how that payload is observed, since the
|
||||
// parked opts themselves are private to the syncer.
|
||||
updateDraft.mockClear()
|
||||
await UserDraftDbSyncer.flush(q)
|
||||
expect(updateDraft).toHaveBeenCalledTimes(1)
|
||||
expect(updateDraft.mock.calls[0][0]).toMatchObject({ requestBody: { value: { v: 'refused' } } })
|
||||
|
||||
// Only the caller that has something to put in its place drops it.
|
||||
UserDraftDbSyncer.dropPending(q)
|
||||
updateDraft.mockClear()
|
||||
await UserDraftDbSyncer.flush(q)
|
||||
expect(updateDraft).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('keeps a parked delete distinguishable from nothing parked', async () => {
|
||||
const q = { workspace: 'w', itemKind: 'variable' as const, path: 'u/me/quiesce_delete' }
|
||||
|
||||
expect(UserDraftDbSyncer.peekPending(q)).toBeUndefined()
|
||||
|
||||
// Reverting to the deployed value, or discarding, parks a delete. Refused, it stays
|
||||
// parked — and a resolution reading it has to keep meaning "remove the row", not fall
|
||||
// back to whatever the server holds.
|
||||
updateDraft.mockResolvedValueOnce({
|
||||
status: 'conflict',
|
||||
current_timestamp: '2020-01-02T00:00:00Z'
|
||||
})
|
||||
await UserDraftDbSyncer.save({ ...q, value: null, immediate: true })
|
||||
|
||||
expect(UserDraftDbSyncer.peekPending(q)).toEqual({ value: null })
|
||||
})
|
||||
|
||||
it('cancels a debounced autosave so it cannot displace the write that follows', async () => {
|
||||
const q = { workspace: 'w', itemKind: 'variable' as const, path: 'u/me/quiesce_c' }
|
||||
vi.useFakeTimers()
|
||||
try {
|
||||
// Debounced rather than immediate: this is the autosave a resolution has to call off, or
|
||||
// it fires mid-resolution and, being conditional, is refused.
|
||||
void UserDraftDbSyncer.save({ ...q, value: { v: 'queued' } })
|
||||
|
||||
await UserDraftDbSyncer.quiesce(q)
|
||||
updateDraft.mockClear()
|
||||
|
||||
// Past the debouncer's 10s ceiling, so a schedule that survived has certainly fired.
|
||||
// Real time would not reach it: the test would pass with the cancelling removed.
|
||||
await vi.advanceTimersByTimeAsync(11000)
|
||||
expect(updateDraft).not.toHaveBeenCalled()
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user