From 04ef675b1d0e8813faeb8759870cec0e25aecccd Mon Sep 17 00:00:00 2001
From: HugoCasa <hugo@casademont.ch>
Date: Tue, 1 Oct 2024 12:28:46 +0200
Subject: [PATCH] skip step better UI (#4465)

---
 backend/windmill-common/src/flow_status.rs      |  3 +++
 backend/windmill-worker/src/worker_flow.rs      | 17 +++++++++++++++++
 .../lib/components/FlowStatusViewerInner.svelte |  9 +++++++--
 frontend/src/lib/components/ModuleStatus.svelte |  3 +++
 .../flows/map/FlowModuleSchemaItem.svelte       | 13 +++++++++++++
 .../src/lib/components/flows/map/MapItem.svelte |  1 +
 frontend/src/lib/components/graph/model.ts      |  1 +
 .../graph/renderers/nodes/ModuleNode.svelte     |  2 +-
 frontend/src/lib/components/graph/util.ts       |  9 +++++++--
 openflow.openapi.yaml                           |  2 ++
 10 files changed, 55 insertions(+), 5 deletions(-)

diff --git a/backend/windmill-common/src/flow_status.rs b/backend/windmill-common/src/flow_status.rs
index 035cadecd8..a5c509d6ef 100644
--- a/backend/windmill-common/src/flow_status.rs
+++ b/backend/windmill-common/src/flow_status.rs
@@ -128,6 +128,7 @@ struct UntaggedFlowStatusModule {
     while_loop: Option<bool>,
     approvers: Option<Vec<Approval>>,
     failed_retries: Option<Vec<Uuid>>,
+    skipped: Option<bool>,
 }
 
 #[derive(Serialize, Debug, Clone)]
@@ -179,6 +180,7 @@ pub enum FlowStatusModule {
         approvers: Vec<Approval>,
         #[serde(skip_serializing_if = "Vec::is_empty")]
         failed_retries: Vec<Uuid>,
+        skipped: bool,
     },
     Failure {
         id: String,
@@ -255,6 +257,7 @@ impl<'de> Deserialize<'de> for FlowStatusModule {
                 branch_chosen: untagged.branch_chosen,
                 approvers: untagged.approvers.unwrap_or_default(),
                 failed_retries: untagged.failed_retries.unwrap_or_default(),
+                skipped: untagged.skipped.unwrap_or(false),
             }),
             "Failure" => Ok(FlowStatusModule::Failure {
                 id: untagged
diff --git a/backend/windmill-worker/src/worker_flow.rs b/backend/windmill-worker/src/worker_flow.rs
index c00fd81a44..3f92cfc829 100644
--- a/backend/windmill-worker/src/worker_flow.rs
+++ b/backend/windmill-worker/src/worker_flow.rs
@@ -561,6 +561,7 @@ pub async fn update_flow_status_after_job_completion_internal<
                             branch_chosen: None,
                             approvers: vec![],
                             failed_retries: vec![],
+                            skipped: false,
                         }
                     } else {
                         success = false;
@@ -698,6 +699,20 @@ pub async fn update_flow_status_after_job_completion_internal<
                     }
                 }
                 if success || (flow_jobs.is_some() && (skip_loop_failures || skip_branch_failure)) {
+                    let is_skipped = if current_module.as_ref().is_some_and(|m| m.skip_if.is_some()) {
+                        sqlx::query_scalar!(
+                            "SELECT job_kind = 'identity' FROM completed_job WHERE id = $1",
+                            job_id_for_status
+                        )
+                        .fetch_one(db)
+                        .await
+                        .map_err(|e| {
+                            Error::InternalErr(format!("error during skip check: {e:#}"))
+                        })?
+                        .unwrap_or(false)
+                    } else {
+                        false
+                    };
                     success = true;
                     (
                         true,
@@ -709,6 +724,7 @@ pub async fn update_flow_status_after_job_completion_internal<
                             branch_chosen,
                             approvers: vec![],
                             failed_retries: old_status.retry.failed_jobs.clone(),
+                            skipped: is_skipped,
                         }),
                     )
                 } else {
@@ -2344,6 +2360,7 @@ async fn push_next_flow_job<R: rsmq_async::RsmqConnection + Send + Sync + Clone>
                 branch_chosen: None,
                 approvers: vec![],
                 failed_retries: vec![],
+                skipped: false,
             }))
             .bind(flow_job.id)
             .execute(db)
