From c88d0c6f355c43a3cf3502c687334ffa8f50c463 Mon Sep 17 00:00:00 2001 From: Asish Kumar Date: Sat, 4 Apr 2026 14:48:23 +0530 Subject: [PATCH 1/3] refactor: consolidate run and work order state definitions Hardcoded run and work order state lists were scattered across multiple files (dashboard_stats.ex, query.ex, impeded_project_helper.ex, dashboard_components.ex, search_params.ex, and tests), creating a maintenance burden where any state change required manual updates in multiple locations that the compiler cannot catch. Add Run.active_states/0 for [:available, :claimed, :started], WorkOrder.states/0 for all valid states, and WorkOrder.active_states/0 for [:pending, :running]. Replace all hardcoded state lists across the codebase with references to these canonical functions. Derive SearchParams.@statuses from WorkOrder.states/0 to keep the filter UI in sync automatically. Closes #4589 Signed-off-by: Asish Kumar --- CHANGELOG.md | 6 ++++++ lib/lightning/dashboard_stats.ex | 15 ++++++++++----- .../runs/prom_ex_plugin/impeded_project_helper.ex | 4 +++- lib/lightning/runs/query.ex | 2 +- lib/lightning/runs/run.ex | 7 +++++++ lib/lightning/workorders/search_params.ex | 3 ++- lib/lightning/workorders/workorder.ex | 14 +++++++++++++- .../live/workflow_live/dashboard_components.ex | 3 ++- test/lightning/digest_email_worker_test.exs | 2 +- test/lightning/janitor_test.exs | 2 +- 10 files changed, 46 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d4bc3c4903..527d572e375 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,12 @@ and this project adheres to ### Changed +- Consolidated run and work order state definitions into single source of truth + by adding `Run.active_states/0`, `WorkOrder.states/0`, and + `WorkOrder.active_states/0` and replacing all hardcoded state lists across the + codebase + [#4589](https://github.com/OpenFn/lightning/issues/4589) + ### Fixed ## [2.16.1-pre1] - 2026-04-04 diff --git a/lib/lightning/dashboard_stats.ex b/lib/lightning/dashboard_stats.ex index 7d90d23c018..0d048da33e0 100644 --- a/lib/lightning/dashboard_stats.ex +++ b/lib/lightning/dashboard_stats.ex @@ -11,6 +11,8 @@ defmodule Lightning.DashboardStats do alias Lightning.Workflows.Workflow alias Lightning.WorkOrder + @wo_active WorkOrder.active_states() + @run_active Run.active_states() @days_back 30 defmodule WorkflowStats do @@ -58,7 +60,10 @@ defmodule Lightning.DashboardStats do last_workorders = batch_get_last_workorders(workflow_ids) last_failed_workorders = - batch_get_last_workorders(workflow_ids, [:pending, :running, :success]) + batch_get_last_workorders( + workflow_ids, + WorkOrder.active_states() ++ [:success] + ) Enum.map(workflows, fn workflow -> wf_id = workflow.id @@ -229,7 +234,7 @@ defmodule Lightning.DashboardStats do {:success, cnt}, acc -> %{acc | success: cnt} - {state, cnt}, acc when state in [:pending, :running] -> + {state, cnt}, acc when state in @wo_active -> Map.update!(acc, :pending, &(&1 + cnt)) {_other, cnt}, acc -> @@ -251,7 +256,7 @@ defmodule Lightning.DashboardStats do {:success, cnt}, acc -> %{acc | success: cnt} - {state, cnt}, acc when state in [:available, :claimed, :started] -> + {state, cnt}, acc when state in @run_active -> Map.update!(acc, :pending, &(&1 + cnt)) {_other, cnt}, acc -> @@ -333,7 +338,7 @@ defmodule Lightning.DashboardStats do :success -> %{current | success: cnt} - s when s in [:pending, :running] -> + s when s in @wo_active -> Map.update!(current, :pending, &(&1 + cnt)) _ -> @@ -361,7 +366,7 @@ defmodule Lightning.DashboardStats do :success -> %{current | success: cnt} - s when s in [:available, :claimed, :started] -> + s when s in @run_active -> Map.update!(current, :pending, &(&1 + cnt)) _ -> diff --git a/lib/lightning/runs/prom_ex_plugin/impeded_project_helper.ex b/lib/lightning/runs/prom_ex_plugin/impeded_project_helper.ex index 0da3572d936..52c1dae8f7e 100644 --- a/lib/lightning/runs/prom_ex_plugin/impeded_project_helper.ex +++ b/lib/lightning/runs/prom_ex_plugin/impeded_project_helper.ex @@ -25,9 +25,11 @@ defmodule Lightning.Runs.PromExPlugin.ImpededProjectHelper do select: w.workflow_id, distinct: true + in_progress = Lightning.Run.active_states() -- [:available] + in_progress_runs_query = from r in Lightning.Run, - where: r.state in [:claimed, :started], + where: r.state in ^in_progress, join: w in assoc(r, :work_order), group_by: w.workflow_id, select: %{workflow_id: w.workflow_id, count: count(r.id)} diff --git a/lib/lightning/runs/query.ex b/lib/lightning/runs/query.ex index 949ceb7b36c..2db83a075e6 100644 --- a/lib/lightning/runs/query.ex +++ b/lib/lightning/runs/query.ex @@ -231,7 +231,7 @@ defmodule Lightning.Runs.Query do # Step 1: Rank runs within each workflow by priority and insertion time ranked_runs_query = from(r in Run, - where: r.state in [:available, :claimed, :started], + where: r.state in ^Run.active_states(), join: wo in assoc(r, :work_order), join: w in assoc(wo, :workflow), join: p in assoc(w, :project) diff --git a/lib/lightning/runs/run.ex b/lib/lightning/runs/run.ex index 9501ca9436c..ba8dcc921ab 100644 --- a/lib/lightning/runs/run.ex +++ b/lib/lightning/runs/run.ex @@ -56,6 +56,13 @@ defmodule Lightning.Run do """ def final_states, do: @final_states + @doc """ + Returns the list of active (in-progress) states for a run. + + These are all non-final states: available, claimed, and started. + """ + def active_states, do: [:available, :claimed, :started] + @doc """ Returns the list of failure states for a run. diff --git a/lib/lightning/workorders/search_params.ex b/lib/lightning/workorders/search_params.ex index b7c59a15cb5..93cd83ecd46 100644 --- a/lib/lightning/workorders/search_params.ex +++ b/lib/lightning/workorders/search_params.ex @@ -21,7 +21,8 @@ defmodule Lightning.WorkOrders.SearchParams do :sort_direction ]} - @statuses ~w(pending running success failed crashed killed cancelled lost exception rejected) + @statuses Lightning.WorkOrder.states() + |> Enum.map(&Atom.to_string/1) @statuses_set MapSet.new(@statuses) @search_fields ~w(id body log dataclip_name) diff --git a/lib/lightning/workorders/workorder.ex b/lib/lightning/workorders/workorder.ex index b95e03ecb04..c472d7de029 100644 --- a/lib/lightning/workorders/workorder.ex +++ b/lib/lightning/workorders/workorder.ex @@ -21,11 +21,23 @@ defmodule Lightning.WorkOrder do workflow: Workflow.t() | Ecto.Association.NotLoaded.t() } + @active_states [:pending, :running] + @state_values Enum.concat( - [:rejected, :pending, :running], + [:rejected | @active_states], Run.final_states() ) + @doc """ + Returns all valid work order states. + """ + def states, do: @state_values + + @doc """ + Returns the list of active (in-progress) work order states. + """ + def active_states, do: @active_states + @derive {Jason.Encoder, only: [ :id, diff --git a/lib/lightning_web/live/workflow_live/dashboard_components.ex b/lib/lightning_web/live/workflow_live/dashboard_components.ex index 9c59f8f6f62..e5264272c5e 100644 --- a/lib/lightning_web/live/workflow_live/dashboard_components.ex +++ b/lib/lightning_web/live/workflow_live/dashboard_components.ex @@ -4,6 +4,7 @@ defmodule LightningWeb.WorkflowLive.DashboardComponents do alias Lightning.DashboardStats.ProjectMetrics alias Lightning.Projects.Project + alias Lightning.WorkOrder alias Lightning.WorkOrders.SearchParams alias LightningWeb.Components.Common alias LightningWeb.WorkflowLive.Helpers @@ -455,7 +456,7 @@ defmodule LightningWeb.WorkflowLive.DashboardComponents do
- <%= if @state in [:pending, :running] do %> + <%= if @state in WorkOrder.active_states() do %> insert(:run, work_order: work_order, From f089a06f426a6af291459380745a34c903828b7e Mon Sep 17 00:00:00 2001 From: Asish Kumar Date: Sat, 4 Apr 2026 15:10:29 +0530 Subject: [PATCH 2/3] style: fix formatting in search_params.ex Align pipe operator to satisfy mix format --check-formatted. Signed-off-by: Asish Kumar --- lib/lightning/workorders/search_params.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/lightning/workorders/search_params.ex b/lib/lightning/workorders/search_params.ex index 93cd83ecd46..a9fca2448c7 100644 --- a/lib/lightning/workorders/search_params.ex +++ b/lib/lightning/workorders/search_params.ex @@ -22,7 +22,7 @@ defmodule Lightning.WorkOrders.SearchParams do ]} @statuses Lightning.WorkOrder.states() - |> Enum.map(&Atom.to_string/1) + |> Enum.map(&Atom.to_string/1) @statuses_set MapSet.new(@statuses) @search_fields ~w(id body log dataclip_name) From 92314b93165c5d486efb6e1aea46ca4639c33b8c Mon Sep 17 00:00:00 2001 From: Asish Kumar Date: Sat, 4 Apr 2026 15:37:17 +0530 Subject: [PATCH 3/3] refactor: use single source of truth for state definitions Derived excluded_states in dashboard_stats from WorkOrder.active_states instead of hardcoding. Introduced @active_states module attribute in Run to share between the function and Ecto.Enum definition. Signed-off-by: Asish Kumar --- lib/lightning/dashboard_stats.ex | 2 +- lib/lightning/runs/run.ex | 14 ++++---------- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/lib/lightning/dashboard_stats.ex b/lib/lightning/dashboard_stats.ex index 0d048da33e0..e407528545f 100644 --- a/lib/lightning/dashboard_stats.ex +++ b/lib/lightning/dashboard_stats.ex @@ -197,7 +197,7 @@ defmodule Lightning.DashboardStats do end defp get_last_failed_workorder(workflow, %{state: :success}) do - excluded_states = [:pending, :running, :success] + excluded_states = @wo_active ++ [:success] get_last_workorder(workflow, excluded_states) end diff --git a/lib/lightning/runs/run.ex b/lib/lightning/runs/run.ex index ba8dcc921ab..9314d368209 100644 --- a/lib/lightning/runs/run.ex +++ b/lib/lightning/runs/run.ex @@ -41,6 +41,8 @@ defmodule Lightning.Run do :final_dataclip_id ]} + @active_states [:available, :claimed, :started] + @final_states [ :success, :failed, @@ -61,7 +63,7 @@ defmodule Lightning.Run do These are all non-final states: available, claimed, and started. """ - def active_states, do: [:available, :claimed, :started] + def active_states, do: @active_states @doc """ Returns the list of failure states for a run. @@ -105,15 +107,7 @@ defmodule Lightning.Run do embeds_one :options, Lightning.Runs.RunOptions field :state, Ecto.Enum, - values: - Enum.concat( - [ - :available, - :claimed, - :started - ], - @final_states - ), + values: Enum.concat(@active_states, @final_states), default: :available field :error_type, :string