From f2693b51eaa13c389d1954bc563f2c19be5957b1 Mon Sep 17 00:00:00 2001 From: Elias Date: Fri, 22 Sep 2023 10:41:15 +0000 Subject: [PATCH] Prevent Deletion of First Job of a Workflow (#1139) * Disable delete button when first job of a workflow * Disable PlusButton for trigger nodes * Add tooltip to delete button and disable it for first job of a workflow * Add IDs for CloseViaEscape hook * Block deletion when first job of workflow * Liveview tests for deleting the first job of a workflow * Update CHANGELOG.md --- CHANGELOG.md | 5 +- assets/js/workflow-diagram/nodes/Trigger.tsx | 6 ++- lib/lightning_web/components/new_inputs.ex | 2 +- .../live/workflow_live/components.ex | 1 + lib/lightning_web/live/workflow_live/edit.ex | 46 +++++++++++++++---- .../live/workflow_live/job_view.ex | 6 ++- .../live/workflow_live/edit_test.exs | 28 +++++++++++ 7 files changed, 79 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0416d2ed28..526726ff6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,11 +12,14 @@ and this project adheres to ### Changed +- Prevent deletion of first job of a workflow + [#1097](https://github.com/OpenFn/Lightning/issues/1097) + ### Fixed - Creating a new user without a password fails and there is no user feedback [#731](https://github.com/OpenFn/Lightning/issues/731) - + ## [v0.9.2] - 2023-09-20 ### Added diff --git a/assets/js/workflow-diagram/nodes/Trigger.tsx b/assets/js/workflow-diagram/nodes/Trigger.tsx index c8e786ec1f..ab356cb4c4 100644 --- a/assets/js/workflow-diagram/nodes/Trigger.tsx +++ b/assets/js/workflow-diagram/nodes/Trigger.tsx @@ -18,7 +18,8 @@ const TriggerNode = ({ sourcePosition = Position.Bottom, ...props }): JSX.Element => { - const toolbar = () => props.data?.allowPlaceholder && ; + // Do not remove yet, we might need this snippet of code when implementing issue #1121 + // const toolbar = () => props.data?.allowPlaceholder && ; const { label, sublabel, tooltip, icon } = getTriggerMeta( props.data as Lightning.TriggerNode ); @@ -32,7 +33,8 @@ const TriggerNode = ({ icon={icon} sourcePosition={sourcePosition} interactive={props.data.trigger.type === 'webhook'} - toolbar={toolbar} + // TODO: put back the toolbar when implementing issue #1121 + toolbar={false} /> ); }; diff --git a/lib/lightning_web/components/new_inputs.ex b/lib/lightning_web/components/new_inputs.ex index 745d2c580a..6d803b7239 100644 --- a/lib/lightning_web/components/new_inputs.ex +++ b/lib/lightning_web/components/new_inputs.ex @@ -15,7 +15,7 @@ defmodule LightningWeb.Components.NewInputs do <.button phx-click="go" class="ml-2">Send! """ attr :id, :string, default: "no-id" - attr :type, :string, default: nil + attr :type, :string, default: "button", values: ["button", "submit"] attr :class, :string, default: nil attr :rest, :global, include: ~w(disabled form name value) attr :tooltip, :any, default: nil diff --git a/lib/lightning_web/live/workflow_live/components.ex b/lib/lightning_web/live/workflow_live/components.ex index e8115d7297..dd0429e05b 100644 --- a/lib/lightning_web/live/workflow_live/components.ex +++ b/lib/lightning_web/live/workflow_live/components.ex @@ -168,6 +168,7 @@ defmodule LightningWeb.WorkflowLive.Components do
<.link + id="close-panel" phx-hook="ClosePanelViaEscape" patch={@cancel_url} class="justify-center hover:text-gray-500" diff --git a/lib/lightning_web/live/workflow_live/edit.ex b/lib/lightning_web/live/workflow_live/edit.ex index 81784ed7b8..5cdaa49bbc 100644 --- a/lib/lightning_web/live/workflow_live/edit.ex +++ b/lib/lightning_web/live/workflow_live/edit.ex @@ -157,7 +157,7 @@ defmodule LightningWeb.WorkflowLive.Edit do phx-change="validate" > <.single_inputs_for - :let={{jf, has_child_edges}} + :let={{jf, has_child_edges, is_first_job}} :if={@selected_job} form={@workflow_form} field={:jobs} @@ -192,14 +192,16 @@ defmodule LightningWeb.WorkflowLive.Edit do
@@ -311,17 +313,24 @@ defmodule LightningWeb.WorkflowLive.Edit do form = assigns[:form] has_child_edges = form.source |> has_child_edges?(assigns[:id]) + is_first_job = form.source |> is_first_job?(assigns[:id]) forms = form |> Phoenix.HTML.Form.inputs_for(assigns[:field]) |> Enum.filter(&(Ecto.Changeset.get_field(&1.source, :id) == assigns[:id])) - assigns = assigns |> assign(forms: forms, has_child_edges: has_child_edges) + assigns = + assigns + |> assign( + forms: forms, + has_child_edges: has_child_edges, + is_first_job: is_first_job + ) ~H""" <%= for f <- @forms do %> - <%= render_slot(@inner_block, {f, @has_child_edges}) %> + <%= render_slot(@inner_block, {f, @has_child_edges, @is_first_job}) %> <% end %> """ end @@ -456,7 +465,8 @@ defmodule LightningWeb.WorkflowLive.Edit do } = socket.assigns with true <- can_edit_job || :not_authorized, - true <- !has_child_edges?(changeset, id) || :has_child_edges do + true <- !has_child_edges?(changeset, id) || :has_child_edges, + true <- !is_first_job?(changeset, id) || :is_first_job do edges_to_delete = Ecto.Changeset.get_assoc(changeset, :edges, :struct) |> Enum.filter(&(&1.target_job_id == id)) @@ -484,6 +494,11 @@ defmodule LightningWeb.WorkflowLive.Edit do {:noreply, socket |> put_flash(:error, "Delete all descendant jobs first.")} + + :is_first_job -> + {:noreply, + socket + |> put_flash(:error, "You can't delete the first job of a workflow.")} end end @@ -699,11 +714,22 @@ defmodule LightningWeb.WorkflowLive.Edit do defp has_child_edges?(workflow_changeset, job_id) do workflow_changeset - |> Ecto.Changeset.get_assoc(:edges, :struct) - |> Enum.filter(&(&1.source_job_id == job_id)) + |> get_filtered_edges(&(&1.source_job_id == job_id)) |> Enum.any?() end + defp is_first_job?(workflow_changeset, job_id) do + workflow_changeset + |> get_filtered_edges(&(&1.source_trigger_id && &1.target_job_id == job_id)) + |> Enum.any?() + end + + defp get_filtered_edges(workflow_changeset, filter_func) do + workflow_changeset + |> Ecto.Changeset.get_assoc(:edges, :struct) + |> Enum.filter(filter_func) + end + defp handle_new_params(socket, params) do %{workflow_params: initial_params, can_edit_job: can_edit_job} = socket.assigns diff --git a/lib/lightning_web/live/workflow_live/job_view.ex b/lib/lightning_web/live/workflow_live/job_view.ex index ac5fa3b77f..07208e821e 100644 --- a/lib/lightning_web/live/workflow_live/job_view.ex +++ b/lib/lightning_web/live/workflow_live/job_view.ex @@ -80,7 +80,11 @@ defmodule LightningWeb.WorkflowLive.JobView do
- <.link patch={@close_url} phx-hook="ClosePanelViaEscape"> + <.link + id={"close-job-edit-view-#{@job.id}"} + patch={@close_url} + phx-hook="ClosePanelViaEscape" + >
diff --git a/test/lightning_web/live/workflow_live/edit_test.exs b/test/lightning_web/live/workflow_live/edit_test.exs index 72b0d6231c..f017289030 100644 --- a/test/lightning_web/live/workflow_live/edit_test.exs +++ b/test/lightning_web/live/workflow_live/edit_test.exs @@ -4,6 +4,7 @@ defmodule LightningWeb.WorkflowLive.EditTest do import Lightning.WorkflowLive.Helpers import Lightning.WorkflowsFixtures import Lightning.JobsFixtures + import Lightning.Factories alias LightningWeb.CredentialLiveHelpers @@ -287,6 +288,33 @@ defmodule LightningWeb.WorkflowLive.EditTest do }) end + @tag role: :editor + test "can't the first job of a workflow", %{conn: conn, project: project} do + trigger = build(:trigger, type: :webhook) + + job = + build(:job, + body: ~s[fn(state => { return {...state, extra: "data"} })], + name: "First Job" + ) + + workflow = + build(:workflow, project: project) + |> with_job(job) + |> with_trigger(trigger) + |> with_edge({trigger, job}) + |> insert() + + {:ok, view, _html} = live(conn, ~p"/projects/#{project}/w/#{workflow}") + + view |> select_node(job) + + assert view |> delete_job_button_is_disabled?(job) + + assert view |> force_event(:delete_node, job) =~ + "You can't delete the first job of a workflow." + end + @tag role: :viewer test "viewers can't edit existing jobs", %{ conn: conn,