[Enhancement] Add sorting, pagination, and new attributes to sources index table (#510)

* WIP - started improving handling of sorting for sources index table

* WIP - Added UI to table to indicate sort column and direction

* Refactored toggle liveview into a livecomponent

* Added sorting for all table attrs

* Added pagination to the sources table

* Added tests for updated liveviews and live components

* Add tests for new helper methods

* Added fancy new CSS to my sources table

* Added size to sources table

* Adds relative div to ensure that sorting arrow doesn't run away

* Fixed da tests

test/pinchflat_web/controllers/sources/index_table_live_test.exs

@@ -1,4 +1,4 @@
-defmodule PinchflatWeb.Sources.IndexTableLiveTest do
+defmodule PinchflatWeb.Sources.SourceLive.IndexTableLiveTest do
   use PinchflatWeb.ConnCase
 
   import Phoenix.LiveViewTest
@@ -6,13 +6,13 @@ defmodule PinchflatWeb.Sources.IndexTableLiveTest do
   import Pinchflat.ProfilesFixtures
 
   alias Pinchflat.Sources.Source
-  alias PinchflatWeb.Sources.IndexTableLive
+  alias PinchflatWeb.Sources.SourceLive.IndexTableLive
 
   describe "initial rendering" do
     test "lists all sources", %{conn: conn} do
       source = source_fixture()
 
-      {:ok, _view, html} = live_isolated(conn, IndexTableLive)
+      {:ok, _view, html} = live_isolated(conn, IndexTableLive, session: create_session())
 
       assert html =~ source.custom_name
     end
@@ -20,7 +20,7 @@ defmodule PinchflatWeb.Sources.IndexTableLiveTest do
     test "omits sources that have marked_for_deletion_at set", %{conn: conn} do
       source = source_fixture(marked_for_deletion_at: DateTime.utc_now())
 
-      {:ok, _view, html} = live_isolated(conn, IndexTableLive)
+      {:ok, _view, html} = live_isolated(conn, IndexTableLive, session: create_session())
 
       refute html =~ source.custom_name
     end
@@ -29,27 +29,102 @@ defmodule PinchflatWeb.Sources.IndexTableLiveTest do
       media_profile = media_profile_fixture(marked_for_deletion_at: DateTime.utc_now())
       source = source_fixture(media_profile_id: media_profile.id)
 
-      {:ok, _view, html} = live_isolated(conn, IndexTableLive)
+      {:ok, _view, html} = live_isolated(conn, IndexTableLive, session: create_session())
 
       refute html =~ source.custom_name
     end
   end
 
-  describe "when a source is enabled or disabled" do
+  describe "when testing sorting" do
+    test "sorts by the custom_name by default", %{conn: conn} do
+      source1 = source_fixture(custom_name: "Source_B")
+      source2 = source_fixture(custom_name: "Source_A")
+
+      {:ok, view, _html} = live_isolated(conn, IndexTableLive, session: create_session())
+      assert render_element(view, "tbody tr:first-child") =~ source2.custom_name
+      assert render_element(view, "tbody tr:last-child") =~ source1.custom_name
+    end
+
+    test "clicking the row will change the sort direction", %{conn: conn} do
+      source1 = source_fixture(custom_name: "Source_B")
+      source2 = source_fixture(custom_name: "Source_A")
+
+      {:ok, view, _html} = live_isolated(conn, IndexTableLive, session: create_session())
+
+      # Click the row to change the sort direction
+      click_element(view, "th", "Name")
+
+      assert render_element(view, "tbody tr:first-child") =~ source1.custom_name
+      assert render_element(view, "tbody tr:last-child") =~ source2.custom_name
+    end
+
+    test "clicking a different row will sort by that attribute", %{conn: conn} do
+      source1 = source_fixture(custom_name: "Source_A", enabled: true)
+      source2 = source_fixture(custom_name: "Source_A", enabled: false)
+
+      {:ok, view, _html} = live_isolated(conn, IndexTableLive, session: create_session())
+
+      # Click the row to change the sort field
+      click_element(view, "th", "Enabled?")
+
+      assert render_element(view, "tbody tr:first-child") =~ source2.custom_name
+      assert render_element(view, "tbody tr:last-child") =~ source1.custom_name
+
+      # Click the row to again change the sort direcation
+      click_element(view, "th", "Enabled?")
+      assert render_element(view, "tbody tr:first-child") =~ source1.custom_name
+      assert render_element(view, "tbody tr:last-child") =~ source2.custom_name
+    end
+  end
+
+  describe "when testing pagination" do
+    test "moving to the next page loads new records", %{conn: conn} do
+      source1 = source_fixture(custom_name: "Source_A")
+      source2 = source_fixture(custom_name: "Source_B")
+
+      session = Map.merge(create_session(), %{"results_per_page" => 1})
+      {:ok, view, _html} = live_isolated(conn, IndexTableLive, session: session)
+
+      assert render_element(view, "tbody") =~ source1.custom_name
+      refute render_element(view, "tbody") =~ source2.custom_name
+
+      click_element(view, "span.pagination-next")
+
+      refute render_element(view, "tbody") =~ source1.custom_name
+      assert render_element(view, "tbody") =~ source2.custom_name
+    end
+  end
+
+  describe "when testing the enable toggle" do
     test "updates the source's enabled status", %{conn: conn} do
       source = source_fixture(enabled: true)
-      {:ok, view, _html} = live_isolated(conn, IndexTableLive)
+      {:ok, view, _html} = live_isolated(conn, IndexTableLive, session: create_session())
 
-      params = %{
-        "event" => "toggle_enabled",
-        "id" => source.id,
-        "value" => "false"
-      }
-
-      # Send an event to the server directly
-      render_change(view, "formless-input", params)
+      view
+      |> element(".enabled_toggle_form")
+      |> render_change(%{source: %{"enabled" => false}})
 
       assert %{enabled: false} = Repo.get!(Source, source.id)
     end
   end
+
+  defp click_element(view, selector, text_filter \\ nil) do
+    view
+    |> element(selector, text_filter)
+    |> render_click()
+  end
+
+  defp render_element(view, selector) do
+    view
+    |> element(selector)
+    |> render()
+  end
+
+  defp create_session do
+    %{
+      "initial_sort_key" => :custom_name,
+      "initial_sort_direction" => :asc,
+      "results_per_page" => 10
+    }
+  end
 end

test/pinchflat_web/controllers/sources/source_enable_toggle_test.exs

@@ -0,0 +1,26 @@
+defmodule PinchflatWeb.Sources.SourceLive.SourceEnableToggleTest do
+  use PinchflatWeb.ConnCase
+
+  import Phoenix.LiveViewTest
+
+  alias PinchflatWeb.Sources.SourceLive.SourceEnableToggle
+
+  describe "initial rendering" do
+    test "renders a toggle in the on position if the source is enabled" do
+      source = %{id: 1, enabled: true}
+
+      html = render_component(SourceEnableToggle, %{id: :foo, source: source})
+
+      # This is checking the Alpine attrs which is a good-enough proxy for the toggle position
+      assert html =~ "{ enabled: true }"
+    end
+
+    test "renders a toggle in the off position if the source is disabled" do
+      source = %{id: 1, enabled: false}
+
+      html = render_component(SourceEnableToggle, %{id: :foo, source: source})
+
+      assert html =~ "{ enabled: false }"
+    end
+  end
+end

test/pinchflat_web/helpers/pagination_helpers_test.exs

@@ -0,0 +1,96 @@
+defmodule PinchflatWeb.Helpers.PaginationHelpersTest do
+  use Pinchflat.DataCase
+  import Pinchflat.SourcesFixtures
+
+  alias Pinchflat.Sources.Source
+  alias PinchflatWeb.Helpers.PaginationHelpers
+
+  describe "get_pagination_attributes/3" do
+    test "returns the correct pagination attributes" do
+      source_fixture()
+      query = from(s in Source, select: s.id)
+      page = 1
+      records_per_page = 10
+
+      pagination_attributes = PaginationHelpers.get_pagination_attributes(query, page, records_per_page)
+
+      assert pagination_attributes.page == 1
+      assert pagination_attributes.total_pages == 1
+      assert pagination_attributes.total_record_count == 1
+      assert pagination_attributes.limit == 10
+      assert pagination_attributes.offset == 0
+    end
+
+    test "returns the correct pagination attributes when there are multiple pages" do
+      source_fixture()
+      source_fixture()
+
+      query = from(s in Source, select: s.id)
+      page = 1
+      records_per_page = 1
+
+      pagination_attributes = PaginationHelpers.get_pagination_attributes(query, page, records_per_page)
+
+      assert pagination_attributes.page == 1
+      assert pagination_attributes.total_pages == 2
+      assert pagination_attributes.total_record_count == 2
+      assert pagination_attributes.limit == 1
+      assert pagination_attributes.offset == 0
+    end
+
+    test "returns the correct attributes when on a page other than the first" do
+      source_fixture()
+      source_fixture()
+
+      query = from(s in Source, select: s.id)
+      page = 2
+      records_per_page = 1
+
+      pagination_attributes = PaginationHelpers.get_pagination_attributes(query, page, records_per_page)
+
+      assert pagination_attributes.page == 2
+      assert pagination_attributes.total_pages == 2
+      assert pagination_attributes.total_record_count == 2
+      assert pagination_attributes.limit == 1
+      assert pagination_attributes.offset == 1
+    end
+  end
+
+  describe "update_page_number/3" do
+    test "increments the page number" do
+      current_page = 1
+      total_pages = 2
+
+      updated_page = PaginationHelpers.update_page_number(current_page, :inc, total_pages)
+
+      assert updated_page == 2
+    end
+
+    test "decrements the page number" do
+      current_page = 2
+      total_pages = 2
+
+      updated_page = PaginationHelpers.update_page_number(current_page, :dec, total_pages)
+
+      assert updated_page == 1
+    end
+
+    test "doesn't overflow the page number" do
+      current_page = 2
+      total_pages = 2
+
+      updated_page = PaginationHelpers.update_page_number(current_page, :inc, total_pages)
+
+      assert updated_page == 2
+    end
+
+    test "doesn't underflow the page number" do
+      current_page = 1
+      total_pages = 2
+
+      updated_page = PaginationHelpers.update_page_number(current_page, :dec, total_pages)
+
+      assert updated_page == 1
+    end
+  end
+end

test/pinchflat_web/helpers/sorting_helpers_test.exs

@@ -0,0 +1,31 @@
+defmodule PinchflatWeb.Helpers.SortingHelpersTest do
+  use Pinchflat.DataCase
+
+  alias PinchflatWeb.Helpers.SortingHelpers
+
+  describe "get_sort_direction/3" do
+    test "returns the correct sort direction when the new sort attribute is the same as the old sort attribute" do
+      old_sort_attr = "name"
+      new_sort_attr = "name"
+      old_sort_direction = :desc
+
+      assert SortingHelpers.get_sort_direction(old_sort_attr, new_sort_attr, old_sort_direction) == :asc
+    end
+
+    test "returns the correct sort direction when the new sort attribute is the same as the old sort attribute in the other direction" do
+      old_sort_attr = "name"
+      new_sort_attr = "name"
+      old_sort_direction = :asc
+
+      assert SortingHelpers.get_sort_direction(old_sort_attr, new_sort_attr, old_sort_direction) == :desc
+    end
+
+    test "returns the correct sort direction when the new sort attribute is different from the old sort attribute" do
+      old_sort_attr = "name"
+      new_sort_attr = "date"
+      old_sort_direction = :asc
+
+      assert SortingHelpers.get_sort_direction(old_sort_attr, new_sort_attr, old_sort_direction) == :asc
+    end
+  end
+end