Skip to content

Commit

Permalink
Prevent Deletion of First Job of a Workflow (#1139)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
elias-ba authored Sep 22, 2023
1 parent fb2011a commit f2693b5
Show file tree
Hide file tree
Showing 7 changed files with 79 additions and 15 deletions.
5 changes: 4 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions assets/js/workflow-diagram/nodes/Trigger.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ const TriggerNode = ({
sourcePosition = Position.Bottom,
...props
}): JSX.Element => {
const toolbar = () => props.data?.allowPlaceholder && <PlusButton />;
// Do not remove yet, we might need this snippet of code when implementing issue #1121
// const toolbar = () => props.data?.allowPlaceholder && <PlusButton />;
const { label, sublabel, tooltip, icon } = getTriggerMeta(
props.data as Lightning.TriggerNode
);
Expand All @@ -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}
/>
);
};
Expand Down
2 changes: 1 addition & 1 deletion lib/lightning_web/components/new_inputs.ex
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ defmodule LightningWeb.Components.NewInputs do
<.button phx-click="go" class="ml-2">Send!</.button>
"""
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
Expand Down
1 change: 1 addition & 0 deletions lib/lightning_web/live/workflow_live/components.ex
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ defmodule LightningWeb.WorkflowLive.Components do
</div>
<div class="flex-none">
<.link
id="close-panel"
phx-hook="ClosePanelViaEscape"
patch={@cancel_url}
class="justify-center hover:text-gray-500"
Expand Down
46 changes: 36 additions & 10 deletions lib/lightning_web/live/workflow_live/edit.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -192,14 +192,16 @@ defmodule LightningWeb.WorkflowLive.Edit do
</div>
<div class="grow flex justify-end">
<label>
<Common.button
color="red"
<.button
id="delete-job-button"
phx-click="delete_node"
phx-value-id={@selected_job.id}
disabled={!@can_edit_job or has_child_edges}
class="focus:ring-red-500 bg-red-600 hover:bg-red-700 disabled:bg-red-300"
disabled={!@can_edit_job or has_child_edges or is_first_job}
tooltip="You can't delete the first job of a workflow"
>
Delete
</Common.button>
</.button>
</label>
</div>
</div>
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion lib/lightning_web/live/workflow_live/job_view.ex
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,11 @@ defmodule LightningWeb.WorkflowLive.JobView do
</div>
<div class="basis-1/3 flex justify-end">
<div class="flex w-14 items-center justify-center">
<.link patch={@close_url} phx-hook="ClosePanelViaEscape">
<.link
id={"close-job-edit-view-#{@job.id}"}
patch={@close_url}
phx-hook="ClosePanelViaEscape"
>
<Heroicons.x_mark class="w-6 h-6 text-gray-500 hover:text-gray-700 hover:cursor-pointer" />
</.link>
</div>
Expand Down
28 changes: 28 additions & 0 deletions test/lightning_web/live/workflow_live/edit_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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&#39;t delete the first job of a workflow."
end

@tag role: :viewer
test "viewers can't edit existing jobs", %{
conn: conn,
Expand Down

0 comments on commit f2693b5

Please sign in to comment.