From 20226a15b669bbd36bda2409126f82a5eda29e75 Mon Sep 17 00:00:00 2001 From: Guilhem Lemouel Date: Wed, 29 Jul 2026 19:53:54 +0200 Subject: [PATCH] refactor: back editor trigger drafts with the per-user draft table Co-Authored-By: Claude Opus 5 (1M context) --- .../tests/schedules.rs | 107 +++++ backend/windmill-api-schedule/src/lib.rs | 32 +- backend/windmill-api/openapi.yaml | 18 + backend/windmill-common/src/user_drafts.rs | 29 ++ backend/windmill-trigger/src/handler.rs | 40 +- backend/windmill-trigger/src/types.rs | 14 +- frontend/src/lib/components/Dev.svelte | 4 +- .../src/lib/components/FlowBuilder.svelte | 117 +++--- .../src/lib/components/FlowGraphViewer.svelte | 4 +- .../src/lib/components/SchedulePanel.svelte | 8 +- .../src/lib/components/ScriptBuilder.svelte | 86 ++-- .../DraftTriggersConfirmationModal.svelte | 47 +-- frontend/src/lib/components/flow_builder.ts | 2 - .../flows/content/FlowEditorPanel.svelte | 4 +- .../flows/content/FlowModuleWrapper.svelte | 22 +- .../flows/content/FlowPathViewer.svelte | 4 +- .../flows/map/FlowModuleSchemaMap.svelte | 4 +- .../graph/renderers/nodes/TriggersNode.svelte | 23 +- frontend/src/lib/components/triggers.ts | 14 +- .../triggers/TriggerEditorToolbar.svelte | 9 +- .../components/triggers/TriggerLabel.svelte | 4 +- .../components/triggers/TriggersEditor.svelte | 53 ++- .../components/triggers/TriggersTable.svelte | 9 +- .../triggers/TriggersWrapper.svelte | 4 - .../amqp/AmqpTriggerEditorInner.svelte | 7 +- .../triggers/amqp/AmqpTriggersPanel.svelte | 8 +- .../azure/AzureTriggerEditorInner.svelte | 7 +- .../triggers/azure/AzureTriggerPanel.svelte | 8 +- .../triggers/draftTriggerPath.test.ts | 33 ++ .../email/EmailTriggerEditorInner.svelte | 7 +- .../triggers/email/EmailTriggerPanel.svelte | 8 +- .../triggers/gcp/GcpTriggerEditorInner.svelte | 7 +- .../triggers/gcp/GcpTriggerPanel.svelte | 8 +- .../triggers/http/RouteEditorInner.svelte | 7 +- .../triggers/http/RoutesPanel.svelte | 8 +- .../kafka/KafkaTriggerEditorInner.svelte | 7 +- .../triggers/kafka/KafkaTriggersPanel.svelte | 8 +- .../mqtt/MqttTriggerEditorInner.svelte | 7 +- .../triggers/mqtt/MqttTriggersPanel.svelte | 8 +- .../nats/NatsTriggerEditorInner.svelte | 7 +- .../triggers/nats/NatsTriggersPanel.svelte | 8 +- .../PostgresTriggerEditorInner.svelte | 7 +- .../postgres/PostgresTriggersPanel.svelte | 8 +- .../schedules/ScheduleEditorInner.svelte | 9 +- .../triggers/sqs/SqsTriggerEditorInner.svelte | 7 +- .../triggers/sqs/SqsTriggerPanel.svelte | 8 +- .../components/triggers/triggers.svelte.ts | 146 ++++--- .../triggers/useTriggerDraftSync.svelte.ts | 17 + frontend/src/lib/components/triggers/utils.ts | 397 ++++++++++-------- .../triggers/webhook/WebhookEditor.svelte | 4 +- .../WebsocketTriggerEditorInner.svelte | 7 +- .../websocket/WebsocketTriggersPanel.svelte | 8 +- frontend/src/lib/legacyDraftTriggers.ts | 55 +++ frontend/src/lib/utils_draft_deploy.ts | 32 +- .../flows/edit/[...path]/+page.svelte | 8 - .../(logged)/flows/get/[...path]/+page.svelte | 4 +- .../(root)/(logged)/schedules/+page.svelte | 6 +- .../scripts/edit/[...path]/+page.svelte | 15 +- .../scripts/get/[...hash]/+page.svelte | 4 +- frontend/src/routes/flows/dev/+page.svelte | 4 +- 60 files changed, 1012 insertions(+), 545 deletions(-) create mode 100644 frontend/src/lib/components/triggers/draftTriggerPath.test.ts create mode 100644 frontend/src/lib/legacyDraftTriggers.ts diff --git a/backend/windmill-api-integration-tests/tests/schedules.rs b/backend/windmill-api-integration-tests/tests/schedules.rs index 89ba0ec2cc..e3807c4676 100644 --- a/backend/windmill-api-integration-tests/tests/schedules.rs +++ b/backend/windmill-api-integration-tests/tests/schedules.rs @@ -234,3 +234,110 @@ async fn test_schedule_endpoints(db: Pool) -> anyhow::Result<()> { Ok(()) } + +/// The flow/script editor lists its own triggers with `path` + `is_flow`, and +/// needs its undeployed ones back. Draft-only rows are synthesized from the +/// `draft` table, which no SQL filter reaches, so they are matched on the draft +/// value's own `script_path`/`is_flow` — and reported under the draft row's key, +/// with `draft_path` carrying the path the user renamed it to. +#[sqlx::test(migrations = "../migrations", fixtures("base"))] +async fn test_list_draft_only_schedules_scoped_to_runnable(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + let list_url = format!("http://localhost:{port}/api/w/test-workspace/schedules/list"); + + // Two drafts on the target flow (one of them renamed away from its key) and + // one on a different runnable. + for (key, value) in [ + ( + "u/test-user/target_schedule", + json!({ + "path": "u/test-user/target_schedule", + "schedule": "0 0 * * * *", + "timezone": "UTC", + "script_path": "f/test/target_flow", + "is_flow": true, + }), + ), + ( + "u/test-user/renamed_key", + json!({ + "path": "u/test-user/nicer_name", + "schedule": "0 0 * * * *", + "timezone": "UTC", + "script_path": "f/test/target_flow", + "is_flow": true, + }), + ), + ( + "u/test-user/other_schedule", + json!({ + "path": "u/test-user/other_schedule", + "schedule": "0 0 * * * *", + "timezone": "UTC", + "script_path": "f/test/other_flow", + "is_flow": true, + }), + ), + ] { + let resp = authed(client().post(format!( + "http://localhost:{port}/api/w/test-workspace/drafts/update/trigger_schedule/{key}" + ))) + .json(&json!({ "value": value })) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), 200, "seeding draft {key}"); + } + + let rows: Vec = authed(client().get(&list_url)) + .query(&[ + ("path", "f/test/target_flow"), + ("is_flow", "true"), + ("include_draft_only", "true"), + ]) + .send() + .await + .unwrap() + .json() + .await?; + + let mut paths: Vec<&str> = rows + .iter() + .map(|r| r["path"].as_str().unwrap()) + .collect::>(); + paths.sort(); + assert_eq!( + paths, + vec!["u/test-user/renamed_key", "u/test-user/target_schedule"], + "only this runnable's drafts, keyed by their draft row" + ); + + let renamed = rows + .iter() + .find(|r| r["path"] == "u/test-user/renamed_key") + .unwrap(); + assert_eq!(renamed["draft_path"], "u/test-user/nicer_name"); + assert_eq!(renamed["draft_only"], true); + let unrenamed = rows + .iter() + .find(|r| r["path"] == "u/test-user/target_schedule") + .unwrap(); + assert!( + unrenamed.get("draft_path").is_none(), + "no draft_path when the key already is the intended path" + ); + + // Without the flag the same query stays deployed-only. + let rows: Vec = authed(client().get(&list_url)) + .query(&[("path", "f/test/target_flow"), ("is_flow", "true")]) + .send() + .await + .unwrap() + .json() + .await?; + assert!(rows.is_empty(), "drafts must not leak without the flag"); + + Ok(()) +} diff --git a/backend/windmill-api-schedule/src/lib.rs b/backend/windmill-api-schedule/src/lib.rs index e9ca4d54fe..3a273cdd17 100644 --- a/backend/windmill-api-schedule/src/lib.rs +++ b/backend/windmill-api-schedule/src/lib.rs @@ -28,8 +28,8 @@ use windmill_common::{ error::{Error, JsonResult, Result}, schedule::Schedule, user_drafts::{ - delete_all_drafts_for_path, fetch_draft_only_list_rows, overlay_or_draft_only, - UserDraftItemKind, WithDraftOverlay, WithDraftQuery, + delete_all_drafts_for_path, draft_targets_runnable, fetch_draft_only_list_rows, + overlay_or_draft_only, UserDraftItemKind, WithDraftOverlay, WithDraftQuery, }, utils::{ escape_ilike_pattern, not_found_if_none, paginate, Pagination, ScheduleType, StripPath, @@ -732,6 +732,13 @@ pub struct ScheduleLight { #[sqlx(default)] #[serde(skip_serializing_if = "Option::is_none")] pub inherited_labels: Option>, + /// The path a synthesized draft-only row will deploy to, when the user + /// renamed it away from the `draft` row's key. `path` stays the key — it is + /// what get/update/delete address — so a renamed draft can still be opened. + /// Same split as flows and apps. `None` when they agree. + #[sqlx(default)] + #[serde(skip_serializing_if = "Option::is_none")] + pub draft_path: Option, } async fn list_schedule( authed: ApiAuthed, @@ -832,8 +839,6 @@ async fn list_schedule( if lsq.include_draft_only.unwrap_or(false) && !authed.is_operator && offset == 0 - && lsq.path.is_none() - && lsq.is_flow.is_none() && lsq.args.is_none() && lsq.path_start.is_none() && lsq.schedule_path.is_none() @@ -853,15 +858,19 @@ async fn list_schedule( for row in draft_only_rows { let v: serde_json::Value = serde_json::from_str(row.value.0.get()).unwrap_or(serde_json::Value::Null); - // Schedule editor's draft mirrors NewSchedule: { path, schedule, timezone, script_path, is_flow, enabled?, summary?, labels? } - let path = v - .get("path") - .and_then(|s| s.as_str()) - .unwrap_or("") - .to_string(); - if path.is_empty() { + if !draft_targets_runnable(&v, lsq.path.as_deref(), lsq.is_flow) { continue; } + // Schedule editor's draft mirrors NewSchedule: { path, schedule, timezone, script_path, is_flow, enabled?, summary?, labels? } + // The `draft` row's key addresses the schedule; the value's `path` is + // only where it will deploy. Reporting the key as `path` is what lets + // a renamed draft still be opened, updated and deleted. + let intended_path = v.get("path").and_then(|s| s.as_str()).unwrap_or(""); + if intended_path.is_empty() { + continue; + } + let draft_path = (intended_path != row.path).then(|| intended_path.to_string()); + let path = row.path.clone(); let schedule = v .get("schedule") .and_then(|x| x.as_str()) @@ -909,6 +918,7 @@ async fn list_schedule( draft_only: Some(true), // Synthesized rows are the authed user's draft. is_draft: Some(true), + draft_path, }); } } diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index 381cf1b999..f13ff7f487 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -23918,6 +23918,10 @@ components: so the home page can render a "Draft" badge. Gated to non-operators + page 0 + no narrowing filters on the backend so picker callers stay deployed-only and pagination stays clean. + On the trigger and schedule list endpoints the `path`/`is_flow` + pair is exempt from that rule: it narrows to a runnable, and the + appended rows are matched on the draft's own `script_path`/`is_flow`, + so a flow/script editor can list its undeployed triggers. schema: type: boolean Id: @@ -27465,6 +27469,13 @@ components: (over a deployed row or a synthesized draft-only row). Frontend appends a `*` to the displayed name. type: boolean + draft_path: + description: | + Where a synthesized draft-only row will deploy to, when it was + renamed away from the `draft` row's key. `path` stays the key — + what get/update/delete address — so a renamed draft is still + addressable. Absent when the two agree. + type: string inherited_labels: type: array items: @@ -27804,6 +27815,13 @@ components: (over a deployed row or a synthesized draft-only row). Frontend appends a `*` to the displayed name. type: boolean + draft_path: + description: | + Where a synthesized draft-only row will deploy to, when it was + renamed away from the `draft` row's key. `path` stays the key — + what get/update/delete address — so a renamed draft is still + addressable. Absent when the two agree. + type: string required: - path - script_path diff --git a/backend/windmill-common/src/user_drafts.rs b/backend/windmill-common/src/user_drafts.rs index 84a0e8c3ea..503f8feca2 100644 --- a/backend/windmill-common/src/user_drafts.rs +++ b/backend/windmill-common/src/user_drafts.rs @@ -398,6 +398,35 @@ pub async fn fetch_draft_only_list_rows( Ok(rows) } +/// Whether a trigger draft's JSON targets the runnable a list query narrows to. +/// The trigger list endpoints narrow by `(path, is_flow)` — meaning the +/// *runnable* the trigger fires, matched in SQL against `script_path`/`is_flow` +/// on deployed rows. Synthesized draft-only rows have no SQL to filter them, so +/// the same pair is read out of the draft value here. A query that narrows on +/// neither matches every draft. +pub fn draft_targets_runnable( + value: &serde_json::Value, + script_path: Option<&str>, + is_flow: Option, +) -> bool { + if let Some(p) = script_path { + if value.get("script_path").and_then(|x| x.as_str()) != Some(p) { + return false; + } + } + if let Some(f) = is_flow { + if value + .get("is_flow") + .and_then(|x| x.as_bool()) + .unwrap_or(false) + != f + { + return false; + } + } + true +} + /// The get-by-path draft choreography, shared by every entity's "get by path" /// route. Given the deployed entity as an `Option` (caller maps its own "not /// found" to `None`): diff --git a/backend/windmill-trigger/src/handler.rs b/backend/windmill-trigger/src/handler.rs index 0c46ff25fd..6a9b17939e 100644 --- a/backend/windmill-trigger/src/handler.rs +++ b/backend/windmill-trigger/src/handler.rs @@ -17,8 +17,9 @@ use windmill_common::{ db::UserDB, error::{Error, JsonResult, Result}, user_drafts::{ - delete_all_drafts_for_path, delete_own_draft_for_path, fetch_draft_only_list_rows, - overlay_or_draft_only, UserDraftItemKind, WithDraftOverlay, WithDraftQuery, + delete_all_drafts_for_path, delete_own_draft_for_path, draft_targets_runnable, + fetch_draft_only_list_rows, overlay_or_draft_only, UserDraftItemKind, WithDraftOverlay, + WithDraftQuery, }, utils::{paginate, Pagination, StripPath}, worker::CLOUD_HOSTED, @@ -599,21 +600,28 @@ async fn list_triggers( Extension(db): Extension, Path(workspace_id): Path, Query(query): Query, -) -> JsonResult> { +) -> JsonResult> { let mut tx = user_db.begin(&authed).await?; - let mut triggers = handler + let deployed = handler .list_triggers(&mut *tx, &workspace_id, Some(&query), Some(&authed.email)) .await?; tx.commit().await?; + // Serialized rather than kept as `T::Trigger`: the synthesized draft rows + // below are the editor's config shape, which does NOT round-trip through + // `T::TriggerConfig` — required fields the editor never writes (`route_path_key` + // on HTTP) and fields a half-configured new trigger hasn't got yet would make + // `from_value` fail, silently hiding the draft. The response body is the same + // either way, so the merge happens on JSON. + let mut triggers = deployed + .into_iter() + .map(|t| serde_json::to_value(t).map_err(Error::from)) + .collect::>>()?; + // Append the authed user's draft-only triggers of this kind; see scripts.rs. - // Best-effort: the editor's TriggerData shape overlaps T::Trigger but a per-kind - // config can deviate, so drop a row on deserialize failure rather than fail the list. if query.include_draft_only.unwrap_or(false) && !authed.is_operator && query.page.unwrap_or(0) == 0 - && query.path.is_none() - && query.is_flow.is_none() && query.path_start.is_none() && query.label.is_none() { @@ -631,6 +639,9 @@ async fn list_triggers( Ok(v) => v, Err(_) => continue, }; + if !draft_targets_runnable(&v, query.path.as_deref(), query.is_flow) { + continue; + } let serde_json::Value::Object(mut map) = v else { continue; }; @@ -657,15 +668,20 @@ async fn list_triggers( map.insert("draft_only".into(), serde_json::Value::Bool(true)); // Synthesized rows are the authed user's draft. map.insert("is_draft".into(), serde_json::Value::Bool(true)); - match serde_json::from_value::(serde_json::Value::Object(map)) { - Ok(t) => triggers.push(t), - Err(_) => continue, + // The `draft` row's key addresses the trigger; the value's `path` is + // only where it will deploy. Surfacing the key as `path` is what lets + // a renamed draft still be opened, updated and deleted. + if let Some(intended) = map.insert("path".into(), row.path.clone().into()) { + if intended.as_str() != Some(row.path.as_str()) { + map.insert("draft_path".into(), intended); + } } + triggers.push(serde_json::Value::Object(map)); } } let allowed = build_scope_path_predicate(&authed, T::scope_domain_name(), "read"); - triggers.retain(|t| allowed(t.trigger_path())); + triggers.retain(|t| allowed(t.get("path").and_then(|p| p.as_str()).unwrap_or(""))); Ok(Json(triggers)) } diff --git a/backend/windmill-trigger/src/types.rs b/backend/windmill-trigger/src/types.rs index dcf9a136be..8bcf7c3c54 100644 --- a/backend/windmill-trigger/src/types.rs +++ b/backend/windmill-trigger/src/types.rs @@ -28,8 +28,11 @@ pub struct StandardTriggerQuery { pub path_start: Option, pub label: Option, /// When true, append per-user draft rows whose path has no - /// deployed trigger of this kind. Same gate as scripts/flows/apps: - /// non-operators, offset 0, no narrowing filters. + /// deployed trigger of this kind. Gated to non-operators at offset 0 + /// with no `path_start`/`label` filter. `path`/`is_flow` ARE honored — + /// they narrow to a runnable, and the appended rows are filtered on the + /// draft's own `script_path`/`is_flow` so the flow/script editor can list + /// its undeployed triggers. pub include_draft_only: Option, } @@ -62,6 +65,13 @@ pub struct BaseTrigger { #[serde(skip_serializing_if = "Option::is_none")] #[sqlx(default)] pub is_draft: Option, + /// The path a synthesized draft-only row will deploy to, when the user + /// renamed it away from the `draft` row's key. `path` stays the key — it + /// is what get/update/delete address — so a renamed draft can still be + /// opened. Same split as flows and apps. `None` when they agree. + #[serde(skip_serializing_if = "Option::is_none")] + #[sqlx(default)] + pub draft_path: Option, } #[derive(Debug, FromRow, Clone, Serialize, Deserialize)] diff --git a/frontend/src/lib/components/Dev.svelte b/frontend/src/lib/components/Dev.svelte index 6d9b21aeb9..178c2b8bc7 100644 --- a/frontend/src/lib/components/Dev.svelte +++ b/frontend/src/lib/components/Dev.svelte @@ -88,6 +88,7 @@ import { ModulesTestStates } from './modulesTest.svelte' import type { GraphModuleState } from './graph' import { setCopilotInfo } from '$lib/aiStore' + import { NO_TRIGGER_DRAFT_TARGET } from '$lib/components/triggers/utils' let { initial = undefined @@ -679,7 +680,8 @@ triggersCount: triggersCount, simplifiedPoll: writable(false), showCaptureHint: writable(undefined), - triggersState: new Triggers() + triggersState: new Triggers(), + draftTarget: () => NO_TRIGGER_DRAFT_TARGET }) let pathStore = writable('') diff --git a/frontend/src/lib/components/FlowBuilder.svelte b/frontend/src/lib/components/FlowBuilder.svelte index d9692f4812..e83d6495b5 100644 --- a/frontend/src/lib/components/FlowBuilder.svelte +++ b/frontend/src/lib/components/FlowBuilder.svelte @@ -3,7 +3,6 @@ FlowService, type Flow, type PathScript, - type OpenFlow, type InputTransform, type TriggersCount, CaptureService, @@ -96,7 +95,13 @@ import DeployButton from './DeployButton.svelte' import { invalidateWorkspacePaths } from './PathNameAutocomplete.svelte' import type { Trigger } from './triggers/utils' - import { deployTriggers, handleSelectTriggerFromKind } from './triggers/utils' + import { + deployTriggers, + handleSelectTriggerFromKind, + repointTriggerDrafts, + triggerHasPendingChanges + } from './triggers/utils' + import type { TriggerDraftTarget } from './triggers/utils' import DraftTriggersConfirmationModal from './common/confirmationModal/DraftTriggersConfirmationModal.svelte' import { Triggers } from './triggers/triggers.svelte' import { StepsInputArgs } from './flows/stepsInputArgs.svelte' @@ -132,7 +137,6 @@ savedPrimarySchedule = undefined, version = undefined, draftBaseVersion = undefined, - draftTriggersFromUrl = undefined, selectedTriggerIndexFromUrl = undefined, children, loadedFromHistoryFromUrl, @@ -246,8 +250,7 @@ // `flowStore.val.path` doesn't track those edits, so without this the // rename wouldn't show up in the diff and the unsaved-changes warning // wouldn't fire when leaving with a pending rename. - path: $pathStore, - draft_triggers: structuredClone(triggersState.getDraftTriggersSnapshot()) + path: $pathStore } } } @@ -289,13 +292,23 @@ primaryScheduleStore.set(schedule) } - export function setDraftTriggers(triggers: Trigger[] | undefined) { - triggersState.setTriggers([ - ...(triggers ?? []), - ...triggersState.triggers.filter((t) => !t.draftConfig) - ]) - loadTriggers() - } + + + // Trigger drafts carry the runnable's path in `script_path`. That path stays + // editable until the first deploy, so follow renames here rather than only at + // deploy time — a draft deployed on its own (triggers page, Review & Deploy) + // reads what is stored. Debounced: the path field rewrites on every keystroke. + let repointTimer: ReturnType | undefined + $effect(() => { + const target = triggerDraftTarget() + if (!target.runnablePath || !target.workspace) return + clearTimeout(repointTimer) + repointTimer = setTimeout( + () => void repointTriggerDrafts($state.snapshot(triggersState.triggers), target), + 800 + ) + return () => clearTimeout(repointTimer) + }) export function setSelectedTriggerIndex(index: number | undefined) { triggersState.selectedTriggerIndex = index @@ -393,7 +406,7 @@ async function saveFlow(deploymentMsg?: string, triggersToDeploy?: Trigger[]): Promise { if (!triggersToDeploy) { // Check if there are draft triggers that need confirmation - const draftTriggers = triggersState.triggers.filter((trigger) => trigger.draftConfig) + const draftTriggers = triggersState.triggers.filter(triggerHasPendingChanges) if (draftTriggers.length > 0) { draftTriggersModalOpen = true confirmDeploymentCallback = async (triggersToDeploy: Trigger[]) => { @@ -474,27 +487,7 @@ }, runnableKind: 'flow' }) - if (triggersToDeploy) { - await deployTriggers( - triggersToDeploy, - opWorkspace, - !!$userStore?.is_admin || !!$userStore?.is_super_admin, - usedTriggerKinds, - $pathStore, - true - ) - } } else { - if (triggersToDeploy) { - await deployTriggers( - triggersToDeploy, - opWorkspace, - !!$userStore?.is_admin || !!$userStore?.is_super_admin, - usedTriggerKinds, - initialPath - ) - } - await FlowService.updateFlow({ workspace: opWorkspace!, path: initialPath, @@ -516,18 +509,31 @@ }) } + // Triggers deploy after the flow, never before: their `script_path` + // points at the path this deploy just created or renamed to. + if (triggersToDeploy?.length) { + const failed = await deployTriggers( + triggersToDeploy, + opWorkspace, + !!$userStore?.is_admin || !!$userStore?.is_super_admin, + usedTriggerKinds, + $pathStore, + true + ) + for (const { trigger, error } of failed) { + sendUserToast(`Could not deploy ${trigger.type} trigger: ${error}`, true) + } + } + // New/updated path now exists server-side — drop the autocomplete // cache so it shows up immediately instead of after the 60s TTL. invalidateWorkspacePaths(opWorkspace!) - const { draft_triggers: _, ...newSavedFlow } = flowStore.val as OpenFlow & { - draft_triggers: Trigger[] - } savedFlow = { - ...structuredClone($state.snapshot(newSavedFlow)), + ...structuredClone($state.snapshot(flowStore.val)), path: $pathStore } as Flow - setDraftTriggers([]) + await loadTriggers() loadingSave = false onDeploy?.({ path: $pathStore }) } catch (err) { @@ -782,32 +788,41 @@ new Triggers( [ { type: 'webhook', path: '', isDraft: false }, - { type: 'default_email', path: '', isDraft: false }, - ...(untrack(() => draftTriggersFromUrl) ?? []) + { type: 'default_email', path: '', isDraft: false } ], untrack(() => selectedTriggerIndexFromUrl) ) ) + const triggerDraftTarget = (): TriggerDraftTarget => ({ + runnablePath: $pathStore, + isFlow: true, + workspace: opWorkspace + }) + setContext('TriggerContext', { triggersCount, simplifiedPoll, showCaptureHint, - triggersState + triggersState, + draftTarget: triggerDraftTarget }) export async function loadTriggers() { - if (initialPath == '') return - $triggersCount = await FlowService.getTriggersCountOfFlow({ - workspace: opWorkspace!, - path: initialPath - }) + // An undeployed flow has no deployed triggers to count, but it can already + // have drafted ones — fetch against the path they were drafted at. + if (initialPath != '') { + $triggersCount = await FlowService.getTriggersCountOfFlow({ + workspace: opWorkspace!, + path: initialPath + }) + } // Initialize triggers using utility function await triggersState.fetchTriggers( triggersCount, opWorkspace, - initialPath, + initialPath || $pathStore, true, $primaryScheduleStore, $userStore @@ -947,7 +962,6 @@ async function openDiffDrawer() { if (!savedFlow) return await syncWithDeployed() - const currentDraftTriggers = structuredClone(triggersState.getDraftTriggersSnapshot()) diffDrawer?.openDrawer() const currentFlow = flowStore.val diffDrawer?.setDiff({ @@ -955,8 +969,7 @@ deployed: deployedValue ?? savedFlow, current: { ...currentFlow, - path: $pathStore, - draft_triggers: currentDraftTriggers + path: $pathStore } }) } @@ -1268,7 +1281,7 @@ t.draftConfig)} + draftTriggers={triggersState.triggers.filter(triggerHasPendingChanges)} isFlow={true} on:canceled={() => { draftTriggersModalOpen = false @@ -1385,7 +1398,7 @@ unifiedSize={headerBtnSize} on:openTriggers={(e) => { select('Trigger') - handleSelectTriggerFromKind(triggersState, triggersCount, initialPath, e.detail.kind) + handleSelectTriggerFromKind(triggersState, triggersCount, triggerDraftTarget(), e.detail.kind) captureOn.set(true) showCaptureHint.set(true) }} diff --git a/frontend/src/lib/components/FlowGraphViewer.svelte b/frontend/src/lib/components/FlowGraphViewer.svelte index 47da29bb57..7e6950a217 100644 --- a/frontend/src/lib/components/FlowGraphViewer.svelte +++ b/frontend/src/lib/components/FlowGraphViewer.svelte @@ -14,6 +14,7 @@ import { untrack } from 'svelte' import { publishLinkedAgentTools } from './flows/flowState' import { linkedToolsScope } from './flows/linkedAgentToolsStore.svelte' + import { NO_TRIGGER_DRAFT_TARGET } from '$lib/components/triggers/utils' interface Props { flow: { @@ -61,7 +62,8 @@ triggersCount, simplifiedPoll: writable(false), showCaptureHint: writable(undefined), - triggersState: new Triggers() + triggersState: new Triggers(), + draftTarget: () => NO_TRIGGER_DRAFT_TARGET }) } diff --git a/frontend/src/lib/components/SchedulePanel.svelte b/frontend/src/lib/components/SchedulePanel.svelte index 334adcb98c..65412583fd 100644 --- a/frontend/src/lib/components/SchedulePanel.svelte +++ b/frontend/src/lib/components/SchedulePanel.svelte @@ -13,9 +13,9 @@ ...restProps } = $props() - function openScheduleEditor(isFlow: boolean, isDraft: boolean) { - if (isDraft) { - scheduleEditor?.openNew(isFlow, path, defaultValues) + function openScheduleEditor(isFlow: boolean) { + if (selectedTrigger.isNew) { + scheduleEditor?.openNew(isFlow, path, { ...defaultValues, path: selectedTrigger.path }) } else { scheduleEditor?.openEdit(selectedTrigger.path, isFlow, defaultValues) } @@ -24,7 +24,7 @@ onMount(() => { selectedTrigger?.type === 'schedule' && scheduleEditor && - openScheduleEditor(isFlow, selectedTrigger.isDraft ?? false) + openScheduleEditor(isFlow) }) diff --git a/frontend/src/lib/components/ScriptBuilder.svelte b/frontend/src/lib/components/ScriptBuilder.svelte index bd30cd6909..c35dcaefa1 100644 --- a/frontend/src/lib/components/ScriptBuilder.svelte +++ b/frontend/src/lib/components/ScriptBuilder.svelte @@ -35,7 +35,6 @@ emptyString, generateRandomString, orderedJsonStringify, - readFieldsRecursively, replaceFalseWithUndefined, type Value } from '$lib/utils' @@ -97,7 +96,14 @@ import CaptureTable from './triggers/CaptureTable.svelte' import type { SavedAndModifiedValue } from './common/confirmationModal/unsavedTypes' import DeployButton from './DeployButton.svelte' - import { type Trigger, deployTriggers, handleSelectTriggerFromKind } from './triggers/utils' + import { + type Trigger, + deployTriggers, + handleSelectTriggerFromKind, + repointTriggerDrafts, + triggerHasPendingChanges + } from './triggers/utils' + import type { TriggerDraftTarget } from './triggers/utils' import DraftTriggersConfirmationModal from './common/confirmationModal/DraftTriggersConfirmationModal.svelte' import { Triggers } from './triggers/triggers.svelte' import type { ScriptBuilderProps } from './script_builder' @@ -153,8 +159,7 @@ return { savedValue: savedScript, modifiedValue: { - ...script, - draft_triggers: structuredClone(triggersState.getDraftTriggersSnapshot()) + ...script } } } @@ -268,13 +273,21 @@ loadTriggers() } - export function setDraftTriggers(triggers: Trigger[] | undefined) { - triggersState.setTriggers([ - ...(triggers ?? []), - ...triggersState.triggers.filter((t) => !t.draftConfig) - ]) - loadTriggers() - } + // Trigger drafts carry the runnable's path in `script_path`. That path stays + // editable until the first deploy, so follow renames here rather than only at + // deploy time — a draft deployed on its own (triggers page, Review & Deploy) + // reads what is stored. Debounced: the path field rewrites on every keystroke. + let repointTimer: ReturnType | undefined + $effect(() => { + const target = triggerDraftTarget() + if (!target.runnablePath || !target.workspace) return + clearTimeout(repointTimer) + repointTimer = setTimeout( + () => void repointTriggerDrafts($state.snapshot(triggersState.triggers), target), + 800 + ) + return () => clearTimeout(repointTimer) + }) onMount(() => { if (functionExports) { @@ -295,7 +308,7 @@ } }) - async function loadTriggers() { + export async function loadTriggers() { if (!initialPath) { return } @@ -318,18 +331,24 @@ const triggersState = $state( new Triggers([ { type: 'webhook', path: '', isDraft: false }, - { type: 'default_email', path: '', isDraft: false }, - ...(script.draft_triggers ?? []) + { type: 'default_email', path: '', isDraft: false } ]) ) const captureOn = writable(undefined) const showCaptureHint = writable(undefined) + const triggerDraftTarget = (): TriggerDraftTarget => ({ + runnablePath: script.path, + isFlow: false, + workspace: opWorkspace + }) + setContext('TriggerContext', { triggersCount, simplifiedPoll, showCaptureHint: showCaptureHint, - triggersState + triggersState, + draftTarget: triggerDraftTarget }) const enterpriseLangs = ['mssql', 'oracledb'] @@ -600,7 +619,7 @@ ): Promise { if (!triggersToDeploy) { // Check if there are draft triggers that need confirmation - const draftTriggers = triggersState.triggers.filter((trigger) => trigger.draftConfig) + const draftTriggers = triggersState.triggers.filter(triggerHasPendingChanges) if (draftTriggers.length > 0) { draftTriggersModalOpen = true confirmDeploymentCallback = async (triggersToDeploy: Trigger[]) => { @@ -700,20 +719,24 @@ }) } - if (triggersToDeploy) { - await deployTriggers( + // Triggers deploy after the script, never before: their `script_path` + // points at the path this deploy just created or renamed to. + if (triggersToDeploy?.length) { + const failed = await deployTriggers( triggersToDeploy, opWorkspace, !!$userStore?.is_admin || !!$userStore?.is_super_admin, usedTriggerKinds, script.path, - true + false ) + for (const { trigger, error } of failed) { + sendUserToast(`Could not deploy ${trigger.type} trigger: ${error}`, true) + } } - const { draft_triggers: _, ...newScript } = structuredClone($state.snapshot(script)) - savedScript = structuredClone($state.snapshot(newScript)) - setDraftTriggers([]) + savedScript = structuredClone($state.snapshot(script)) + await loadTriggers() if (!disableHistoryChange) { history.replaceState(history.state, '', `/scripts/edit/${script.path}`) @@ -781,8 +804,6 @@ } await syncWithDeployed() - const currentDraftTriggers = structuredClone(triggersState.getDraftTriggersSnapshot()) - const deployed = deployedValue ?? savedScript // `script` comes from the full DB row (since #9351), so it carries `lock`/`extra_perms` // while the deployed side is trimmed in `syncWithDeployed` — null them here to match @@ -791,8 +812,7 @@ const current = { ...script, lock: undefined, - extra_perms: undefined, - draft_triggers: currentDraftTriggers + extra_perms: undefined } if (current.assets && !current.assets.length) delete current.assets @@ -942,7 +962,7 @@ function openTriggers(ev) { metadataOpen = true selectedTab = 'triggers' - handleSelectTriggerFromKind(triggersState, triggersCount, initialPath, ev.detail.kind) + handleSelectTriggerFromKind(triggersState, triggersCount, triggerDraftTarget(), ev.detail.kind) captureOn.set(true) } @@ -1018,16 +1038,6 @@ }) }) - // Mirror the draft triggers (held in a separate `triggersState` $state) - // back into `script.draft_triggers` so the UserDraft autosave — which - // deep-tracks `script` — picks them up. Pre-PR ScriptBuilder ran its own - // localStorage autosave that explicitly snapshotted triggersState; the - // switch to a unified UserDraft handle dropped that bridge. - $effect(() => { - readFieldsRecursively(triggersState.triggers) - script.draft_triggers = triggersState.getDraftTriggersSnapshot() - }) - loadWorkerTags() async function loadWorkerTags() { if (usesLocalTags) { @@ -1054,7 +1064,7 @@ t.draftConfig)} + draftTriggers={triggersState.triggers.filter(triggerHasPendingChanges)} on:canceled={() => { draftTriggersModalOpen = false }} diff --git a/frontend/src/lib/components/common/confirmationModal/DraftTriggersConfirmationModal.svelte b/frontend/src/lib/components/common/confirmationModal/DraftTriggersConfirmationModal.svelte index cfebae0e9f..9d9ab3fdc3 100644 --- a/frontend/src/lib/components/common/confirmationModal/DraftTriggersConfirmationModal.svelte +++ b/frontend/src/lib/components/common/confirmationModal/DraftTriggersConfirmationModal.svelte @@ -5,7 +5,7 @@ import DataTable from '$lib/components/table/DataTable.svelte' import { twMerge } from 'tailwind-merge' import TriggerLabel from '$lib/components/triggers/TriggerLabel.svelte' - import { triggerIconMap } from '$lib/components/triggers/utils' + import { triggerDraftKind, triggerIconMap } from '$lib/components/triggers/utils' import { Star } from 'lucide-svelte' import ToggleButtonGroup from '../toggleButton-v2/ToggleButtonGroup.svelte' import ToggleButton from '../toggleButton-v2/ToggleButton.svelte' @@ -27,26 +27,23 @@ confirmed: { selectedTriggers: Trigger[] } }>() - function toggleTrigger(trigger: Trigger, selected: 'discard' | 'deploy') { - if (selected === 'discard') { - if (trigger.isDraft) { - selectedTriggers = selectedTriggers.filter((t) => !t.isDraft || t.id !== trigger.id) - } else { - selectedTriggers = selectedTriggers.filter( - (t) => t.isDraft || t.type !== trigger.type || t.path !== trigger.path - ) - } + /** Identity across the list. Draft-backed triggers are keyed by their draft + * path; the kinds without a draft row have no path until they deploy, so they + * fall back to the client id assigned when they were added. */ + function triggerKey(trigger: Trigger): string { + return `${trigger.type}:${trigger.path ?? trigger.id ?? ''}` + } + + function toggleTrigger(trigger: Trigger, selected: 'skip' | 'deploy') { + if (selected === 'skip') { + selectedTriggers = selectedTriggers.filter((t) => triggerKey(t) !== triggerKey(trigger)) } else if (!isSelected(selectedTriggers, trigger)) { selectedTriggers = [...selectedTriggers, trigger] } } function isSelected(triggers: Trigger[], trigger: Trigger): boolean { - if (trigger.isDraft) { - return triggers.some((t) => t.id === trigger.id) - } else { - return triggers.some((t) => t.path === trigger.path && t.type === trigger.type) - } + return triggers.some((t) => triggerKey(t) === triggerKey(trigger)) } function checkSavePermissions(trigger: Trigger) { @@ -57,7 +54,10 @@ !$userStore?.is_super_admin && trigger.isDraft - const invalidConfig = !trigger.draftConfig?.canSave + // Only the kinds without a draft row carry their config here. For the rest + // it lives server-side, where an incomplete one surfaces as a deploy error — + // non-destructive, since a failed deploy leaves the draft in place. + const invalidConfig = !triggerDraftKind(trigger.type) && !trigger.draftConfig?.canSave return invalidConfig ? 'invalid-config' : adminOnly ? 'admin-only' : 'deploy' } @@ -70,7 +70,7 @@
- {`${isFlow ? 'Your flow' : 'Your script'} has draft triggers. Select which draft triggers to deploy with the ${isFlow ? 'flow' : 'script'}. Undeployed - draft triggers will be permanently deleted.`} + {`${isFlow ? 'Your flow' : 'Your script'} has undeployed trigger changes. Select which to deploy alongside the ${isFlow ? 'flow' : 'script'}; the rest stay as drafts.`}
5 ? 'h-[300px]' : ''}> @@ -124,19 +123,19 @@
toggleTrigger(trigger, e.detail)} > {#snippet children({ item })} - No draft triggers found + No undeployed trigger changes {/if} diff --git a/frontend/src/lib/components/flow_builder.ts b/frontend/src/lib/components/flow_builder.ts index ce848c6f83..076bb999fa 100644 --- a/frontend/src/lib/components/flow_builder.ts +++ b/frontend/src/lib/components/flow_builder.ts @@ -1,7 +1,6 @@ import type { Flow, OpenFlow } from '$lib/gen' import type { StateStore } from '$lib/utils' import type { FlowState } from './flows/flowState' -import type { Trigger } from './triggers/utils' import type { DiffDrawerI } from './diff_drawer' import type { FlowBuilderWhitelabelCustomUi } from './custom_ui' import type { ScheduleTrigger } from './triggers' @@ -27,7 +26,6 @@ export type FlowBuilderProps = { /** flow_version the draft was forked from; when set, the deploy-time staleness * check compares it (not the load-time head `version`) against the latest. */ draftBaseVersion?: number | undefined - draftTriggersFromUrl?: Trigger[] | undefined selectedTriggerIndexFromUrl?: number | undefined children?: import('svelte').Snippet loadedFromHistoryFromUrl?: { diff --git a/frontend/src/lib/components/flows/content/FlowEditorPanel.svelte b/frontend/src/lib/components/flows/content/FlowEditorPanel.svelte index 471790d539..9494f757d0 100644 --- a/frontend/src/lib/components/flows/content/FlowEditorPanel.svelte +++ b/frontend/src/lib/components/flows/content/FlowEditorPanel.svelte @@ -77,7 +77,7 @@ const selectedId = $derived(selectionManager.getSelectedId()) - const { showCaptureHint, triggersState, triggersCount } = + const { showCaptureHint, triggersState, triggersCount, draftTarget } = getContext('TriggerContext') function checkDup(modules: FlowModule[]): string | undefined { @@ -124,7 +124,7 @@ disabled={disabledFlowInputs} on:openTriggers={(ev) => { selectionManager.selectId('Trigger') - handleSelectTriggerFromKind(triggersState, triggersCount, savedFlow?.path, ev.detail.kind) + handleSelectTriggerFromKind(triggersState, triggersCount, draftTarget(), ev.detail.kind) showCaptureHint.set(true) }} on:applyArgs diff --git a/frontend/src/lib/components/flows/content/FlowModuleWrapper.svelte b/frontend/src/lib/components/flows/content/FlowModuleWrapper.svelte index 0cae21b630..db35703403 100644 --- a/frontend/src/lib/components/flows/content/FlowModuleWrapper.svelte +++ b/frontend/src/lib/components/flows/content/FlowModuleWrapper.svelte @@ -26,7 +26,7 @@ getContext('FlowEditorContext') const selectedId = $derived(selectionManager.getSelectedId()) - const { triggersState, triggersCount } = getContext('TriggerContext') + const { triggersState, triggersCount, draftTarget } = getContext('TriggerContext') let scriptKind: 'script' | 'trigger' | 'approval' = $state('script') let scriptTemplate: 'pgsql' | 'mysql' | 'script' | 'docker' | 'powershell' = $state('script') @@ -67,25 +67,9 @@ args: {}, schedule: formatCron('0 */15 * * *'), timezone: Intl.DateTimeFormat().resolvedOptions().timeZone, - enabled: true, - is_flow: true - } - triggersState.addDraftTrigger(triggersCount, 'schedule', undefined, primaryCfg) - } else if (triggersState.triggers[primaryIndex].draftConfig) { - //If there is a primary schedule draft update it - const newCfg = { ...triggersState.triggers[primaryIndex].draftConfig } - let updated = false - if (!newCfg.schedule) { - newCfg.schedule = formatCron('0 */15 * * *') - updated = true - } - if (!newCfg.enabled) { - newCfg.enabled = true - updated = true - } - if (updated) { - triggersState.triggers[primaryIndex].draftConfig = newCfg + enabled: true } + triggersState.addDraftTrigger(triggersCount, 'schedule', draftTarget(), primaryCfg) } module.stop_after_if = { diff --git a/frontend/src/lib/components/flows/content/FlowPathViewer.svelte b/frontend/src/lib/components/flows/content/FlowPathViewer.svelte index 8ef7b12dee..290de2fc24 100644 --- a/frontend/src/lib/components/flows/content/FlowPathViewer.svelte +++ b/frontend/src/lib/components/flows/content/FlowPathViewer.svelte @@ -10,6 +10,7 @@ import { getContext, setContext } from 'svelte' import type { FlowEditorContext } from '../types' import { writable } from 'svelte/store' + import { NO_TRIGGER_DRAFT_TARGET } from '$lib/components/triggers/utils' interface Props { path: string @@ -32,7 +33,8 @@ triggersCount: triggersCount, simplifiedPoll: writable(false), showCaptureHint: writable(undefined), - triggersState: new Triggers() + triggersState: new Triggers(), + draftTarget: () => NO_TRIGGER_DRAFT_TARGET }) async function loadFlow(path: string) { diff --git a/frontend/src/lib/components/flows/map/FlowModuleSchemaMap.svelte b/frontend/src/lib/components/flows/map/FlowModuleSchemaMap.svelte index 82f91306c0..3d5be3dd2b 100644 --- a/frontend/src/lib/components/flows/map/FlowModuleSchemaMap.svelte +++ b/frontend/src/lib/components/flows/map/FlowModuleSchemaMap.svelte @@ -136,7 +136,7 @@ let opWs = $derived(opWorkspace?.() ?? $workspaceStore) const moveManager = new MoveManager() - const { triggersCount, triggersState } = getContext('TriggerContext') + const { triggersCount, triggersState, draftTarget } = getContext('TriggerContext') const { flowPropPickerConfig } = getContext('PropPickerContext') @@ -820,7 +820,7 @@ }) } if (detail.kind == 'trigger') { - setScheduledPollSchedule(triggersState, triggersCount) + setScheduledPollSchedule(triggersState, triggersCount, draftTarget()) } if (detail.flow?.path) { loadLastJob(detail.flow.path, module.id) diff --git a/frontend/src/lib/components/graph/renderers/nodes/TriggersNode.svelte b/frontend/src/lib/components/graph/renderers/nodes/TriggersNode.svelte index 58f95d4b33..3f698d26ef 100644 --- a/frontend/src/lib/components/graph/renderers/nodes/TriggersNode.svelte +++ b/frontend/src/lib/components/graph/renderers/nodes/TriggersNode.svelte @@ -32,20 +32,17 @@ const { selectionManager } = getGraphContext() - const { triggersCount, triggersState } = $state(getContext('TriggerContext')) + const { triggersCount, triggersState, draftTarget } = $state( + getContext('TriggerContext') + ) function getScheduleCfg(primary: Trigger | undefined, triggersCount: TriggersCount | undefined) { - return primary?.draftConfig - ? { - enabled: primary?.draftConfig?.enabled, - schedule: primary?.draftConfig?.schedule + return primary?.lightConfig + ? { enabled: primary.lightConfig.enabled, schedule: primary.lightConfig.schedule } + : { + enabled: !!triggersCount?.primary_schedule, + schedule: triggersCount?.primary_schedule?.schedule } - : primary?.lightConfig - ? { enabled: primary?.lightConfig?.enabled, schedule: primary?.lightConfig?.schedule } - : { - enabled: !!triggersCount?.primary_schedule, - schedule: triggersCount?.primary_schedule?.schedule - } } let colorClasses = $derived( @@ -89,7 +86,7 @@ triggersState.selectedTriggerIndex = triggerIndex }} onAddDraftTrigger={async (type: TriggerType) => { - const newTrigger = triggersState.addDraftTrigger(triggersCount, type) + const newTrigger = await triggersState.addDraftTrigger(triggersCount, type, draftTarget()) data?.eventHandlers?.select('Trigger') await tick() triggersState.selectedTriggerIndex = newTrigger @@ -123,7 +120,7 @@