Skip to content

Sandbox merge no longer deletes a parent workflow added after the fork #4940

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
elias-ba wants to merge 1 commit into
base: main
Choose a base branch
from
+183 −55
Open

Sandbox merge no longer deletes a parent workflow added after the fork #4940

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
31 changes: 24 additions & 7 deletions lib/lightning_web/live/sandbox_live/components.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,32 +370,49 @@ defmodule LightningWeb.SandboxLive.Components do
</span>
<span
:if={wf.is_changed && !wf.is_new && !wf.is_deleted}
id={"merge-wf-changed-#{wf.id}"}
class="flex items-center gap-1 text-xs font-medium text-green-700"
title="This workflow has been modified in the sandbox"
phx-hook="Tooltip"
aria-label="This workflow has been modified in the sandbox"
>
Changed
</span>
<span
:if={wf.is_diverged}
id={"merge-wf-diverged-#{wf.id}"}
class="flex items-center gap-1 text-xs font-medium text-amber-700"
title="This workflow was modified in the target project - this change will be lost"
phx-hook="Tooltip"
aria-label="This workflow was modified in the target project - this change will be lost"
>
<.icon name="hero-exclamation-triangle-mini" class="h-3.5 w-3.5" />
Diverged
</span>
<span
:if={wf.is_new}
id={"merge-wf-new-#{wf.id}"}
class="flex items-center gap-1 text-xs font-medium text-blue-700"
title="This workflow doesn't exist in the target — it will be created"
phx-hook="Tooltip"
aria-label="This workflow doesn't exist in the target — it will be created"
>
New
</span>
<span
:if={wf.is_deleted}
class="flex items-center gap-1 text-xs font-medium text-red-700"
title="This workflow was deleted in the sandbox — selecting it will delete it from the target"
:if={wf.is_deleted && wf.added_after_fork}
id={"merge-wf-added-#{wf.id}"}
class="flex items-center gap-1 text-xs font-medium text-gray-500"
phx-hook="Tooltip"
aria-label="Added to the project after this sandbox was created. Check to delete it from the project."
>
Deleted in sandbox
Added to the project after this sandbox was created
</span>
<span
:if={wf.is_deleted && !wf.added_after_fork}
id={"merge-wf-existed-#{wf.id}"}
class="flex items-center gap-1 text-xs font-medium text-gray-500"
phx-hook="Tooltip"
aria-label="In the project but not in this sandbox. Check to delete it from the project."
>
In the project but not in this sandbox
</span>
</li>
</ul>
Expand Down
75 changes: 41 additions & 34 deletions lib/lightning_web/live/sandbox_live/index.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -890,49 +898,54 @@ 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)
|> Enum.map(fn wf ->
%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)

(source_entries ++ deleted_entries)
|> 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),
Expand Down Expand Up @@ -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 ->
Expand Down
127 changes: 113 additions & 14 deletions test/lightning_web/live/sandbox_live/index_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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", %{
Expand Down Expand Up @@ -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}"
}
}
)
Expand Down Expand Up @@ -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}"
}
}
)
Expand Down