From fe1d3db89f4fb1d6da447437e2610f4e93a3c792 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Wo=CC=88ginger?= Date: Mon, 29 Jan 2024 22:11:11 +0100 Subject: [PATCH 1/6] split nodes changeset in different use cases --- lib/radiator/outline.ex | 41 +++++++------------ lib/radiator/outline/node.ex | 44 ++++++++++++++------- lib/radiator_web/live/episode_live/index.ex | 2 +- test/radiator/outline_test.exs | 13 ++---- 4 files changed, 49 insertions(+), 51 deletions(-) diff --git a/lib/radiator/outline.ex b/lib/radiator/outline.ex index a7faf4d9..18b4b3b8 100644 --- a/lib/radiator/outline.ex +++ b/lib/radiator/outline.ex @@ -15,12 +15,6 @@ defmodule Radiator.Outline do |> Notify.broadcast_node_action(:insert, socket_id) end - def update(%Node{} = node, attrs, socket_id \\ nil) do - node - |> update_node(attrs) - |> Notify.broadcast_node_action(:update, socket_id) - end - def delete(%Node{} = node, socket_id \\ nil) do node |> delete_node() @@ -107,28 +101,36 @@ defmodule Radiator.Outline do {:error, %Ecto.Changeset{}} """ - def create_node(attrs \\ %{}) do + def create_node(attrs \\ %{}, socket_id \\ nil) do %Node{} - |> Node.changeset(attrs) + |> Node.insert_changeset(attrs) + |> Repo.insert() + |> Notify.broadcast_node_action(:insert, socket_id) + end + + def create_node(attrs, %{id: id}) do + %Node{creator_id: id} + |> Node.insert_changeset(attrs) |> Repo.insert() end @doc """ - Updates a node. + Updates a nodes content. ## Examples - iex> update_node(node, %{field: new_value}) + iex> update_node_content(node, %{content: new_value}) {:ok, %Node{}} - iex> update_node(node, %{field: bad_value}) + iex> update_node_content(node, %{content: nil}) {:error, %Ecto.Changeset{}} """ - def update_node(%Node{} = node, attrs) do + def update_node_content(%Node{} = node, attrs, socket_id \\ nil) do node - |> Node.changeset(attrs) + |> Node.update_content_changeset(attrs) |> Repo.update() + |> Notify.broadcast_node_action(:update, socket_id) end @doc """ @@ -147,17 +149,4 @@ defmodule Radiator.Outline do node |> Repo.delete() end - - @doc """ - Returns an `%Ecto.Changeset{}` for tracking node changes. - - ## Examples - - iex> change_node(node) - %Ecto.Changeset{data: %Node{}} - - """ - def change_node(%Node{} = node, attrs \\ %{}) do - Node.changeset(node, attrs) - end end diff --git a/lib/radiator/outline/node.ex b/lib/radiator/outline/node.ex index 8fb151ac..0ce640c6 100644 --- a/lib/radiator/outline/node.ex +++ b/lib/radiator/outline/node.ex @@ -10,6 +10,7 @@ defmodule Radiator.Outline.Node do @derive {Jason.Encoder, only: [:uuid, :content, :creator_id, :parent_id, :prev_id]} @primary_key {:uuid, :binary_id, autogenerate: true} + schema "outline_nodes" do field :content, :string field :creator_id, :integer @@ -21,25 +22,38 @@ defmodule Radiator.Outline.Node do timestamps(type: :utc_datetime) end - @required_fields [ - :episode_id - ] - - @optional_fields [ - :content, - :creator_id, - :parent_id, - :prev_id - ] + @doc """ + A changeset for inserting a new node + Work in progress. Since we currently ignore the tree structure, there is + no concept for a root node. + Also questionable wether a node really needs a content from beginning. So probably a root + doesnt have a content + Another issue might be we need to create the uuid upfront and pass it here + """ + def insert_changeset(node, attributes) do + node + |> cast(attributes, [:content, :episode_id, :creator_id, :parent_id, :prev_id]) + |> update_change(:content, &trim/1) + |> validate_required([:content, :episode_id]) + end - @all_fields @optional_fields ++ @required_fields + @doc """ + Changeset for moving a node + Only the parent_id is allowed and expected to be changed + """ + def move_changeset(node, attrs) do + node + |> cast(attrs, [:parent_id]) + end - @doc false - def changeset(node, attrs) do + @doc """ + Changeset for updating the content of a node + """ + def update_content_changeset(node, attrs) do node - |> cast(attrs, @all_fields) + |> cast(attrs, [:content]) |> update_change(:content, &trim/1) - |> validate_required(@required_fields) + |> validate_required([:content]) end defp trim(content) when is_binary(content), do: String.trim(content) diff --git a/lib/radiator_web/live/episode_live/index.ex b/lib/radiator_web/live/episode_live/index.ex index eaca1329..403837a0 100644 --- a/lib/radiator_web/live/episode_live/index.ex +++ b/lib/radiator_web/live/episode_live/index.ex @@ -61,7 +61,7 @@ defmodule RadiatorWeb.EpisodeLive.Index do case Outline.get_node(uuid) do nil -> nil - node -> Outline.update(node, attrs, socket.id) + node -> Outline.update_node_content(node, attrs, socket.id) end socket diff --git a/test/radiator/outline_test.exs b/test/radiator/outline_test.exs index c44e6ea9..849d70e5 100644 --- a/test/radiator/outline_test.exs +++ b/test/radiator/outline_test.exs @@ -51,17 +51,17 @@ defmodule Radiator.OutlineTest do assert {:error, %Ecto.Changeset{}} = Outline.create_node(@invalid_attrs) end - test "update_node/2 with valid data updates the node" do + test "update_node_content/2 with valid data updates the node" do node = node_fixture() update_attrs = %{content: "some updated content"} - assert {:ok, %Node{} = node} = Outline.update_node(node, update_attrs) + assert {:ok, %Node{} = node} = Outline.update_node_content(node, update_attrs) assert node.content == "some updated content" end - test "update_node/2 with invalid data returns error changeset" do + test "update_node_content/2 with invalid data returns error changeset" do node = node_fixture() - assert {:error, %Ecto.Changeset{}} = Outline.update_node(node, @invalid_attrs) + assert {:error, %Ecto.Changeset{}} = Outline.update_node_content(node, %{content: nil}) assert node == Outline.get_node!(node.uuid) end @@ -70,10 +70,5 @@ defmodule Radiator.OutlineTest do assert {:ok, %Node{}} = Outline.delete_node(node) assert_raise Ecto.NoResultsError, fn -> Outline.get_node!(node.uuid) end end - - test "change_node/1 returns a node changeset" do - node = node_fixture() - assert %Ecto.Changeset{} = Outline.change_node(node) - end end end From 1423608b3b097ac90c844b3908973059f52f723d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Wo=CC=88ginger?= Date: Mon, 29 Jan 2024 22:55:38 +0100 Subject: [PATCH 2/6] add tree node function --- lib/radiator/outline.ex | 94 +++++++++++++++++++++++-- lib/radiator/outline/node.ex | 11 +-- priv/repo/seeds.exs | 5 +- test/radiator/outline_test.exs | 121 +++++++++++++++++++++++++++++---- 4 files changed, 200 insertions(+), 31 deletions(-) diff --git a/lib/radiator/outline.ex b/lib/radiator/outline.ex index 18b4b3b8..809edab3 100644 --- a/lib/radiator/outline.ex +++ b/lib/radiator/outline.ex @@ -89,6 +89,88 @@ defmodule Radiator.Outline do |> Repo.get(id) end + @doc """ + Gets all nodes of an episode as a tree. + Uses a Common Table Expression (CTE) to recursively query the database. + Sets the level of each node in the tree. Level 0 are the root nodes (without a parent) + Returns a list with all nodes of the episode sorted by the level. + ## Examples + + iex> get_node_tree(123) + [%Node{}, %Node{}, ..] + + SQL: + WITH RECURSIVE node_tree AS ( + SELECT uuid, content, parent_id, prev_id, 0 AS level + FROM outline_nodes + WHERE episode_id = ?::integer and parent_id is NULL + UNION ALL + SELECT outline_nodes.uuid, outline_nodes.content, outline_nodes.parent_id, outline_nodes.prev_id, node_tree.level + 1 + FROM outline_nodes + JOIN node_tree ON outline_nodes.parent_id = node_tree.uuid + ) + SELECT * FROM node_tree; + """ + def get_node_tree(episode_id) do + node_tree_initial_query = + Node + |> where([n], is_nil(n.parent_id)) + |> where([n], n.episode_id == ^episode_id) + |> select([n], %{ + uuid: n.uuid, + content: n.content, + parent_id: n.parent_id, + prev_id: n.prev_id, + level: 0 + }) + + node_tree_recursion_query = + from outline_node in "outline_nodes", + join: node_tree in "node_tree", + on: outline_node.parent_id == node_tree.uuid, + select: [ + outline_node.uuid, + outline_node.content, + outline_node.parent_id, + outline_node.prev_id, + node_tree.level + 1 + ] + + node_tree_query = + node_tree_initial_query + |> union_all(^node_tree_recursion_query) + + tree = + "node_tree" + |> recursive_ctes(true) + |> with_cte("node_tree", as: ^node_tree_query) + |> select([n], %{ + uuid: n.uuid, + content: n.content, + parent_id: n.parent_id, + prev_id: n.prev_id, + level: n.level + }) + |> Repo.all() + |> Enum.map(fn %{ + uuid: uuid, + content: content, + parent_id: parent_id, + prev_id: prev_id, + level: level + } -> + %Node{ + uuid: binaray_uuid_to_ecto_uuid(uuid), + content: content, + parent_id: binaray_uuid_to_ecto_uuid(parent_id), + prev_id: binaray_uuid_to_ecto_uuid(prev_id), + level: level + } + end) + + {:ok, tree} + end + @doc """ Creates a node. @@ -108,12 +190,6 @@ defmodule Radiator.Outline do |> Notify.broadcast_node_action(:insert, socket_id) end - def create_node(attrs, %{id: id}) do - %Node{creator_id: id} - |> Node.insert_changeset(attrs) - |> Repo.insert() - end - @doc """ Updates a nodes content. @@ -149,4 +225,10 @@ defmodule Radiator.Outline do node |> Repo.delete() end + + defp binaray_uuid_to_ecto_uuid(nil), do: nil + + defp binaray_uuid_to_ecto_uuid(uuid) do + Ecto.UUID.load!(uuid) + end end diff --git a/lib/radiator/outline/node.ex b/lib/radiator/outline/node.ex index 0ce640c6..6403a343 100644 --- a/lib/radiator/outline/node.ex +++ b/lib/radiator/outline/node.ex @@ -10,12 +10,12 @@ defmodule Radiator.Outline.Node do @derive {Jason.Encoder, only: [:uuid, :content, :creator_id, :parent_id, :prev_id]} @primary_key {:uuid, :binary_id, autogenerate: true} - schema "outline_nodes" do field :content, :string field :creator_id, :integer field :parent_id, Ecto.UUID field :prev_id, Ecto.UUID + field :level, :integer, virtual: true belongs_to :episode, Episode @@ -37,15 +37,6 @@ defmodule Radiator.Outline.Node do |> validate_required([:content, :episode_id]) end - @doc """ - Changeset for moving a node - Only the parent_id is allowed and expected to be changed - """ - def move_changeset(node, attrs) do - node - |> cast(attrs, [:parent_id]) - end - @doc """ Changeset for updating the content of a node """ diff --git a/priv/repo/seeds.exs b/priv/repo/seeds.exs index 03a5e3b0..89df6abe 100644 --- a/priv/repo/seeds.exs +++ b/priv/repo/seeds.exs @@ -19,7 +19,7 @@ alias Radiator.{Accounts, Outline, Podcast} {:ok, show} = Podcast.create_show(%{title: "Tech Weekly", network_id: network.id}) -{:ok, _episode} = +{:ok, past_episode} = Podcast.create_episode(%{title: "past episode", show_id: show.id}) {:ok, current_episode} = @@ -60,3 +60,6 @@ alias Radiator.{Accounts, Outline, Podcast} episode_id: current_episode.id, prev_id: node211.uuid }) + +{:ok, past_parent_node} = + Outline.create_node(%{content: "Old Content", episode_id: past_episode.id}) diff --git a/test/radiator/outline_test.exs b/test/radiator/outline_test.exs index 849d70e5..55aea0cf 100644 --- a/test/radiator/outline_test.exs +++ b/test/radiator/outline_test.exs @@ -2,16 +2,17 @@ defmodule Radiator.OutlineTest do use Radiator.DataCase alias Radiator.Outline + alias Radiator.Outline.Node + alias Radiator.PodcastFixtures + alias Radiator.Repo - describe "outline_nodes" do - alias Radiator.Outline.Node + import Radiator.OutlineFixtures + import Ecto.Query, warn: false - import Radiator.OutlineFixtures - alias Radiator.PodcastFixtures + @invalid_attrs %{episode_id: nil} - @invalid_attrs %{episode_id: nil} - - test "list_nodes/0 returns all nodes" do + describe "list_nodes/0" do + test "returns all nodes" do node1 = node_fixture() node2 = node_fixture() @@ -25,13 +26,17 @@ defmodule Radiator.OutlineTest do assert Outline.list_nodes_by_episode(node1.episode_id) == [node1] assert Outline.list_nodes_by_episode(node2.episode_id) == [node2] end + end - test "get_node!/1 returns the node with given id" do + describe "get_node!/1" do + test "returns the node with given id" do node = node_fixture() assert Outline.get_node!(node.uuid) == node end + end - test "create_node/1 with valid data creates a node" do + describe "create_node/1" do + test "with valid data creates a node" do episode = PodcastFixtures.episode_fixture() valid_attrs = %{content: "some content", episode_id: episode.id} @@ -39,7 +44,7 @@ defmodule Radiator.OutlineTest do assert node.content == "some content" end - test "create_node/1 trims whitespace from content" do + test "trims whitespace from content" do episode = PodcastFixtures.episode_fixture() valid_attrs = %{content: " some content ", episode_id: episode.id} @@ -47,11 +52,23 @@ defmodule Radiator.OutlineTest do assert node.content == "some content" end - test "create_node/1 with invalid data returns error changeset" do + test "can have a creator" do + episode = PodcastFixtures.episode_fixture() + user = %{id: 2} + valid_attrs = %{content: "some content", episode_id: episode.id, creator_id: user.id} + + assert {:ok, %Node{} = node} = Outline.create_node(valid_attrs, user) + assert node.content == "some content" + assert node.creator_id == user.id + end + + test "with invalid data returns error changeset" do assert {:error, %Ecto.Changeset{}} = Outline.create_node(@invalid_attrs) end + end - test "update_node_content/2 with valid data updates the node" do + describe "update_node_content/2" do + test "with valid data updates the node" do node = node_fixture() update_attrs = %{content: "some updated content"} @@ -59,16 +76,92 @@ defmodule Radiator.OutlineTest do assert node.content == "some updated content" end - test "update_node_content/2 with invalid data returns error changeset" do + test "with invalid data returns error changeset" do node = node_fixture() assert {:error, %Ecto.Changeset{}} = Outline.update_node_content(node, %{content: nil}) assert node == Outline.get_node!(node.uuid) end + end - test "delete_node/1 deletes the node" do + describe "delete_node/1" do + 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 end + + 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 + assert {:ok, tree} = Outline.get_node_tree(episode_id) + + all_nodes = + Node + |> where([n], n.episode_id == ^episode_id) + |> Repo.all() + + assert Enum.count(tree) == Enum.count(all_nodes) + + Enum.each(tree, fn node -> + assert node.uuid == + List.first(Enum.filter(all_nodes, fn n -> n.uuid == node.uuid end)).uuid + end) + end + + test "does not return a node from another episode", %{ + parent: parent + } do + episode_id = parent.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 + {: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 + end + + assert node.level >= current_level + node.level + end) + end + end + + 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) + + nested_node_2 = + node_fixture(episode_id: episode.id, parent_id: node_3.uuid, prev_id: nested_node_1.uuid) + + %{ + node_1: node_1, + node_2: node_2, + node_3: node_3, + node_4: node_4, + node_5: node_5, + node_6: node_6, + nested_node_1: nested_node_1, + nested_node_2: nested_node_2, + parent: parent + } + end end From f7c46c94ad4fcdca9f4357bae391cbfa4e701522 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 3/6] enhance delete function to keep the tree in a consistent state fix related to #515 --- lib/radiator/outline.ex | 60 ++++++++++++++++ lib/radiator/outline/node.ex | 5 ++ test/radiator/outline_test.exs | 121 +++++++++++++++++++++++++++++++-- 3 files changed, 181 insertions(+), 5 deletions(-) diff --git a/lib/radiator/outline.ex b/lib/radiator/outline.ex index 809edab3..6de47cc0 100644 --- a/lib/radiator/outline.ex +++ b/lib/radiator/outline.ex @@ -70,6 +70,39 @@ 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 + + @doc """ + Returns all child nodes of a given node. + ## Examples + iex> get_all_child_nodes(%Node{}) + [%Node{}, %Node{}] + + """ + def get_all_child_nodes(node) do + Node + |> where([n], n.parent_id == ^node.uuid) + |> Repo.all() + end + @doc """ Gets a single node. @@ -222,6 +255,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 we dont have too much levels in a tree + node + |> get_all_child_nodes() + |> Enum.each(fn child_node -> + delete_node(child_node) + end) + node |> Repo.delete() end 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 55aea0cf..b626ac8d 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,127 @@ 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 "get_all_child_nodes/1" do + setup :complex_node_fixture + + test "returns all child nodes", %{ + node_3: node_3, + nested_node_1: nested_node_1, + nested_node_2: nested_node_2 + } do + assert Outline.get_all_child_nodes(node_3) == [nested_node_1, nested_node_2] + end + + test "returns an empty list if there are no child nodes", %{node_1: node_1} do + assert Outline.get_all_child_nodes(node_1) == [] + 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 +212,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) From 3d47cfbb9059747636ca893c9379198f97f98f3a 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 4/6] insert node (tree aware) --- lib/radiator/outline.ex | 124 +++++++++++- test/radiator/outline_test.exs | 360 ++++++++++++++++++++++++++++----- 2 files changed, 430 insertions(+), 54 deletions(-) diff --git a/lib/radiator/outline.ex b/lib/radiator/outline.ex index 6de47cc0..3988aa64 100644 --- a/lib/radiator/outline.ex +++ b/lib/radiator/outline.ex @@ -51,6 +51,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. @@ -122,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. @@ -197,7 +232,8 @@ defmodule Radiator.Outline do content: content, parent_id: binaray_uuid_to_ecto_uuid(parent_id), prev_id: binaray_uuid_to_ecto_uuid(prev_id), - level: level + level: level, + episode_id: episode_id } end) @@ -223,6 +259,77 @@ defmodule Radiator.Outline do |> Notify.broadcast_node_action(:insert, socket_id) end + @doc """ + Inserts a node. + + ## Examples + + iex> insert_node(%{content: 'foo'}) + {:ok, %Node{}} + + iex> insert_node(%{content: value}) + {: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) do + Repo.transaction(fn -> + 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 = + Node + |> where(episode_id: ^episode_id) + |> where_prev_node_equals(prev_node_id) + |> where_parent_node_equals(parent_node_id) + |> Repo.one() + + 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 + 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. @@ -262,16 +369,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 @@ -286,6 +386,12 @@ defmodule Radiator.Outline do |> Repo.delete() end + defp get_node_id(nil), do: nil + + defp get_node_id(%Node{} = node) do + node.uuid + end + defp binaray_uuid_to_ecto_uuid(nil), do: nil defp binaray_uuid_to_ecto_uuid(uuid) do diff --git a/test/radiator/outline_test.exs b/test/radiator/outline_test.exs index b626ac8d..992898d9 100644 --- a/test/radiator/outline_test.exs +++ b/test/radiator/outline_test.exs @@ -69,7 +69,7 @@ defmodule Radiator.OutlineTest do describe "update_node_content/2" do test "with valid data updates the node" do node = node_fixture() - update_attrs = %{content: "some updated content"} + update_attrs = %{"content" => "some updated content"} assert {:ok, %Node{} = node} = Outline.update_node_content(node, update_attrs) assert node.content == "some updated content" @@ -77,7 +77,7 @@ defmodule Radiator.OutlineTest do test "with invalid data returns error changeset" do node = node_fixture() - assert {:error, %Ecto.Changeset{}} = Outline.update_node_content(node, %{content: nil}) + assert {:error, %Ecto.Changeset{}} = Outline.update_node_content(node, %{"content" => nil}) assert node == Outline.get_node!(node.uuid) end end @@ -110,6 +110,179 @@ defmodule Radiator.OutlineTest do end end + describe "insert_node/1" 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, + "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 + + 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, + "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 + + 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, + "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 + + 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, + "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 + 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, + "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 + 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, + "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 + 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 + # another node in another episode without parent and prev node + node_fixture(parent_id: nil, prev_id: nil) + 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, + "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) + 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, + "parent_node" => parent_node.uuid, + "prev_node" => bad_parent_node.uuid + } + + {:error, _error_message} = + Outline.insert_node(node_attrs) + 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, + "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 + end + end + describe "delete_node/1" do setup :complex_node_fixture @@ -138,19 +311,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 +323,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 +348,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 +363,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,45 +378,160 @@ 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 node.level end) end + + test "associated the correct level", %{ + node_1: node_1, + node_2: node_2, + node_3: node_3, + node_4: node_4, + node_5: node_5, + node_6: node_6, + nested_node_1: nested_node_1, + nested_node_2: nested_node_2, + parent_node: parent_node + } do + {:ok, tree} = Outline.get_node_tree(parent_node.episode_id) + assert_level_for_node(tree, parent_node, 0) + assert_level_for_node(tree, node_1, 1) + assert_level_for_node(tree, node_2, 1) + assert_level_for_node(tree, node_3, 1) + assert_level_for_node(tree, node_4, 1) + assert_level_for_node(tree, node_5, 1) + assert_level_for_node(tree, node_6, 1) + assert_level_for_node(tree, nested_node_1, 2) + assert_level_for_node(tree, nested_node_2, 2) + end + + test "tree can have more than one parent node", %{ + parent_node: parent_node + } do + episode_id = parent_node.episode_id + + other_parent_node = + node_fixture( + parent_id: nil, + prev_id: parent_node.uuid, + episode_id: episode_id, + content: "also a parent" + ) + + third_parent_node = + node_fixture( + parent_id: nil, + prev_id: other_parent_node.uuid, + episode_id: episode_id, + content: "even another root element" + ) + + {:ok, tree} = Outline.get_node_tree(parent_node.episode_id) + assert_level_for_node(tree, parent_node, 0) + assert_level_for_node(tree, other_parent_node, 0) + assert_level_for_node(tree, third_parent_node, 0) + end + end + + defp assert_level_for_node(tree, node, level) do + node = Enum.filter(tree, fn n -> n.uuid == node.uuid end) |> List.first() + assert node.level == level end 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, + content: "root of all evil" + ) + + 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 +542,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 From b60a3ee2c135a5011f8bdee84eaac94310b4b7f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Wo=CC=88ginger?= Date: Tue, 26 Mar 2024 22:16:46 +0100 Subject: [PATCH 5/6] remove notifications for now will be moved to evebt_consumer (or so) --- lib/radiator/outline.ex | 13 ++++--------- test/radiator_web/live/episode_live_test.exs | 16 +++++++--------- 2 files changed, 11 insertions(+), 18 deletions(-) diff --git a/lib/radiator/outline.ex b/lib/radiator/outline.ex index 3988aa64..64d0ba16 100644 --- a/lib/radiator/outline.ex +++ b/lib/radiator/outline.ex @@ -6,19 +6,16 @@ defmodule Radiator.Outline do import Ecto.Query, warn: false alias Radiator.Outline.Node - alias Radiator.Outline.Notify alias Radiator.Repo - def create(attrs \\ %{}, socket_id \\ nil) do + def create(attrs \\ %{}, _socket_id \\ nil) do attrs |> create_node() - |> Notify.broadcast_node_action(:insert, socket_id) end - def delete(%Node{} = node, socket_id \\ nil) do + def delete(%Node{} = node, _socket_id \\ nil) do node |> delete_node() - |> Notify.broadcast_node_action(:delete, socket_id) end @doc """ @@ -252,11 +249,10 @@ defmodule Radiator.Outline do {:error, %Ecto.Changeset{}} """ - def create_node(attrs \\ %{}, socket_id \\ nil) do + def create_node(attrs \\ %{}, _socket_id \\ nil) do %Node{} |> Node.insert_changeset(attrs) |> Repo.insert() - |> Notify.broadcast_node_action(:insert, socket_id) end @doc """ @@ -342,11 +338,10 @@ defmodule Radiator.Outline do {:error, %Ecto.Changeset{}} """ - def update_node_content(%Node{} = node, attrs, socket_id \\ nil) do + def update_node_content(%Node{} = node, attrs, _socket_id \\ nil) do node |> Node.update_content_changeset(attrs) |> Repo.update() - |> Notify.broadcast_node_action(:update, socket_id) end @doc """ diff --git a/test/radiator_web/live/episode_live_test.exs b/test/radiator_web/live/episode_live_test.exs index 1396de36..c927d029 100644 --- a/test/radiator_web/live/episode_live_test.exs +++ b/test/radiator_web/live/episode_live_test.exs @@ -67,7 +67,7 @@ defmodule RadiatorWeb.EpisodeLiveTest do test "insert a new node", %{conn: conn, show: show} do {:ok, live, _html} = live(conn, ~p"/admin/podcast/#{show.id}") - {:ok, other_live, _html} = live(conn, ~p"/admin/podcast/#{show.id}") + {:ok, _other_live, _html} = live(conn, ~p"/admin/podcast/#{show.id}") temp_id = "f894d2ed-9447-4eef-8c31-fc52372b3bbe" params = %{"temp_id" => temp_id, "content" => "new node temp content"} @@ -82,12 +82,12 @@ defmodule RadiatorWeb.EpisodeLiveTest do assert_reply(live, ^node_with_temp_id) - assert_push_event(other_live, "insert", ^node) + # FIXME: assert_push_event(other_live, "insert", ^node) end test "update node", %{conn: conn, show: show, episode: episode} do {:ok, live, _html} = live(conn, ~p"/admin/podcast/#{show.id}") - {:ok, other_live, _html} = live(conn, ~p"/admin/podcast/#{show.id}") + {:ok, _other_live, _html} = live(conn, ~p"/admin/podcast/#{show.id}") node = node_fixture(%{episode_id: episode.id}) @@ -104,22 +104,20 @@ defmodule RadiatorWeb.EpisodeLiveTest do assert updated_node.uuid == params.uuid assert updated_node.content == params.content - - assert_push_event(other_live, "update", ^updated_node) + # FIXME: assert_push_event(other_live, "update", ^updated_node) end test "delete node", %{conn: conn, show: show, episode: episode} do {:ok, live, _html} = live(conn, ~p"/admin/podcast/#{show.id}") - {:ok, other_live, _html} = live(conn, ~p"/admin/podcast/#{show.id}") + {:ok, _other_live, _html} = live(conn, ~p"/admin/podcast/#{show.id}") node = node_fixture(%{episode_id: episode.id}) params = Map.from_struct(node) assert live |> render_hook(:delete_node, params) - assert_push_event(other_live, "delete", %{uuid: deleted_uuid}) - - assert deleted_uuid == node.uuid + # FIXME: assert_push_event(other_live, "delete", %{uuid: deleted_uuid}) + # FIXME: assert deleted_uuid == node.uuid end end end From 101032c49efa983c5900c9151dec1d524ab3b00b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Wo=CC=88ginger?= Date: Fri, 29 Mar 2024 20:20:15 +0100 Subject: [PATCH 6/6] split outline module into node_repository and outline in outline all tree aware functions --- lib/radiator/outline.ex | 282 +++++------------- lib/radiator/outline/event_consumer.ex | 2 +- lib/radiator/outline/node_repository.ex | 148 +++++++++ .../controllers/api/outline_controller.ex | 2 +- lib/radiator_web/live/episode_live/index.ex | 11 +- .../radiator/outline/node_repository_test.exs | 78 +++++ test/radiator/outline_test.exs | 127 +++----- .../api/outline_controller_test.exs | 9 +- test/radiator_web/live/episode_live_test.exs | 14 +- test/support/fixtures/outline_fixtures.ex | 4 +- 10 files changed, 358 insertions(+), 319 deletions(-) create mode 100644 lib/radiator/outline/node_repository.ex create mode 100644 test/radiator/outline/node_repository_test.exs diff --git a/lib/radiator/outline.ex b/lib/radiator/outline.ex index 64d0ba16..bc2c83b5 100644 --- a/lib/radiator/outline.ex +++ b/lib/radiator/outline.ex @@ -6,80 +6,109 @@ defmodule Radiator.Outline do import Ecto.Query, warn: false alias Radiator.Outline.Node + alias Radiator.Outline.NodeRepository alias Radiator.Repo - def create(attrs \\ %{}, _socket_id \\ nil) do - attrs - |> create_node() - end - - def delete(%Node{} = node, _socket_id \\ nil) do - node - |> delete_node() - end - @doc """ - Returns the list of nodes. + Inserts a node. ## Examples - iex> list_nodes() - [%Node{}, ...] + iex> insert_node(%{content: 'foo'}) + {:ok, %Node{}} + + iex> insert_node(%{content: value}) + {:error, :parent_and_prev_not_consistent} """ - def list_nodes do - Node - |> Repo.all() + # 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) do + Repo.transaction(fn -> + 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 = + Node + |> where(episode_id: ^episode_id) + |> where_prev_node_equals(prev_node_id) + |> where_parent_node_equals(parent_node_id) + |> Repo.one() + + with parent_node <- NodeRepository.get_node_if(parent_node_id), + prev_node <- NodeRepository.get_node_if(prev_node_id), + true <- parent_and_prev_consistent?(parent_node, prev_node), + {:ok, node} <- NodeRepository.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 @doc """ - Returns the list of nodes for an episode. + Updates a nodes content. ## Examples - iex> list_nodes(123) - [%Node{}, ...] + iex> update_node_content(node, %{content: new_value}) + {:ok, %Node{}} - """ + iex> update_node_content(node, %{content: nil}) + {:error, %Ecto.Changeset{}} - def list_nodes_by_episode(episode_id) do - Node - |> where([p], p.episode_id == ^episode_id) - |> Repo.all() + """ + def update_node_content(%Node{} = node, attrs, _socket_id \\ nil) do + node + |> Node.update_content_changeset(attrs) + |> Repo.update() end @doc """ - Returns the the number of nodes for an episode - + Removes a node from the tree and deletes it from the repository. + Recursivly deletes all children if there are some. ## 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 + iex> remove_node(node) + {:ok, %Node{}} - @doc """ - Gets a single node. + iex> remove_node(node) + {:error, %Ecto.Changeset{}} - Raises `Ecto.NoResultsError` if the Node does not exist. + """ + def remove_node(%Node{} = node, _socket_id \\ nil) do + next_node = + Node + |> where([n], n.prev_id == ^node.uuid) + |> Repo.one() - ## Examples + prev_node = get_prev_node(node) - iex> get_node!(123) - %Node{} + if next_node do + next_node + |> Node.move_node_changeset(%{prev_id: get_node_id(prev_node)}) + |> Repo.update() + end - iex> get_node!(456) - ** (Ecto.NoResultsError) + # no tail recursion but we dont have too much levels in a tree + node + |> get_all_child_nodes() + |> Enum.each(fn child_node -> + remove_node(child_node) + end) - """ - def get_node!(id) do - Node - |> Repo.get!(id) + # finally delete the node itself from the database + NodeRepository.delete_node(node) end @doc """ @@ -115,45 +144,6 @@ defmodule Radiator.Outline do |> Repo.all() end - @doc """ - Gets a single node. - - Returns `nil` if the Node does not exist. - - ## Examples - - iex> get_node(123) - %Node{} - - iex> get_node(456) - nil - - """ - def get_node(id) do - Node - |> 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. @@ -237,71 +227,6 @@ defmodule Radiator.Outline do {:ok, tree} end - @doc """ - Creates a node. - - ## Examples - - iex> create_node(%{field: value}) - {:ok, %Node{}} - - iex> create_node(%{field: bad_value}) - {:error, %Ecto.Changeset{}} - - """ - def create_node(attrs \\ %{}, _socket_id \\ nil) do - %Node{} - |> Node.insert_changeset(attrs) - |> Repo.insert() - end - - @doc """ - Inserts a node. - - ## Examples - - iex> insert_node(%{content: 'foo'}) - {:ok, %Node{}} - - iex> insert_node(%{content: value}) - {: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) do - Repo.transaction(fn -> - 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 = - Node - |> where(episode_id: ^episode_id) - |> where_prev_node_equals(prev_node_id) - |> where_parent_node_equals(parent_node_id) - |> Repo.one() - - 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 - 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 @@ -326,61 +251,6 @@ defmodule Radiator.Outline do 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. - - ## Examples - - iex> update_node_content(node, %{content: new_value}) - {:ok, %Node{}} - - iex> update_node_content(node, %{content: nil}) - {:error, %Ecto.Changeset{}} - - """ - def update_node_content(%Node{} = node, attrs, _socket_id \\ nil) do - node - |> Node.update_content_changeset(attrs) - |> Repo.update() - end - - @doc """ - Deletes a node. - - ## Examples - - iex> delete_node(node) - {:ok, %Node{}} - - iex> delete_node(node) - {:error, %Ecto.Changeset{}} - - """ - def delete_node(%Node{} = node) do - next_node = - Node - |> where([n], n.prev_id == ^node.uuid) - |> Repo.one() - - prev_node = get_prev_node(node) - - if next_node do - next_node - |> Node.move_node_changeset(%{prev_id: get_node_id(prev_node)}) - |> Repo.update() - end - - # no tail recursion but we dont have too much levels in a tree - node - |> get_all_child_nodes() - |> Enum.each(fn child_node -> - delete_node(child_node) - end) - - node - |> Repo.delete() - end - defp get_node_id(nil), do: nil defp get_node_id(%Node{} = node) do diff --git a/lib/radiator/outline/event_consumer.ex b/lib/radiator/outline/event_consumer.ex index 4d7d71a7..da9a5c63 100644 --- a/lib/radiator/outline/event_consumer.ex +++ b/lib/radiator/outline/event_consumer.ex @@ -23,7 +23,7 @@ defmodule Radiator.Outline.EventConsumer do defp process_event(%InsertNodeEvent{payload: payload} = _event) do payload - |> Outline.create_node() + |> Outline.insert_node() |> handle_insert_result() # validate diff --git a/lib/radiator/outline/node_repository.ex b/lib/radiator/outline/node_repository.ex new file mode 100644 index 00000000..2ed58cec --- /dev/null +++ b/lib/radiator/outline/node_repository.ex @@ -0,0 +1,148 @@ +defmodule Radiator.Outline.NodeRepository do + @moduledoc """ + Repository functions for the Node module. + Simple not tree aware node database actions. Mostly used internal and by tests. + """ + import Ecto.Query, warn: false + + alias Radiator.Outline.Node + alias Radiator.Repo + + @doc """ + Creates a node in the repository. + + ## Examples + + iex> create_node(%{field: value}) + {:ok, %Node{}} + + iex> create_node(%{field: bad_value}) + {:error, %Ecto.Changeset{}} + + """ + def create_node(attrs \\ %{}) do + %Node{} + |> Node.insert_changeset(attrs) + |> Repo.insert() + end + + @doc """ + Deletes a node from the repository. + + ## Examples + + iex> delete_node(%{field: value}) + {:ok, %Node{}} + + iex> delete_node(%{field: bad_value}) + {:error, %Ecto.Changeset{}} + + """ + def delete_node(%Node{} = node) do + node + |> Repo.delete() + end + + @doc """ + Returns the list of nodes. + + ## Examples + + iex> list_nodes() + [%Node{}, ...] + + """ + def list_nodes do + Node + |> Repo.all() + end + + @doc """ + Returns the list of nodes for an episode. + + ## Examples + + iex> list_nodes(123) + [%Node{}, ...] + + """ + + def list_nodes_by_episode(episode_id) do + Node + |> where([p], p.episode_id == ^episode_id) + |> 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. + + Returns `nil` if the Node does not exist. + + ## Examples + + iex> get_node(123) + %Node{} + + iex> get_node(456) + nil + + """ + def get_node(id) do + Node + |> 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 a single node. + + Raises `Ecto.NoResultsError` if the Node does not exist. + + ## Examples + + iex> get_node!(123) + %Node{} + + iex> get_node!(456) + ** (Ecto.NoResultsError) + + """ + def get_node!(id) do + Node + |> Repo.get!(id) + end +end diff --git a/lib/radiator_web/controllers/api/outline_controller.ex b/lib/radiator_web/controllers/api/outline_controller.ex index 91889f72..87d7cde4 100644 --- a/lib/radiator_web/controllers/api/outline_controller.ex +++ b/lib/radiator_web/controllers/api/outline_controller.ex @@ -33,7 +33,7 @@ defmodule RadiatorWeb.Api.OutlineController do defp create_node(_, _, nil), do: {:error, :episode} defp create_node(user, content, episode_id) do - Outline.create(%{ + Outline.insert_node(%{ "content" => content, "creator_id" => user.id, "episode_id" => episode_id diff --git a/lib/radiator_web/live/episode_live/index.ex b/lib/radiator_web/live/episode_live/index.ex index 403837a0..97fce833 100644 --- a/lib/radiator_web/live/episode_live/index.ex +++ b/lib/radiator_web/live/episode_live/index.ex @@ -2,6 +2,7 @@ defmodule RadiatorWeb.EpisodeLive.Index do use RadiatorWeb, :live_view alias Radiator.Outline + alias Radiator.Outline.NodeRepository alias Radiator.Podcast alias RadiatorWeb.Endpoint @@ -50,7 +51,7 @@ defmodule RadiatorWeb.EpisodeLive.Index do episode = socket.assigns.selected_episode attrs = Map.merge(params, %{"creator_id" => user.id, "episode_id" => episode.id}) - case Outline.create(attrs, socket.id) do + case Outline.insert_node(attrs) do {:ok, node} -> socket |> reply(:reply, Map.put(node, :temp_id, temp_id)) _ -> socket |> reply(:noreply) end @@ -59,7 +60,7 @@ defmodule RadiatorWeb.EpisodeLive.Index do def handle_event("update_node", %{"uuid" => uuid} = params, socket) do attrs = Map.merge(%{"parent_id" => nil, "prev_id" => nil}, params) - case Outline.get_node(uuid) do + case NodeRepository.get_node(uuid) do nil -> nil node -> Outline.update_node_content(node, attrs, socket.id) end @@ -69,9 +70,9 @@ defmodule RadiatorWeb.EpisodeLive.Index do end def handle_event("delete_node", %{"uuid" => uuid}, socket) do - case Outline.get_node(uuid) do + case NodeRepository.get_node(uuid) do nil -> nil - node -> Outline.delete(node, socket.id) + node -> Outline.remove_node(node, socket.id) end socket @@ -110,6 +111,6 @@ defmodule RadiatorWeb.EpisodeLive.Index do Podcast.get_current_episode_for_show(show_id) end - defp get_nodes(%{id: id}), do: Outline.list_nodes_by_episode(id) + defp get_nodes(%{id: id}), do: NodeRepository.list_nodes_by_episode(id) defp get_nodes(_), do: [] end diff --git a/test/radiator/outline/node_repository_test.exs b/test/radiator/outline/node_repository_test.exs new file mode 100644 index 00000000..0fc179ce --- /dev/null +++ b/test/radiator/outline/node_repository_test.exs @@ -0,0 +1,78 @@ +defmodule Radiator.Outline.NodeRepositoryTest do + use Radiator.DataCase + + alias Radiator.Outline.Node + alias Radiator.Outline.NodeRepository + alias Radiator.PodcastFixtures + + import Radiator.OutlineFixtures + import Ecto.Query, warn: false + + @invalid_attrs %{episode_id: nil} + + describe "create_node/1" do + test "with valid data creates a node" do + episode = PodcastFixtures.episode_fixture() + valid_attrs = %{content: "some content", episode_id: episode.id} + + assert {:ok, %Node{} = node} = NodeRepository.create_node(valid_attrs) + assert node.content == "some content" + end + + test "trims whitespace from content" do + episode = PodcastFixtures.episode_fixture() + valid_attrs = %{content: " some content ", episode_id: episode.id} + + assert {:ok, %Node{} = node} = NodeRepository.create_node(valid_attrs) + assert node.content == "some content" + end + + test "can have a creator" do + episode = PodcastFixtures.episode_fixture() + user = %{id: 2} + valid_attrs = %{content: "some content", episode_id: episode.id, creator_id: user.id} + + assert {:ok, %Node{} = node} = NodeRepository.create_node(valid_attrs) + assert node.content == "some content" + assert node.creator_id == user.id + end + + test "with invalid data returns error changeset" do + assert {:error, %Ecto.Changeset{}} = NodeRepository.create_node(@invalid_attrs) + end + end + + describe "get_node!/1" do + test "returns the node with given id" do + node = node_fixture() + assert NodeRepository.get_node!(node.uuid) == node + end + end + + describe "list_nodes/0" do + test "returns all nodes" do + node1 = node_fixture() + node2 = node_fixture() + + assert NodeRepository.list_nodes() == [node1, node2] + end + end + + describe "list_nodes_by_episode/1" do + test "list_nodes/1 returns only nodes of this episode" do + node1 = node_fixture() + node2 = node_fixture() + + assert NodeRepository.list_nodes_by_episode(node1.episode_id) == [node1] + assert NodeRepository.list_nodes_by_episode(node2.episode_id) == [node2] + end + end + + describe "delete_node/1" do + test "deletes the node" do + node = node_fixture() + assert {:ok, %Node{}} = NodeRepository.delete_node(node) + assert_raise Ecto.NoResultsError, fn -> NodeRepository.get_node!(node.uuid) end + end + end +end diff --git a/test/radiator/outline_test.exs b/test/radiator/outline_test.exs index 992898d9..f1cce3a6 100644 --- a/test/radiator/outline_test.exs +++ b/test/radiator/outline_test.exs @@ -3,69 +3,12 @@ defmodule Radiator.OutlineTest do alias Radiator.Outline alias Radiator.Outline.Node + alias Radiator.Outline.NodeRepository alias Radiator.PodcastFixtures import Radiator.OutlineFixtures import Ecto.Query, warn: false - @invalid_attrs %{episode_id: nil} - - describe "list_nodes/0" do - test "returns all nodes" do - node1 = node_fixture() - node2 = node_fixture() - - assert Outline.list_nodes() == [node1, node2] - end - - test "list_nodes/1 returns only nodes of this episode" do - node1 = node_fixture() - node2 = node_fixture() - - assert Outline.list_nodes_by_episode(node1.episode_id) == [node1] - assert Outline.list_nodes_by_episode(node2.episode_id) == [node2] - end - end - - describe "get_node!/1" do - test "returns the node with given id" do - node = node_fixture() - assert Outline.get_node!(node.uuid) == node - end - end - - describe "create_node/1" do - test "with valid data creates a node" do - episode = PodcastFixtures.episode_fixture() - valid_attrs = %{content: "some content", episode_id: episode.id} - - assert {:ok, %Node{} = node} = Outline.create_node(valid_attrs) - assert node.content == "some content" - end - - test "trims whitespace from content" do - episode = PodcastFixtures.episode_fixture() - valid_attrs = %{content: " some content ", episode_id: episode.id} - - assert {:ok, %Node{} = node} = Outline.create_node(valid_attrs) - assert node.content == "some content" - end - - test "can have a creator" do - episode = PodcastFixtures.episode_fixture() - user = %{id: 2} - valid_attrs = %{content: "some content", episode_id: episode.id, creator_id: user.id} - - assert {:ok, %Node{} = node} = Outline.create_node(valid_attrs, user) - assert node.content == "some content" - assert node.creator_id == user.id - end - - test "with invalid data returns error changeset" do - assert {:error, %Ecto.Changeset{}} = Outline.create_node(@invalid_attrs) - end - end - describe "update_node_content/2" do test "with valid data updates the node" do node = node_fixture() @@ -78,7 +21,7 @@ defmodule Radiator.OutlineTest do test "with invalid data returns error changeset" do node = node_fixture() assert {:error, %Ecto.Changeset{}} = Outline.update_node_content(node, %{"content" => nil}) - assert node == Outline.get_node!(node.uuid) + assert node == NodeRepository.get_node!(node.uuid) end end @@ -117,7 +60,7 @@ defmodule Radiator.OutlineTest do node_3: node_3, nested_node_1: nested_node_1 } do - count_nodes = Outline.count_nodes_by_episode(node_3.episode_id) + count_nodes = NodeRepository.count_nodes_by_episode(node_3.episode_id) node_attrs = %{ "content" => "new node", @@ -127,7 +70,7 @@ defmodule Radiator.OutlineTest do } Outline.insert_node(node_attrs) - new_count_nodes = Outline.count_nodes_by_episode(node_3.episode_id) + new_count_nodes = NodeRepository.count_nodes_by_episode(node_3.episode_id) assert new_count_nodes == count_nodes + 1 end @@ -175,9 +118,9 @@ defmodule Radiator.OutlineTest do {:ok, new_node} = Outline.insert_node(node_attrs) - assert Outline.get_node!(nested_node_2.uuid).prev_id == new_node.uuid + assert NodeRepository.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 + assert NodeRepository.get_node!(nested_node_1.uuid).prev_id == nil end test "inserted node can be inserted at the end", %{ @@ -194,9 +137,9 @@ defmodule Radiator.OutlineTest do {:ok, new_node} = Outline.insert_node(node_attrs) - assert Outline.get_node!(nested_node_2.uuid).prev_id == nested_node_1.uuid + assert NodeRepository.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 + assert NodeRepository.get_node!(nested_node_1.uuid).prev_id == nil end test "without a prev node inserted node will be first in list", %{ @@ -213,8 +156,8 @@ defmodule Radiator.OutlineTest do {: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 - assert Outline.get_node!(nested_node_2.uuid).prev_id == nested_node_1.uuid + assert NodeRepository.get_node!(nested_node_1.uuid).prev_id == new_node.uuid + assert NodeRepository.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", %{ @@ -227,7 +170,7 @@ defmodule Radiator.OutlineTest do assert new_node.prev_id == nil assert new_node.parent_id == nil - assert Outline.get_node!(parent_node.uuid).prev_id == new_node.uuid + assert NodeRepository.get_node!(parent_node.uuid).prev_id == new_node.uuid end test "parent node and prev node need to be consistent", %{ @@ -267,7 +210,7 @@ defmodule Radiator.OutlineTest do parent_node: parent_node, nested_node_1: nested_node_1 } do - count_nodes = Outline.count_nodes_by_episode(parent_node.episode_id) + count_nodes = NodeRepository.count_nodes_by_episode(parent_node.episode_id) node_attrs = %{ "content" => "new node", @@ -277,19 +220,19 @@ defmodule Radiator.OutlineTest do } {:error, _error_message} = Outline.insert_node(node_attrs) - new_count_nodes = Outline.count_nodes_by_episode(parent_node.episode_id) + new_count_nodes = NodeRepository.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 + describe "remove_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 + assert {:ok, %Node{}} = Outline.remove_node(node) + assert_raise Ecto.NoResultsError, fn -> NodeRepository.get_node!(node.uuid) end end test "next node must be updated", %{ @@ -299,10 +242,10 @@ defmodule Radiator.OutlineTest do } do assert node_4.prev_id == node_3.uuid - assert {:ok, %Node{}} = Outline.delete_node(node_3) + assert {:ok, %Node{}} = Outline.remove_node(node_3) # reload nodes - node_4 = Outline.get_node!(node_4.uuid) - node_2 = Outline.get_node!(node_2.uuid) + node_4 = NodeRepository.get_node!(node_4.uuid) + node_2 = NodeRepository.get_node!(node_2.uuid) assert node_4.prev_id == node_2.uuid end @@ -311,9 +254,9 @@ defmodule Radiator.OutlineTest do node_6: node_6 } do episode_id = node_6.episode_id - count_nodes = Outline.count_nodes_by_episode(episode_id) - assert {:ok, %Node{}} = Outline.delete_node(node_6) - new_count_nodes = Outline.count_nodes_by_episode(episode_id) + count_nodes = NodeRepository.count_nodes_by_episode(episode_id) + assert {:ok, %Node{}} = Outline.remove_node(node_6) + new_count_nodes = NodeRepository.count_nodes_by_episode(episode_id) assert new_count_nodes == count_nodes - 1 end @@ -323,12 +266,12 @@ defmodule Radiator.OutlineTest do } do episode_id = node_1.episode_id - count_nodes = Outline.count_nodes_by_episode(episode_id) - assert {:ok, %Node{}} = Outline.delete_node(node_1) - new_count_nodes = Outline.count_nodes_by_episode(episode_id) + count_nodes = NodeRepository.count_nodes_by_episode(episode_id) + assert {:ok, %Node{}} = Outline.remove_node(node_1) + new_count_nodes = NodeRepository.count_nodes_by_episode(episode_id) assert new_count_nodes == count_nodes - 1 - node_2 = Outline.get_node!(node_2.uuid) + node_2 = NodeRepository.get_node!(node_2.uuid) assert node_2.prev_id == nil end @@ -337,10 +280,10 @@ defmodule Radiator.OutlineTest do nested_node_1: nested_node_1, nested_node_2: nested_node_2 } do - assert {:ok, %Node{}} = Outline.delete_node(node_3) + assert {:ok, %Node{}} = Outline.remove_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 + assert_raise Ecto.NoResultsError, fn -> NodeRepository.get_node!(nested_node_1.uuid) end + assert_raise Ecto.NoResultsError, fn -> NodeRepository.get_node!(nested_node_2.uuid) end end test "when top parent gets deleted the whole tree will be gone", %{ @@ -350,13 +293,13 @@ defmodule Radiator.OutlineTest do nested_node_2: nested_node_2, parent_node: parent_node } do - assert {:ok, %Node{}} = Outline.delete_node(parent_node) + assert {:ok, %Node{}} = Outline.remove_node(parent_node) # 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 + assert_raise Ecto.NoResultsError, fn -> NodeRepository.get_node!(node_1.uuid) end + assert_raise Ecto.NoResultsError, fn -> NodeRepository.get_node!(node_4.uuid) end + assert_raise Ecto.NoResultsError, fn -> NodeRepository.get_node!(node_6.uuid) end + assert_raise Ecto.NoResultsError, fn -> NodeRepository.get_node!(nested_node_2.uuid) end end end @@ -367,7 +310,7 @@ defmodule Radiator.OutlineTest 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) + all_nodes = NodeRepository.list_nodes_by_episode(episode_id) assert Enum.count(tree) == Enum.count(all_nodes) diff --git a/test/radiator_web/controllers/api/outline_controller_test.exs b/test/radiator_web/controllers/api/outline_controller_test.exs index 68b90eb1..acacbc08 100644 --- a/test/radiator_web/controllers/api/outline_controller_test.exs +++ b/test/radiator_web/controllers/api/outline_controller_test.exs @@ -4,7 +4,8 @@ defmodule RadiatorWeb.Api.OutlineControllerTest do import Radiator.AccountsFixtures import Radiator.PodcastFixtures - alias Radiator.{Accounts, Outline, Podcast} + alias Radiator.{Accounts, Podcast} + alias Radiator.Outline.NodeRepository describe "POST /api/v1/outline" do setup %{conn: conn} do @@ -31,7 +32,7 @@ defmodule RadiatorWeb.Api.OutlineControllerTest do %{"uuid" => uuid} = json_response(conn, 200) assert %{content: "new node content", creator_id: ^user_id} = - Outline.get_node!(uuid) + NodeRepository.get_node!(uuid) end test "created node is connected to episode", %{ @@ -45,7 +46,9 @@ defmodule RadiatorWeb.Api.OutlineControllerTest do %{"uuid" => uuid} = json_response(conn, 200) episode_id = Podcast.get_current_episode_for_show(show_id).id - assert %{content: "new node content", episode_id: ^episode_id} = Outline.get_node!(uuid) + + assert %{content: "new node content", episode_id: ^episode_id} = + NodeRepository.get_node!(uuid) end test "can't create node when content is missing", %{conn: conn} do diff --git a/test/radiator_web/live/episode_live_test.exs b/test/radiator_web/live/episode_live_test.exs index c927d029..2702898d 100644 --- a/test/radiator_web/live/episode_live_test.exs +++ b/test/radiator_web/live/episode_live_test.exs @@ -6,7 +6,7 @@ defmodule RadiatorWeb.EpisodeLiveTest do import Radiator.PodcastFixtures import Radiator.OutlineFixtures - alias Radiator.Outline + alias Radiator.Outline.NodeRepository describe "Episode page is restricted" do setup do @@ -60,7 +60,7 @@ defmodule RadiatorWeb.EpisodeLiveTest do test "lists all nodes", %{conn: conn, show: show} do {:ok, live, _html} = live(conn, ~p"/admin/podcast/#{show.id}") - nodes = Outline.list_nodes() + nodes = NodeRepository.list_nodes() assert_push_event(live, "list", %{nodes: ^nodes}) end @@ -75,14 +75,12 @@ defmodule RadiatorWeb.EpisodeLiveTest do assert live |> render_hook(:create_node, params) node = - Outline.list_nodes() + NodeRepository.list_nodes() |> Enum.find(&(&1.content == "new node temp content")) node_with_temp_id = Map.put(node, :temp_id, temp_id) assert_reply(live, ^node_with_temp_id) - - # FIXME: assert_push_event(other_live, "insert", ^node) end test "update node", %{conn: conn, show: show, episode: episode} do @@ -99,12 +97,11 @@ defmodule RadiatorWeb.EpisodeLiveTest do assert live |> render_hook(:update_node, params) updated_node = - Outline.list_nodes() + NodeRepository.list_nodes() |> Enum.find(&(&1.content == "update node content")) assert updated_node.uuid == params.uuid assert updated_node.content == params.content - # FIXME: assert_push_event(other_live, "update", ^updated_node) end test "delete node", %{conn: conn, show: show, episode: episode} do @@ -115,9 +112,6 @@ defmodule RadiatorWeb.EpisodeLiveTest do params = Map.from_struct(node) assert live |> render_hook(:delete_node, params) - - # FIXME: assert_push_event(other_live, "delete", %{uuid: deleted_uuid}) - # FIXME: assert deleted_uuid == node.uuid end end end diff --git a/test/support/fixtures/outline_fixtures.ex b/test/support/fixtures/outline_fixtures.ex index 0bc54a54..f7b400ee 100644 --- a/test/support/fixtures/outline_fixtures.ex +++ b/test/support/fixtures/outline_fixtures.ex @@ -3,6 +3,8 @@ defmodule Radiator.OutlineFixtures do This module defines test helpers for creating entities via the `Radiator.Outline` context. """ + + alias Radiator.Outline.NodeRepository alias Radiator.Podcast alias Radiator.PodcastFixtures @@ -18,7 +20,7 @@ defmodule Radiator.OutlineFixtures do content: "some content", episode_id: episode.id }) - |> Radiator.Outline.create_node() + |> NodeRepository.create_node() node end