mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-13 08:02:47 +00:00
fix(schedules): baseline the enabled state the server accepted, not the one the toggle is claiming
The toggle sets `enabled` before its request and puts it back if that fails, so the form's value is a claim until it lands — and a save adopting it wrote an uncommitted toggle into its baseline, where cancelling the request left it: a false dirty state, and a Discard offering a value the server never took. The committed value is tracked and both the baseline and what counts as written take it, so the settle compares like for like and drops its draft instead of leaving a row behind. The reload queue keys by workspace as well as page: a draft landing in one says nothing about the same page in another, and reloading it there costs that frame its scroll. A restore reports the path it restored rather than leaving the caller to read a live one its own await has outlived. 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
eb80261258
commit
45c53a37da
@@ -30,9 +30,11 @@
|
||||
}: {
|
||||
workspace?: string
|
||||
disableChatOffset?: boolean
|
||||
/** A version was restored, in the workspace the history was pointed at — which
|
||||
* `WsSpecificVersions` can make a linked one rather than the acting workspace. */
|
||||
onRestored?: (workspace: string) => void
|
||||
/** A version was restored, at this path and in the workspace the history was
|
||||
* pointed at — which `WsSpecificVersions` can make a linked one rather than the
|
||||
* acting workspace. Both reported rather than left for the caller to read off
|
||||
* its own live state, which the restore's await has already outlived. */
|
||||
onRestored?: (workspace: string, path: string) => void
|
||||
/** Fires after Save has written, with the path it wrote to — which is not the
|
||||
* one it was opened on when the user renamed it — and the workspace and path
|
||||
* it started on. For a caller showing state derived from the resource;
|
||||
@@ -330,7 +332,7 @@
|
||||
editorGeneration++
|
||||
// Its own callback rather than the `refresh` event: callers bind that to
|
||||
// reopening a picker (EditorBar), which a restore should not trigger.
|
||||
onRestored?.(historyWorkspace)
|
||||
if (path) onRestored?.(historyWorkspace, path)
|
||||
}}
|
||||
/>
|
||||
{/if}
|
||||
|
||||
@@ -45,7 +45,7 @@
|
||||
workspace={workspaceId}
|
||||
{onBack}
|
||||
{onRemoved}
|
||||
onRestored={(ws) => path && onSavedTo?.(path, path, ws)}
|
||||
onRestored={(ws, restored) => onSavedTo?.(restored, restored, ws)}
|
||||
onSaved={(saved, from, fromWs) => {
|
||||
if (saved && from && fromWs) onSavedTo?.(saved, from, fromWs)
|
||||
}}
|
||||
|
||||
@@ -131,6 +131,10 @@
|
||||
let initNewPath = $state(false)
|
||||
let path: string = $state('')
|
||||
let enabled: boolean = $state(false)
|
||||
// What the server has accepted for `enabled`. The toggle sets `enabled` before
|
||||
// its request and puts it back if that fails, so the form's value is a claim
|
||||
// until then — and a save must not adopt a claim as its deployed baseline.
|
||||
let deployedEnabled = false
|
||||
let pathError = $state('')
|
||||
let summary = $state('')
|
||||
let labels: string[] | undefined = $state(undefined)
|
||||
@@ -545,6 +549,7 @@
|
||||
initialCronVersion = cronVersion
|
||||
isLatestCron = cronVersion == 'v2'
|
||||
enabled = cfg.enabled
|
||||
deployedEnabled = cfg.enabled
|
||||
schedule = cfg.schedule
|
||||
initialSchedule = schedule
|
||||
timezone = cfg.timezone
|
||||
@@ -632,16 +637,18 @@
|
||||
if (wasCreate) {
|
||||
const sentEnabled = scheduleCfg.enabled
|
||||
scheduleCfg.enabled = true
|
||||
deployedEnabled = true
|
||||
if (enabled === sentEnabled) enabled = true
|
||||
} else {
|
||||
// An update's payload carries no `enabled` (see saveScheduleFromCfg), so
|
||||
// this write did not move it: what counts as written keeps the value the
|
||||
// server has accepted, and the baseline below with it.
|
||||
scheduleCfg.enabled = deployedEnabled
|
||||
}
|
||||
// What was sent is the deployed value now. Adopted here rather than by
|
||||
// remounting: the form stays editable during the write, and a remount would
|
||||
// re-read over an edit made then — which `settleDraftAfterWrite` keeps.
|
||||
initialConfig = structuredClone(scheduleCfg)
|
||||
// An update's payload carries no `enabled` (see saveScheduleFromCfg): the
|
||||
// toggle deploys that field on its own, so the deployed value is whatever it
|
||||
// last set, not what this save's click-time snapshot happens to hold.
|
||||
if (!wasCreate) initialConfig.enabled = enabled
|
||||
// An edit made while the write was in flight is kept rather than settled away
|
||||
// — and the host must not remount over it, which is the only thing that can
|
||||
// tell it so.
|
||||
@@ -778,6 +785,7 @@
|
||||
}
|
||||
sendUserToast(`${nEnabled ? 'enabled' : 'disabled'} schedule ${path}`)
|
||||
// Deployed state moved, so the baseline the banner compares against does too.
|
||||
deployedEnabled = nEnabled
|
||||
if (initialConfig) initialConfig.enabled = nEnabled
|
||||
// This request carried the enabled flag alone: anything else the form has
|
||||
// diverged into is an edit of the user's, which a remount would drop.
|
||||
|
||||
@@ -588,7 +588,13 @@
|
||||
const owner = getRuntime(s.id)?.previewTabs
|
||||
if (!owner) continue
|
||||
const workspace = getEffectiveWorkspaceId(s)
|
||||
for (const tab of tabsToReload(owner.tabs, pages)) {
|
||||
// The queue names the workspace each page was touched in; this session only
|
||||
// cares about the ones touched in its own.
|
||||
const prefix = `${workspace}\u0000`
|
||||
const mine = new Set(
|
||||
[...pages].filter((p) => p.startsWith(prefix)).map((p) => p.slice(prefix.length))
|
||||
)
|
||||
for (const tab of tabsToReload(owner.tabs, mine)) {
|
||||
const key = tabKey(s.id, tab.id)
|
||||
// A list tab shows every row, so any mutation on its page is its
|
||||
// business. A hosted entity tab shows one item, and is told apart here.
|
||||
@@ -639,7 +645,7 @@
|
||||
// otherwise. Queued rather than reloaded here, so one deploy's writes and
|
||||
// this report collapse into a single reload.
|
||||
if (!entity) {
|
||||
queuePageReload(page)
|
||||
queuePageReload(ev.workspace, page)
|
||||
continue
|
||||
}
|
||||
if (entity.path !== ev.path) continue
|
||||
@@ -657,9 +663,12 @@
|
||||
}
|
||||
// One queue for every reason a previewed list page goes stale — a chat tool, a
|
||||
// hosted editor's write, a draft landing afterwards — so the same frame is not
|
||||
// reloaded several times for one deploy.
|
||||
function queuePageReload(page: string) {
|
||||
pendingPages.add(page)
|
||||
// reloaded several times for one deploy. Keyed by workspace too: a write in one
|
||||
// says nothing about the same page in another, and reloading it there costs the
|
||||
// frame its scroll and everything else it holds.
|
||||
function queuePageReload(workspace: string | undefined, page: string) {
|
||||
if (!workspace) return
|
||||
pendingPages.add(`${workspace}\u0000${page}`)
|
||||
clearTimeout(reloadHandle)
|
||||
reloadHandle = setTimeout(flushReload, 500)
|
||||
}
|
||||
@@ -675,9 +684,9 @@
|
||||
// reloads against writes, reload when one lands: a draft written after a save —
|
||||
// an edit made while it was in flight — reaches the list this way too.
|
||||
$effect(() => {
|
||||
return UserDraftDbSyncer.onAnySaved(({ itemKind }) => {
|
||||
return UserDraftDbSyncer.onAnySaved(({ workspace, itemKind }) => {
|
||||
const page = entityListPage(itemKind as EntityEditorKind)?.path
|
||||
if (page) queuePageReload(page)
|
||||
if (page) queuePageReload(workspace, page)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -686,7 +695,7 @@
|
||||
setToolCompletionListener((name, args, workspace) => {
|
||||
const { pages, entity, path } = toolReloadEffect(name, args)
|
||||
if (pages.length === 0) return
|
||||
for (const p of pages) pendingPages.add(p)
|
||||
for (const p of pages) pendingPages.add(`${workspace}\u0000${p}`)
|
||||
// A write reaches a hosted editor through the draft cell it holds, but an
|
||||
// editor still loading holds none yet and the seed no-opped past it. The
|
||||
// miss is recorded when it happens — asking now would be too late, since
|
||||
|
||||
Reference in New Issue
Block a user