mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-13 08:02:47 +00:00
fix(sessions): mount a hosted entity editor per item, and scope its reports to its workspace
The schedule and variable views re-pointed one live editor at each new path, so the previous item's load — `openEdit`, `editVariable` and everything they await — finished into the new item's form, baseline, draft cell and permissions, last response winning. Keying each on its path gives every item its own instance, with nothing of the next one's to land on. The resource host already worked this way. A report's guard also matched kind and path but not workspace, so re-scoping an unsent session left a rename from the old workspace free to retarget the new one's tab onto a path that exists only where it was written. 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
5373cc2984
commit
8202413bb9
@@ -32,10 +32,10 @@
|
||||
disableChatOffset?: boolean
|
||||
onRestored?: () => 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 one it was opened
|
||||
* on. For a caller showing state derived from the resource; `onRestored` only
|
||||
* covers restoring a version. */
|
||||
onSaved?: (savedPath?: string, fromPath?: string) => void
|
||||
* 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;
|
||||
* `onRestored` only covers restoring a version. */
|
||||
onSaved?: (savedPath?: string, fromPath?: string, fromWorkspace?: string) => void
|
||||
/**
|
||||
* False renders the editor in place instead of in a drawer, for a host that
|
||||
* gives it a pane of its own (a session's resource tab). Same convention as
|
||||
@@ -47,9 +47,9 @@
|
||||
* user should be able to get back to (a session tab that took over the list). */
|
||||
onBack?: () => void
|
||||
/** The resource is gone — a draft-only one whose draft was discarded — with
|
||||
* the path it was showing. A host addressing it by path (a session tab) has
|
||||
* to stop showing it. */
|
||||
onRemoved?: (fromPath: string) => void
|
||||
* the workspace and path it was showing. A host addressing it by those (a
|
||||
* session tab) has to stop showing it. */
|
||||
onRemoved?: (fromPath: string, fromWorkspace: string) => void
|
||||
} = $props()
|
||||
|
||||
let drawer: Drawer | undefined = $state()
|
||||
@@ -180,7 +180,8 @@
|
||||
getCurrent={() => resourceEditor?.localDraftCurrent()}
|
||||
onDiscard={() => {
|
||||
const from = path
|
||||
if (resourceEditor?.discardLocalDraft() === false && from) onRemoved?.(from)
|
||||
if (resourceEditor?.discardLocalDraft() === false && from)
|
||||
onRemoved?.(from, effectiveWorkspace)
|
||||
}}
|
||||
disabled={!canWriteSelected}
|
||||
/>
|
||||
@@ -213,10 +214,11 @@
|
||||
unifiedSize="md"
|
||||
startIcon={{ icon: Save }}
|
||||
on:click={async () => {
|
||||
// The path this save started from. Read before the await: an inline host can
|
||||
// re-point the editor at another resource while the write is in flight, and
|
||||
// the caller needs to know which one the result belongs to.
|
||||
// Where this save started. Read before the await: an inline host can re-point
|
||||
// the editor at another resource, or re-scope the session to another
|
||||
// workspace, while the write is in flight.
|
||||
const from = path
|
||||
const fromWs = effectiveWorkspace
|
||||
// Closed before the write is awaited, the way it always was: `save()` toasts its
|
||||
// own failures and never rejects, so waiting would only add visible lag to every
|
||||
// caller of this drawer. `onSaved` still fires after the write lands.
|
||||
@@ -236,7 +238,7 @@
|
||||
const savedPath = livePath ?? path
|
||||
if (savedPath) path = savedPath
|
||||
editorGeneration++
|
||||
onSaved?.(savedPath, from)
|
||||
onSaved?.(savedPath, from, fromWs)
|
||||
}}
|
||||
disabled={!canSave}
|
||||
>
|
||||
|
||||
@@ -58,12 +58,12 @@
|
||||
* user should be able to get back to (a session tab that took over the list). */
|
||||
onBack?: () => void
|
||||
/** Fires after a save, with the path it wrote to — which is not the one it was
|
||||
* opened on when the user renamed it — and the one it was opened on. */
|
||||
onSaved?: (savedPath?: string, fromPath?: string) => void
|
||||
* opened on when the user renamed it — and the workspace and path it started on. */
|
||||
onSaved?: (savedPath?: string, fromPath?: string, fromWorkspace?: string) => void
|
||||
/** The variable is gone — a draft-only one whose draft was discarded — with the
|
||||
* path it was showing. A host addressing it by path (a session tab) has to stop
|
||||
* showing it. */
|
||||
onRemoved?: (fromPath: string) => void
|
||||
* workspace and path it was showing. A host addressing it by those (a session
|
||||
* tab) has to stop showing it. */
|
||||
onRemoved?: (fromPath: string, fromWorkspace: string) => void
|
||||
} = $props()
|
||||
let curWs = $derived(workspace ?? $workspaceStore)
|
||||
|
||||
@@ -282,6 +282,7 @@
|
||||
// would then be that variable's: the writes would send its state under this
|
||||
// one's path, and the baseline below would overwrite its own.
|
||||
const from = editPath
|
||||
const fromWs = curWs
|
||||
const savedPath = current?.path ?? from
|
||||
const payloads = dirtyWorkspaces.map((ws) => ({
|
||||
ws,
|
||||
@@ -346,7 +347,7 @@
|
||||
if (savedPath && savedPath !== editPath) editPath = savedPath
|
||||
drawer?.closeDrawer()
|
||||
}
|
||||
onSaved?.(savedPath, from)
|
||||
onSaved?.(savedPath, from, fromWs)
|
||||
} catch (err) {
|
||||
sendUserToast(`Could not save variable: ${err.body}`, true)
|
||||
}
|
||||
@@ -368,7 +369,7 @@
|
||||
// A draft-only variable has no deployed row under the draft, so discarding
|
||||
// it removed the variable: `initialStates` holds a synthesized stand-in,
|
||||
// not a baseline to fall back to.
|
||||
if (!existedInitially[selected] && from) onRemoved?.(from)
|
||||
if (!existedInitially[selected] && from && curWs) onRemoved?.(from, curWs)
|
||||
}}
|
||||
disabled={!can_write}
|
||||
/>
|
||||
|
||||
@@ -184,9 +184,14 @@
|
||||
// time the tab may hold something else entirely. The report is about the item it
|
||||
// started on, so acting on anything else re-points an unrelated editor — and
|
||||
// builds the destination out of its location.
|
||||
function stillShowing(kind: EntityEditorKind, path: string): boolean {
|
||||
function stillShowing(kind: EntityEditorKind, path: string, ws: string): boolean {
|
||||
const now = resolvePreviewTab(tab.url)
|
||||
return now.kind === 'entity' && now.entityKind === kind && now.path === path
|
||||
return (
|
||||
now.kind === 'entity' &&
|
||||
now.entityKind === kind &&
|
||||
now.path === path &&
|
||||
ws === workspaceId
|
||||
)
|
||||
}
|
||||
|
||||
// The item is gone (a draft-only one whose draft was discarded), so the tab has
|
||||
@@ -195,8 +200,8 @@
|
||||
// another tab, which must not be the one sent to the list.
|
||||
const returnToList = $derived(
|
||||
slot.kind === 'entity' && runtime
|
||||
? (kind: EntityEditorKind, fromPath: string) => {
|
||||
if (!stillShowing(kind, fromPath)) return
|
||||
? (kind: EntityEditorKind, fromPath: string, fromWs: string) => {
|
||||
if (!stillShowing(kind, fromPath, fromWs)) return
|
||||
runtime.previewTabs.retargetTabTo(tab.id, entityListHref(whereIs(tab)))
|
||||
}
|
||||
: undefined
|
||||
@@ -207,8 +212,8 @@
|
||||
// path — so they have to move with it, or they name an item that no longer exists.
|
||||
const retargetTo = $derived(
|
||||
slot.kind === 'entity' && runtime
|
||||
? (kind: EntityEditorKind, newPath: string, fromPath: string) => {
|
||||
if (!stillShowing(kind, fromPath)) return
|
||||
? (kind: EntityEditorKind, newPath: string, fromPath: string, fromWs: string) => {
|
||||
if (!stillShowing(kind, fromPath, fromWs)) return
|
||||
runtime.previewTabs.retargetTabTo(tab.id, entityEditorHref(whereIs(tab), newPath))
|
||||
}
|
||||
: undefined
|
||||
@@ -402,8 +407,8 @@
|
||||
path={slot.path}
|
||||
{workspaceId}
|
||||
onBack={backToList}
|
||||
onRemoved={(from) => returnToList?.('trigger_schedule', from)}
|
||||
onRenamed={(to, from) => retargetTo?.('trigger_schedule', to, from)}
|
||||
onRemoved={(from, ws) => returnToList?.('trigger_schedule', from, ws)}
|
||||
onRenamed={(to, from, ws) => retargetTo?.('trigger_schedule', to, from, ws)}
|
||||
/>
|
||||
{/await}
|
||||
{:else if slot.entityKind === 'resource'}
|
||||
@@ -414,8 +419,8 @@
|
||||
path={slot.path}
|
||||
{workspaceId}
|
||||
onBack={backToList}
|
||||
onRemoved={(from) => returnToList?.('resource', from)}
|
||||
onRenamed={(to, from) => retargetTo?.('resource', to, from)}
|
||||
onRemoved={(from, ws) => returnToList?.('resource', from, ws)}
|
||||
onRenamed={(to, from, ws) => retargetTo?.('resource', to, from, ws)}
|
||||
/>
|
||||
{/await}
|
||||
{:else if slot.entityKind === 'variable'}
|
||||
@@ -426,8 +431,8 @@
|
||||
path={slot.path}
|
||||
{workspaceId}
|
||||
onBack={backToList}
|
||||
onRemoved={(from) => returnToList?.('variable', from)}
|
||||
onRenamed={(to, from) => retargetTo?.('variable', to, from)}
|
||||
onRemoved={(from, ws) => returnToList?.('variable', from, ws)}
|
||||
onRenamed={(to, from, ws) => retargetTo?.('variable', to, from, ws)}
|
||||
/>
|
||||
{/await}
|
||||
{/if}
|
||||
|
||||
@@ -19,12 +19,12 @@
|
||||
/** The item at `fromPath` is gone — a draft-only one whose draft was discarded.
|
||||
* Distinct from `onBack`, which moves whichever tab is active: only this tab is
|
||||
* the one to send back, and only while it still shows that item. */
|
||||
onRemoved?: (fromPath: string) => void
|
||||
onRemoved?: (fromPath: string, fromWorkspace: string) => void
|
||||
/** The item at `fromPath` was saved under a different path. The tab addresses
|
||||
* it by path — as do its label, the chat's ACTIVE PREVIEW and the draft key —
|
||||
* so the tab has to follow, or all four keep naming an item that no longer
|
||||
* exists. */
|
||||
onRenamed?: (newPath: string, fromPath: string) => void
|
||||
onRenamed?: (newPath: string, fromPath: string, fromWorkspace: string) => void
|
||||
} = $props()
|
||||
|
||||
let editor = $state<ResourceEditorDrawer | undefined>()
|
||||
@@ -45,7 +45,7 @@
|
||||
workspace={workspaceId}
|
||||
{onBack}
|
||||
{onRemoved}
|
||||
onSaved={(saved, from) => {
|
||||
if (saved && from && saved !== from) onRenamed?.(saved, from)
|
||||
onSaved={(saved, from, fromWs) => {
|
||||
if (saved && from && fromWs && saved !== from) onRenamed?.(saved, from, fromWs)
|
||||
}}
|
||||
/>
|
||||
|
||||
@@ -23,12 +23,12 @@
|
||||
* Distinct from `onBack`, which moves whichever tab is active: the discard
|
||||
* awaits a reload of the runnable, by which time the user may be looking at
|
||||
* another tab, or have pointed this one somewhere else. */
|
||||
onRemoved?: (fromPath: string) => void
|
||||
onRemoved?: (fromPath: string, fromWorkspace: string) => void
|
||||
/** The item at `fromPath` was saved under a different path. The tab addresses
|
||||
* it by path — as do its label, the chat's ACTIVE PREVIEW and the draft key —
|
||||
* so the tab has to follow, or all four keep naming an item that no longer
|
||||
* exists. */
|
||||
onRenamed?: (newPath: string, fromPath: string) => void
|
||||
onRenamed?: (newPath: string, fromPath: string, fromWorkspace: string) => void
|
||||
} = $props()
|
||||
|
||||
// Captured at init, so it must read the current prop rather than close over it.
|
||||
@@ -42,18 +42,14 @@
|
||||
// value the save just replaced. Remounting re-reads the saved schedule.
|
||||
let generation = $state(0)
|
||||
|
||||
// Load whenever the tab is pointed at another schedule; the component keeps
|
||||
// its identity across that, as it does for the drawer's row-to-row switch.
|
||||
// `isFlow` is a first guess only — loadScheduleCfg sets it from the loaded
|
||||
// config — so the tab needs no knowledge of the target beyond the path.
|
||||
// Loads the schedule this tab holds, on the instance the `{#key}` below just
|
||||
// mounted for it — `editor` is rebound per instance, so this re-runs per path
|
||||
// and per generation. `isFlow` is a first guess only — loadScheduleCfg sets it
|
||||
// from the loaded config — so the tab needs no knowledge beyond the path.
|
||||
$effect(() => {
|
||||
const p = path
|
||||
const e = editor
|
||||
// `generation` is tracked so a remount re-opens: `editor` is rebound to the
|
||||
// fresh instance, but reading it alone would not say the instance changed.
|
||||
generation
|
||||
if (!p || !e) return
|
||||
untrack(() => void e.openEdit(p, false))
|
||||
if (!path || !e) return
|
||||
untrack(() => void e.openEdit(path, false))
|
||||
})
|
||||
</script>
|
||||
|
||||
@@ -61,7 +57,10 @@
|
||||
<!-- useDrawer=false renders the editor as a Section, the same inline form the
|
||||
script/flow trigger panel mounts (see SchedulePanel). `customLabel` is that
|
||||
Section's header, which is where the way back belongs. -->
|
||||
{#key generation}
|
||||
<!-- Keyed on the path as well: `openEdit` loads asynchronously, and re-pointing a
|
||||
live editor would leave the previous schedule's load to finish into the new
|
||||
one's form and deployed baseline. -->
|
||||
{#key `${path}#${generation}`}
|
||||
<!-- No `allowDraft`: that switches the toolbar to the trigger-panel branch,
|
||||
whose deploy button is gated on the `trigger`/`isDeployed` a script or
|
||||
flow editor supplies for a trigger staged next to it. A standalone tab
|
||||
@@ -72,7 +71,7 @@
|
||||
useDrawer={false}
|
||||
showDraftBanner
|
||||
{onRemoved}
|
||||
onUpdate={(saved: string | undefined, from: string) => {
|
||||
onUpdate={(saved: string | undefined, from: string, fromWs: string) => {
|
||||
// The write landed after the tab was pointed at another schedule: the
|
||||
// remount below would take that one back through a load it never asked
|
||||
// for, and the rename is not its rename.
|
||||
@@ -80,7 +79,7 @@
|
||||
generation++
|
||||
// A draft-only schedule opens with its path editable (saving CREATEs),
|
||||
// so the save can land somewhere other than where the tab is pointed.
|
||||
if (saved && saved !== from) onRenamed?.(saved, from)
|
||||
if (saved && saved !== from) onRenamed?.(saved, from, fromWs)
|
||||
}}
|
||||
>
|
||||
{#snippet customLabel()}
|
||||
|
||||
@@ -19,33 +19,38 @@
|
||||
/** The item at `fromPath` is gone — a draft-only one whose draft was discarded.
|
||||
* Distinct from `onBack`, which moves whichever tab is active: only this tab is
|
||||
* the one to send back, and only while it still shows that item. */
|
||||
onRemoved?: (fromPath: string) => void
|
||||
onRemoved?: (fromPath: string, fromWorkspace: string) => void
|
||||
/** The item at `fromPath` was saved under a different path. The tab addresses
|
||||
* it by path — as do its label, the chat's ACTIVE PREVIEW and the draft key —
|
||||
* so the tab has to follow, or all four keep naming an item that no longer
|
||||
* exists. */
|
||||
onRenamed?: (newPath: string, fromPath: string) => void
|
||||
onRenamed?: (newPath: string, fromPath: string, fromWorkspace: string) => void
|
||||
} = $props()
|
||||
|
||||
let editor = $state<VariableEditor | undefined>()
|
||||
|
||||
// Re-selects when the tab is pointed at another variable; the component keeps
|
||||
// its identity across that, as it does for the drawer's row-to-row switch.
|
||||
// Selects the variable this tab holds, on the instance the `{#key}` below just
|
||||
// mounted for it — `editor` is rebound per instance, so this re-runs per path.
|
||||
$effect(() => {
|
||||
const p = path
|
||||
const e = editor
|
||||
if (!p || !e) return
|
||||
untrack(() => e.editVariable(p))
|
||||
if (!path || !e) return
|
||||
untrack(() => e.editVariable(path))
|
||||
})
|
||||
</script>
|
||||
|
||||
<VariableEditor
|
||||
bind:this={editor}
|
||||
useDrawer={false}
|
||||
workspace={workspaceId}
|
||||
{onBack}
|
||||
{onRemoved}
|
||||
onSaved={(saved, from) => {
|
||||
if (saved && from && saved !== from) onRenamed?.(saved, from)
|
||||
}}
|
||||
/>
|
||||
<!-- Keyed on the path: the editor loads asynchronously, and re-pointing a live one
|
||||
would leave the previous item's load to finish into the new item's state — its
|
||||
draft cell, its baseline, its permissions. A fresh instance per path has none
|
||||
of that to overwrite. -->
|
||||
{#key path}
|
||||
<VariableEditor
|
||||
bind:this={editor}
|
||||
useDrawer={false}
|
||||
workspace={workspaceId}
|
||||
{onBack}
|
||||
{onRemoved}
|
||||
onSaved={(saved, from, fromWs) => {
|
||||
if (saved && from && fromWs && saved !== from) onRenamed?.(saved, from, fromWs)
|
||||
}}
|
||||
/>
|
||||
{/key}
|
||||
|
||||
@@ -70,8 +70,8 @@
|
||||
// session's schedule tab — opts in, or nothing says an edit is unsaved.
|
||||
showDraftBanner = false,
|
||||
// The schedule is gone — a draft-only one whose draft the banner discarded —
|
||||
// with the path it was showing. A host addressing it by path (a session tab)
|
||||
// has to stop showing it.
|
||||
// with the workspace and path it was showing. A host addressing it by those
|
||||
// (a session tab) has to stop showing it.
|
||||
onRemoved = undefined
|
||||
} = $props()
|
||||
|
||||
@@ -615,12 +615,13 @@
|
||||
|
||||
async function scheduleScript(): Promise<void> {
|
||||
const previousPath = initialPath
|
||||
const previousWs = wsId
|
||||
const scheduleCfg = getScheduleCfg()
|
||||
deploymentLoading = true
|
||||
const isSaved = await saveScheduleFromCfg(scheduleCfg, edit, wsId!)
|
||||
if (isSaved) {
|
||||
draftSync.discard(previousPath, scheduleCfg)
|
||||
onUpdate?.(scheduleCfg.path, previousPath)
|
||||
onUpdate?.(scheduleCfg.path, previousPath, previousWs)
|
||||
drawer?.closeDrawer()
|
||||
}
|
||||
deploymentLoading = false
|
||||
@@ -719,6 +720,7 @@
|
||||
async function handleToggleEnabled(nEnabled: boolean) {
|
||||
const previousEnabled = enabled
|
||||
const path = initialPath
|
||||
const ws = wsId
|
||||
enabled = nEnabled
|
||||
if (!trigger?.draftConfig) {
|
||||
const ok = await withForkConflictRetry(
|
||||
@@ -735,7 +737,7 @@
|
||||
return
|
||||
}
|
||||
sendUserToast(`${nEnabled ? 'enabled' : 'disabled'} schedule ${path}`)
|
||||
onUpdate?.(path, path)
|
||||
onUpdate?.(path, path, ws)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1470,9 +1472,10 @@
|
||||
// and an inline host can re-point the editor in the meantime — after
|
||||
// which this discard's outcome is no longer about what is on screen.
|
||||
const from = initialPath
|
||||
const fromWs = wsId
|
||||
const wasDraftOnly = draftOnly
|
||||
await draftSync.resetToDeployed(from)
|
||||
if (wasDraftOnly && from && initialPath === from) onRemoved?.(from)
|
||||
if (wasDraftOnly && from && fromWs && initialPath === from) onRemoved?.(from, fromWs)
|
||||
}}
|
||||
disabled={!can_write}
|
||||
/>
|
||||
|
||||
Reference in New Issue
Block a user