diff --git a/frontend/src/lib/components/FlowStatusViewerInner.svelte b/frontend/src/lib/components/FlowStatusViewerInner.svelte
index 0232548183..2dcc2a030b 100644
--- a/frontend/src/lib/components/FlowStatusViewerInner.svelte
+++ b/frontend/src/lib/components/FlowStatusViewerInner.svelte
@@ -437,7 +437,8 @@
 						flow_jobs: mod.flow_jobs,
 						flow_jobs_success: mod.flow_jobs_success,
 						iteration_total: mod.iterator?.itered?.length,
-						retries: mod?.failed_retries?.length
+						retries: mod?.failed_retries?.length,
+						skipped: mod.skipped
 						// retries: $flowStateStore?.raw_flow
 					},
 					force
@@ -1054,7 +1055,11 @@
 											<span class="pl-1 text-tertiary text-lg pt-4">Selected subflow</span>
 										{/if}
 										<div class="px-2 flex gap-2 min-w-0 w-full">
-											<ModuleStatus type={node.type} scheduled_for={node.scheduled_for} />
+											<ModuleStatus
+												type={node.type}
+												scheduled_for={node.scheduled_for}
+												skipped={node.skipped}
+											/>
 											{#if node.duration_ms}
 												<Badge>
 													<Hourglass class="mr-2" size={10} />
diff --git a/frontend/src/lib/components/ModuleStatus.svelte b/frontend/src/lib/components/ModuleStatus.svelte
index b5f4e8b230..c01e793454 100644
--- a/frontend/src/lib/components/ModuleStatus.svelte
+++ b/frontend/src/lib/components/ModuleStatus.svelte
@@ -7,6 +7,7 @@
 
 	export let type: FlowStatusModule['type']
 	export let scheduled_for: Date | undefined
+	export let skipped: boolean = false
 </script>
 
 {#if type == 'WaitingForEvents'}
@@ -28,6 +29,8 @@
 			Job is waiting for an executor
 		{/if}
 	</span>
+{:else if skipped}
+	<Badge color="blue">Skipped</Badge>
 {:else if type == 'Success'}
 	<Badge color="green">Success</Badge>
 {:else if type == 'Failure'}
diff --git a/frontend/src/lib/components/flows/map/FlowModuleSchemaItem.svelte b/frontend/src/lib/components/flows/map/FlowModuleSchemaItem.svelte
index bbab6a6f7e..a0896add9a 100644
--- a/frontend/src/lib/components/flows/map/FlowModuleSchemaItem.svelte
+++ b/frontend/src/lib/components/flows/map/FlowModuleSchemaItem.svelte
@@ -13,6 +13,7 @@
 		PhoneIncoming,
 		Repeat,
 		Square,
+		SkipForward,
 		Voicemail,
 		X
 	} from 'lucide-svelte'
@@ -33,6 +34,7 @@
 	export let retry: boolean = false
 	export let cache: boolean = false
 	export let earlyStop: boolean = false
+	export let skip: boolean = false
 	export let suspend: boolean = false
 	export let sleep: boolean = false
 	export let mock: boolean = false
@@ -181,6 +183,17 @@
 				<svelte:fragment slot="text">Early stop/break</svelte:fragment>
 			</Popover>
 		{/if}
+		{#if skip}
+			<Popover notClickable>
+				<div
+					transition:fade|local={{ duration: 200 }}
+					class="center-center bg-surface rounded border border-gray-400 text-secondary px-1 py-0.5"
+				>
+					<SkipForward size={12} />
+				</div>
+				<svelte:fragment slot="text">Skip</svelte:fragment>
+			</Popover>
+		{/if}
 		{#if suspend}
 			<Popover notClickable>
 				<div
diff --git a/frontend/src/lib/components/flows/map/MapItem.svelte b/frontend/src/lib/components/flows/map/MapItem.svelte
index a171cd3344..647dd9ab72 100644
--- a/frontend/src/lib/components/flows/map/MapItem.svelte
+++ b/frontend/src/lib/components/flows/map/MapItem.svelte
@@ -48,6 +48,7 @@
 		selected: $selectedId === mod.id,
 		retry: mod.retry?.constant != undefined || mod.retry?.exponential != undefined,
 		earlyStop: mod.stop_after_if != undefined || mod.stop_after_all_iters_if != undefined,
+		skip: Boolean(mod.skip_if),
 		suspend: Boolean(mod.suspend),
 		sleep: Boolean(mod.sleep),
 		cache: Boolean(mod.cache_ttl),
diff --git a/frontend/src/lib/components/graph/model.ts b/frontend/src/lib/components/graph/model.ts
index 34f7a4c4a3..74ffa8ff49 100644
--- a/frontend/src/lib/components/graph/model.ts
+++ b/frontend/src/lib/components/graph/model.ts
@@ -60,6 +60,7 @@ export type GraphModuleState = {
 	started_at?: number
 	suspend_count?: number
 	isListJob?: boolean
+	skipped?: boolean
 }
 
 export type NestedNodes = GraphItem[]
diff --git a/frontend/src/lib/components/graph/renderers/nodes/ModuleNode.svelte b/frontend/src/lib/components/graph/renderers/nodes/ModuleNode.svelte
index c733890161..89116f9317 100644
--- a/frontend/src/lib/components/graph/renderers/nodes/ModuleNode.svelte
+++ b/frontend/src/lib/components/graph/renderers/nodes/ModuleNode.svelte
@@ -56,7 +56,7 @@
 			  '/' +
 			  (state?.iteration_total ?? '?')
 			: ''}
-		bgColor={getStateColor(type, darkMode, true)}
+		bgColor={getStateColor(type, darkMode, true, state?.skipped)}
 		modules={data.modules ?? []}
 		moving={data.moving}
 		duration_ms={state?.duration_ms}
diff --git a/frontend/src/lib/components/graph/util.ts b/frontend/src/lib/components/graph/util.ts
index dc3b255bb3..fb2ac009a1 100644
--- a/frontend/src/lib/components/graph/util.ts
+++ b/frontend/src/lib/components/graph/util.ts
@@ -12,11 +12,16 @@ export const NODE = {
 export function getStateColor(
 	state: FlowStatusModule['type'] | undefined,
 	isDark: boolean,
-	nonVirtualItem?: boolean
+	nonVirtualItem?: boolean,
+	isSkipped?: boolean
 ): string {
 	switch (state) {
 		case 'Success':
-			return isDark ? '#059669' : 'rgb(193, 255, 216)'
+			if (isSkipped) {
+				return isDark ? '#1E3A8A' : 'rgb(191, 219, 254)'
+			} else {
+				return isDark ? '#059669' : 'rgb(193, 255, 216)'
+			}
 		case 'Failure':
 			return isDark ? '#dc2626' : 'rgb(248 113 113)'
 		case 'InProgress':
diff --git a/openflow.openapi.yaml b/openflow.openapi.yaml
index d544bcd50e..e95266c52f 100644
--- a/openflow.openapi.yaml
+++ b/openflow.openapi.yaml
@@ -549,4 +549,6 @@ components:
           items:
             type: string
             format: uuid
+        skipped:
+          type: boolean
       required: [type]