Sort nodes (#546)

* tests will break

lib/radiator/outline.ex
- insert_node:69

* fix insert_node: the moved node need to have the same parent as before

* fix and test for sorting nodes

* WIP

* `list_nodes_by_episode`

* group by parent id

* mix format

---------

Co-authored-by: sorax <[email protected]>

lib/radiator/outline.ex

@@ -28,20 +28,33 @@ defmodule Radiator.Outline do
   @doc """
   Returns a list of direct child nodes in correct order.
   """
-
   def order_child_nodes(%Node{} = node) do
     node
     |> get_all_child_nodes()
     |> order_sibling_nodes()
   end
 
+  @doc """
+  Orders a given list of nodes by their prev_id.
+  """
   def order_sibling_nodes(nodes) do
     nodes
     |> Enum.map(fn node -> {node.prev_id, node} end)
     |> Map.new()
     |> order_nodes_by_index(nil, [])
   end
 
+  @doc """
+  Returns a list of all child nodes.
+  """
+  def list_nodes_by_episode_sorted(episode_id) do
+    episode_id
+    |> NodeRepository.list_nodes_by_episode()
+    |> Enum.group_by(& &1.parent_id)
+    |> Enum.map(fn {_parent_id, children} -> order_sibling_nodes(children) end)
+    |> List.flatten()
+  end
+
   defp order_nodes_by_index(index, prev_id, collection) do
     case index[prev_id] do
       %{uuid: uuid} = node -> order_nodes_by_index(index, uuid, [node | collection])
@@ -83,7 +96,7 @@ defmodule Radiator.Outline do
            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_if(next_node, nil, node.uuid),
+           {:ok, _node_to_move} <- move_node_if(next_node, parent_node_id, node.uuid),
            {:ok, node} <- move_node_if(node, parent_node_id, prev_node_id) do
         %NodeRepoResult{node: node, next_id: get_node_id(next_node)}
       else
@@ -147,6 +160,7 @@ defmodule Radiator.Outline do
       {:ok, _new_next_node} = move_node_if(new_next_node, new_parent_id, get_node_id(node))
 
       Map.merge(node_repo_result, %{
+        node: node,
         old_next_id: get_node_id(old_next_node),
         old_prev_id: get_node_id(prev_node),
         next_id: get_node_id(new_next_node)

lib/radiator/outline/node_repository.ex

@@ -71,6 +71,9 @@ defmodule Radiator.Outline.NodeRepository do
     Node
     |> where([p], p.episode_id == ^episode_id)
     |> Repo.all()
+    |> Enum.group_by(& &1.parent_id)
+    |> Enum.map(fn {_parent_id, children} -> Radiator.Outline.order_sibling_nodes(children) end)
+    |> List.flatten()
   end
 
   @doc """

lib/radiator_web/live/episode_live/index.ex

@@ -1,7 +1,8 @@
 defmodule RadiatorWeb.EpisodeLive.Index do
   use RadiatorWeb, :live_view
 
-  alias Radiator.Outline.{Dispatch, NodeRepository}
+  alias Radiator.Outline
+  alias Radiator.Outline.Dispatch
 
   alias Radiator.Outline.Event.{
     NodeContentChangedEvent,
@@ -227,7 +228,7 @@ defmodule RadiatorWeb.EpisodeLive.Index do
     []
   end
 
-  defp get_nodes(%{id: id}), do: NodeRepository.list_nodes_by_episode(id)
+  defp get_nodes(%{id: id}), do: Outline.list_nodes_by_episode_sorted(id)
   defp get_nodes(_), do: []
 
   defp generate_event_id(id), do: Ecto.UUID.generate() <> ":" <> id

test/radiator/outline_test.exs

@@ -78,6 +78,24 @@ defmodule Radiator.OutlineTest do
       assert node4.prev_id == node3_1_uuid
     end
 
+    test "the moved node gets moved to its right place", %{
+      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_id" => node_3.uuid,
+        "prev_id" => nested_node_1.uuid
+      }
+
+      {:ok, %{node: new_node}} = Outline.insert_node(node_attrs)
+      nested_node_2 = Repo.reload!(nested_node_2)
+      assert nested_node_2.prev_id == new_node.uuid
+      assert nested_node_2.parent_id == node_3.uuid
+    end
+
     test "creates a new node in the tree", %{
       node_3: node_3,
       nested_node_1: nested_node_1
@@ -681,9 +699,38 @@ defmodule Radiator.OutlineTest do
   describe "order_child_nodes/1" do
     setup :complex_node_fixture
 
-    test "get child nodes in correct order", %{parent_node: parent_node} do
-      assert parent_node |> Outline.order_child_nodes() |> Enum.map(& &1.content) ==
-               ["node_1", "node_2", "node_3", "node_4", "node_5", "node_6"]
+    test "get child nodes in correct order" do
+      episode = PodcastFixtures.episode_fixture()
+
+      node_1 =
+        node_fixture(
+          episode_id: episode.id,
+          parent_id: nil,
+          prev_id: nil,
+          content: "node_1"
+        )
+
+      node_3 =
+        node_fixture(
+          episode_id: episode.id,
+          parent_id: node_1.uuid,
+          prev_id: nil,
+          content: "node_3"
+        )
+
+      node_2 =
+        node_fixture(
+          episode_id: episode.id,
+          parent_id: node_1.uuid,
+          prev_id: node_3.uuid,
+          content: "node_2"
+        )
+
+      {:ok, %NodeRepoResult{} = _result} =
+        Outline.move_node(node_3.uuid, node_2.uuid, node_1.uuid)
+
+      assert node_1 |> Outline.order_child_nodes() |> Enum.map(& &1.content) ==
+               ["node_2", "node_3"]
     end
   end
 

test/radiator_web/live/episode_live_test.exs

@@ -6,6 +6,7 @@ defmodule RadiatorWeb.EpisodeLiveTest do
   import Radiator.PodcastFixtures
   import Radiator.OutlineFixtures
 
+  alias Radiator.Outline
   alias Radiator.Outline.Node
   alias Radiator.Outline.NodeRepository
 
@@ -53,20 +54,46 @@ defmodule RadiatorWeb.EpisodeLiveTest do
       show = show_fixture()
       episode = episode_fixture(%{show_id: show.id})
 
-      node = node_fixture(%{episode_id: episode.id})
+      node_1 =
+        node_fixture(
+          episode_id: episode.id,
+          parent_id: nil,
+          prev_id: nil,
+          content: "node_1"
+        )
+
+      node_3 =
+        node_fixture(
+          episode_id: episode.id,
+          parent_id: node_1.uuid,
+          prev_id: nil,
+          content: "node_3"
+        )
+
+      node_2 =
+        node_fixture(
+          episode_id: episode.id,
+          parent_id: node_1.uuid,
+          prev_id: node_3.uuid,
+          content: "node_2"
+        )
+
+      Outline.move_node(node_3.uuid, node_2.uuid, node_1.uuid)
 
-      %{conn: log_in_user(conn, user), show: show, episode: episode, node: node}
+      %{conn: log_in_user(conn, user), show: show, episode: episode, nodes: [node_3, node_2]}
     end
 
-    test "lists all nodes", %{conn: conn, show: show} do
+    test "lists all nodes", %{conn: conn, show: show, episode: episode} do
       {:ok, live, _html} = live(conn, ~p"/admin/podcast/#{show.id}")
 
-      nodes = NodeRepository.list_nodes()
+      nodes =
+        episode.id
+        |> Outline.list_nodes_by_episode_sorted()
 
       assert_push_event(live, "list", %{nodes: ^nodes})
     end
 
-    test "insert a new node", %{conn: conn, show: show, node: node} do
+    test "insert a new node", %{conn: conn, show: show, nodes: [node | _]} do
       {:ok, live, _html} = live(conn, ~p"/admin/podcast/#{show.id}")
       {:ok, other_live, _html} = live(conn, ~p"/admin/podcast/#{show.id}")
 
@@ -75,7 +102,6 @@ defmodule RadiatorWeb.EpisodeLiveTest do
       params = %{
         "uuid" => uuid,
         "content" => "new node content",
-        # "parent_id" => nil,
         "prev_id" => node.uuid
       }
 
@@ -92,7 +118,7 @@ defmodule RadiatorWeb.EpisodeLiveTest do
       assert_push_event(other_live, "insert", %{node: ^new_node})
     end
 
-    test "update node content", %{conn: conn, show: show, node: %{uuid: uuid}} do
+    test "update node content", %{conn: conn, show: show, nodes: [%{uuid: uuid} | _]} do
       {:ok, live, _html} = live(conn, ~p"/admin/podcast/#{show.id}")
       {:ok, other_live, _html} = live(conn, ~p"/admin/podcast/#{show.id}")
 
@@ -109,8 +135,8 @@ defmodule RadiatorWeb.EpisodeLiveTest do
       assert_push_event(other_live, "change_content", %{node: %{uuid: ^uuid, content: ^content}})
     end
 
-    test "move node", %{conn: conn, show: show, episode: episode, node: node1} do
-      %{uuid: uuid1, parent_id: parent_id1} = node1
+    test "move node", %{conn: conn, show: show, episode: episode, nodes: [node_1 | _]} do
+      %{uuid: uuid1, parent_id: parent_id1} = node_1
 
       uuid2 = Ecto.UUID.generate()
 
@@ -140,7 +166,7 @@ defmodule RadiatorWeb.EpisodeLiveTest do
       })
     end
 
-    test "delete node", %{conn: conn, show: show, node: %{uuid: uuid}} do
+    test "delete node", %{conn: conn, show: show, nodes: [%{uuid: uuid} | _]} do
       {:ok, live, _html} = live(conn, ~p"/admin/podcast/#{show.id}")
       {:ok, other_live, _html} = live(conn, ~p"/admin/podcast/#{show.id}")