fix: saving workflow with same name as deleted workflow

CHANGELOG.md

@@ -21,6 +21,8 @@ and this project adheres to
 
 ### Fixed
 
+- Fix saving workflow with same name as deleted workflow
+  [#4165](https://github.com/OpenFn/lightning/pull/4165)
 - Fix validation error states not changing after undo (Ctrl+Z) on Workflow
   Settings [#4182](https://github.com/OpenFn/lightning/issues/4182))
 

assets/js/collaborative-editor/types/workflow.ts

@@ -17,7 +17,7 @@ import type { Trigger as TriggerType } from './trigger';
 /**
  * Zod schema for workflow validation
  *
- * Mirrors backend validation from lib/lightning/workflows/workflow.ex:81-89
+ * Mirrors backend validation from lib/lightning/workflows/workflow.ex:81-103
  */
 export const WorkflowSchema = z.object({
   id: z.string().uuid(),

lib/lightning/workflows.ex

@@ -462,12 +462,16 @@ defmodule Lightning.Workflows do
         where: t.workflow_id == ^workflow.id
       )
 
+    new_name = resolve_name_for_pending_deletion(workflow)
+
     Multi.new()
     |> Multi.update(
       :workflow,
-      Workflow.request_deletion_changeset(workflow, %{
+      workflow
+      |> Workflow.request_deletion_changeset(%{
         "deleted_at" => DateTime.utc_now()
       })
+      |> Ecto.Changeset.put_change(:name, new_name)
     )
     |> Multi.insert(:audit, Audit.marked_for_deletion(workflow.id, actor))
     |> Multi.update_all(
@@ -486,6 +490,33 @@ defmodule Lightning.Workflows do
     end)
   end
 
+  defp resolve_name_for_pending_deletion(%Workflow{
+         name: name,
+         project_id: project_id
+       }) do
+    base_name = "#{name}_del"
+
+    existing_names =
+      from(w in Workflow,
+        where:
+          w.project_id == ^project_id and
+            (w.name == ^base_name or like(w.name, ^"#{base_name}%")),
+        select: w.name
+      )
+      |> Repo.all()
+      |> MapSet.new()
+
+    find_available_name(base_name, existing_names)
+  end
+
+  defp find_available_name(base_name, existing_names, n \\ 0) do
+    candidate = if n == 0, do: base_name, else: "#{base_name}#{n}"
+
+    if MapSet.member?(existing_names, candidate),
+      do: find_available_name(base_name, existing_names, n + 1),
+      else: candidate
+  end
+
   defp notify_of_affected_kafka_triggers(%{triggers: triggers}) do
     triggers
     |> Enum.filter(&(&1.type == :kafka))

lib/lightning/workflows/workflow.ex

@@ -84,7 +84,8 @@ defmodule Lightning.Workflows.Workflow do
     |> validate_number(:concurrency, greater_than_or_equal_to: 1)
     |> validate_required([:name])
     |> unique_constraint([:name, :project_id],
-      message: "a workflow with this name already exists in this project."
+      message:
+        "A workflow with this name already exists (possibly pending deletion) in this project."
     )
   end
 

test/lightning/workflows/workflow_test.exs

@@ -79,7 +79,7 @@ defmodule Lightning.Workflows.WorkflowTest do
         |> Workflow.changeset(%{name: "w1", project_id: p1.id})
         |> Repo.insert()
 
-      assert "a workflow with this name already exists in this project." in errors_on(
+      assert "A workflow with this name already exists (possibly pending deletion) in this project." in errors_on(
                cs
              ).name
 

test/lightning/workflows_test.exs

@@ -52,7 +52,7 @@ defmodule Lightning.WorkflowsTest do
 
       assert %{
                name: [
-                 "a workflow with this name already exists in this project."
+                 "A workflow with this name already exists (possibly pending deletion) in this project."
                ]
              } = errors_on(changeset)
 
@@ -833,6 +833,88 @@ defmodule Lightning.WorkflowsTest do
       assert_received %KafkaTriggerUpdated{trigger_id: ^kafka_trigger_2_id}
       assert_received %KafkaTriggerUpdated{trigger_id: ^kafka_trigger_1_id}
     end
+
+    test "mark_for_deletion/3 renames workflow with _del suffix" do
+      # Use a separate project to avoid pollution from setup
+      project = insert(:project)
+      user = insert(:user)
+      w1 = insert(:workflow, project: project, name: "Test Workflow")
+
+      assert {:ok, %{workflow: workflow}} = Workflows.mark_for_deletion(w1, user)
+      assert workflow.name == "Test Workflow_del"
+
+      # Test incrementing when deleting another workflow with the same name
+      w2 = insert(:workflow, project: project, name: "Test Workflow")
+
+      assert {:ok, %{workflow: workflow2}} =
+               Workflows.mark_for_deletion(w2, user)
+
+      assert workflow2.name == "Test Workflow_del1"
+
+      # Test incrementing again
+      w3 = insert(:workflow, project: project, name: "Test Workflow")
+
+      assert {:ok, %{workflow: workflow3}} =
+               Workflows.mark_for_deletion(w3, user)
+
+      assert workflow3.name == "Test Workflow_del2"
+    end
+
+    test "mark_for_deletion/3 does not conflict with active workflows ending in _del" do
+      # An active workflow named "water_gap_analysis_del" (where _del is Delaware)
+      # should not interfere with deleting "water_gap_analysis"
+      project = insert(:project)
+      user = insert(:user)
+
+      # Active workflow with name ending in _del (not deleted)
+      _delaware_workflow =
+        insert(:workflow, project: project, name: "water_gap_analysis_del")
+
+      # Workflow to be deleted
+      w1 = insert(:workflow, project: project, name: "water_gap_analysis")
+
+      # Should still get _del suffix since the existing _del workflow is not deleted
+      assert {:ok, %{workflow: deleted}} = Workflows.mark_for_deletion(w1, user)
+      assert deleted.name == "water_gap_analysis_del1"
+
+      assert {:ok, %{workflow: deleted}} = Workflows.mark_for_deletion(w1, user)
+      assert deleted.name == "water_gap_analysis_del2"
+    end
+
+    test "allows reusing workflow name after marking for deletion, then validates error when using deleted workflow name" do
+      project = insert(:project)
+      user = insert(:user)
+      w1 = insert(:workflow, project: project, name: "My Workflow")
+
+      # Mark workflow for deletion
+      assert {:ok, %{workflow: deleted_workflow}} =
+               Workflows.mark_for_deletion(w1, user)
+
+      assert deleted_workflow.name == "My Workflow_del"
+
+      # Should be able to create a new workflow with the original name
+      assert {:ok, new_workflow} =
+               Workflows.save_workflow(
+                 %{name: "My Workflow", project_id: project.id},
+                 user
+               )
+
+      assert new_workflow.name == "My Workflow"
+      assert new_workflow.deleted_at == nil
+
+      # Should NOT be able to create another workflow with the deleted workflow's name
+      assert {:error, changeset} =
+               Workflows.save_workflow(
+                 %{name: "My Workflow_del", project_id: project.id},
+                 user
+               )
+
+      assert errors_on(changeset) == %{
+               name: [
+                 "A workflow with this name already exists (possibly pending deletion) in this project."
+               ]
+             }
+    end
   end
 
   describe "get_workflows_for/2" do

test/lightning_web/live/workflow_live/edit_test.exs

@@ -1229,7 +1229,9 @@ defmodule LightningWeb.WorkflowLive.EditTest do
         })
         |> render_submit()
 
-      assert html =~ "a workflow with this name already exists in this project."
+      assert html =~
+               "A workflow with this name already exists (possibly pending deletion) in this project."
+
       assert html =~ "Workflow could not be saved"
 
       assert view