From b35f0217be6917561f804aff6fdd410900b7d288 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20W=C3=B6ginger?= Date: Tue, 2 Apr 2024 23:20:03 +0200 Subject: [PATCH] fixup, no extra argument for prev and parent node they are already in attributes --- lib/radiator/outline.ex | 34 ++++++++++--- test/radiator/outline_test.exs | 90 +++++++++++++++++++++++++++------- 2 files changed, 99 insertions(+), 25 deletions(-) diff --git a/lib/radiator/outline.ex b/lib/radiator/outline.ex index 4493b062..3988aa64 100644 --- a/lib/radiator/outline.ex +++ b/lib/radiator/outline.ex @@ -137,6 +137,26 @@ defmodule Radiator.Outline do |> Repo.get(id) end + @doc """ + Gets a single node where id can be nil. + + Returns `nil` if the Node does not exist. + + ## Examples + + iex> get_node_if(123) + %Node{} + + iex> get_node_if(456) + nil + + iex> get_node_if(nil) + nil + + """ + def get_node_if(nil), do: nil + def get_node_if(node), do: get_node(node) + @doc """ Gets all nodes of an episode as a tree. Uses a Common Table Expression (CTE) to recursively query the database. @@ -244,10 +264,10 @@ defmodule Radiator.Outline do ## Examples - iex> insert_node(%{content: 'foo'}, %Node{} = parent_node, %Node{} = prev_node) + iex> insert_node(%{content: 'foo'}) {:ok, %Node{}} - iex> insert_node(%{content: value}, %Node{} = parent_node, %Node{parent_id: nil} = prev_node) + iex> insert_node(%{content: value}) {:error, :parent_and_prev_not_consistent} """ @@ -256,10 +276,10 @@ defmodule Radiator.Outline do # if a previous node is given, the new node will be inserted after the previous node # if no parent is given, the new node will be inserted as a root node # if no previous node is given, the new node will be inserted as the first child of the parent node - def insert_node(attrs, parent_node \\ nil, prev_node \\ nil) do + def insert_node(attrs) do Repo.transaction(fn -> - prev_node_id = get_node_id(prev_node) - parent_node_id = get_node_id(parent_node) + prev_node_id = attrs["prev_node"] + parent_node_id = attrs["parent_node"] episode_id = attrs["episode_id"] # find Node which has been previously connected to prev_node node_to_move = @@ -269,7 +289,9 @@ defmodule Radiator.Outline do |> where_parent_node_equals(parent_node_id) |> Repo.one() - with true <- parent_and_prev_consistent?(parent_node, prev_node), + with parent_node <- get_node_if(parent_node_id), + prev_node <- get_node_if(prev_node_id), + true <- parent_and_prev_consistent?(parent_node, prev_node), {:ok, node} <- create_node(attrs), {:ok, _node_to_move} <- move_node_(node_to_move, nil, node.uuid), {:ok, node} <- move_node_(node, parent_node_id, prev_node_id) do diff --git a/test/radiator/outline_test.exs b/test/radiator/outline_test.exs index 8a8509df..992898d9 100644 --- a/test/radiator/outline_test.exs +++ b/test/radiator/outline_test.exs @@ -110,7 +110,7 @@ defmodule Radiator.OutlineTest do end end - describe "insert_node/3" do + describe "insert_node/1" do setup :complex_node_fixture test "creates a new node in the tree", %{ @@ -119,8 +119,14 @@ defmodule Radiator.OutlineTest do } do count_nodes = Outline.count_nodes_by_episode(node_3.episode_id) - node_attrs = %{"content" => "new node", "episode_id" => node_3.episode_id} - Outline.insert_node(node_attrs, node_3, nested_node_1) + node_attrs = %{ + "content" => "new node", + "episode_id" => node_3.episode_id, + "parent_node" => node_3.uuid, + "prev_node" => nested_node_1.uuid + } + + Outline.insert_node(node_attrs) new_count_nodes = Outline.count_nodes_by_episode(node_3.episode_id) assert new_count_nodes == count_nodes + 1 end @@ -129,8 +135,14 @@ defmodule Radiator.OutlineTest do node_3: node_3, nested_node_1: nested_node_1 } do - node_attrs = %{"content" => "new node", "episode_id" => node_3.episode_id} - {:ok, new_node} = Outline.insert_node(node_attrs, node_3, nested_node_1) + node_attrs = %{ + "content" => "new node", + "episode_id" => node_3.episode_id, + "parent_node" => node_3.uuid, + "prev_node" => nested_node_1.uuid + } + + {:ok, new_node} = Outline.insert_node(node_attrs) assert new_node.parent_id == node_3.uuid end @@ -138,8 +150,14 @@ defmodule Radiator.OutlineTest do node_3: node_3, nested_node_1: nested_node_1 } do - node_attrs = %{"content" => "new node", "episode_id" => node_3.episode_id} - {:ok, new_node} = Outline.insert_node(node_attrs, node_3, nested_node_1) + node_attrs = %{ + "content" => "new node", + "episode_id" => node_3.episode_id, + "parent_node" => node_3.uuid, + "prev_node" => nested_node_1.uuid + } + + {:ok, new_node} = Outline.insert_node(node_attrs) assert new_node.prev_id == nested_node_1.uuid end @@ -148,8 +166,14 @@ defmodule Radiator.OutlineTest do nested_node_1: nested_node_1, nested_node_2: nested_node_2 } do - node_attrs = %{"content" => "new node", "episode_id" => node_3.episode_id} - {:ok, new_node} = Outline.insert_node(node_attrs, node_3, nested_node_1) + node_attrs = %{ + "content" => "new node", + "episode_id" => node_3.episode_id, + "parent_node" => node_3.uuid, + "prev_node" => nested_node_1.uuid + } + + {:ok, new_node} = Outline.insert_node(node_attrs) assert Outline.get_node!(nested_node_2.uuid).prev_id == new_node.uuid assert new_node.prev_id == nested_node_1.uuid @@ -161,8 +185,14 @@ defmodule Radiator.OutlineTest do nested_node_1: nested_node_1, nested_node_2: nested_node_2 } do - node_attrs = %{"content" => "new node", "episode_id" => node_3.episode_id} - {:ok, new_node} = Outline.insert_node(node_attrs, node_3, nested_node_2) + node_attrs = %{ + "content" => "new node", + "episode_id" => node_3.episode_id, + "parent_node" => node_3.uuid, + "prev_node" => nested_node_2.uuid + } + + {:ok, new_node} = Outline.insert_node(node_attrs) assert Outline.get_node!(nested_node_2.uuid).prev_id == nested_node_1.uuid assert new_node.prev_id == nested_node_2.uuid @@ -174,8 +204,13 @@ defmodule Radiator.OutlineTest do nested_node_1: nested_node_1, nested_node_2: nested_node_2 } do - node_attrs = %{"content" => "new node", "episode_id" => node_3.episode_id} - {:ok, new_node} = Outline.insert_node(node_attrs, node_3) + node_attrs = %{ + "content" => "new node", + "episode_id" => node_3.episode_id, + "parent_node" => node_3.uuid + } + + {:ok, new_node} = Outline.insert_node(node_attrs) assert new_node.prev_id == nil assert Outline.get_node!(nested_node_1.uuid).prev_id == new_node.uuid @@ -200,10 +235,15 @@ defmodule Radiator.OutlineTest do nested_node_1: nested_node_1 } do # new node cannot be inserted at level 1 and wants the lined in level 2 - node_attrs = %{"content" => "new node", "episode_id" => parent_node.episode_id} + node_attrs = %{ + "content" => "new node", + "episode_id" => parent_node.episode_id, + "parent_node" => parent_node.uuid, + "prev_node" => nested_node_1.uuid + } {:error, "Insert node failed. Parent and prev node are not consistent."} = - Outline.insert_node(node_attrs, parent_node, nested_node_1) + Outline.insert_node(node_attrs) end test "parent node and prev node need to be consistent (2)", %{ @@ -212,10 +252,15 @@ defmodule Radiator.OutlineTest do bad_parent_node = node_fixture(episode_id: parent_node.episode_id, parent_id: nil, prev_id: nil) - node_attrs = %{"content" => "new node", "episode_id" => parent_node.episode_id} + node_attrs = %{ + "content" => "new node", + "episode_id" => parent_node.episode_id, + "parent_node" => parent_node.uuid, + "prev_node" => bad_parent_node.uuid + } {:error, _error_message} = - Outline.insert_node(node_attrs, parent_node, bad_parent_node) + Outline.insert_node(node_attrs) end test "in case of error no node gets inserted", %{ @@ -223,8 +268,15 @@ defmodule Radiator.OutlineTest do nested_node_1: nested_node_1 } do count_nodes = Outline.count_nodes_by_episode(parent_node.episode_id) - node_attrs = %{"content" => "new node", "episode_id" => parent_node.episode_id} - {:error, _error_message} = Outline.insert_node(node_attrs, parent_node, nested_node_1) + + node_attrs = %{ + "content" => "new node", + "episode_id" => parent_node.episode_id, + "parent_node" => parent_node.uuid, + "prev_node" => nested_node_1.uuid + } + + {:error, _error_message} = Outline.insert_node(node_attrs) new_count_nodes = Outline.count_nodes_by_episode(parent_node.episode_id) # count stays the same assert new_count_nodes == count_nodes