From a32f2576864a9e1f59c44cc262c85db3a5b7cc64 Mon Sep 17 00:00:00 2001 From: Diego Imbert Date: Tue, 12 May 2026 22:42:50 +0200 Subject: [PATCH] feat(frontend): staleness modal for the script editor's local autosave MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the script editor's toast-based "Discard / Show diff" pattern with a dedicated modal that surfaces *why* the local autosave is out of date: a new DB draft on the server, or a new deployed version. Adds `checkStaleness` (UserDraftMeta vs current backend revs, draft-rev priority) and a `setMeta({ force: true })` mode so the "Keep current draft" acknowledgement persists even when it happens to be the entry's first state mutation — under `saveInitialValue: false` an ack-only setMeta would otherwise be skipped and the modal would re-fire on next mount. The modal lives at LocalDraftStaleModal.svelte; the script editor wires it as a template for the remaining editors. Other editors (flows, apps, raw_apps, resources, variables, triggers) still use the previous toast pattern and will be migrated in follow-up commits. --- .../LocalDraftStaleModal.svelte | 106 ++++++++++++++++ frontend/src/lib/userDraft.svelte.ts | 63 +++++++++- frontend/src/lib/userDraft.test.ts | 62 ++++++++++ .../scripts/edit/[...path]/+page.svelte | 115 ++++++++++++------ 4 files changed, 305 insertions(+), 41 deletions(-) create mode 100644 frontend/src/lib/components/common/confirmationModal/LocalDraftStaleModal.svelte diff --git a/frontend/src/lib/components/common/confirmationModal/LocalDraftStaleModal.svelte b/frontend/src/lib/components/common/confirmationModal/LocalDraftStaleModal.svelte new file mode 100644 index 0000000000..5ecc13f5a5 --- /dev/null +++ b/frontend/src/lib/components/common/confirmationModal/LocalDraftStaleModal.svelte @@ -0,0 +1,106 @@ + + + + +{#if open} + +{/if} diff --git a/frontend/src/lib/userDraft.svelte.ts b/frontend/src/lib/userDraft.svelte.ts index 32018b3cdc..0dca13ce52 100644 --- a/frontend/src/lib/userDraft.svelte.ts +++ b/frontend/src/lib/userDraft.svelte.ts @@ -113,6 +113,55 @@ function extractMeta(stored: StoredDraft | undefined): UserDraftMeta { return meta } +/** + * Compares the rev metadata recorded against the local draft to the current + * backend revs. Returns the staleness cause, or `null` when the local draft + * is still based on the latest backend state we know about. + * + * - Entries with no recorded meta (legacy entries written before this field + * existed) report `null` — we can't tell if they're stale, and we'd rather + * trust the local autosave than spam the user with false positives. + * - DB-draft staleness wins over deployed-version staleness: a remote DB + * draft is the more recent state to reconcile against. + * - If a DB draft existed when the local autosave was created but now no + * longer exists on the remote (someone discarded it), we report `version` + * because the deployed version is now the canonical "latest saved". + */ +export type UserDraftStalenessCause = 'draft' | 'version' + +export function checkStaleness( + meta: UserDraftMeta, + currentRev: string | number | undefined, + currentDraftRev?: string | number | undefined +): UserDraftStalenessCause | null { + if (meta.remoteRev === undefined && meta.remoteDraftRev === undefined) return null + if (meta.remoteDraftRev !== currentDraftRev) { + return currentDraftRev !== undefined ? 'draft' : 'version' + } + if (currentRev !== undefined && meta.remoteRev !== currentRev) return 'version' + return null +} + +/** + * Synchronously write to localStorage, bypassing the entry's setter. Used by + * `setMeta({ force: true })` so a "Keep current draft" modal acknowledgement + * persists even when it happens to be the first state mutation of the entry + * (which `useLocalStorageValue`'s `saveInitialValue: false` contract would + * otherwise skip). + */ +function persistDirect(key: string, value: V | undefined, meta: UserDraftMeta): void { + try { + const next = wrap(value, meta) + if (next === undefined) { + localStorage.removeItem(key) + } else { + localStorage.setItem(key, JSON.stringify(next)) + } + } catch (e) { + console.error('UserDraft: localStorage write failed', e) + } +} + function readPersisted(key: string): StoredDraft | undefined { try { const raw = localStorage.getItem(key) @@ -172,8 +221,15 @@ export type UserDraftHandle = { * Used after the "Keep current draft" modal action to ack the new remote * rev so the staleness modal doesn't fire again until the remote moves * further. + * + * Pass `{ force: true }` to also persist the new meta to localStorage + * synchronously, bypassing `saveInitialValue: false`'s skip of the first + * state mutation. Without `force`, an ack that happens to be the first + * write on the entry would only update in-memory state and be lost on + * next mount — re-triggering the staleness modal even after the user + * just acknowledged it. */ - setMeta(meta: UserDraftMeta): void + setMeta(meta: UserDraftMeta, opts?: { force?: boolean }): void } export const UserDraft = { @@ -314,10 +370,13 @@ export const UserDraft = { setDraftAndMeta(value: V | undefined, meta: UserDraftMeta): void { sharedEntry.state.val = wrap(value, meta) }, - setMeta(meta: UserDraftMeta): void { + setMeta(meta: UserDraftMeta, opts?: { force?: boolean }): void { const current = sharedEntry.state.val as StoredDraft | undefined if (current === undefined) return sharedEntry.state.val = wrap(current.value, meta) + if (opts?.force && !isLocalOnly(path)) { + persistDirect(localStorageKey(ws, itemKind, path), current.value, meta) + } } } } diff --git a/frontend/src/lib/userDraft.test.ts b/frontend/src/lib/userDraft.test.ts index 69d25457b3..f897c35f87 100644 --- a/frontend/src/lib/userDraft.test.ts +++ b/frontend/src/lib/userDraft.test.ts @@ -364,6 +364,68 @@ describe('UserDraft — rev metadata for staleness checks', () => { expect(handle.draft).toBe('no-rev') expect(handle.meta).toEqual({}) }) + + it('setMeta({ force: true }) persists immediately, bypassing the first-write skip', () => { + localStorage.setItem( + 'userdraft/w/test_ws/flow/u/me/forceack', + JSON.stringify({ value: 'edited', remoteRev: 'v1' }) + ) + const handle = UserDraft.use('flow', 'u/me/forceack') + + // Without force, this is the entry's first state mutation and gets + // swallowed by saveInitialValue=false — localStorage would still + // hold the old remoteRev. + handle.setMeta({ remoteRev: 'v2' }, { force: true }) + + expect(handle.meta).toEqual({ remoteRev: 'v2' }) + expect(localStorage.getItem('userdraft/w/test_ws/flow/u/me/forceack')).toBe( + JSON.stringify({ value: 'edited', remoteRev: 'v2' }) + ) + }) +}) + +describe('checkStaleness', () => { + let checkStaleness: ( + meta: { remoteRev?: string | number; remoteDraftRev?: string | number }, + currentRev: string | number | undefined, + currentDraftRev?: string | number | undefined + ) => 'draft' | 'version' | null + + beforeEach(async () => { + // Re-import to dodge ESM caching surprises across test files. + ;({ checkStaleness } = await import('./userDraft.svelte')) + }) + + it('returns null for legacy entries with no recorded rev', () => { + expect(checkStaleness({}, 'h1', '2026-01-01')).toBeNull() + }) + + it('returns null when meta matches current revs exactly', () => { + expect(checkStaleness({ remoteRev: 'h1', remoteDraftRev: 'd1' }, 'h1', 'd1')).toBeNull() + expect(checkStaleness({ remoteRev: 'h1' }, 'h1', undefined)).toBeNull() + }) + + it('returns "draft" when a newer DB draft was pushed on the remote', () => { + expect(checkStaleness({ remoteRev: 'h1', remoteDraftRev: 'd1' }, 'h1', 'd2')).toBe('draft') + }) + + it('returns "draft" when the remote gained a DB draft that we didn\'t baseline against', () => { + expect(checkStaleness({ remoteRev: 'h1' }, 'h1', 'd1')).toBe('draft') + }) + + it('returns "version" when the deployed rev moved and draft revs match', () => { + expect(checkStaleness({ remoteRev: 'h1' }, 'h2', undefined)).toBe('version') + }) + + it('returns "version" when the baseline draft was deleted on the remote (no current draft)', () => { + expect(checkStaleness({ remoteRev: 'h1', remoteDraftRev: 'd1' }, 'h1', undefined)).toBe( + 'version' + ) + }) + + it('prefers "draft" over "version" when both have changed', () => { + expect(checkStaleness({ remoteRev: 'h1', remoteDraftRev: 'd1' }, 'h2', 'd2')).toBe('draft') + }) }) describe('UserDraft.use() — reference counting & cleanup', () => { diff --git a/frontend/src/routes/(root)/(logged)/scripts/edit/[...path]/+page.svelte b/frontend/src/routes/(root)/(logged)/scripts/edit/[...path]/+page.svelte index 5fb30187d3..33639dfbb3 100644 --- a/frontend/src/routes/(root)/(logged)/scripts/edit/[...path]/+page.svelte +++ b/frontend/src/routes/(root)/(logged)/scripts/edit/[...path]/+page.svelte @@ -9,12 +9,13 @@ import { sendUserToast } from '$lib/toast' import DiffDrawer from '$lib/components/DiffDrawer.svelte' import UnsavedConfirmationModal from '$lib/components/common/confirmationModal/UnsavedConfirmationModal.svelte' + import LocalDraftStaleModal from '$lib/components/common/confirmationModal/LocalDraftStaleModal.svelte' import type { ScheduleTrigger } from '$lib/components/triggers' import type { Trigger } from '$lib/components/triggers/utils' import { get } from 'svelte/store' import { untrack } from 'svelte' import { page } from '$app/state' - import { UserDraft } from '$lib/userDraft.svelte' + import { UserDraft, checkStaleness, type UserDraftMeta } from '$lib/userDraft.svelte' type EditableScript = NewScript & { draft_triggers?: Trigger[] } @@ -41,6 +42,43 @@ let savedPrimarySchedule: ScheduleTrigger | undefined = $state(undefined) + // Local-draft staleness modal: opened when the remote (deployed or DB + // draft) has moved on since the user's autosave was created. + let staleModalOpen = $state(false) + let staleModalCause = $state<'draft' | 'version'>('version') + let pendingBaseline: { baseline: EditableScript; revs: UserDraftMeta } | undefined = undefined + + function applyBaseline(baseline: EditableScript): void { + initialPath = baseline.path + scriptBuilder?.setDraftTriggers(baseline.draft_triggers) + scriptBuilder?.setCode(baseline.content) + if (baseline['primary_schedule']) { + savedPrimarySchedule = baseline['primary_schedule'] + scriptBuilder?.setPrimarySchedule(savedPrimarySchedule) + } + } + + function onStaleLoadLatest(): void { + if (!pendingBaseline) { + staleModalOpen = false + return + } + const { baseline, revs } = pendingBaseline + UserDraft.remove('script', draftPath) + scriptHandle.setDraftAndMeta(baseline, revs) + applyBaseline(baseline) + pendingBaseline = undefined + staleModalOpen = false + } + + function onStaleKeepDraft(): void { + if (pendingBaseline) { + scriptHandle.setMeta(pendingBaseline.revs, { force: true }) + } + pendingBaseline = undefined + staleModalOpen = false + } + /** Increments per `loadScript` call. Stale loads (e.g. when picker * navigation races a draft-discard reload) bail at the next checkpoint * after their captured token no longer matches. */ @@ -65,9 +103,14 @@ savedScript = structuredClone($state.snapshot(scriptWithDraft)) const localDraft = scriptHandle.draft + const previousMeta = scriptHandle.meta const backendDraft = scriptWithDraft.draft ? ({ ...scriptWithDraft.draft } as EditableScript) : undefined + const newRevs: UserDraftMeta = { + remoteRev: scriptWithDraft.hash, + remoteDraftRev: scriptWithDraft.draft_created_at + } // Compute the fully-baked initial value once so the assignment // below is a single write — otherwise post-load mutations like @@ -81,64 +124,52 @@ } if (localDraft != undefined) { - // Local autosave wins; offer a diff against the latest saved - // version (backend draft if any, otherwise deployed) so the - // user can discard it. const reference = backendDraft ?? scriptWithDraft const referenceClean = cleanValueProperties(reference) const localClean = cleanValueProperties(localDraft) if (orderedJsonStringify(referenceClean) === orderedJsonStringify(localClean)) { // Local matches the saved version — silently drop it and use the saved one. - scriptHandle.draft = bakedBaseline + UserDraft.remove('script', draftPath) + scriptHandle.setDraftAndMeta(bakedBaseline, newRevs) } else { - sendUserToast('Script loaded from local autosave', false, [ - { - label: 'Discard local autosave', - callback: () => { - scriptHandle.draft = bakedBaseline - } - }, - { - label: 'Show diff', - callback: async () => { - diffDrawer?.openDrawer() - diffDrawer?.setDiff({ - mode: 'simple', - original: referenceClean, - current: localClean, - title: `${backendDraft ? 'Latest saved draft' : 'Deployed'} <> Autosave`, - button: { - text: 'Discard autosave', - onClick: () => { - scriptHandle.draft = bakedBaseline - } - } - }) - } - } - ]) + const cause = checkStaleness(previousMeta, newRevs.remoteRev, newRevs.remoteDraftRev) + if (cause) { + // Remote moved on since the local autosave was written — + // surface the choice via modal. The local draft stays on + // screen until the user picks. + pendingBaseline = { baseline: bakedBaseline, revs: newRevs } + staleModalCause = cause + staleModalOpen = true + } else if ( + previousMeta.remoteRev === undefined && + previousMeta.remoteDraftRev === undefined + ) { + // Legacy entry (no meta recorded) — backfill so future + // loads can detect staleness even if the user doesn't edit. + scriptHandle.setMeta(newRevs, { force: true }) + } } } else if (backendDraft) { - scriptHandle.draft = bakedBaseline - if (scriptHandle.draft?.['primary_schedule']) { - savedPrimarySchedule = scriptHandle.draft['primary_schedule'] + scriptHandle.setDraftAndMeta(bakedBaseline, newRevs) + if (bakedBaseline['primary_schedule']) { + savedPrimarySchedule = bakedBaseline['primary_schedule'] scriptBuilder?.setPrimarySchedule(savedPrimarySchedule) } - scriptBuilder?.setDraftTriggers(scriptHandle.draft.draft_triggers) + scriptBuilder?.setDraftTriggers(bakedBaseline.draft_triggers) if (!scriptWithDraft.draft_only) { const reloadAction = async () => { await DraftService.deleteDraft({ workspace: $workspaceStore!, kind: 'script', - path: scriptHandle.draft!.path + path: bakedBaseline.path }) UserDraft.remove('script', draftPath) - goto(`/scripts/edit/${scriptHandle.draft!.path}`) + goto(`/scripts/edit/${bakedBaseline.path}`) loadScript() } const deployed = cleanValueProperties(scriptWithDraft) - const draft = cleanValueProperties(scriptHandle.draft) + const draft = cleanValueProperties(bakedBaseline) sendUserToast('Script loaded from latest saved draft', false, [ { label: 'Discard draft reset to deployed version', @@ -160,7 +191,7 @@ ]) } } else { - scriptHandle.draft = bakedBaseline + scriptHandle.setDraftAndMeta(bakedBaseline, newRevs) } } @@ -214,6 +245,12 @@ + {#if scriptHandle.draft}