diff --git a/CHANGELOG.md b/CHANGELOG.md index 90e82d0b6c..6086d7108b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,20 @@ and this project adheres to ### Changed +- When a run is `:claimed` by a worker, set its parent work order to `:running` + rather than leaving it in `:pending`. + [#4635](https://github.com/OpenFn/lightning/issues/4635) + + When a run is claimed by a worker, there's no stopping it. From the platform's + perspective, the parent work order should be moved from `:pending` to + `:running`, even though there's an underlying technical difference between the + run states `:claimed` and `:started`. As a result, the "Cancel" button on the + history view disappears once a run has been claimed. + + This shift is also visible to external consumers: the `/api/workorders` + endpoint and workflow channel subscribers will now report `running` where they + previously reported `pending` for work orders whose run has been claimed. + ### Fixed - Bump `@openfn/ws-worker` from diff --git a/lib/lightning/workorders/query.ex b/lib/lightning/workorders/query.ex index f5069d8e93..8d61c1ee79 100644 --- a/lib/lightning/workorders/query.ex +++ b/lib/lightning/workorders/query.ex @@ -7,15 +7,19 @@ defmodule Lightning.WorkOrders.Query do alias Lightning.Run # Create a copy of the WorkOrder state enum to use in the query, or else - # we would need to unnecessarly join the workorders table just for casting. + # we would need to unnecessarily join the workorders table just for casting. # @workorder_state Ecto.ParameterizedType.init(Ecto.Enum, # values: Ecto.Enum.values(Lightning.WorkOrder, :state) # ) + # Order matters: this is a priority list used with `WITH ORDINALITY` below, + # where lower ordinals win in ascending sort. Do not alphabetize. + # `started` and `claimed` both imply the workorder is `:running`, so they + # must outrank `available`, which implies `:pending`. @unfinished_state_order ~w[ started - available claimed + available ] @doc """ @@ -29,9 +33,10 @@ defmodule Lightning.WorkOrders.Query do - The current Run is unioned onto the unfinished runs with a null ordinality. - The runs are ordered by state in the following order - `started > available > claimed > null` - - The run states are then mapped to the workorder state enum, so `available` - and `claimed` are both mapped to `pending` and `started` is mapped to `running` + `started > claimed > available > null` + - The run states are then mapped to the workorder state enum: `available` + is mapped to `pending` (the only run state Lightning can still cancel + atomically), while `claimed` and `started` are both mapped to `running`. > The `null` ordinality ensures that the current run is always last in the > ordering. @@ -71,7 +76,7 @@ defmodule Lightning.WorkOrders.Query do """ CASE ? WHEN 'available' THEN 'pending' - WHEN 'claimed' THEN 'pending' + WHEN 'claimed' THEN 'running' WHEN 'started' THEN 'running' ELSE ? END diff --git a/test/lightning/work_orders_test.exs b/test/lightning/work_orders_test.exs index efb383b49e..ba1445b948 100644 --- a/test/lightning/work_orders_test.exs +++ b/test/lightning/work_orders_test.exs @@ -2582,6 +2582,24 @@ defmodule Lightning.WorkOrdersTest do assert work_order.state == :running end + + test "sets the workorders state to running once a run is claimed" do + dataclip = insert(:dataclip) + %{triggers: [trigger]} = workflow = insert(:simple_workflow) + + %{runs: [run]} = + work_order_for(trigger, workflow: workflow, dataclip: dataclip) + |> insert() + + {:ok, work_order} = WorkOrders.update_state(run) + assert work_order.state == :pending + + {:ok, run} = + Repo.update(run |> Ecto.Changeset.change(state: :claimed)) + + {:ok, work_order} = WorkOrders.update_state(run) + assert work_order.state == :running + end end describe "get_workorders_with_runs/2" do diff --git a/test/lightning/workorders/query_test.exs b/test/lightning/workorders/query_test.exs index cc9fe89f3f..600bf03194 100644 --- a/test/lightning/workorders/query_test.exs +++ b/test/lightning/workorders/query_test.exs @@ -27,7 +27,7 @@ defmodule Lightning.WorkOrders.QueryTest do Repo.update(change(first_run, state: :claimed)) - assert Query.state_for(first_run) |> Repo.one() == %{state: "pending"} + assert Query.state_for(first_run) |> Repo.one() == %{state: "running"} Repo.update(change(first_run, state: :started)) @@ -57,5 +57,20 @@ defmodule Lightning.WorkOrders.QueryTest do # Running wins over pending. assert %{state: "running"} = Query.state_for(third_run) |> Repo.one() end + + test "a claimed sibling wins over an available run", context do + [_claimed_run, available_run] = + [:claimed, :available] + |> Enum.map(fn state -> + insert(:run, + work_order: context.work_order, + dataclip: context.dataclip, + starting_trigger: context.trigger, + state: state + ) + end) + + assert %{state: "running"} = Query.state_for(available_run) |> Repo.one() + end end end diff --git a/test/lightning_web/live/run_live/index_test.exs b/test/lightning_web/live/run_live/index_test.exs index ddaa8a9ac8..c0afd4bcfe 100644 --- a/test/lightning_web/live/run_live/index_test.exs +++ b/test/lightning_web/live/run_live/index_test.exs @@ -1070,6 +1070,48 @@ defmodule LightningWeb.RunLive.IndexTest do assert html =~ "Work order could not be cancelled" end + @tag role: :editor + test "cancel button disappears when run is claimed while page is live", + %{conn: conn, project: project, workflow: workflow} do + trigger = List.first(workflow.triggers) + dataclip = insert(:dataclip, project: project) + + pending_wo = + insert(:workorder, + workflow: workflow, + trigger: trigger, + dataclip: dataclip, + state: :pending, + last_activity: DateTime.utc_now() + ) + |> with_run( + state: :available, + dataclip: dataclip, + starting_trigger: trigger + ) + + {:ok, view, _html} = + live_async( + conn, + Routes.project_run_index_path(conn, :index, project.id, + filters: %{pending: true, running: true} + ) + ) + + assert has_element?(view, "button#cancel-wo-#{pending_wo.id}") + + run = List.first(pending_wo.runs) + + {:ok, run} = + run |> Ecto.Changeset.change(state: :claimed) |> Repo.update() + + {:ok, _wo} = WorkOrders.update_state(run) + + render(view) + + refute has_element?(view, "button#cancel-wo-#{pending_wo.id}") + end + @tag role: :editor test "cancel-run with nonexistent run shows error", %{conn: conn, project: project} do