2703 final cleanup (#2874)

* Remove last usage in test * Remove get_or_create_latest_for/1 * Update CHANGELOG --------- Co-authored-by: Stuart Corbishley <[email protected]>
OpenFn · Jan 29, 2025 · 0931854 · 0931854
1 parent b517532
commit 0931854
Show file tree

Hide file tree

Showing 4 changed files with 5 additions and 154 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -31,6 +31,8 @@ and this project adheres to
 - Indexes to foreign keys on `workflow_edges` and `steps` tables to try and
   alleviate slow loading of the job editor.
   [#2617](https://github.com/OpenFn/lightning/issues/2617)
+- Remove `Snapshot.get_or_create_latest_for`.
+  [#2703](https://github.com/OpenFn/lightning/issues/2703)
 - Add temporary events to allow Lightning to log metrics reported by editors.
   [#2617](https://github.com/OpenFn/lightning/issues/2617)
 - Audit when workflow deletion is requested.

diff --git a/lib/lightning/workflows/snapshot.ex b/lib/lightning/workflows/snapshot.ex
@@ -12,7 +12,6 @@ defmodule Lightning.Workflows.Snapshot do
 
   alias Lightning.Projects.ProjectCredential
   alias Lightning.Repo
-  alias Lightning.Workflows.Audit
   alias Lightning.Workflows.WebhookAuthMethod
   alias Lightning.Workflows.Workflow
 
@@ -169,9 +168,7 @@ defmodule Lightning.Workflows.Snapshot do
   Get the latest snapshot for a workflow, based on the lock_version.
 
   It returns the latest snapshot regardless of the lock_version of the
-  workflow passed in. This is intentional to ensure that
-  `get_or_create_latest_for/1` doesn't attempt to create a new snapshot if the
-  workflow has been updated elsewhere.
+  workflow passed in.
   """
   @spec get_current_for(Workflow.t()) :: t() | nil
   def get_current_for(%Workflow{} = workflow) do
@@ -197,70 +194,6 @@ defmodule Lightning.Workflows.Snapshot do
     |> Repo.one()
   end
 
-  @doc """
-  Get the latest snapshot for a workflow, or create one if it doesn't exist.
-  """
-  @spec get_or_create_latest_for(Workflow.t(), struct()) ::
-          {:ok, t()} | {:error, Ecto.Changeset.t()}
-  def get_or_create_latest_for(workflow, actor) do
-    Multi.new()
-    |> get_or_create_latest_for(workflow, actor)
-    |> Repo.transaction()
-    |> case do
-      {:ok, %{snapshot: snapshot}} -> {:ok, snapshot}
-      {:error, _name, error, _multi} -> {:error, error}
-    end
-  end
-
-  @spec get_or_create_latest_for(
-          Multi.t(),
-          binary() | :snapshot,
-          Workflow.t(),
-          struct()
-        ) ::
-          Multi.t()
-  def get_or_create_latest_for(multi, name \\ :snapshot, workflow, actor) do
-    unique_op = "_existing#{System.unique_integer()}"
-
-    multi
-    |> Multi.one(unique_op, get_current_query(workflow))
-    |> Multi.merge(fn %{^unique_op => snapshot} ->
-      return_or_create(name, snapshot, workflow, actor)
-    end)
-  end
-
-  defp return_or_create(name, snapshot, workflow, actor) do
-    if snapshot do
-      Multi.new() |> Multi.put(name, snapshot)
-    else
-      unique_op = "_workflow#{System.unique_integer()}"
-
-      audit_snapshot =
-        fn %{^name => %{id: snapshot_id}} ->
-          Audit.snapshot_created(workflow.id, snapshot_id, actor)
-        end
-
-      Multi.new()
-      |> Multi.one(
-        unique_op,
-        from(w in Workflow,
-          where: w.id == ^workflow.id,
-          preload: [:jobs, :triggers, :edges],
-          lock: "FOR UPDATE"
-        )
-      )
-      |> Multi.merge(fn %{^unique_op => workflow} ->
-        if workflow do
-          Multi.new()
-          |> Multi.insert(name, build(workflow))
-          |> Multi.insert(String.to_atom("audit_of_#{name}"), audit_snapshot)
-        else
-          Multi.new() |> Multi.error(:workflow, :no_workflow)
-        end
-      end)
-    end
-  end
-
   @spec include_latest_snapshot(Multi.t(), binary() | :snapshot, Workflow.t()) ::
           Multi.t()
   def include_latest_snapshot(multi, name \\ :snapshot, workflow) do

diff --git a/test/lightning/workflows/snapshots_test.exs b/test/lightning/workflows/snapshots_test.exs
@@ -1,7 +1,6 @@
 defmodule Lightning.Workflows.SnapshotsTest do
   use Lightning.DataCase, async: true
 
-  alias Lightning.Auditing.Audit
   alias Lightning.Workflows
 
   import Lightning.Factories
@@ -118,85 +117,6 @@ defmodule Lightning.Workflows.SnapshotsTest do
     end
   end
 
-  describe "get_or_create_latest_for" do
-    setup do
-      %{actor: insert(:project_repo_connection)}
-    end
-
-    test "without a workflow", %{actor: actor} do
-      workflow = build(:simple_workflow, id: Ecto.UUID.generate())
-
-      {:error, :no_workflow} =
-        Workflows.Snapshot.get_or_create_latest_for(workflow, actor)
-    end
-
-    test "without an existing snapshot", %{actor: actor} do
-      workflow = insert(:simple_workflow)
-
-      assert {:ok, snapshot} =
-               Workflows.Snapshot.get_or_create_latest_for(workflow, actor)
-
-      assert snapshot == Workflows.Snapshot.get_current_for(workflow)
-    end
-
-    test "creates an audit entry if a snapshot was created", %{
-      actor: %{id: actor_id} = actor
-    } do
-      %{id: workflow_id} = workflow = insert(:simple_workflow)
-
-      assert {:ok, %{id: snapshot_id}} =
-               Workflows.Snapshot.get_or_create_latest_for(workflow, actor)
-
-      audit = Repo.one!(Audit)
-
-      assert %{
-               event: "snapshot_created",
-               item_id: ^workflow_id,
-               actor_id: ^actor_id,
-               changes: %{
-                 after: %{"snapshot_id" => ^snapshot_id}
-               }
-             } = audit
-    end
-
-    test "called with multi - creates an audit entry if snapshot was created", %{
-      actor: %{id: actor_id} = actor
-    } do
-      %{id: workflow_id} = workflow = insert(:simple_workflow)
-
-      {:ok, %{snapshot: %{id: snapshot_id}}} =
-        Workflows.Snapshot.get_or_create_latest_for(
-          Ecto.Multi.new(),
-          workflow,
-          actor
-        )
-        |> Repo.transaction()
-
-      audit = Repo.one!(Audit)
-
-      assert %{
-               event: "snapshot_created",
-               item_id: ^workflow_id,
-               actor_id: ^actor_id,
-               changes: %{
-                 after: %{"snapshot_id" => ^snapshot_id}
-               }
-             } = audit
-    end
-
-    test "with an existing snapshot", %{actor: actor} do
-      workflow = insert(:simple_workflow)
-
-      {:ok, existing} =
-        Workflows.Snapshot.get_or_create_latest_for(workflow, actor)
-
-      {:ok, latest} =
-        Workflows.Snapshot.get_or_create_latest_for(workflow, actor)
-
-      assert existing == latest
-    end
-  end
-
   describe "include_latest_snapshot/3" do
     setup do
       workflow = insert(:simple_workflow) |> with_snapshot()

diff --git a/test/lightning_web/live/workflow_live/edit_test.exs b/test/lightning_web/live/workflow_live/edit_test.exs
@@ -1791,7 +1791,6 @@ defmodule LightningWeb.WorkflowLive.EditTest do
 
     test "workflow state toggle tooltip messages vary by trigger type", %{
       conn: conn,
-      user: user,
       project: project
     } do
       cron_trigger = build(:trigger, type: :cron, enabled: false)
@@ -1814,12 +1813,9 @@ defmodule LightningWeb.WorkflowLive.EditTest do
         |> with_edge({webhook_trigger, job_2})
         |> insert()
 
-      Lightning.Workflows.Snapshot.get_or_create_latest_for(cron_workflow, user)
+      Lightning.Workflows.Snapshot.create(cron_workflow)
 
-      Lightning.Workflows.Snapshot.get_or_create_latest_for(
-        webhook_workflow,
-        user
-      )
+      Lightning.Workflows.Snapshot.create(webhook_workflow)
 
       {:ok, view, _html} =
         live(