From 0828aabdb94c4536b67f85e816aa9575d98f6c0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Wo=CC=88ginger?= Date: Tue, 20 Feb 2024 19:33:30 +0100 Subject: [PATCH] enhance delete function to keep the tree in a consistent state fix related to #515 --- lib/radiator/outline.ex | 53 +++++++++++++++++ lib/radiator/outline/node.ex | 5 ++ test/radiator/outline_test.exs | 105 +++++++++++++++++++++++++++++++-- 3 files changed, 158 insertions(+), 5 deletions(-) diff --git a/lib/radiator/outline.ex b/lib/radiator/outline.ex index ae4ec508..b5fde39b 100644 --- a/lib/radiator/outline.ex +++ b/lib/radiator/outline.ex @@ -59,6 +59,32 @@ defmodule Radiator.Outline do |> Repo.get!(id) end + @doc """ + Returns the previous node of a given node in the outline tree. + Returns `nil` if prev_id of the node is nil. + + ## Examples + iex> get_prev_node(%Node{prev_id: nil}) + nil + + iex> get_prev_node(%Node{prev_id: 42}) + %Node{uuid: 42} + + """ + def get_prev_node(node) when is_nil(node.prev_id), do: nil + + def get_prev_node(node) do + Node + |> where([n], n.uuid == ^node.prev_id) + |> Repo.one() + end + + def get_all_child_nodes(node) do + Node + |> where([n], n.parent_id == ^node.uuid) + |> Repo.all() + end + @doc """ Gets a single node. @@ -218,6 +244,33 @@ defmodule Radiator.Outline do """ def delete_node(%Node{} = node) do + next_node = + Node + |> where([n], n.prev_id == ^node.uuid) + |> Repo.one() + + 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}) + |> Repo.update() + end + + # no tail recursion but dont have to much levels in a tree + node + |> get_all_child_nodes() + |> Enum.each(fn child_node -> + delete_node(child_node) + end) + node |> Repo.delete() |> broadcast_node_action(:delete) diff --git a/lib/radiator/outline/node.ex b/lib/radiator/outline/node.ex index 6403a343..827fe7e3 100644 --- a/lib/radiator/outline/node.ex +++ b/lib/radiator/outline/node.ex @@ -47,6 +47,11 @@ defmodule Radiator.Outline.Node do |> validate_required([:content]) end + def move_node_changeset(node, attrs) do + node + |> cast(attrs, [:parent_id, :prev_id]) + end + defp trim(content) when is_binary(content), do: String.trim(content) defp trim(content), do: content end diff --git a/test/radiator/outline_test.exs b/test/radiator/outline_test.exs index aea58822..ad229133 100644 --- a/test/radiator/outline_test.exs +++ b/test/radiator/outline_test.exs @@ -4,7 +4,6 @@ defmodule Radiator.OutlineTest do alias Radiator.Outline alias Radiator.Outline.Node alias Radiator.PodcastFixtures - alias Radiator.Repo import Radiator.OutlineFixtures import Ecto.Query, warn: false @@ -83,12 +82,111 @@ defmodule Radiator.OutlineTest do end end + describe "get_prev_node/1" do + setup :complex_node_fixture + + test "returns the previous node", %{node_2: node_2, node_3: node_3} do + assert Outline.get_prev_node(node_3) == node_2 + end + + test "returns nil if there is no previous node", %{node_1: node_1} do + assert Outline.get_prev_node(node_1) == nil + end + end + describe "delete_node/1" do + setup :complex_node_fixture + test "deletes the node" do node = node_fixture() assert {:ok, %Node{}} = Outline.delete_node(node) assert_raise Ecto.NoResultsError, fn -> Outline.get_node!(node.uuid) end end + + test "next node must be updated", %{ + node_2: node_2, + node_3: node_3, + node_4: node_4 + } do + assert node_4.prev_id == node_3.uuid + + assert {:ok, %Node{}} = Outline.delete_node(node_3) + # reload nodes + node_4 = Outline.get_node!(node_4.uuid) + node_2 = Outline.get_node!(node_2.uuid) + + assert node_4.prev_id == node_2.uuid + end + + test "works for last element in list", %{ + node_6: node_6 + } do + episode_id = node_6.episode_id + + count_nodes = + episode_id + |> Outline.list_nodes_by_episode() + |> Enum.count() + + assert {:ok, %Node{}} = Outline.delete_node(node_6) + + new_count_nodes = + episode_id + |> Outline.list_nodes_by_episode() + |> Enum.count() + + assert new_count_nodes == count_nodes - 1 + end + + test "works for first element in list", %{ + node_1: node_1, + node_2: node_2 + } do + episode_id = node_1.episode_id + + count_nodes = + episode_id + |> Outline.list_nodes_by_episode() + |> Enum.count() + + assert {:ok, %Node{}} = Outline.delete_node(node_1) + + new_count_nodes = + episode_id + |> Outline.list_nodes_by_episode() + |> Enum.count() + + assert new_count_nodes == count_nodes - 1 + node_2 = Outline.get_node!(node_2.uuid) + assert node_2.prev_id == nil + end + + test "delete also child elements", %{ + node_3: node_3, + nested_node_1: nested_node_1, + nested_node_2: nested_node_2 + } do + assert {:ok, %Node{}} = Outline.delete_node(node_3) + + assert_raise Ecto.NoResultsError, fn -> Outline.get_node!(nested_node_1.uuid) end + assert_raise Ecto.NoResultsError, fn -> Outline.get_node!(nested_node_2.uuid) end + end + + test "when top parent gets deleted the whole tree will be gone", %{ + node_1: node_1, + node_4: node_4, + node_6: node_6, + nested_node_2: nested_node_2, + parent: parent + } do + assert {:ok, %Node{}} = Outline.delete_node(parent) + + # test some of elements in the tree + assert_raise Ecto.NoResultsError, fn -> Outline.get_node!(node_1.uuid) end + assert_raise Ecto.NoResultsError, fn -> Outline.get_node!(node_4.uuid) end + assert_raise Ecto.NoResultsError, fn -> Outline.get_node!(node_6.uuid) end + assert_raise Ecto.NoResultsError, fn -> Outline.get_node!(nested_node_2.uuid) end + end end describe "get_node_tree/1" do @@ -98,10 +196,7 @@ defmodule Radiator.OutlineTest do episode_id = parent.episode_id assert {:ok, tree} = Outline.get_node_tree(episode_id) - all_nodes = - Node - |> where([n], n.episode_id == ^episode_id) - |> Repo.all() + all_nodes = Outline.list_nodes_by_episode(episode_id) assert Enum.count(tree) == Enum.count(all_nodes)