mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-08 16:03:27 +00:00
fix: count a click as an edit, and keep an unjudged row from sealing the sweep
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ScVgqGpuyDMWdzVNPm7Q5f
This commit is contained in:
co-authored by
Claude Opus 5
parent
92ebe73494
commit
09bb9f41b6
@@ -109,13 +109,10 @@
|
||||
workspaceSpecs.push({ ws, defaultValue })
|
||||
}
|
||||
|
||||
// A workspace stays gated until the user puts something into its form: the
|
||||
// resource type's schema is what fills in properties the stored value never
|
||||
// had, and that is not an edit. While gated the autosave is suspended and
|
||||
// the deployed baseline absorbs whatever the form settles on, so opening a
|
||||
// resource whose type gained a property leaves no draft behind. See
|
||||
// `onUserInput`. A workspace opened ON a saved draft keeps its baseline —
|
||||
// the divergence there is the user's own, from an earlier session.
|
||||
// Gated per workspace until the user puts something into that workspace's
|
||||
// form (see `onUserInput`): the autosave stays suspended and the deployed
|
||||
// baseline absorbs whatever the form settles on. A workspace opened ON a
|
||||
// saved draft keeps its baseline — that divergence is the user's own.
|
||||
let userEdited: Record<string, boolean> = $state({})
|
||||
let openedOnDraft: Record<string, boolean> = $state({})
|
||||
const suspendedWorkspaces = new Set<string>()
|
||||
@@ -132,16 +129,11 @@
|
||||
}
|
||||
}
|
||||
|
||||
// Input that arrives before this workspace's handle exists cannot be an edit
|
||||
// to its form — the form is not on screen yet — but it would open the gate,
|
||||
// and the gating effect would then un-suspend the moment the fetch lands,
|
||||
// in time for the schema's materialized values to POST as a draft.
|
||||
//
|
||||
// The same holds for the rest of the load: the schema arrives separately and
|
||||
// materializes on arrival, so until it settles a bare click is not enough to
|
||||
// call this an edit. `Path`, the labels and the description ARE editable
|
||||
// through that window though — they render above the schema form's skeleton
|
||||
// — so a real value event still opens the gate and keeps that edit.
|
||||
// Nothing counts until this workspace's form is on screen, and while the
|
||||
// schema is still arriving a precursor alone does not: it would open the gate
|
||||
// just in time for the schema's materialized values to POST. `Path`, the
|
||||
// labels and the description render above that skeleton and stay editable
|
||||
// throughout, so a real value event still counts and keeps the edit.
|
||||
onUserInput((kind) => {
|
||||
if (!selected || !(selected in states)) return
|
||||
if (kind === 'precursor' && loadingSchema) return
|
||||
|
||||
@@ -113,14 +113,10 @@ export function useTriggerDraftSync(opts: TriggerDraftSyncOptions): TriggerDraft
|
||||
})
|
||||
const handle = $derived(handles[0])
|
||||
|
||||
// The form settles on values nobody entered: the arguments `SchemaForm`
|
||||
// renders come from the runnable's schema, so one that gained a property
|
||||
// fills it in — an empty string, the first option of a required enum — the
|
||||
// moment the drawer opens. Until the user actually puts something in, the
|
||||
// baseline absorbs whatever the form settles on and nothing persists, so an
|
||||
// untouched trigger is never reported as having unsaved changes. A drawer
|
||||
// opened ON a restored draft absorbs nothing: that divergence is the user's
|
||||
// own, from an earlier session. See `onUserInput`.
|
||||
// Gated until the user puts something in (see `onUserInput`): the baseline
|
||||
// absorbs whatever the form settles on and nothing persists, so an untouched
|
||||
// trigger never reports unsaved changes. A drawer opened ON a restored draft
|
||||
// absorbs nothing — that divergence is the user's own.
|
||||
let settledBaseline: Cfg | undefined = $state(undefined)
|
||||
let userEdited = $state(false)
|
||||
let openedOnDraft = $state(false)
|
||||
|
||||
@@ -12,10 +12,14 @@ import { onDestroy } from 'svelte'
|
||||
export type UserInputKind = 'value' | 'precursor'
|
||||
|
||||
/** Events that ARE an edit. `drop` and `paste` matter on their own: text
|
||||
* dragged in from another application, or an assistive technology activating a
|
||||
* control, produce no pointer or key event in this document at all. */
|
||||
* dragged in from another application produces no pointer or key event in this
|
||||
* document at all. */
|
||||
const VALUE_EVENTS = ['input', 'change', 'drop', 'paste'] as const
|
||||
const PRECURSOR_EVENTS = ['pointerdown', 'keydown'] as const
|
||||
/** `click` is here for the controls that mutate state from a click handler and
|
||||
* fire no native value event — ArgInput's "Add item", say. A mouse always sends
|
||||
* `pointerdown` first, but an assistive technology can activate one with a
|
||||
* trusted `click` alone, and that is a real edit with nothing else to catch it. */
|
||||
const PRECURSOR_EVENTS = ['pointerdown', 'keydown', 'click'] as const
|
||||
|
||||
/**
|
||||
* A draft is supposed to record what the USER changed, but an editor built
|
||||
|
||||
@@ -103,11 +103,16 @@ describe('pruneMeaninglessDrafts', () => {
|
||||
expect(discardDraft).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('leaves a draft alone when its diff cannot be fetched', async () => {
|
||||
it('leaves a draft alone when its diff cannot be fetched, and retries it later', async () => {
|
||||
listDrafts.mockResolvedValue([row()])
|
||||
getDraftDiffValues.mockRejectedValue(new Error('boom'))
|
||||
await pruneMeaninglessDrafts('main', 'me@x.dev')
|
||||
expect(discardDraft).not.toHaveBeenCalled()
|
||||
// A row that could not be judged is not a row that carries changes, so
|
||||
// the pass must stay open rather than strand it.
|
||||
getDraftDiffValues.mockResolvedValue(diff())
|
||||
await pruneMeaninglessDrafts('main', 'me@x.dev')
|
||||
expect(discardedPaths()).toEqual(['u/me/r'])
|
||||
})
|
||||
|
||||
it('conditions the delete on the timestamp it judged, so a row that moved is spared', async () => {
|
||||
|
||||
@@ -1,14 +1,11 @@
|
||||
/**
|
||||
* One-off sweep that drops drafts carrying no changes.
|
||||
*
|
||||
* Before the editors gated their autosave on real user input (`onUserInput`),
|
||||
* merely opening an item whose schema had moved on saved a draft — workspaces
|
||||
* accumulated dozens that nobody wrote. The gate stops new ones; the ones
|
||||
* already stored need this pass to clear. Runs once per (workspace, user) per
|
||||
* browser, after the localStorage→DB migration so anything it just uploaded is
|
||||
* swept too.
|
||||
* `onUserInput` stops new ones from being written; the ones already stored need
|
||||
* this pass to clear. Runs once per (workspace, user) per browser, after the
|
||||
* localStorage→DB migration so anything it just uploaded is swept too.
|
||||
*
|
||||
* Scoped to the kinds whose editors this gate covers. A script, flow or app
|
||||
* Scoped to the kinds whose editors that gate covers. A script, flow or app
|
||||
* draft only ever came from an explicit edit, so there is no phantom to clear
|
||||
* there — and `getDraftDiffValues` would fetch each one's full deployed payload
|
||||
* at login to prove it.
|
||||
@@ -69,7 +66,12 @@ function busyLocally(workspace: string, kind: UserDraftItemKind, path: string):
|
||||
return UserDraftDbSyncer.getState({ workspace, itemKind: kind, path }).state !== 'none'
|
||||
}
|
||||
|
||||
async function carriesNoChanges(workspace: string, { kind, path }: Candidate): Promise<boolean> {
|
||||
/** `undefined` when the diff could not be fetched — distinct from `false`, so
|
||||
* the caller can leave the pass open rather than strand a row it never judged. */
|
||||
async function carriesNoChanges(
|
||||
workspace: string,
|
||||
{ kind, path }: Candidate
|
||||
): Promise<boolean | undefined> {
|
||||
try {
|
||||
const { deployed, draft, hasDraft, noDeployed } = await getDraftDiffValues(
|
||||
kind,
|
||||
@@ -82,7 +84,7 @@ async function carriesNoChanges(workspace: string, { kind, path }: Candidate): P
|
||||
if (!hasDraft || noDeployed) return false
|
||||
return draftValuesEqual(draft, deployed)
|
||||
} catch {
|
||||
return false
|
||||
return undefined
|
||||
}
|
||||
}
|
||||
|
||||
@@ -141,7 +143,9 @@ export async function pruneMeaninglessDrafts(workspace: string, userKey: string)
|
||||
|
||||
const empty: Candidate[] = []
|
||||
await mapWithLimit(candidates, CONCURRENCY, async (c) => {
|
||||
if (await carriesNoChanges(workspace, c)) empty.push(c)
|
||||
const verdict = await carriesNoChanges(workspace, c)
|
||||
if (verdict === undefined) unresolved++
|
||||
else if (verdict) empty.push(c)
|
||||
})
|
||||
|
||||
let discarded = 0
|
||||
|
||||
Reference in New Issue
Block a user