From a4cbff78056cb08c860ed4a71e884995305cd6ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Wo=CC=88ginger?= Date: Wed, 28 Feb 2024 20:27:46 +0100 Subject: [PATCH] insert node (tree aware) --- lib/radiator/outline.ex | 98 +++++++++++-- test/radiator/outline_test.exs | 242 +++++++++++++++++++++++++++------ 2 files changed, 288 insertions(+), 52 deletions(-) diff --git a/lib/radiator/outline.ex b/lib/radiator/outline.ex index 6d9870d1..4c141d69 100644 --- a/lib/radiator/outline.ex +++ b/lib/radiator/outline.ex @@ -40,6 +40,21 @@ defmodule Radiator.Outline do |> Repo.all() end + @doc """ + Returns the the number of nodes for an episode + + ## Examples + + iex> count_nodes_by_episode(123) + 3 + + """ + def count_nodes_by_episode(episode_id) do + episode_id + |> list_nodes_by_episode() + |> Enum.count() + end + @doc """ Gets a single node. @@ -219,6 +234,74 @@ defmodule Radiator.Outline do |> broadcast_node_action(:insert) end + @doc """ + Inserts a node. + + ## Examples + + iex> insert_node(%{content: 'foo'}, %Node{} = parent_node, %Node{} = prev_node) + {:ok, %Node{}} + + iex> insert_node(%{content: value}, %Node{} = parent_node, %Node{parent_id: nil} = prev_node) + {:error, :parent_and_prev_not_consistent} + + """ + # creates a node and inserts it into the outline tree + # if a parent node is given, the new node will be inserted as a child of the parent node + # 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 + Repo.transaction(fn -> + prev_node_id = get_node_id(prev_node) + parent_node_id = get_node_id(parent_node) + + # find Node which has been previously connected to prev_node + node_to_move = + Node + |> where_prev_node_equals(prev_node_id) + |> where_parent_node_equals(parent_node_id) + |> Repo.one() + + with 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 + node + else + false -> + Repo.rollback("Insert node failed. Parent and prev node are not consistent.") + + {:error, _} -> + Repo.rollback("Insert node failed. Unkown error") + end + end) + end + + defp move_node_(nil, _parent_node_id, _prev_node_id), do: {:ok, nil} + + defp move_node_(node, parent_node_id, prev_node_id) do + node + |> Node.move_node_changeset(%{ + parent_id: parent_node_id, + prev_id: prev_node_id + }) + |> Repo.update() + end + + defp parent_and_prev_consistent?(_, nil), do: true + defp parent_and_prev_consistent?(nil, _), do: true + + defp parent_and_prev_consistent?(parent, prev) do + parent.uuid == prev.parent_id + end + + defp where_prev_node_equals(node, nil), do: where(node, [n], is_nil(n.prev_id)) + defp where_prev_node_equals(node, prev_id), do: where(node, [n], n.prev_id == ^prev_id) + + defp where_parent_node_equals(node, nil), do: where(node, [n], is_nil(n.parent_id)) + defp where_parent_node_equals(node, parent_id), do: where(node, [n], n.parent_id == ^parent_id) + @doc """ Updates a nodes content. @@ -258,16 +341,9 @@ defmodule Radiator.Outline do prev_node = get_prev_node(node) - prev_uuid = - if prev_node do - prev_node.uuid - else - nil - end - if next_node do next_node - |> Node.move_node_changeset(%{prev_id: prev_uuid}) + |> Node.move_node_changeset(%{prev_id: get_node_id(prev_node)}) |> Repo.update() end @@ -283,6 +359,12 @@ defmodule Radiator.Outline do |> broadcast_node_action(:delete) end + defp get_node_id(nil), do: nil + + defp get_node_id(%Node{} = node) do + node.uuid + end + defp broadcast_node_action({:ok, node}, action) do PubSub.broadcast(Radiator.PubSub, @topic, {action, node}) {:ok, node} diff --git a/test/radiator/outline_test.exs b/test/radiator/outline_test.exs index 5f4414da..03120f09 100644 --- a/test/radiator/outline_test.exs +++ b/test/radiator/outline_test.exs @@ -110,6 +110,125 @@ defmodule Radiator.OutlineTest do end end + describe "insert_node/3" do + setup :complex_node_fixture + + test "creates a new node in the tree", %{ + node_3: node_3, + nested_node_1: nested_node_1 + } 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) + new_count_nodes = Outline.count_nodes_by_episode(node_3.episode_id) + assert new_count_nodes == count_nodes + 1 + end + + test "the parent gets set", %{ + 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) + assert new_node.parent_id == node_3.uuid + end + + test "the prev gets set", %{ + 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) + assert new_node.prev_id == nested_node_1.uuid + end + + test "all nodes in same level are correctly connected", %{ + node_3: node_3, + 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) + + assert Outline.get_node!(nested_node_2.uuid).prev_id == new_node.uuid + assert new_node.prev_id == nested_node_1.uuid + assert Outline.get_node!(nested_node_1.uuid).prev_id == nil + end + + test "inserted node can be inserted at the end", %{ + node_3: node_3, + 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) + + assert Outline.get_node!(nested_node_2.uuid).prev_id == nested_node_1.uuid + assert new_node.prev_id == nested_node_2.uuid + assert Outline.get_node!(nested_node_1.uuid).prev_id == nil + end + + test "without a prev node inserted node will be first in list", %{ + node_3: node_3, + 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) + + assert new_node.prev_id == nil + assert Outline.get_node!(nested_node_1.uuid).prev_id == new_node.uuid + assert Outline.get_node!(nested_node_2.uuid).prev_id == nested_node_1.uuid + end + + test "without a parent node the inserted node will be put at the top", %{ + parent_node: parent_node + } do + node_attrs = %{content: "new node", episode_id: parent_node.episode_id} + {:ok, new_node} = Outline.insert_node(node_attrs) + + assert new_node.prev_id == nil + assert new_node.parent_id == nil + assert Outline.get_node!(parent_node.uuid).prev_id == new_node.uuid + end + + test "parent node and prev node need to be consistent", %{ + parent_node: parent_node, + 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} + + {:error, "Insert node failed. Parent and prev node are not consistent."} = + Outline.insert_node(node_attrs, parent_node, nested_node_1) + end + + test "parent node and prev node need to be consistent (2)", %{ + parent_node: parent_node + } 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} + + {:error, _error_message} = + Outline.insert_node(node_attrs, parent_node, bad_parent_node) + end + + test "in case of error no node gets inserted", %{ + parent_node: parent_node, + 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) + new_count_nodes = Outline.count_nodes_by_episode(parent_node.episode_id) + # count stays the same + assert new_count_nodes == count_nodes + end + end + describe "delete_node/1" do setup :complex_node_fixture @@ -138,19 +257,9 @@ defmodule Radiator.OutlineTest do node_6: node_6 } do episode_id = node_6.episode_id - - count_nodes = - episode_id - |> Outline.list_nodes_by_episode() - |> Enum.count() - + count_nodes = Outline.count_nodes_by_episode(episode_id) assert {:ok, %Node{}} = Outline.delete_node(node_6) - - new_count_nodes = - episode_id - |> Outline.list_nodes_by_episode() - |> Enum.count() - + new_count_nodes = Outline.count_nodes_by_episode(episode_id) assert new_count_nodes == count_nodes - 1 end @@ -160,19 +269,11 @@ defmodule Radiator.OutlineTest do } do episode_id = node_1.episode_id - count_nodes = - episode_id - |> Outline.list_nodes_by_episode() - |> Enum.count() - + count_nodes = Outline.count_nodes_by_episode(episode_id) assert {:ok, %Node{}} = Outline.delete_node(node_1) - - new_count_nodes = - episode_id - |> Outline.list_nodes_by_episode() - |> Enum.count() - + new_count_nodes = Outline.count_nodes_by_episode(episode_id) assert new_count_nodes == count_nodes - 1 + node_2 = Outline.get_node!(node_2.uuid) assert node_2.prev_id == nil end @@ -193,9 +294,9 @@ defmodule Radiator.OutlineTest do node_4: node_4, node_6: node_6, nested_node_2: nested_node_2, - parent: parent + parent_node: parent_node } do - assert {:ok, %Node{}} = Outline.delete_node(parent) + assert {:ok, %Node{}} = Outline.delete_node(parent_node) # test some of elements in the tree assert_raise Ecto.NoResultsError, fn -> Outline.get_node!(node_1.uuid) end @@ -208,8 +309,8 @@ defmodule Radiator.OutlineTest do describe "get_node_tree/1" do setup :complex_node_fixture - test "returns all nodes from a episode", %{parent: parent} do - episode_id = parent.episode_id + test "returns all nodes from a episode", %{parent_node: parent_node} do + episode_id = parent_node.episode_id assert {:ok, tree} = Outline.get_node_tree(episode_id) all_nodes = Outline.list_nodes_by_episode(episode_id) @@ -223,23 +324,23 @@ defmodule Radiator.OutlineTest do end test "does not return a node from another episode", %{ - parent: parent + parent_node: parent_node } do - episode_id = parent.episode_id + episode_id = parent_node.episode_id other_node = node_fixture(parent_id: nil, prev_id: nil, content: "other content") assert other_node.episode_id != episode_id {:ok, tree} = Outline.get_node_tree(episode_id) assert Enum.filter(tree, fn n -> n.uuid == other_node.uuid end) == [] end - test "returns nodes sorted by level", %{parent: parent} do - episode_id = parent.episode_id + test "returns nodes sorted by level", %{parent_node: parent_node} do + episode_id = parent_node.episode_id {:ok, tree} = Outline.get_node_tree(episode_id) Enum.reduce(tree, 0, fn node, current_level -> if node.parent_id != nil do - parent = Enum.find(tree, fn n -> n.uuid == node.parent_id end) - assert parent.level + 1 == node.level + parent_node = Enum.find(tree, fn n -> n.uuid == node.parent_id end) + assert parent_node.level + 1 == node.level end assert node.level >= current_level @@ -250,18 +351,71 @@ defmodule Radiator.OutlineTest do defp complex_node_fixture(_) do episode = PodcastFixtures.episode_fixture() - parent = node_fixture(episode_id: episode.id, parent_id: nil, prev_id: nil) - node_1 = node_fixture(episode_id: episode.id, parent_id: parent.uuid, prev_id: nil) - node_2 = node_fixture(episode_id: episode.id, parent_id: parent.uuid, prev_id: node_1.uuid) - node_3 = node_fixture(episode_id: episode.id, parent_id: parent.uuid, prev_id: node_2.uuid) - node_4 = node_fixture(episode_id: episode.id, parent_id: parent.uuid, prev_id: node_3.uuid) - node_5 = node_fixture(episode_id: episode.id, parent_id: parent.uuid, prev_id: node_4.uuid) - node_6 = node_fixture(episode_id: episode.id, parent_id: parent.uuid, prev_id: node_5.uuid) - - nested_node_1 = node_fixture(episode_id: episode.id, parent_id: node_3.uuid, prev_id: nil) + parent_node = node_fixture(episode_id: episode.id, parent_id: nil, prev_id: nil) + + node_1 = + node_fixture( + episode_id: episode.id, + parent_id: parent_node.uuid, + prev_id: nil, + content: "node_1" + ) + + node_2 = + node_fixture( + episode_id: episode.id, + parent_id: parent_node.uuid, + prev_id: node_1.uuid, + content: "node_2" + ) + + node_3 = + node_fixture( + episode_id: episode.id, + parent_id: parent_node.uuid, + prev_id: node_2.uuid, + content: "node_3" + ) + + node_4 = + node_fixture( + episode_id: episode.id, + parent_id: parent_node.uuid, + prev_id: node_3.uuid, + content: "node_4" + ) + + node_5 = + node_fixture( + episode_id: episode.id, + parent_id: parent_node.uuid, + prev_id: node_4.uuid, + content: "node_5" + ) + + node_6 = + node_fixture( + episode_id: episode.id, + parent_id: parent_node.uuid, + prev_id: node_5.uuid, + content: "node_6" + ) + + nested_node_1 = + node_fixture( + episode_id: episode.id, + parent_id: node_3.uuid, + prev_id: nil, + content: "nested_node_1" + ) nested_node_2 = - node_fixture(episode_id: episode.id, parent_id: node_3.uuid, prev_id: nested_node_1.uuid) + node_fixture( + episode_id: episode.id, + parent_id: node_3.uuid, + prev_id: nested_node_1.uuid, + content: "nested_node_2" + ) %{ node_1: node_1, @@ -272,7 +426,7 @@ defmodule Radiator.OutlineTest do node_6: node_6, nested_node_1: nested_node_1, nested_node_2: nested_node_2, - parent: parent + parent_node: parent_node } end end