diff --git a/CHANGELOG.md b/CHANGELOG.md index 8467e44c16..5924570475 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,8 @@ and this project adheres to - Error when the logger receives a boolean [#2666](https://github.com/OpenFn/lightning/issues/2666) +- Disable save and run actions on deleted workflows + [#2170](https://github.com/OpenFn/lightning/issues/2170) ## [v2.10.1] - 2024-11-13 diff --git a/lib/lightning/work_orders.ex b/lib/lightning/work_orders.ex index 0fc9feab1d..539df6cb51 100644 --- a/lib/lightning/work_orders.ex +++ b/lib/lightning/work_orders.ex @@ -78,7 +78,8 @@ defmodule Lightning.WorkOrders do create_for(job, workflow: workflow, dataclip: dataclip, user: user) """ @spec create_for(Trigger.t() | Job.t(), Multi.t(), [work_order_option()]) :: - {:ok, WorkOrder.t()} | {:error, Ecto.Changeset.t(WorkOrder.t())} + {:ok, WorkOrder.t()} + | {:error, Ecto.Changeset.t(WorkOrder.t()) | :workflow_deleted} def create_for(target, multi \\ Multi.new(), opts) def create_for(%Trigger{} = trigger, multi, opts) do @@ -118,6 +119,13 @@ defmodule Lightning.WorkOrders do def create_for(%Manual{} = manual) do Multi.new() + |> Multi.run(:workflow_deleted?, fn _repo, _changes -> + if manual.workflow.deleted_at do + {:error, :workflow_deleted} + else + {:ok, false} + end + end) |> get_or_insert_dataclip(manual) |> Multi.put(:workflow, manual.workflow) |> get_or_create_snapshot() @@ -283,7 +291,7 @@ defmodule Lightning.WorkOrders do Step.t() | Ecto.UUID.t(), [work_order_option(), ...] ) :: - {:ok, Run.t()} | {:error, Ecto.Changeset.t()} + {:ok, Run.t()} | {:error, Ecto.Changeset.t() | :workflow_deleted} def retry(%Run{id: run_id}, %Step{id: step_id}, opts) do retry(run_id, step_id, opts) end @@ -310,6 +318,13 @@ defmodule Lightning.WorkOrders do preload: [:job] ) end) + |> Multi.run(:workflow_deleted?, fn _repo, %{run: run} -> + if run.work_order.workflow.deleted_at do + {:error, :workflow_deleted} + else + {:ok, false} + end + end) |> Multi.run(:input_dataclip_id, fn _repo, %{step: %Step{input_dataclip_id: input_dataclip_id}} -> {:ok, input_dataclip_id} @@ -501,7 +516,10 @@ defmodule Lightning.WorkOrders do retry(run_step.run_id, run_step.step_id, opts) end) - {:ok, Enum.count(results, fn result -> match?({:ok, _}, result) end), 0} + success_count = + Enum.count(results, fn result -> match?({:ok, _}, result) end) + + {:ok, success_count, Enum.count(results) - success_count} end end @@ -672,6 +690,8 @@ defmodule Lightning.WorkOrders do defp fetch_retriable_workorders(workorder_ids) do workorder_ids |> workorders_with_dataclips_query() + |> join(:inner, [wo], wf in assoc(wo, :workflow), as: :workflow) + |> where([workflow: wf], is_nil(wf.deleted_at)) |> Repo.all() end diff --git a/lib/lightning/workflows.ex b/lib/lightning/workflows.ex index b319a89c7d..761bacaa49 100644 --- a/lib/lightning/workflows.ex +++ b/lib/lightning/workflows.ex @@ -54,9 +54,18 @@ defmodule Lightning.Workflows do def get_workflow(id), do: Repo.get(Workflow, id) @spec save_workflow(Ecto.Changeset.t(Workflow.t()) | map()) :: - {:ok, Workflow.t()} | {:error, Ecto.Changeset.t(Workflow.t())} + {:ok, Workflow.t()} + | {:error, Ecto.Changeset.t(Workflow.t())} + | {:error, :workflow_deleted} def save_workflow(%Ecto.Changeset{data: %Workflow{}} = changeset) do Multi.new() + |> Multi.run(:validate, fn _repo, _changes -> + if is_nil(changeset.data.deleted_at) do + {:ok, true} + else + {:error, :workflow_deleted} + end + end) |> Multi.insert_or_update(:workflow, changeset) |> then(fn multi -> if changeset.changes == %{} do @@ -86,6 +95,9 @@ defmodule Lightning.Workflows do end) {:error, false} + + {:error, _action, reason, _changes} -> + {:error, reason} end end diff --git a/lib/lightning_web/live/run_live/index.ex b/lib/lightning_web/live/run_live/index.ex index 9c5f298fd0..49cdff140b 100644 --- a/lib/lightning_web/live/run_live/index.ex +++ b/lib/lightning_web/live/run_live/index.ex @@ -337,6 +337,11 @@ defmodule LightningWeb.RunLive.Index do socket |> put_flash(:error, error_text)} + {:error, :workflow_deleted} -> + {:noreply, + socket + |> put_flash(:error, "Runs for deleted workflows cannot be retried")} + {:error, _changeset} -> {:noreply, socket @@ -359,7 +364,8 @@ defmodule LightningWeb.RunLive.Index do "New run#{if count > 1, do: "s", else: ""} enqueued for #{count} workorder#{if count > 1, do: "s", else: ""}" |> then(fn msg -> if discarded_count > 0 do - msg <> " (#{discarded_count} were discarded due to wiped dataclip)" + msg <> + " (#{discarded_count} were discarded due to wiped dataclip/workflow being deleted)" else msg end diff --git a/lib/lightning_web/live/workflow_live/edit.ex b/lib/lightning_web/live/workflow_live/edit.ex index 800197f075..65a1141423 100644 --- a/lib/lightning_web/live/workflow_live/edit.ex +++ b/lib/lightning_web/live/workflow_live/edit.ex @@ -27,7 +27,6 @@ defmodule LightningWeb.WorkflowLive.Edit do alias Lightning.Workflows.Trigger alias Lightning.Workflows.Workflow alias Lightning.WorkOrders - alias LightningWeb.Components.Form alias LightningWeb.WorkflowLive.Helpers alias LightningWeb.WorkflowNewLive.WorkflowParams alias Phoenix.LiveView.JS @@ -146,6 +145,7 @@ defmodule LightningWeb.WorkflowLive.Edit do <.offline_indicator /> <.save_workflow_button + id="top-bar-save-workflow-btn" changeset={@changeset} can_edit_workflow={@can_edit_workflow} snapshot_version_tag={@snapshot_version_tag} @@ -323,6 +323,7 @@ defmodule LightningWeb.WorkflowLive.Edit do /> <.with_changes_indicator changeset={@changeset}> <.save_workflow_button + id="inspector-save-workflow-btn" changeset={@changeset} can_edit_workflow={@can_edit_workflow} snapshot_version_tag={@snapshot_version_tag} @@ -447,7 +448,8 @@ defmodule LightningWeb.WorkflowLive.Edit do <.job_form on_change={&send_form_changed/1} editable={ - @can_edit_workflow && @snapshot_version_tag == "latest" && + is_nil(@workflow.deleted_at) && @can_edit_workflow && + @snapshot_version_tag == "latest" && @has_presence_edit_priority } form={jf} @@ -472,12 +474,14 @@ defmodule LightningWeb.WorkflowLive.Edit do phx-value-id={@selected_job.id} class="focus:ring-red-500 bg-red-600 hover:bg-red-700 disabled:bg-red-300" disabled={ - !@can_edit_workflow or @has_child_edges or @is_first_job or + !is_nil(@workflow.deleted_at) or !@can_edit_workflow or + @has_child_edges or @is_first_job or @snapshot_version_tag != "latest" || !@has_presence_edit_priority } tooltip={ - deletion_tooltip_message( + job_deletion_tooltip_message( + is_struct(@workflow.deleted_at), @can_edit_workflow, @has_child_edges, @is_first_job @@ -524,7 +528,7 @@ defmodule LightningWeb.WorkflowLive.Edit do form={tf} on_change={&send_form_changed/1} disabled={ - !@can_edit_workflow or + !is_nil(@workflow.deleted_at) or !@can_edit_workflow or @snapshot_version_tag != "latest" || !@has_presence_edit_priority } @@ -556,7 +560,7 @@ defmodule LightningWeb.WorkflowLive.Edit do <.edge_form form={ef} disabled={ - !@can_edit_workflow or + !is_nil(@workflow.deleted_at) or !@can_edit_workflow or @snapshot_version_tag != "latest" || !@has_presence_edit_priority } @@ -772,12 +776,16 @@ defmodule LightningWeb.WorkflowLive.Edit do defp processing(_run), do: false - defp deletion_tooltip_message( + defp job_deletion_tooltip_message( + workflow_deleted, can_edit_job, has_child_edges, is_first_job ) do cond do + workflow_deleted -> + "You cannot modify a deleted workflow" + !can_edit_job -> "You are not authorized to delete this step." @@ -1403,20 +1411,9 @@ defmodule LightningWeb.WorkflowLive.Edit do end def handle_event("save", params, socket) do - %{ - project: project, - workflow_params: initial_params, - can_edit_workflow: can_edit_workflow, - snapshot_version_tag: tag, - has_presence_edit_priority: has_presence_edit_priority - } = - socket.assigns + %{project: project, workflow_params: initial_params} = socket.assigns - with true <- can_edit_workflow || :not_authorized, - true <- tag == "latest" || :view_only, - true <- - has_presence_edit_priority || - :presence_low_priority do + with :ok <- check_user_can_save_workflow(socket) do next_params = case params do %{"workflow" => params} -> @@ -1456,6 +1453,14 @@ defmodule LightningWeb.WorkflowLive.Edit do {:error, %{text: message}} -> {:noreply, put_flash(socket, :error, message)} + {:error, :workflow_deleted} -> + {:noreply, + put_flash( + socket, + :error, + "Oops! You cannot modify a deleted workflow" + )} + {:error, %Ecto.Changeset{} = changeset} -> {:noreply, socket @@ -1464,27 +1469,6 @@ defmodule LightningWeb.WorkflowLive.Edit do |> put_flash(:error, "Workflow could not be saved") |> push_patches_applied(initial_params)} end - else - :view_only -> - {:noreply, - socket - |> put_flash( - :error, - "Cannot save in snapshot mode, switch to the latest version." - )} - - :presence_low_priority -> - {:noreply, - socket - |> put_flash( - :error, - "Cannot save in view-only mode" - )} - - :not_authorized -> - {:noreply, - socket - |> put_flash(:error, "You are not authorized to perform this action.")} end end @@ -1558,6 +1542,10 @@ defmodule LightningWeb.WorkflowLive.Edit do {:error, %{text: message}} -> {:noreply, put_flash(socket, :error, message)} + {:error, :workflow_deleted} -> + {:noreply, + put_flash(socket, :error, "Cannot rerun a deleted a workflow")} + {:error, _changeset} -> {:noreply, socket @@ -1585,9 +1573,6 @@ defmodule LightningWeb.WorkflowLive.Edit do selected_job: selected_job, current_user: current_user, workflow_params: workflow_params, - can_edit_workflow: can_edit_workflow, - can_run_workflow: can_run_workflow, - snapshot_version_tag: tag, has_presence_edit_priority: has_presence_edit_priority, workflow: workflow, manual_run_form: form @@ -1610,54 +1595,52 @@ defmodule LightningWeb.WorkflowLive.Edit do get_workflow_by_id(workflow.id) end - with true <- (can_run_workflow && can_edit_workflow) || :not_authorized, - true <- tag == "latest" || :view_only, - {:ok, %{workorder: workorder, workflow: workflow}} <- - Helpers.run_workflow( + with :ok <- check_user_can_manual_run_workflow(socket) do + case Helpers.run_workflow( workflow_or_changeset, params, project: project, selected_job: selected_job, created_by: current_user ) do - %{runs: [run]} = workorder + {:ok, %{workorder: workorder, workflow: workflow}} -> + %{runs: [run]} = workorder - Runs.subscribe(run) + Runs.subscribe(run) - snapshot = snapshot_by_version(workflow.id, workflow.lock_version) + snapshot = snapshot_by_version(workflow.id, workflow.lock_version) - {:noreply, - socket - |> assign_workflow(workflow, snapshot) - |> follow_run(run) - |> push_event("push-hash", %{"hash" => "log"})} - else - {:error, %Ecto.Changeset{data: %WorkOrders.Manual{}} = changeset} -> - {:noreply, - socket - |> assign_manual_run_form(changeset)} + {:noreply, + socket + |> assign_workflow(workflow, snapshot) + |> follow_run(run) + |> push_event("push-hash", %{"hash" => "log"})} - {:error, %Ecto.Changeset{data: %Workflow{}} = changeset} -> - { - :noreply, - socket - |> assign_changeset(changeset) - |> mark_validated() - |> put_flash(:error, "Workflow could not be saved") - } + {:error, %Ecto.Changeset{data: %WorkOrders.Manual{}} = changeset} -> + {:noreply, + socket + |> assign_manual_run_form(changeset)} - :not_authorized -> - {:noreply, - socket - |> put_flash(:error, "You are not authorized to perform this action.")} + {:error, %Ecto.Changeset{data: %Workflow{}} = changeset} -> + { + :noreply, + socket + |> assign_changeset(changeset) + |> mark_validated() + |> put_flash(:error, "Workflow could not be saved") + } - :view_only -> - {:noreply, - socket - |> put_flash(:error, "Cannot run in snapshot mode, switch to latest.")} + {:error, %{text: message}} -> + {:noreply, put_flash(socket, :error, message)} - {:error, %{text: message}} -> - {:noreply, put_flash(socket, :error, message)} + {:error, :workflow_deleted} -> + {:noreply, + put_flash( + socket, + :error, + "Oops! You cannot modify a deleted workflow" + )} + end end end @@ -1755,14 +1738,75 @@ defmodule LightningWeb.WorkflowLive.Edit do {:noreply, socket} end + defp check_user_can_manual_run_workflow(socket) do + case socket.assigns do + %{ + can_edit_workflow: true, + can_run_workflow: true, + snapshot_version_tag: "latest" + } -> + :ok + + %{can_edit_workflow: false} -> + {:noreply, + socket + |> put_flash(:error, "You are not authorized to perform this action.")} + + %{can_run_workflow: false} -> + {:noreply, + socket + |> put_flash(:error, "You are not authorized to perform this action.")} + + _snapshot_not_latest -> + {:noreply, + socket + |> put_flash(:error, "Cannot run in snapshot mode, switch to latest.")} + end + end + + defp check_user_can_save_workflow(socket) do + case socket.assigns do + %{ + can_edit_workflow: true, + has_presence_edit_priority: true, + snapshot_version_tag: "latest" + } -> + :ok + + %{can_edit_workflow: false} -> + {:noreply, + socket + |> put_flash(:error, "You are not authorized to perform this action.")} + + %{has_presence_edit_priority: false} -> + {:noreply, + socket + |> put_flash( + :error, + "Cannot save in view-only mode" + )} + + _snapshot_not_latest -> + {:noreply, + socket + |> put_flash( + :error, + "Cannot save in snapshot mode, switch to the latest version." + )} + end + end + defp maybe_disable_canvas(socket) do %{ has_presence_edit_priority: has_edit_priority, snapshot_version_tag: version, - can_edit_workflow: can_edit_workflow + can_edit_workflow: can_edit_workflow, + workflow: workflow } = socket.assigns - disabled = !(has_edit_priority && version == "latest" && can_edit_workflow) + disabled = + !(is_nil(workflow.deleted_at) && has_edit_priority && version == "latest" && + can_edit_workflow) push_event(socket, "set-disabled", %{disabled: disabled}) end @@ -1905,6 +1949,9 @@ defmodule LightningWeb.WorkflowLive.Edit do %{manual_run_form: nil} -> true + %{workflow: %{deleted_at: deleted_at}} when is_struct(deleted_at) -> + true + %{ manual_run_form: manual_run_form, changeset: changeset, @@ -2297,39 +2344,62 @@ defmodule LightningWeb.WorkflowLive.Edit do """ end + attr :id, :string, required: true attr :can_edit_workflow, :boolean, required: true attr :changeset, Ecto.Changeset, required: true attr :snapshot_version_tag, :string, required: true attr :has_presence_priority, :boolean, required: true defp save_workflow_button(assigns) do - %{ - can_edit_workflow: can_edit_workflow, - changeset: changeset, - snapshot_version_tag: snapshot_version_tag, - has_presence_priority: has_presence_priority - } = assigns + {disabled, tooltip} = + case assigns do + %{ + changeset: %{valid?: true, data: %{deleted_at: nil}}, + can_edit_workflow: true, + snapshot_version_tag: "latest", + has_presence_priority: true + } -> + {false, nil} + + %{changeset: %{data: %{deleted_at: deleted_at}}} + when is_struct(deleted_at) -> + {true, "Workflow has been deleted"} + + %{can_edit_workflow: false} -> + {true, "You do not have permission to edit this workflow"} + + %{changeset: %{valid?: false}} -> + {true, "You have some unresolved errors in your workflow"} + + %{snapshot_version_tag: tag} when tag != "latest" -> + {true, "You cannot edit an old snapshot of a workflow"} + + _other -> + {true, nil} + end assigns = assigns |> assign( - disabled: - !can_edit_workflow or !changeset.valid? or - snapshot_version_tag != "latest" || !has_presence_priority + disabled: disabled, + tooltip: tooltip ) ~H""" -