From 981d9ed0ab51ee8e0ede0ebffc7550131f54732b Mon Sep 17 00:00:00 2001 From: "Elias W. BA" Date: Mon, 6 Jul 2026 10:07:34 +0000 Subject: [PATCH] fix: keep parent workflows absent from a sandbox on merge A workflow added to a project after a sandbox was branched was flagged for deletion and pre-selected on the sandbox merge screen, so a routine merge could silently delete it. Workflows present in the project but not the sandbox now default to unchecked (deletion is opt-in), the merge no longer takes the merge-everything-and-delete-unmatched path, and the merge screen labels those rows honestly instead of "Deleted in sandbox". --- CHANGELOG.md | 5 + .../live/sandbox_live/components.ex | 31 ++++- lib/lightning_web/live/sandbox_live/index.ex | 75 ++++++----- .../live/sandbox_live/index_test.exs | 127 ++++++++++++++++-- 4 files changed, 183 insertions(+), 55 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 69c839d736..df57698372 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,11 @@ and this project adheres to ### Fixed +- Sandbox merge no longer deletes a workflow that was added to the project after + the sandbox was branched. Workflows present in the project but not the sandbox + now default to kept (removal is opt-in) and are labelled clearly in the merge + screen. [#4919](https://github.com/OpenFn/lightning/issues/4919) + ## [2.16.8] - 2026-07-01 ## [2.16.8-pre] - 2026-06-18 diff --git a/lib/lightning_web/live/sandbox_live/components.ex b/lib/lightning_web/live/sandbox_live/components.ex index 69412da638..b851eff111 100644 --- a/lib/lightning_web/live/sandbox_live/components.ex +++ b/lib/lightning_web/live/sandbox_live/components.ex @@ -370,32 +370,49 @@ defmodule LightningWeb.SandboxLive.Components do Changed <.icon name="hero-exclamation-triangle-mini" class="h-3.5 w-3.5" /> Diverged New - Deleted in sandbox + Added to the project after this sandbox was created + + + In the project but not in this sandbox diff --git a/lib/lightning_web/live/sandbox_live/index.ex b/lib/lightning_web/live/sandbox_live/index.ex index 242d3f88a1..1ec704eaeb 100644 --- a/lib/lightning_web/live/sandbox_live/index.ex +++ b/lib/lightning_web/live/sandbox_live/index.ex @@ -14,7 +14,15 @@ defmodule LightningWeb.SandboxLive.Index do require Logger defmodule MergeWorkflow do - defstruct [:id, :name, :is_changed, :is_diverged, :is_new, :is_deleted] + defstruct [ + :id, + :name, + :is_changed, + :is_diverged, + :is_new, + :is_deleted, + added_after_fork: false + ] end on_mount {LightningWeb.Hooks, :project_scope} @@ -890,6 +898,7 @@ defmodule LightningWeb.SandboxLive.Index do } end) + # Target-only workflows default to unchecked so a merge never silently deletes them. deleted_entries = target_workflows |> Enum.reject(fn wf -> MapSet.member?(source_workflow_names, wf.name) end) @@ -897,10 +906,11 @@ defmodule LightningWeb.SandboxLive.Index do %MergeWorkflow{ id: wf.id, name: wf.name, - is_changed: true, + is_changed: false, is_diverged: false, is_new: false, - is_deleted: true + is_deleted: true, + added_after_fork: workflow_added_after_fork?(wf, source) } end) @@ -908,31 +918,34 @@ defmodule LightningWeb.SandboxLive.Index do |> Enum.sort_by(fn wf -> wf.name end) end - defp resolve_selected_workflow_ids(assigns) do - all_ids = MapSet.new(assigns.merge_source_workflows, fn wf -> wf.id end) + defp workflow_added_after_fork?(%{inserted_at: %DateTime{} = wf_inserted}, %{ + inserted_at: %DateTime{} = fork_time + }) do + DateTime.compare(wf_inserted, fork_time) == :gt + end - if MapSet.equal?(assigns.merge_selected_workflow_ids, all_ids) do - {nil, nil} - else - deleted_ids = - assigns.merge_source_workflows - |> Enum.filter(fn wf -> wf.is_deleted end) - |> MapSet.new(fn wf -> wf.id end) + defp workflow_added_after_fork?(_workflow, _source), do: false - selected = assigns.merge_selected_workflow_ids + # Always pass an explicit selection so unchecked target-only workflows are kept, not deleted. + defp resolve_selected_workflow_ids(assigns) do + deleted_ids = + assigns.merge_source_workflows + |> Enum.filter(fn wf -> wf.is_deleted end) + |> MapSet.new(fn wf -> wf.id end) - selected_source_ids = - selected - |> Enum.reject(&MapSet.member?(deleted_ids, &1)) - |> Enum.to_list() + selected = assigns.merge_selected_workflow_ids - selected_deleted_ids = - selected - |> Enum.filter(&MapSet.member?(deleted_ids, &1)) - |> Enum.to_list() + selected_source_ids = + selected + |> Enum.reject(&MapSet.member?(deleted_ids, &1)) + |> Enum.to_list() - {selected_source_ids, selected_deleted_ids} - end + selected_deleted_ids = + selected + |> Enum.filter(&MapSet.member?(deleted_ids, &1)) + |> Enum.to_list() + + {selected_source_ids, selected_deleted_ids} end defp preload_merge_projects(source, nil), @@ -991,17 +1004,11 @@ defmodule LightningWeb.SandboxLive.Index do ) do maybe_commit_to_github(target, "pre-merge commit") - opts = - if selected_workflow_ids do - %{ - selected_workflow_ids: selected_workflow_ids, - deleted_target_workflow_ids: deleted_target_workflow_ids - } - else - %{} - end - - opts = Map.put(opts, :selected_credential_ids, selected_credential_ids) + opts = %{ + selected_workflow_ids: selected_workflow_ids, + deleted_target_workflow_ids: deleted_target_workflow_ids, + selected_credential_ids: selected_credential_ids + } case Sandboxes.merge(source, target, actor, opts) do {:ok, _updated_target} = success -> diff --git a/test/lightning_web/live/sandbox_live/index_test.exs b/test/lightning_web/live/sandbox_live/index_test.exs index c927d80b6b..eefb711154 100644 --- a/test/lightning_web/live/sandbox_live/index_test.exs +++ b/test/lightning_web/live/sandbox_live/index_test.exs @@ -3579,16 +3579,22 @@ defmodule LightningWeb.SandboxLive.IndexTest do assert MapSet.member?(assigns.merge_selected_workflow_ids, changed_data.id) end - test "target-only workflows appear in list with is_deleted flag and badge", + test "target-only workflows appear in list unchecked by default", %{ conn: conn, parent: parent, sandbox: sandbox } do - # Parent has "Alpha" and "Gamma" — sandbox only has "Alpha" - # so "Gamma" was deleted in the sandbox + # Gamma existed before the fork, so it is in the project but not the sandbox. _parent_alpha = insert(:workflow, project: parent, name: "Alpha") - _parent_gamma = insert(:workflow, project: parent, name: "Gamma") + + _parent_gamma = + insert(:workflow, + project: parent, + name: "Gamma", + inserted_at: DateTime.add(sandbox.inserted_at, -3600, :second) + ) + _sandbox_alpha = insert(:workflow, project: sandbox, name: "Alpha") {:ok, view, _} = live(conn, ~p"/projects/#{parent.id}/sandboxes") @@ -3603,16 +3609,107 @@ defmodule LightningWeb.SandboxLive.IndexTest do gamma_data = Enum.find(assigns.merge_source_workflows, &(&1.name == "Gamma")) - assert gamma_data - assert gamma_data.is_deleted - refute gamma_data.is_new - refute gamma_data.is_diverged + assert %{ + is_deleted: true, + is_new: false, + is_diverged: false, + is_changed: false, + added_after_fork: false + } = gamma_data - # The gamma workflow's ID in the list is the target (parent) workflow ID - assert MapSet.member?(assigns.merge_selected_workflow_ids, gamma_data.id) + refute MapSet.member?(assigns.merge_selected_workflow_ids, gamma_data.id) - # Badge shown in HTML - assert html =~ "Deleted in sandbox" + assert html =~ "In the project but not in this sandbox" + end + + test "target-only workflow added after the fork is labelled and kept", + %{conn: conn, parent: parent, sandbox: sandbox} do + _parent_alpha = insert(:workflow, project: parent, name: "Alpha") + + _parent_added = + insert(:workflow, + project: parent, + name: "Added Later", + inserted_at: DateTime.add(sandbox.inserted_at, 3600, :second) + ) + + _sandbox_alpha = insert(:workflow, project: sandbox, name: "Alpha") + + {:ok, view, _} = live(conn, ~p"/projects/#{parent.id}/sandboxes") + + html = + view + |> element("#branch-rewire-sandbox-#{sandbox.id} button") + |> render_click() + + assigns = :sys.get_state(view.pid).socket.assigns + + added_data = + Enum.find(assigns.merge_source_workflows, &(&1.name == "Added Later")) + + assert %{is_deleted: true, is_changed: false, added_after_fork: true} = + added_data + + refute MapSet.member?(assigns.merge_selected_workflow_ids, added_data.id) + + assert html =~ "Added to the project after this sandbox was created" + refute html =~ "In the project but not in this sandbox" + end + + test "explicitly checking a target-only workflow deletes it on merge", + %{conn: conn, parent: parent, sandbox: sandbox} do + parent_alpha = insert(:workflow, project: parent, name: "Alpha") + + parent_gamma = + insert(:workflow, + project: parent, + name: "Gamma", + inserted_at: DateTime.add(sandbox.inserted_at, -3600, :second) + ) + + _sandbox_alpha = insert(:workflow, project: sandbox, name: "Alpha") + + {:ok, view, _} = live(conn, ~p"/projects/#{parent.id}/sandboxes") + + view + |> element("#branch-rewire-sandbox-#{sandbox.id} button") + |> render_click() + + render_click(view, "toggle-workflow", %{"id" => parent_gamma.id}) + + render_click(view, "confirm-merge", %{ + "merge" => %{"target_id" => parent.id} + }) + + assert Lightning.Repo.reload(parent_gamma).deleted_at + refute Lightning.Repo.reload(parent_alpha).deleted_at + end + + test "target-only workflow is kept when left unchecked on merge", + %{conn: conn, parent: parent, sandbox: sandbox} do + parent_alpha = insert(:workflow, project: parent, name: "Alpha") + + parent_added = + insert(:workflow, + project: parent, + name: "Added Later", + inserted_at: DateTime.add(sandbox.inserted_at, 3600, :second) + ) + + _sandbox_alpha = insert(:workflow, project: sandbox, name: "Alpha") + + {:ok, view, _} = live(conn, ~p"/projects/#{parent.id}/sandboxes") + + view + |> element("#branch-rewire-sandbox-#{sandbox.id} button") + |> render_click() + + render_click(view, "confirm-merge", %{ + "merge" => %{"target_id" => parent.id} + }) + + refute Lightning.Repo.reload(parent_added).deleted_at + refute Lightning.Repo.reload(parent_alpha).deleted_at end test "workflow selection UI shows per-row status badges", %{ @@ -4142,7 +4239,8 @@ defmodule LightningWeb.SandboxLive.IndexTest do apiSecretName: api_secret_name(parent), branch: repo_connection.branch, pathToConfig: path_to_config(repo_connection), - commitMessage: "Merged sandbox #{sandbox.name}" + commitMessage: "Merged sandbox #{sandbox.name}", + snapshots: "#{snapshot.id}" } } ) @@ -4209,7 +4307,8 @@ defmodule LightningWeb.SandboxLive.IndexTest do apiSecretName: api_secret_name(parent), branch: repo_connection.branch, pathToConfig: path_to_config(repo_connection), - commitMessage: "Merged sandbox #{sandbox.name}" + commitMessage: "Merged sandbox #{sandbox.name}", + snapshots: "#{snapshot.id}" } } )