From c720302221f07bfd5d0daf19e7c0776c418ae14c Mon Sep 17 00:00:00 2001 From: Mika Cohen Date: Mon, 29 Jun 2026 14:31:32 -0600 Subject: [PATCH 01/12] Add shared Flop list pagination --- .gitignore | 2 + AGENTS.md | 4 + lib/reencodarr_web.ex | 1 + lib/reencodarr_web/live/flop_list.ex | 228 ++++++++++++++++++ lib/reencodarr_web/live/list_pagination.ex | 15 -- test/reencodarr/flop_fixtures_test.exs | 37 +++ .../flop_list_test_helpers_test.exs | 25 ++ test/reencodarr_web/live/flop_list_test.exs | 142 +++++++++++ .../live/list_pagination_test.exs | 111 --------- test/support/flop_fixtures.ex | 41 ++++ test/support/flop_list_test_helpers.ex | 57 +++++ 11 files changed, 537 insertions(+), 126 deletions(-) create mode 100644 lib/reencodarr_web/live/flop_list.ex delete mode 100644 lib/reencodarr_web/live/list_pagination.ex create mode 100644 test/reencodarr/flop_fixtures_test.exs create mode 100644 test/reencodarr_web/flop_list_test_helpers_test.exs create mode 100644 test/reencodarr_web/live/flop_list_test.exs delete mode 100644 test/reencodarr_web/live/list_pagination_test.exs create mode 100644 test/support/flop_fixtures.ex create mode 100644 test/support/flop_list_test_helpers.ex diff --git a/.gitignore b/.gitignore index 41695964..b7a7ba40 100644 --- a/.gitignore +++ b/.gitignore @@ -44,3 +44,5 @@ config/dev.overrides.exs # Log files /logs/ .expert/ + +.envrc diff --git a/AGENTS.md b/AGENTS.md index 34329ad0..256c8f99 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -62,6 +62,10 @@ - Use `meck` where the current test suite already uses it for external command mocking. - When changing sync, parser, state-machine, or LiveView behavior, add or update tests near the affected module. +## List LiveViews (Flop) +- Paginated list pages (`VideosLive`, `FailuresLive`, `BadFilesLive`) use `handle_params`, URL query params, and `ReencodarrWeb.Live.FlopList` (`flop_pagination`, `patch_with_page`, `parse_page`). +- List APIs return `{items, %Flop.Meta{}}`: `Media.list_videos_paginated/1`, `Media.list_failures/1`, `Media.list_bad_file_issues/2`. + ## File/Module Pointers - `lib/reencodarr/sync.ex` - Sonarr/Radarr sync and batch upserts. - `lib/reencodarr/media/video_upsert.ex` - guarded upsert logic, bitrate/VMAF handling. diff --git a/lib/reencodarr_web.ex b/lib/reencodarr_web.ex index d01e3dc0..04841e93 100644 --- a/lib/reencodarr_web.ex +++ b/lib/reencodarr_web.ex @@ -85,6 +85,7 @@ defmodule ReencodarrWeb do import Phoenix.HTML # Core UI components and translation import ReencodarrWeb.CoreComponents + import ReencodarrWeb.Live.FlopList import Flop.Phoenix, except: [table: 1] use Gettext, backend: ReencodarrWeb.Gettext diff --git a/lib/reencodarr_web/live/flop_list.ex b/lib/reencodarr_web/live/flop_list.ex new file mode 100644 index 00000000..96744d0d --- /dev/null +++ b/lib/reencodarr_web/live/flop_list.ex @@ -0,0 +1,228 @@ +defmodule ReencodarrWeb.Live.FlopList do + @moduledoc """ + Shared LiveView helpers for Flop-backed list pagination. + + The list LiveViews keep their filters in URL params and pass Flop metadata to + this module for consistent labels, page links, and page parsing. + """ + + use Phoenix.Component + + import Flop.Phoenix + + alias Reencodarr.Core.Parsers + + attr :id, :string, default: "flop-pagination" + attr :meta, Flop.Meta, required: true + attr :path, :any, default: nil + attr :base_path, :string, default: nil + attr :query, :map, default: %{} + attr :mode, :atom, default: :simple, values: [:simple, :full] + attr :page_links, :integer, default: 5 + attr :target, :string, default: nil + attr :class, :string, default: nil + + @doc """ + Renders pagination controls for a `%Flop.Meta{}` result. + + Pass either a Flop `:path` callback or a `:base_path` plus current `:query`. + The `:simple` mode renders previous/next links only; `:full` also renders page + number links. + """ + def flop_pagination(assigns) do + path = pagination_path(assigns) + page_links = if assigns.mode == :simple, do: :none, else: assigns.page_links + + assigns = + assign(assigns, + path: path, + page_links: page_links, + label: pagination_label(assigns.meta), + nav_id: "#{assigns.id}-nav" + ) + + ~H""" +
+ {@label} + <.pagination + meta={@meta} + path={@path} + target={@target} + page_links={@page_links} + page_link_attrs={page_link_attrs()} + current_page_link_attrs={current_page_link_attrs()} + disabled_link_attrs={disabled_link_attrs()} + id={@nav_id} + data-role="flop-pagination-nav" + > + <:previous attrs={nav_button_attrs("flop-pagination-prev")}>Previous + <:next attrs={nav_button_attrs("flop-pagination-next")}>Next + +
+ """ + end + + @doc """ + Builds the compact result range label shown beside pagination controls. + """ + @spec pagination_label(Flop.Meta.t()) :: String.t() + def pagination_label(%Flop.Meta{} = meta) do + per_page = meta.page_size || 1 + total = meta.total_count || 0 + page = meta.current_page || 1 + + page = + if total > 0 do + page |> max(1) |> min(total_pages(total, per_page)) + else + max(page, 1) + end + + pagination_range_label(page, per_page, total) + end + + @doc """ + Returns the number of pages needed for a result count and page size. + + Empty result sets still return `1` so URL parsing and LiveView assigns have a + stable page value. + """ + @spec total_pages(non_neg_integer(), pos_integer()) :: pos_integer() + def total_pages(total, _per_page) when total <= 0, do: 1 + def total_pages(total, per_page), do: max(ceil(total / per_page), 1) + + @doc """ + Parses a positive page number from request params. + + When `:total` and `:per_page` are provided, the page is clamped to the + available range. + """ + @spec parse_page(map(), pos_integer(), keyword()) :: pos_integer() + def parse_page(params, default, opts \\ []) do + page = params |> Map.get("page", "#{default}") |> Parsers.parse_int(default) |> max(1) + + with total when is_integer(total) <- Keyword.get(opts, :total), + per_page when is_integer(per_page) <- Keyword.get(opts, :per_page) do + page |> max(1) |> min(total_pages(total, per_page)) + else + _ -> page + end + end + + @doc """ + Parses a page size from request params and falls back unless it is allowed. + """ + @spec parse_per_page(map(), pos_integer(), [pos_integer()]) :: pos_integer() + def parse_per_page(params, default, allowed) do + params + |> Map.get("per_page", "#{default}") + |> Parsers.parse_int(default) + |> then(&if(&1 in allowed, do: &1, else: default)) + end + + @doc """ + Returns normalized pagination assigns from request params. + """ + @spec pagination_assigns(map(), pos_integer(), [pos_integer()], pos_integer()) :: %{ + page: pos_integer(), + per_page: pos_integer() + } + def pagination_assigns(params, default_per_page, allowed_per_pages, default_page \\ 1) do + %{ + page: parse_page(params, default_page), + per_page: parse_per_page(params, default_per_page, allowed_per_pages) + } + end + + defp pagination_range_label(page, per_page, total) when total > 0 do + first = (page - 1) * per_page + 1 + last = min(page * per_page, total) + "#{first}-#{last} of #{total}" + end + + defp pagination_range_label(_, _, _), do: "0 results" + + @doc """ + Flop path callback that preserves the current list query while changing page. + """ + @spec flop_page_path(String.t(), map(), keyword()) :: String.t() + def flop_page_path(base_path, query, flop_params) do + page = Keyword.get(flop_params, :page, 1) + patch_with_page(base_path, query, page) + end + + @doc """ + Returns a LiveView patch path with `page` merged into existing query params. + + Page `1` is omitted from the URL so the canonical first page stays clean. + """ + @spec patch_with_page(String.t(), map(), pos_integer()) :: String.t() + def patch_with_page(base_path, query, page) when is_binary(base_path) and is_map(query) do + query + |> stringify_query() + |> maybe_put_page(page) + |> drop_empty() + |> case do + %{} = params when map_size(params) == 0 -> base_path + params -> base_path <> "?" <> URI.encode_query(params) + end + end + + defp pagination_path(%{base_path: base_path, path: nil, query: query}) + when is_binary(base_path) do + {__MODULE__, :flop_page_path, [base_path, query]} + end + + defp pagination_path(%{path: path}) when not is_nil(path), do: path + + defp pagination_path(_assigns) do + raise ArgumentError, "flop_pagination requires :path or :base_path" + end + + defp stringify_query(query) do + Map.new(query, fn {key, value} -> {to_string(key), value} end) + end + + defp maybe_put_page(query, 1), do: Map.delete(query, "page") + defp maybe_put_page(query, page), do: Map.put(query, "page", to_string(page)) + + defp drop_empty(query) do + Map.reject(query, fn {_key, value} -> value in [nil, ""] end) + end + + defp nav_button_attrs(role) do + [ + class: nav_button_classes(), + "data-role": role + ] + end + + defp nav_button_classes, + do: + "px-3 py-1 bg-gray-700 rounded text-gray-300 hover:bg-gray-600 disabled:opacity-40 disabled:cursor-not-allowed" + + defp page_link_attrs, + do: [ + class: page_link_classes(), + "data-role": "flop-pagination-page" + ] + + defp current_page_link_attrs, + do: [ + class: current_page_link_classes(), + "data-role": "flop-pagination-page" + ] + + defp disabled_link_attrs, do: [class: "opacity-40 cursor-not-allowed"] + + defp page_link_classes, + do: + "px-3 py-1.5 text-sm font-medium rounded text-gray-300 bg-gray-700 border border-gray-600 hover:bg-gray-600 transition-colors" + + defp current_page_link_classes, + do: "px-3 py-1.5 text-sm font-medium rounded text-white bg-blue-600 border border-blue-600" +end diff --git a/lib/reencodarr_web/live/list_pagination.ex b/lib/reencodarr_web/live/list_pagination.ex deleted file mode 100644 index 1cb046f3..00000000 --- a/lib/reencodarr_web/live/list_pagination.ex +++ /dev/null @@ -1,15 +0,0 @@ -defmodule ReencodarrWeb.Live.ListPagination do - @moduledoc false - - @spec max_page(non_neg_integer(), pos_integer()) :: pos_integer() - def max_page(total, per_page), do: max(ceil(total / per_page), 1) - - @spec pagination_label(pos_integer(), pos_integer(), non_neg_integer()) :: String.t() - def pagination_label(page, per_page, total) when total > 0 do - first = (page - 1) * per_page + 1 - last = min(page * per_page, total) - "#{first}-#{last} of #{total}" - end - - def pagination_label(_, _, _), do: "0 results" -end diff --git a/test/reencodarr/flop_fixtures_test.exs b/test/reencodarr/flop_fixtures_test.exs new file mode 100644 index 00000000..d521919a --- /dev/null +++ b/test/reencodarr/flop_fixtures_test.exs @@ -0,0 +1,37 @@ +defmodule Reencodarr.FlopFixturesTest do + use ExUnit.Case, async: true + + alias Reencodarr.FlopFixtures + alias Reencodarr.Media.Video + + test "meta_fixture/1 defaults to empty first page" do + meta = FlopFixtures.meta_fixture() + + assert meta.schema == Video + assert meta.current_page == 1 + assert meta.page_size == 20 + assert meta.total_count == 0 + assert meta.total_pages == 0 + refute meta.has_next_page? + refute meta.has_previous_page? + end + + test "meta_fixture/1 builds multi-page meta" do + meta = FlopFixtures.meta_fixture(page: 2, page_size: 10, total_count: 25) + + assert meta.current_page == 2 + assert meta.total_pages == 3 + assert meta.has_previous_page? + assert meta.has_next_page? + assert meta.previous_page == 1 + assert meta.next_page == 3 + end + + test "meta_fixture/1 clamps page beyond total" do + meta = FlopFixtures.meta_fixture(page: 99, page_size: 10, total_count: 15) + + assert meta.current_page == 2 + assert meta.total_pages == 2 + refute meta.has_next_page? + end +end diff --git a/test/reencodarr_web/flop_list_test_helpers_test.exs b/test/reencodarr_web/flop_list_test_helpers_test.exs new file mode 100644 index 00000000..d7d9df2a --- /dev/null +++ b/test/reencodarr_web/flop_list_test_helpers_test.exs @@ -0,0 +1,25 @@ +defmodule ReencodarrWeb.FlopListTestHelpersTest do + use ExUnit.Case, async: true + + alias Reencodarr.FlopFixtures + alias ReencodarrWeb.FlopListTestHelpers + + test "current_page_from_meta/1 reads flop meta page" do + meta = FlopFixtures.meta_fixture(page: 3, page_size: 20, total_count: 60) + assert FlopListTestHelpers.current_page_from_meta(meta) == 3 + end + + test "assert_filter_in_url/3 matches query params" do + assert :ok = + FlopListTestHelpers.assert_filter_in_url( + "/failures?stage=analysis&search=foo", + "stage", + "analysis" + ) + end + + test "pagination_label_from_html/1 parses flop label" do + html = ~s(1-20 of 42) + assert FlopListTestHelpers.pagination_label_from_html(html) == "1-20 of 42" + end +end diff --git a/test/reencodarr_web/live/flop_list_test.exs b/test/reencodarr_web/live/flop_list_test.exs new file mode 100644 index 00000000..08df445b --- /dev/null +++ b/test/reencodarr_web/live/flop_list_test.exs @@ -0,0 +1,142 @@ +defmodule ReencodarrWeb.Live.FlopListTest do + use ExUnit.Case, async: true + + import Phoenix.LiveViewTest + + alias Reencodarr.FlopFixtures + alias ReencodarrWeb.Live.FlopList + + describe "flop_pagination/1" do + test "simple mode renders label and prev/next without page numbers" do + meta = FlopFixtures.meta_fixture(page: 2, page_size: 50, total_count: 120) + + html = + render_component(&FlopList.flop_pagination/1, %{ + id: "videos-flop-pagination", + meta: meta, + path: "/videos", + mode: :simple + }) + + assert html =~ "id=\"videos-flop-pagination\"" + assert html =~ ~s(data-role="flop-pagination-label") + assert html =~ "51-100 of 120" + assert html =~ ~s(data-role="flop-pagination-prev") + assert html =~ ~s(data-role="flop-pagination-next") + assert html =~ "bg-gray-700" + refute html =~ ~s(aria-current="page") + end + + test "base_path builds patch links preserving custom query params" do + meta = FlopFixtures.meta_fixture(page: 1, page_size: 50, total_count: 120) + + html = + render_component(&FlopList.flop_pagination/1, %{ + meta: meta, + base_path: "/videos", + query: %{ + "sort_by" => "updated_at", + "sort_dir" => "desc", + "per_page" => 50 + }, + mode: :simple + }) + + assert html =~ "sort_by=updated_at" + assert html =~ "sort_dir=desc" + assert html =~ "per_page=50" + end + + test "full mode renders numbered page links" do + meta = FlopFixtures.meta_fixture(page: 5, page_size: 20, total_count: 200) + + html = + render_component(&FlopList.flop_pagination/1, %{ + id: "failures-flop-pagination", + meta: meta, + path: "/failures", + mode: :full, + page_links: 5 + }) + + assert html =~ "id=\"failures-flop-pagination\"" + assert html =~ ~s(data-role="flop-pagination-page") + assert html =~ "bg-blue-600" + end + + test "simple mode disables previous on first page" do + meta = FlopFixtures.meta_fixture(page: 1, page_size: 10, total_count: 25) + + html = + render_component(&FlopList.flop_pagination/1, %{ + meta: meta, + path: "/videos", + mode: :simple + }) + + assert html =~ "disabled" + assert html =~ "opacity-40" + end + end + + describe "pagination_label/1" do + test "delegates to range label from meta" do + meta = FlopFixtures.meta_fixture(page: 1, page_size: 10, total_count: 5) + + assert FlopList.pagination_label(meta) == "1-5 of 5" + end + + test "clamps oversized current_page before building the label" do + meta = %Flop.Meta{current_page: 999_999, page_size: 10, total_count: 25} + + assert FlopList.pagination_label(meta) == "21-25 of 25" + end + end + + describe "patch_with_page/3" do + test "omits page param on first page" do + assert FlopList.patch_with_page("/videos", %{"per_page" => 50}, 1) == + "/videos?per_page=50" + end + + test "includes page param when not on first page" do + assert FlopList.patch_with_page("/videos", %{"per_page" => 50}, 2) == + "/videos?page=2&per_page=50" + end + + test "returns bare path when merged query is empty" do + assert FlopList.patch_with_page("/bad-files", %{}, 1) == "/bad-files" + end + end + + describe "parse_page/3" do + test "clamps page to total pages when total and per_page given" do + assert FlopList.parse_page(%{"page" => "99"}, 1, total: 25, per_page: 10) == 3 + end + end + + describe "parse_per_page/3" do + test "falls back to default for invalid per_page" do + assert FlopList.parse_per_page(%{"per_page" => "13"}, 25, [25, 50, 100]) == 25 + end + end + + describe "pagination_assigns/4" do + test "returns assign-ready page and per_page values" do + assert FlopList.pagination_assigns(%{"page" => "4", "per_page" => "50"}, 25, [25, 50]) == + %{page: 4, per_page: 50} + end + + test "coerces invalid values to safe defaults" do + assert FlopList.pagination_assigns(%{"page" => "0", "per_page" => "13"}, 25, [25, 50]) == + %{page: 1, per_page: 25} + end + end + + describe "total_pages/2" do + test "returns at least one page" do + assert FlopList.total_pages(0, 10) == 1 + assert FlopList.total_pages(21, 10) == 3 + end + end +end diff --git a/test/reencodarr_web/live/list_pagination_test.exs b/test/reencodarr_web/live/list_pagination_test.exs deleted file mode 100644 index d9ef8486..00000000 --- a/test/reencodarr_web/live/list_pagination_test.exs +++ /dev/null @@ -1,111 +0,0 @@ -defmodule ReencodarrWeb.Live.ListPaginationTest do - @moduledoc """ - Tests for the ListPagination helper module. - """ - use ExUnit.Case, async: true - - alias ReencodarrWeb.Live.ListPagination - - describe "max_page/2" do - test "returns 1 for zero total items" do - assert ListPagination.max_page(0, 10) == 1 - end - - test "returns 1 when items fit in one page" do - assert ListPagination.max_page(10, 10) == 1 - end - - test "returns 1 for fewer items than per_page" do - assert ListPagination.max_page(5, 10) == 1 - end - - test "returns 2 when one item overflows to second page" do - assert ListPagination.max_page(11, 10) == 2 - end - - test "returns 2 when items fill exactly two pages" do - assert ListPagination.max_page(20, 10) == 2 - end - - test "returns 3 when one item overflows to third page" do - assert ListPagination.max_page(21, 10) == 3 - end - - test "returns correct page count for large datasets" do - assert ListPagination.max_page(100, 10) == 10 - assert ListPagination.max_page(250, 50) == 5 - assert ListPagination.max_page(1000, 25) == 40 - end - - test "always returns at least 1 even with 0 items" do - assert ListPagination.max_page(0, 1) >= 1 - assert ListPagination.max_page(0, 100) >= 1 - end - - test "handles single item per page" do - assert ListPagination.max_page(1, 1) == 1 - assert ListPagination.max_page(5, 1) == 5 - assert ListPagination.max_page(10, 1) == 10 - end - - test "result is always a positive integer" do - for total <- [0, 1, 5, 10, 100, 999], - per_page <- [1, 10, 25, 50, 100] do - result = ListPagination.max_page(total, per_page) - assert is_integer(result), "Expected integer for max_page(#{total}, #{per_page})" - assert result >= 1, "Expected >= 1 for max_page(#{total}, #{per_page})" - end - end - end - - describe "pagination_label/3" do - test "returns '0 results' when total is 0" do - assert ListPagination.pagination_label(1, 10, 0) == "0 results" - end - - test "returns range for first page with full page of items" do - assert ListPagination.pagination_label(1, 10, 10) == "1-10 of 10" - end - - test "returns range for first page with partial items" do - assert ListPagination.pagination_label(1, 10, 5) == "1-5 of 5" - end - - test "returns correct range for second page" do - assert ListPagination.pagination_label(2, 10, 15) == "11-15 of 15" - end - - test "returns correct range for second full page" do - assert ListPagination.pagination_label(2, 10, 20) == "11-20 of 20" - end - - test "last page with partial items shows correct range" do - assert ListPagination.pagination_label(3, 10, 25) == "21-25 of 25" - end - - test "first item on a page is always (page-1)*per_page + 1" do - # Page 1: first = 1 - assert ListPagination.pagination_label(1, 25, 30) =~ "1-" - # Page 2: first = 26 - assert ListPagination.pagination_label(2, 25, 30) =~ "26-" - end - - test "total is always shown correctly after 'of'" do - assert ListPagination.pagination_label(1, 10, 42) =~ "of 42" - assert ListPagination.pagination_label(2, 10, 42) =~ "of 42" - end - - test "last item does not exceed total" do - # Page 5, per_page 10, total 42 → should show 41-42 not 41-50 - assert ListPagination.pagination_label(5, 10, 42) == "41-42 of 42" - end - - test "with per_page=20 defaults as in failures_live" do - # Matches FailuresLive @per_page default - assert ListPagination.pagination_label(1, 20, 0) == "0 results" - assert ListPagination.pagination_label(1, 20, 15) == "1-15 of 15" - assert ListPagination.pagination_label(1, 20, 20) == "1-20 of 20" - assert ListPagination.pagination_label(2, 20, 25) == "21-25 of 25" - end - end -end diff --git a/test/support/flop_fixtures.ex b/test/support/flop_fixtures.ex new file mode 100644 index 00000000..c6696be9 --- /dev/null +++ b/test/support/flop_fixtures.ex @@ -0,0 +1,41 @@ +defmodule Reencodarr.FlopFixtures do + @moduledoc false + + alias Reencodarr.Media.Video + + @doc """ + Builds a `%Flop.Meta{}` for component and LiveView tests without hitting the DB. + """ + @spec meta_fixture(keyword()) :: Flop.Meta.t() + def meta_fixture(opts \\ []) do + page = Keyword.get(opts, :page, 1) + page_size = Keyword.get(opts, :page_size, 20) + total_count = Keyword.get(opts, :total_count, 0) + total_pages = total_pages(total_count, page_size) + current_page = min(max(page, 1), max(total_pages, 1)) + + %Flop.Meta{ + schema: Keyword.get(opts, :schema, Video), + current_page: current_page, + page_size: page_size, + total_count: total_count, + total_pages: total_pages, + previous_page: if(current_page > 1, do: current_page - 1), + next_page: if(current_page < total_pages, do: current_page + 1), + has_previous_page?: current_page > 1, + has_next_page?: current_page < total_pages, + flop: %Flop{ + offset: (current_page - 1) * page_size, + limit: page_size, + page: current_page, + page_size: page_size + } + } + end + + defp total_pages(0, _page_size), do: 0 + + defp total_pages(total_count, page_size) when total_count > 0 do + div(total_count + page_size - 1, page_size) + end +end diff --git a/test/support/flop_list_test_helpers.ex b/test/support/flop_list_test_helpers.ex new file mode 100644 index 00000000..9c7e3d83 --- /dev/null +++ b/test/support/flop_list_test_helpers.ex @@ -0,0 +1,57 @@ +defmodule ReencodarrWeb.FlopListTestHelpers do + @moduledoc false + + import Phoenix.LiveViewTest + + @spec pagination_label_from_html(String.t()) :: String.t() | nil + def pagination_label_from_html(html) do + case Regex.run(~r/data-role="flop-pagination-label"[^>]*>([^<]+)/, html) do + [_, label] -> String.trim(label) + _ -> nil + end + end + + @spec current_page_from_html(String.t()) :: pos_integer() | nil + def current_page_from_html(html) do + case Regex.run(~r/aria-current="page"[^>]*>\s*(\d+)/s, html) do + [_, page] -> String.to_integer(page) + _ -> if String.contains?(html, ~s(data-role="flop-pagination")), do: 1, else: nil + end + end + + @spec current_page_from_meta(Flop.Meta.t()) :: pos_integer() + def current_page_from_meta(%Flop.Meta{current_page: page}) when is_integer(page), do: page + def current_page_from_meta(_meta), do: 1 + + @spec total_pages_from_html(String.t()) :: pos_integer() + def total_pages_from_html(html) do + case Regex.scan(~r/data-role="flop-pagination-page"/, html) do + [] -> 1 + pages -> length(pages) + end + end + + @spec assert_filter_in_url(String.t(), String.t(), String.t()) :: :ok + def assert_filter_in_url(path, key, value) do + query = path |> URI.parse() |> Map.get(:query, "") |> URI.decode_query() + + if Map.get(query, key) != value do + raise ExUnit.AssertionError, + "expected #{inspect(key)}=#{inspect(value)} in #{path}, got #{inspect(query)}" + end + + :ok + end + + @spec assert_flop_patch(Phoenix.LiveViewTest.view(), String.t()) :: :ok + def assert_flop_patch(view, expected_path) do + assert_patch(view, expected_path) + end + + @spec click_flop_next(Phoenix.LiveViewTest.view()) :: String.t() + def click_flop_next(view) do + view + |> element("a[data-role='flop-pagination-next']") + |> render_click() + end +end From d1ef9027d4f28ed8210daa93720970a31126f839 Mon Sep 17 00:00:00 2001 From: Mika Cohen Date: Mon, 29 Jun 2026 14:31:55 -0600 Subject: [PATCH 02/12] Move list queries to Flop-backed media APIs --- lib/reencodarr/bad_files/state.ex | 96 +++--- lib/reencodarr/media.ex | 286 ++++++++++++++---- lib/reencodarr/media/bad_file_issue.ex | 12 + test/reencodarr/bad_file_issue_test.exs | 3 +- .../media/list_bad_file_issues_test.exs | 48 +++ test/reencodarr/media/list_failures_test.exs | 75 +++++ 6 files changed, 415 insertions(+), 105 deletions(-) create mode 100644 test/reencodarr/media/list_bad_file_issues_test.exs create mode 100644 test/reencodarr/media/list_failures_test.exs diff --git a/lib/reencodarr/bad_files/state.ex b/lib/reencodarr/bad_files/state.ex index 196906e6..e80cbb66 100644 --- a/lib/reencodarr/bad_files/state.ex +++ b/lib/reencodarr/bad_files/state.ex @@ -9,21 +9,16 @@ defmodule Reencodarr.BadFiles.State do @spec load(map()) :: map() def load(assigns) do - filters = [ - service: assigns.service_filter, - kind: assigns.kind_filter, - search: assigns.search_query - ] - {active_statuses, resolved_statuses} = statuses_for_filter(assigns.status_filter) - active_issues = fetch_active_issues(filters, active_statuses, assigns) - active_total = fetch_active_total(filters, active_statuses) + {active_issues, meta} = fetch_active_issues(active_statuses, assigns) + active_total = meta.total_count || 0 issue_summary = Media.bad_file_issue_summary() - resolved_issues = fetch_resolved_issues(filters, resolved_statuses, assigns.show_resolved) + resolved_issues = fetch_resolved_issues(assigns, resolved_statuses, assigns.show_resolved) issues = active_issues ++ resolved_issues %{ issues: issues, + meta: meta, tracked_count: issue_summary.open + issue_summary.queued + issue_summary.processing + issue_summary.waiting_for_replacement + issue_summary.failed + issue_summary.resolved, @@ -36,48 +31,69 @@ defmodule Reencodarr.BadFiles.State do } end - defp fetch_active_issues(_filters, [], _assigns), do: [] - - defp fetch_active_issues(filters, active_statuses, assigns) do - Media.list_bad_file_issues( - filters ++ - [ - statuses: active_statuses, - limit: assigns.per_page, - offset: (assigns.page - 1) * assigns.per_page - ] - ) + def active_statuses_for_filter("all"), do: @active_statuses + def active_statuses_for_filter("resolved"), do: [] + + def active_statuses_for_filter(status_filter) do + status = String.to_existing_atom(status_filter) + + if status in @active_statuses, do: [status], else: @active_statuses + rescue + ArgumentError -> @active_statuses end - defp fetch_active_total(_filters, []), do: 0 + def list_active_issues(assigns, opts \\ []) do + assigns + |> Map.put(:page, Keyword.get(opts, :page, 1)) + |> Map.put(:per_page, Keyword.get(opts, :per_page, 250)) + |> list_issues(active_statuses_for_filter(assigns.status_filter)) + |> elem(0) + end - defp fetch_active_total(filters, active_statuses) do - Media.count_bad_file_issues(Keyword.put_new(filters, :statuses, active_statuses)) + def flop_params(assigns) do + %{ + "page" => to_string(assigns.page), + "page_size" => to_string(assigns.per_page), + "service" => assigns.service_filter, + "kind" => assigns.kind_filter, + "search" => assigns.search_query + } end - defp fetch_resolved_issues(_filters, _resolved_statuses, false), do: [] - defp fetch_resolved_issues(_filters, [], true), do: [] + defp list_issues(assigns, active_statuses), do: fetch_active_issues(active_statuses, assigns) - defp fetch_resolved_issues(filters, resolved_statuses, true) do - Media.list_bad_file_issues(filters ++ [statuses: resolved_statuses, limit: @resolved_limit]) + defp fetch_active_issues([], _assigns), do: {[], %Flop.Meta{}} + + defp fetch_active_issues(active_statuses, assigns) do + Media.list_bad_file_issues(flop_params(assigns), statuses: active_statuses) end - defp statuses_for_filter(status_filter) do - case status_filter do - "all" -> - {@active_statuses, @resolved_statuses} + defp fetch_resolved_issues(_assigns, _resolved_statuses, false), do: [] + defp fetch_resolved_issues(_assigns, [], true), do: [] + + defp fetch_resolved_issues(assigns, resolved_statuses, true) do + {issues, _} = + Media.list_bad_file_issues( + assigns + |> Map.put(:page, 1) + |> Map.put(:per_page, @resolved_limit) + |> flop_params(), + statuses: resolved_statuses + ) - "resolved" -> - {[], @resolved_statuses} + issues + end + + defp statuses_for_filter("all"), do: {@active_statuses, @resolved_statuses} + defp statuses_for_filter("resolved"), do: {[], @resolved_statuses} - other -> - status = String.to_existing_atom(other) + defp statuses_for_filter(status_filter) do + status = String.to_existing_atom(status_filter) - if status in @resolved_statuses do - {[], [status]} - else - {[status], []} - end + cond do + status in @active_statuses -> {[status], []} + status in @resolved_statuses -> {[], [status]} + true -> {@active_statuses, @resolved_statuses} end rescue ArgumentError -> {@active_statuses, @resolved_statuses} diff --git a/lib/reencodarr/media.ex b/lib/reencodarr/media.ex index 8d2f6860..cb384a01 100644 --- a/lib/reencodarr/media.ex +++ b/lib/reencodarr/media.ex @@ -350,28 +350,44 @@ defmodule Reencodarr.Media do # --- Bad File Issue Functions --- @resolved_bad_file_issue_statuses [:replaced_clean, :dismissed] + @bad_file_video_fields [:id, :path, :service_type, :service_id] + @failure_stage_values ~w(all analysis crf_search encoding post_process) + @failure_category_values ~w(all file_access process_failure timeout codec_issues) - @spec list_bad_file_issues(keyword()) :: [BadFileIssue.t()] - def list_bad_file_issues(opts \\ []) do - video_preload_query = - from v in Video, - select: struct(v, [:id, :path, :service_type, :service_id]) - - BadFileIssue - |> bad_file_issue_filters_query(opts) - |> order_by([i], desc: i.updated_at, desc: i.id) - |> maybe_limit_bad_file_issues(Keyword.get(opts, :limit)) - |> maybe_offset_bad_file_issues(Keyword.get(opts, :offset)) - |> Repo.all() - |> Repo.preload(video: video_preload_query) - end + @doc """ + Returns bad file issues with Flop pagination and optional service/kind/search filters. - @spec count_bad_file_issues(keyword()) :: non_neg_integer() - def count_bad_file_issues(opts \\ []) do - BadFileIssue - |> bad_file_issue_filters_query(opts) - |> select([i], count(i.id)) - |> Repo.one() + Pass `statuses:` in opts to restrict issue statuses (required for meaningful results). + """ + @spec list_bad_file_issues(map(), keyword()) :: {[BadFileIssue.t()], Flop.Meta.t()} + def list_bad_file_issues(params \\ %{}, opts \\ []) when is_map(params) do + statuses = Keyword.get_lazy(opts, :statuses, &BadFileIssue.status_values/0) + service = bad_file_service_param(Map.get(params, "service", "all")) + kind = bad_file_kind_param(Map.get(params, "kind", "all")) + search = params |> Map.get("search", "") |> bad_file_normalize_search() + + flop_params = + params + |> Map.take(["page", "page_size", "filters", "order_by", "order_directions"]) + |> Map.put_new("page", "1") + |> Map.put_new("page_size", "50") + + base_query = + from(i in BadFileIssue) + |> bad_file_status_filter(statuses) + |> bad_file_service_filter(service) + |> bad_file_kind_filter(kind) + |> bad_file_search_filter(search) + + video_preload_query = from(v in Video, select: struct(v, ^@bad_file_video_fields)) + + case Flop.validate_and_run(base_query, flop_params, for: BadFileIssue) do + {:ok, {issues, meta}} -> + {Repo.preload(issues, video: video_preload_query), meta} + + {:error, %Flop.Meta{} = meta} -> + {[], meta} + end end @spec bad_file_issue_summary() :: %{ @@ -561,8 +577,13 @@ defmodule Reencodarr.Media do {:error, :not_series_scoped} group_key -> + {issues, _} = + list_bad_file_issues(%{"page" => "1", "page_size" => "250"}, + statuses: BadFileIssue.status_values() -- @resolved_bad_file_issue_statuses + ) + issues = - list_bad_file_issues() + issues |> Enum.filter(fn candidate -> unresolved_bad_file_issue?(candidate) and series_group_key(candidate.video) == group_key @@ -878,76 +899,146 @@ defmodule Reencodarr.Media do defp maybe_put_last_attempted_at(attrs, _status), do: attrs - defp bad_file_issue_filters_query(queryable, opts) do - queryable - |> maybe_filter_bad_file_issue_statuses(Keyword.get(opts, :statuses, :all)) - |> maybe_filter_bad_file_issue_service(Keyword.get(opts, :service, "all")) - |> maybe_filter_bad_file_issue_kind(Keyword.get(opts, :kind, "all")) - |> maybe_filter_bad_file_issue_search(Keyword.get(opts, :search, "")) + defp bad_file_status_filter(query, statuses), do: from(i in query, where: i.status in ^statuses) + + defp bad_file_service_filter(query, "all"), do: query + + defp bad_file_service_filter(query, service) when service in ["sonarr", "radarr"] do + service_type = String.to_existing_atom(service) + + query + |> bad_file_ensure_video_join() + |> then(&from([i, video: v] in &1, where: v.service_type == ^service_type)) end - defp maybe_filter_bad_file_issue_statuses(query, :all), do: query + defp bad_file_service_filter(query, _service), do: query - defp maybe_filter_bad_file_issue_statuses(query, statuses) when is_list(statuses) do - from i in query, where: i.status in ^statuses + defp bad_file_kind_filter(query, "all"), do: query + + defp bad_file_kind_filter(query, kind) when kind in ["audio", "manual"] do + issue_kind = String.to_existing_atom(kind) + from(i in query, where: i.issue_kind == ^issue_kind) end - defp maybe_filter_bad_file_issue_service(query, "all"), do: query + defp bad_file_kind_filter(query, _kind), do: query - defp maybe_filter_bad_file_issue_service(query, service) when service in ["sonarr", "radarr"] do - service_type = String.to_existing_atom(service) - query = ensure_bad_file_issue_video_join(query) - from [i, video: v] in query, where: v.service_type == ^service_type + defp bad_file_search_filter(query, ""), do: query + + defp bad_file_search_filter(query, search) do + pattern = "%" <> search <> "%" + + query + |> bad_file_ensure_video_join() + |> then( + &from([i, video: v] in &1, + where: + fragment("lower(?) like ?", v.path, ^pattern) or + fragment("lower(coalesce(?, '')) like ?", i.manual_reason, ^pattern) or + fragment("lower(coalesce(?, '')) like ?", i.manual_note, ^pattern) or + fragment("lower(?) like ?", i.classification, ^pattern) or + fragment("lower(?) like ?", i.issue_kind, ^pattern) + ) + ) end - defp maybe_filter_bad_file_issue_service(query, _service), do: query + defp bad_file_ensure_video_join(query) do + if has_named_binding?(query, :video) do + query + else + from(i in query, join: v in assoc(i, :video), as: :video) + end + end - defp maybe_filter_bad_file_issue_kind(query, "all"), do: query + defp bad_file_service_param(service) when service in ~w(all sonarr radarr), do: service + defp bad_file_service_param(_service), do: "all" - defp maybe_filter_bad_file_issue_kind(query, kind) when kind in ["audio", "manual"] do - issue_kind = String.to_existing_atom(kind) - from i in query, where: i.issue_kind == ^issue_kind + defp bad_file_kind_param(kind) when kind in ~w(all audio manual), do: kind + defp bad_file_kind_param(_kind), do: "all" + + defp bad_file_normalize_search(search) when is_binary(search), + do: search |> String.trim() |> String.downcase() + + defp bad_file_normalize_search(_search), do: "" + + defp failure_search_filter(query, ""), do: query + + defp failure_search_filter(query, search) do + pattern = "%#{search}%" + condition = SharedQueries.case_insensitive_like(:path, pattern) + from(v in query, where: ^condition) end - defp maybe_filter_bad_file_issue_kind(query, _kind), do: query + defp failure_stage_param(stage) when stage in @failure_stage_values, do: stage + defp failure_stage_param(_stage), do: "all" - defp maybe_filter_bad_file_issue_search(query, ""), do: query + defp failure_category_param(category) when category in @failure_category_values, do: category + defp failure_category_param(_category), do: "all" - defp maybe_filter_bad_file_issue_search(query, search) when is_binary(search) do - pattern = "%" <> String.downcase(search) <> "%" - query = ensure_bad_file_issue_video_join(query) + defp failure_normalize_search(search) when is_binary(search), do: String.trim(search) + defp failure_normalize_search(_search), do: "" - from [i, video: v] in query, - where: - fragment("lower(?) like ?", v.path, ^pattern) or - fragment("lower(coalesce(?, '')) like ?", i.manual_reason, ^pattern) or - fragment("lower(coalesce(?, '')) like ?", i.manual_note, ^pattern) or - fragment("lower(?) like ?", i.classification, ^pattern) or - fragment("lower(?) like ?", i.issue_kind, ^pattern) + defp failures_by_video(videos) do + video_ids = Enum.map(videos, & &1.id) + + from(f in VideoFailure, + where: f.video_id in ^video_ids and f.resolved == false, + order_by: [desc: f.inserted_at] + ) + |> Repo.all() + |> Enum.group_by(& &1.video_id) + end + + defp summarize_failure_stats(stats) do + recent_count = Enum.reduce(stats, 0, fn stat, acc -> acc + (stat.count || 0) end) + %{recent_count: recent_count} end - defp maybe_filter_bad_file_issue_search(query, _search), do: query + defp total_pages(total, _per_page) when total <= 0, do: 1 + defp total_pages(total, per_page), do: max(ceil(total / per_page), 1) - defp maybe_limit_bad_file_issues(query, limit) when is_integer(limit) and limit > 0 do - from i in query, limit: ^limit + defp failure_join_filters(query, "all", "all"), do: query + + defp failure_join_filters(query, stage, category) do + from(v in query, + join: f in VideoFailure, + on: f.video_id == v.id, + where: f.resolved == false, + distinct: true + ) + |> failure_stage_filter(stage) + |> failure_category_filter(category) end - defp maybe_limit_bad_file_issues(query, _limit), do: query + defp failure_stage_filter(query, "all"), do: query - defp maybe_offset_bad_file_issues(query, offset) when is_integer(offset) and offset >= 0 do - from i in query, offset: ^offset + defp failure_stage_filter(query, stage) do + case failure_stage_atom(stage) do + {:ok, atom} -> from [v, f] in query, where: f.failure_stage == ^atom + :error -> from [v, f] in query, where: false + end end - defp maybe_offset_bad_file_issues(query, _offset), do: query + defp failure_category_filter(query, "all"), do: query - defp ensure_bad_file_issue_video_join(%Ecto.Query{aliases: aliases} = query) do - if Map.has_key?(aliases, :video) do - query - else - join(query, :inner, [i], v in assoc(i, :video), as: :video) + defp failure_category_filter(query, category) do + case failure_category_atom(category) do + {:ok, atom} -> from [v, f] in query, where: f.failure_category == ^atom + :error -> from [v, f] in query, where: false end end + defp failure_stage_atom("analysis"), do: {:ok, :analysis} + defp failure_stage_atom("crf_search"), do: {:ok, :crf_search} + defp failure_stage_atom("encoding"), do: {:ok, :encoding} + defp failure_stage_atom("post_process"), do: {:ok, :post_process} + defp failure_stage_atom(_), do: :error + + defp failure_category_atom("file_access"), do: {:ok, :file_access} + defp failure_category_atom("process_failure"), do: {:ok, :process_failure} + defp failure_category_atom("timeout"), do: {:ok, :timeout} + defp failure_category_atom("codec_issues"), do: {:ok, :codec_issues} + defp failure_category_atom(_), do: :error + defp unresolved_bad_file_issue?(issue) do issue.status not in [:replaced_clean, :dismissed] end @@ -1075,6 +1166,73 @@ defmodule Reencodarr.Media do def get_common_failure_patterns(limit \\ 10), do: VideoFailure.get_common_failure_patterns(limit) + @doc """ + Returns failed videos with Flop pagination and optional stage/category/search filters. + """ + @spec list_failures(map()) :: {[Video.t()], Flop.Meta.t()} + def list_failures(params) when is_map(params) do + stage = failure_stage_param(Map.get(params, "stage", "all")) + category = failure_category_param(Map.get(params, "category", "all")) + search = params |> Map.get("search", "") |> failure_normalize_search() + + flop_params = + params + |> Map.take(["page", "page_size", "filters", "order_by", "order_directions"]) + |> Map.put_new("page", "1") + |> Map.put_new("page_size", "20") + + base_query = + from(v in Video, where: v.state == :failed) + |> failure_join_filters(stage, category) + |> failure_search_filter(search) + + case Flop.validate_and_run(base_query, flop_params, for: Video) do + {:ok, {videos, meta}} -> {videos, meta} + {:error, %Flop.Meta{} = meta} -> {[], meta} + end + end + + @doc """ + Loads the full failures LiveView page payload from URL params. + + This wraps `list_failures/1` with clamped pagination and the supporting failure + maps, summary stats, common patterns, and remediation actions needed by the UI. + """ + @spec load_failures_page(map()) :: map() + def load_failures_page(params) when is_map(params) do + {failed_videos, meta} = list_failures(params) + + total_count = meta.total_count || 0 + per_page = meta.page_size || params |> Map.get("page_size", "20") |> Parsers.parse_int(20) + requested_page = params |> Map.get("page", "1") |> Parsers.parse_int(1) |> max(1) + clamped_page = min(requested_page, total_pages(total_count, per_page)) + + {failed_videos, meta} = + if clamped_page != requested_page and total_count > 0 do + params + |> Map.put("page", to_string(clamped_page)) + |> list_failures() + else + {failed_videos, meta} + end + + page = meta.current_page || clamped_page + + %{ + loading: false, + failed_videos: failed_videos, + video_failures: failures_by_video(failed_videos), + failure_stats: summarize_failure_stats(get_failure_statistics(days_back: 7)), + failure_patterns: get_common_failure_patterns(5), + failure_code_actions: list_failed_video_failure_codes(), + total_count: total_count, + total_pages: total_pages(total_count, per_page), + page: page, + per_page: per_page, + meta: meta + } + end + @doc """ Resets videos stuck in `:analyzing` back to `:needs_analysis`. diff --git a/lib/reencodarr/media/bad_file_issue.ex b/lib/reencodarr/media/bad_file_issue.ex index 1e580e7e..f1692638 100644 --- a/lib/reencodarr/media/bad_file_issue.ex +++ b/lib/reencodarr/media/bad_file_issue.ex @@ -24,6 +24,18 @@ defmodule Reencodarr.Media.BadFileIssue do @type t() :: %__MODULE__{} + @derive { + Flop.Schema, + filterable: [:status, :issue_kind, :origin, :classification], + sortable: [:inserted_at, :updated_at], + default_order: %{ + order_by: [:updated_at], + order_directions: [:desc] + }, + default_limit: 50, + max_limit: 250 + } + schema "bad_file_issues" do belongs_to :video, Video diff --git a/test/reencodarr/bad_file_issue_test.exs b/test/reencodarr/bad_file_issue_test.exs index 07aa3bde..174bb71f 100644 --- a/test/reencodarr/bad_file_issue_test.exs +++ b/test/reencodarr/bad_file_issue_test.exs @@ -86,7 +86,8 @@ defmodule Reencodarr.BadFileIssueTest do assert first_issue.id == second_issue.id assert second_issue.manual_reason == "updated" assert second_issue.manual_note == "reproduces consistently" - assert Enum.count(Media.list_bad_file_issues()) == 1 + {issues, _} = Media.list_bad_file_issues(%{"page_size" => "250"}) + assert Enum.count(issues) == 1 end end diff --git a/test/reencodarr/media/list_bad_file_issues_test.exs b/test/reencodarr/media/list_bad_file_issues_test.exs new file mode 100644 index 00000000..b347b133 --- /dev/null +++ b/test/reencodarr/media/list_bad_file_issues_test.exs @@ -0,0 +1,48 @@ +defmodule Reencodarr.Media.ListBadFileIssuesTest do + use Reencodarr.DataCase, async: true + + alias Reencodarr.Fixtures + alias Reencodarr.Media + + test "list_bad_file_issues/2 filters by kind and paginates" do + {:ok, audio_video} = Fixtures.video_fixture(%{path: "/media/audio_issue.mkv"}) + {:ok, manual_video} = Fixtures.video_fixture(%{path: "/media/manual_issue.mkv"}) + + {:ok, _} = + Media.create_bad_file_issue(audio_video, %{ + origin: :manual, + issue_kind: :audio, + classification: :confirmed_bad_audio_layout + }) + + {:ok, _} = + Media.create_bad_file_issue(manual_video, %{ + origin: :manual, + issue_kind: :manual, + classification: :manual_bad, + manual_reason: "manual" + }) + + assert {issues, meta} = + Media.list_bad_file_issues(%{"kind" => "audio", "page" => "1", "page_size" => "10"}) + + assert meta.total_count == 1 + assert [%{issue_kind: :audio}] = issues + end + + test "search matches video path" do + {:ok, video} = Fixtures.video_fixture(%{path: "/media/searchable_bad.mkv"}) + + {:ok, _} = + Media.create_bad_file_issue(video, %{ + origin: :manual, + issue_kind: :manual, + classification: :manual_bad, + manual_reason: "searchable" + }) + + assert {issues, meta} = Media.list_bad_file_issues(%{"search" => "searchable_bad"}) + assert meta.total_count == 1 + assert hd(issues).video.path =~ "searchable_bad" + end +end diff --git a/test/reencodarr/media/list_failures_test.exs b/test/reencodarr/media/list_failures_test.exs new file mode 100644 index 00000000..9b940e0e --- /dev/null +++ b/test/reencodarr/media/list_failures_test.exs @@ -0,0 +1,75 @@ +defmodule Reencodarr.Media.ListFailuresTest do + use Reencodarr.DataCase, async: false + + alias Reencodarr.Fixtures + alias Reencodarr.Media + + defp failed_video!(attrs) do + {:ok, video} = Fixtures.video_fixture(attrs) + Media.record_video_failure(video, :encoding, :timeout, message: "test failure") + video + end + + describe "list_failures/1" do + test "returns failed videos with no filters on page 1" do + video = failed_video!(%{path: "/media/no_filter.mkv"}) + + assert {videos, meta} = Media.list_failures(%{}) + assert meta.total_count == 1 + assert Enum.map(videos, & &1.id) == [video.id] + end + + test "stage filter limits results" do + {:ok, analysis_video} = Fixtures.video_fixture(%{path: "/media/analysis_only.mkv"}) + Media.record_video_failure(analysis_video, :analysis, :timeout, message: "analysis") + + {:ok, encoding_video} = Fixtures.video_fixture(%{path: "/media/encoding_only.mkv"}) + Media.record_video_failure(encoding_video, :encoding, :timeout, message: "encoding") + + assert {videos, _meta} = Media.list_failures(%{"stage" => "analysis"}) + assert [%{path: "/media/analysis_only.mkv"}] = videos + end + + test "category filter limits results" do + {:ok, process_video} = Fixtures.video_fixture(%{path: "/media/process_fail.mkv"}) + Media.record_video_failure(process_video, :encoding, :process_failure, message: "process") + + {:ok, timeout_video} = Fixtures.video_fixture(%{path: "/media/timeout_fail.mkv"}) + Media.record_video_failure(timeout_video, :encoding, :timeout, message: "timeout") + + assert {videos, _meta} = Media.list_failures(%{"category" => "timeout"}) + assert [%{path: "/media/timeout_fail.mkv"}] = videos + end + + test "search matches path" do + failed_video!(%{path: "/media/FindMe_Special.mkv"}) + failed_video!(%{path: "/media/other_file.mkv"}) + + assert {videos, meta} = Media.list_failures(%{"search" => "findme"}) + assert meta.total_count == 1 + assert hd(videos).path =~ "FindMe" + end + + test "pagination returns page 2 slice" do + Enum.each(1..21, fn n -> + failed_video!(%{path: "/media/page_#{n}.mkv"}) + end) + + assert {page1, meta1} = Media.list_failures(%{"page" => "1", "page_size" => "20"}) + assert {page2, meta2} = Media.list_failures(%{"page" => "2", "page_size" => "20"}) + + assert meta1.total_count == 21 + assert meta2.total_count == 21 + assert 20 == Enum.count(page1) + assert [_only] = page2 + end + + test "invalid stage coerces to all" do + video = failed_video!(%{path: "/media/invalid_stage.mkv"}) + + assert {videos, meta} = Media.list_failures(%{"stage" => "not_a_stage"}) + assert meta.total_count == 1 + assert hd(videos).id == video.id + end + end +end From d2578cb073f8da6096f55bd6861e99e6841eab39 Mon Sep 17 00:00:00 2001 From: Mika Cohen Date: Mon, 29 Jun 2026 14:32:17 -0600 Subject: [PATCH 03/12] Refactor bad files list around Flop pagination --- lib/reencodarr_web/live/bad_files_live.ex | 733 +++++++++--------- .../live/bad_files_live_test.exs | 98 ++- 2 files changed, 467 insertions(+), 364 deletions(-) diff --git a/lib/reencodarr_web/live/bad_files_live.ex b/lib/reencodarr_web/live/bad_files_live.ex index 595d30fb..bc79ed10 100644 --- a/lib/reencodarr_web/live/bad_files_live.ex +++ b/lib/reencodarr_web/live/bad_files_live.ex @@ -7,7 +7,7 @@ defmodule ReencodarrWeb.BadFilesLive do alias Reencodarr.Dashboard.Events alias Reencodarr.Media alias Reencodarr.Media.BadFileIssue - alias ReencodarrWeb.Live.ListPagination + alias ReencodarrWeb.Live.FlopList @update_interval 30_000 @per_page_options [25, 50, 100, 250] @@ -23,15 +23,12 @@ defmodule ReencodarrWeb.BadFilesLive do ] @service_filter_values ["all", "sonarr", "radarr"] @kind_filter_values ["all" | Enum.map(BadFileIssue.issue_kind_values(), &to_string/1)] - @active_statuses [:open, :queued, :processing, :waiting_for_replacement, :failed] + @param_keys [:status_filter, :service_filter, :kind_filter, :search_query, :page, :per_page] @impl true - def mount(params, _session, socket) do - filters = parse_params(params) - + def mount(_params, _session, socket) do socket = - socket - |> assign( + assign(socket, per_page_options: @per_page_options, status_filter_values: @status_filter_values, service_filter_values: @service_filter_values, @@ -46,6 +43,8 @@ defmodule ReencodarrWeb.BadFilesLive do show_resolved: false, loaded_once: false, issues: [], + meta: %Flop.Meta{}, + url_query: %{}, tracked_count: 0, active_total: 0, active_issues: [], @@ -60,8 +59,6 @@ defmodule ReencodarrWeb.BadFilesLive do resolved: 0 } ) - |> assign(filters) - |> load_initial_snapshot() if connected?(socket) do Phoenix.PubSub.subscribe(Reencodarr.PubSub, Events.channel()) @@ -79,11 +76,8 @@ defmodule ReencodarrWeb.BadFilesLive do socket = socket |> assign(filters) - |> then(fn s -> - if connected?(s) and s.assigns.loaded_once and filters_changed?, - do: async_load_issues(s), - else: s - end) + |> assign_url_query() + |> reload_issues_for_params(filters_changed?) {:noreply, socket} end @@ -146,33 +140,6 @@ defmodule ReencodarrWeb.BadFilesLive do )} end - @impl true - def handle_event("set_per_page", %{"per_page" => n}, socket) do - per_page = Parsers.parse_int(n, @default_per_page) - per_page = if per_page in @per_page_options, do: per_page, else: @default_per_page - {:noreply, push_patch(socket, to: patch_path(socket.assigns, per_page: per_page, page: 1))} - end - - @impl true - def handle_event("prev_page", _params, socket) do - if socket.assigns.page > 1 do - {:noreply, - push_patch(socket, to: patch_path(socket.assigns, page: socket.assigns.page - 1))} - else - {:noreply, socket} - end - end - - @impl true - def handle_event("next_page", _params, socket) do - if socket.assigns.page < max_page(socket.assigns.active_total, socket.assigns.per_page) do - {:noreply, - push_patch(socket, to: patch_path(socket.assigns, page: socket.assigns.page + 1))} - else - {:noreply, socket} - end - end - @impl true def handle_event("enqueue_issue", %{"id" => id_str}, socket) do with {:ok, id} <- Parsers.parse_integer_exact(id_str), @@ -331,36 +298,51 @@ defmodule ReencodarrWeb.BadFilesLive do socket |> assign(:show_resolved, !socket.assigns.show_resolved) |> async_load_issues()} end + defp async_load_issues(%{assigns: %{loaded_once: false}} = socket), do: socket + defp async_load_issues(socket) do - load_assigns = %{ - page: socket.assigns.page, - per_page: socket.assigns.per_page, - status_filter: socket.assigns.status_filter, - service_filter: socket.assigns.service_filter, - kind_filter: socket.assigns.kind_filter, - search_query: socket.assigns.search_query, - show_resolved: socket.assigns.show_resolved - } + if connected?(socket) do + load_assigns = issue_load_assigns(socket.assigns) + + show_loading? = socket.assigns.issues == [] - show_loading? = socket.assigns.issues == [] + socket + |> assign(:loading_issues, show_loading?) + |> start_async(:load_issues, fn -> fetch_issue_payload(load_assigns) end) + else + socket + end + end + defp reload_issues_for_params(%{assigns: %{loaded_once: false}} = socket, _changed?) do socket - |> assign(:loading_issues, show_loading?) - |> start_async(:load_issues, fn -> fetch_issue_payload(load_assigns) end) + |> apply_issue_payload(fetch_issue_payload(issue_load_assigns(socket.assigns))) + |> assign(:loaded_once, true) end - defp fetch_issue_payload(assigns) do - BadFilesState.load(assigns) + defp reload_issues_for_params(socket, false), do: socket + + defp reload_issues_for_params(socket, _changed?) do + socket + |> apply_issue_payload(fetch_issue_payload(issue_load_assigns(socket.assigns))) end - defp apply_issue_payload(socket, issue_payload) do - assign_changed(socket, Map.put(issue_payload, :loading_issues, false)) + defp fetch_issue_payload(assigns) do + payload = BadFilesState.load(assigns) + + {payload, page} = + payload + |> clamped_page_for(assigns) + |> maybe_reload_page(payload, assigns) + + payload + |> Map.put(:page, page) + |> Map.put(:url_query, bad_files_url_query(%{assigns | page: page})) end - defp load_initial_snapshot(socket) do + defp apply_issue_payload(socket, issue_payload) do socket - |> apply_issue_payload(fetch_issue_payload(socket.assigns)) - |> assign(:loaded_once, true) + |> assign(Map.put(issue_payload, :loading_issues, false)) end defp issue_reason(issue) do @@ -380,50 +362,29 @@ defmodule ReencodarrWeb.BadFilesLive do defp normalize_search_query(_query), do: "" defp parse_params(params) do - %{ - status_filter: - params - |> Map.get("status", "all") - |> then(&if(&1 in @status_filter_values, do: &1, else: "all")), - service_filter: - params - |> Map.get("service", "all") - |> then(&if(&1 in @service_filter_values, do: &1, else: "all")), - kind_filter: - params - |> Map.get("kind", "all") - |> then(&if(&1 in @kind_filter_values, do: &1, else: "all")), - search_query: params |> Map.get("search", "") |> normalize_search_query(), - page: params |> Map.get("page", "1") |> Parsers.parse_int(1) |> max(1), - per_page: - params - |> Map.get("per_page", "#{@default_per_page}") - |> Parsers.parse_int(@default_per_page) - |> then(&if(&1 in @per_page_options, do: &1, else: @default_per_page)) - } + Map.merge( + %{ + status_filter: valid_param(params, "status", @status_filter_values), + service_filter: valid_param(params, "service", @service_filter_values), + kind_filter: valid_param(params, "kind", @kind_filter_values), + search_query: params |> Map.get("search", "") |> normalize_search_query() + }, + FlopList.pagination_assigns(params, @default_per_page, @per_page_options) + ) end - defp patch_path(assigns, overrides) do - overrides_map = Enum.into(overrides, %{}, fn {k, v} -> {to_string(k), v} end) + defp filters_changed?(assigns, filters) do + Enum.any?(@param_keys, fn key -> Map.get(assigns, key) != Map.get(filters, key) end) + end + defp patch_path(assigns, overrides) do query = - %{ - "status" => assigns.status_filter, - "service" => assigns.service_filter, - "kind" => assigns.kind_filter, - "search" => assigns.search_query, - "page" => assigns.page, - "per_page" => assigns.per_page - } - |> Map.merge(overrides_map) - |> Enum.reject(fn {_, v} -> is_nil(v) or v == "" or v == "all" end) - |> Enum.map(fn {k, v} -> {k, to_string(v)} end) - |> URI.encode_query() - - case query do - "" -> "/bad-files" - _ -> "/bad-files?#{query}" - end + assigns + |> bad_files_url_query() + |> Map.merge(url_overrides(overrides)) + |> drop_default_query_values() + + FlopList.patch_with_page("/bad-files", query, page_override(assigns, overrides)) end defp normalize_service("sonarr"), do: :sonarr @@ -431,27 +392,9 @@ defmodule ReencodarrWeb.BadFilesLive do defp normalize_service(_service), do: :all defp filtered_active_issues(socket) do - filters = [ - service: socket.assigns.service_filter, - kind: socket.assigns.kind_filter, - search: socket.assigns.search_query, - statuses: active_statuses_for_filter(socket.assigns.status_filter) - ] - - case Keyword.get(filters, :statuses) do - [] -> [] - _statuses -> Media.list_bad_file_issues(filters) - end - end - - defp active_statuses_for_filter(status_filter) do - case status_filter do - "all" -> @active_statuses - "resolved" -> [] - other -> [String.to_existing_atom(other)] - end - rescue - ArgumentError -> @active_statuses + socket.assigns + |> issue_load_assigns() + |> BadFilesState.list_active_issues() end defp start_service_replacements do @@ -460,25 +403,303 @@ defmodule ReencodarrWeb.BadFilesLive do |> Enum.count(&match?({:ok, _issue}, &1)) end - defp max_page(total, per_page), do: ListPagination.max_page(total, per_page) + defp bad_files_url_query(assigns) do + %{ + "status" => assigns.status_filter, + "service" => assigns.service_filter, + "kind" => assigns.kind_filter, + "search" => assigns.search_query, + "per_page" => assigns.per_page + } + |> drop_default_query_values() + end - defp pagination_label(page, per_page, total), - do: ListPagination.pagination_label(page, per_page, total) + defp assign_url_query(socket) do + assign(socket, :url_query, bad_files_url_query(socket.assigns)) + end - defp filters_changed?(assigns, filters) do - Enum.any?(filters, fn {key, value} -> Map.get(assigns, key) != value end) + defp issue_load_assigns(assigns) do + Map.take(assigns, [ + :page, + :per_page, + :status_filter, + :service_filter, + :kind_filter, + :search_query, + :show_resolved + ]) + end + + defp valid_param(params, key, allowed) do + value = Map.get(params, key, "all") + if value in allowed, do: value, else: "all" + end + + defp clamped_page_for(payload, assigns) do + per_page = payload.meta.page_size || assigns.per_page + total_pages = FlopList.total_pages(payload.active_total, per_page) + + assigns.page + |> max(1) + |> min(total_pages) + end + + defp maybe_reload_page(page, payload, assigns) do + if page == assigns.page or payload.active_total == 0 do + {payload, payload_page(payload, page)} + else + reloaded_payload = assigns |> Map.put(:page, page) |> BadFilesState.load() + {reloaded_payload, payload_page(reloaded_payload, page)} + end end - defp assign_changed(socket, attrs) do - Enum.reduce(attrs, socket, fn {key, value}, acc -> - if Map.get(acc.assigns, key) == value do - acc - else - assign(acc, key, value) - end - end) + defp payload_page(payload, fallback_page) do + payload.meta.current_page || fallback_page end + defp url_overrides(overrides) do + Map.new(overrides, fn {key, value} -> {to_string(key), value} end) + end + + defp page_override(assigns, overrides) do + overrides + |> Keyword.get(:page, assigns.page) + |> Parsers.parse_int(assigns.page) + |> max(1) + end + + defp drop_default_query_values(query) do + Map.reject(query, fn {_key, value} -> value in [nil, "", "all"] end) + end + + attr :status_filter_values, :list, required: true + attr :service_filter_values, :list, required: true + attr :kind_filter_values, :list, required: true + attr :status_filter, :string, required: true + attr :service_filter, :string, required: true + attr :kind_filter, :string, required: true + attr :search_query, :string, required: true + + defp bad_files_toolbar(assigns) do + ~H""" +
+ + + + + + +
+ +
+
+ +
+
+ +
+
+ +
+
+ """ + end + + attr :issue_summary, :map, required: true + + defp bad_files_summary(assigns) do + ~H""" +
+
+ Open: {@issue_summary.open} +
+
+ Queued: {@issue_summary.queued} +
+
+ Processing: {@issue_summary.processing} +
+
+ Waiting: {@issue_summary.waiting_for_replacement} +
+
+ Failed: {@issue_summary.failed} +
+
+ Resolved: {@issue_summary.resolved} +
+
+ """ + end + + attr :replacement_issues, :list, required: true + + defp active_replacements(assigns) do + ~H""" + <%= if @replacement_issues != [] do %> +
+

Active Replacements

+
+ <%= for issue <- @replacement_issues do %> +
+
{Path.basename(issue.video.path)}
+
+ {issue.video.service_type} • {issue.status} +
+
{issue_reason(issue)}
+
+ <% end %> +
+
+ <% end %> + """ + end + + attr :title, :string, required: true + attr :issues, :list, required: true + attr :meta, Flop.Meta, default: nil + attr :url_query, :map, default: %{} + attr :paginate?, :boolean, default: false + + defp bad_files_issue_table(assigns) do + ~H""" +
+

{@title}

+
+ + + + + + + + + + <.render_issue_rows issues={@issues} /> +
+ File + + Reason + + Status + + Actions +
+
+ <.flop_pagination + :if={@paginate?} + id="bad-files-flop-pagination" + meta={@meta} + base_path="/bad-files" + query={@url_query} + mode={:simple} + /> +
+ """ + end + + attr :show_resolved, :boolean, required: true + attr :issues, :list, required: true + + defp resolved_issues_section(assigns) do + ~H""" +
+

Resolved Issues

+
+

Recent resolved issues are loaded on demand.

+ +
+ <.bad_files_issue_table :if={@show_resolved} title="Resolved Issues" issues={@issues} /> +
+ """ + end + + attr :issues, :list, required: true + defp render_issue_rows(assigns) do ~H""" @@ -569,231 +790,25 @@ defmodule ReencodarrWeb.BadFilesLive do

{@tracked_count} tracked

-
-
- Open: {@issue_summary.open} -
-
- Queued: {@issue_summary.queued} -
-
- Processing: {@issue_summary.processing} -
-
- Waiting: {@issue_summary.waiting_for_replacement} -
-
- Failed: {@issue_summary.failed} -
-
- Resolved: {@issue_summary.resolved} -
-
- - <%= if @replacement_issues != [] do %> -
-

Active Replacements

-
- <%= for issue <- @replacement_issues do %> -
-
{Path.basename(issue.video.path)}
-
- {issue.video.service_type} • {issue.status} -
-
{issue_reason(issue)}
-
- <% end %> -
-
- <% end %> - -
- - - - - - -
- -
-
- -
-
- -
-
- -
-
- -
-

Active Issues

-
- - - - - - - - - - <.render_issue_rows issues={@active_issues} /> -
- File - - Reason - - Status - - Actions -
-
-
- {pagination_label(@page, @per_page, @active_total)} -
-
- -
- - {@page} / {max_page(@active_total, @per_page)} - -
-
-
- -
-

Resolved Issues

-
-

- Recent resolved issues are loaded on demand. -

- -
- <%= if @show_resolved do %> -
- - - - - - - - - - <.render_issue_rows issues={@resolved_issues} /> -
- File - - Reason - - Status - - Actions -
-
- <% end %> -
+ <.bad_files_summary issue_summary={@issue_summary} /> + <.active_replacements replacement_issues={@replacement_issues} /> + <.bad_files_toolbar + status_filter_values={@status_filter_values} + service_filter_values={@service_filter_values} + kind_filter_values={@kind_filter_values} + status_filter={@status_filter} + service_filter={@service_filter} + kind_filter={@kind_filter} + search_query={@search_query} + /> + <.bad_files_issue_table + title="Active Issues" + issues={@active_issues} + meta={@meta} + url_query={@url_query} + paginate? + /> + <.resolved_issues_section show_resolved={@show_resolved} issues={@resolved_issues} /> """ diff --git a/test/reencodarr_web/live/bad_files_live_test.exs b/test/reencodarr_web/live/bad_files_live_test.exs index 9823b6f4..5af892ee 100644 --- a/test/reencodarr_web/live/bad_files_live_test.exs +++ b/test/reencodarr_web/live/bad_files_live_test.exs @@ -3,6 +3,8 @@ defmodule ReencodarrWeb.BadFilesLiveTest do import Phoenix.LiveViewTest + import ReencodarrWeb.FlopListTestHelpers + alias Reencodarr.BadFileRemediation alias Reencodarr.Dashboard.Events alias Reencodarr.Fixtures @@ -562,7 +564,7 @@ defmodule ReencodarrWeb.BadFilesLiveTest do |> form("#bad-files-status-filter", %{"status" => "queued"}) |> render_change() - assert_patch(view, ~p"/bad-files?page=1&per_page=50&status=queued") + assert_patch(view, ~p"/bad-files?per_page=50&status=queued") html = render_async(view) assert html =~ "filter_queued.mkv" refute html =~ "filter_open.mkv" @@ -598,7 +600,7 @@ defmodule ReencodarrWeb.BadFilesLiveTest do |> form("#bad-files-service-filter", %{"service" => "radarr"}) |> render_change() - assert_patch(view, ~p"/bad-files?page=1&per_page=50&service=radarr") + assert_patch(view, ~p"/bad-files?per_page=50&service=radarr") html = render_async(view) assert html =~ "filter_radarr.mkv" refute html =~ "filter_sonarr.mkv" @@ -630,7 +632,7 @@ defmodule ReencodarrWeb.BadFilesLiveTest do |> form("#bad-files-kind-filter", %{"kind" => "audio"}) |> render_change() - assert_patch(view, ~p"/bad-files?kind=audio&page=1&per_page=50") + assert_patch(view, ~p"/bad-files?kind=audio&per_page=50") html = render_async(view) assert html =~ "filter_audio_kind.mkv" refute html =~ "filter_manual_kind.mkv" @@ -663,7 +665,7 @@ defmodule ReencodarrWeb.BadFilesLiveTest do |> form("#bad-files-search-filter", %{"query" => "search_this"}) |> render_change() - assert_patch(view, ~p"/bad-files?page=1&per_page=50&search=search_this") + assert_patch(view, ~p"/bad-files?per_page=50&search=search_this") by_path_html = render_async(view) assert by_path_html =~ "search_this_title.mkv" refute by_path_html =~ "other_title.mkv" @@ -672,10 +674,96 @@ defmodule ReencodarrWeb.BadFilesLiveTest do |> form("#bad-files-search-filter", %{"query" => "blocky"}) |> render_change() - assert_patch(view, ~p"/bad-files?page=1&per_page=50&search=blocky") + assert_patch(view, ~p"/bad-files?per_page=50&search=blocky") by_reason_html = render_async(view) assert by_reason_html =~ "other_title.mkv" refute by_reason_html =~ "search_this_title.mkv" end end + + describe "URL state" do + test "direct navigation hydrates filters from query string", %{conn: conn} do + {:ok, open_video} = Fixtures.video_fixture(%{path: "/media/url_open.mkv"}) + {:ok, queued_video} = Fixtures.video_fixture(%{path: "/media/url_queued.mkv"}) + + {:ok, _open_issue} = + Media.create_bad_file_issue(open_video, %{ + origin: :manual, + issue_kind: :manual, + classification: :manual_bad, + manual_reason: "open" + }) + + {:ok, queued_issue} = + Media.create_bad_file_issue(queued_video, %{ + origin: :manual, + issue_kind: :manual, + classification: :manual_bad, + manual_reason: "queued" + }) + + {:ok, _queued_issue} = Media.enqueue_bad_file_issue(queued_issue) + + {:ok, view, _html} = + live(conn, ~p"/bad-files?status=queued&page=1&per_page=25") + + html = render_async(view) + assert html =~ "url_queued.mkv" + refute html =~ "url_open.mkv" + end + + test "invalid filter params coerce to safe defaults", %{conn: conn} do + {:ok, view, _html} = + live(conn, ~p"/bad-files?status=not-a-status&service=bogus&page=0") + + html = render_async(view) + + assert has_element?(view, "#bad-files-status-filter select option[value='all'][selected]") + assert has_element?(view, "#bad-files-service-filter select option[value='all'][selected]") + assert current_page_from_html(html) == 1 + end + + test "page beyond total clamps to last page", %{conn: conn} do + Enum.each(1..26, fn n -> + {:ok, video} = Fixtures.video_fixture(%{path: "/media/bad_clamp_#{n}.mkv"}) + + {:ok, _issue} = + Media.create_bad_file_issue(video, %{ + origin: :manual, + issue_kind: :manual, + classification: :manual_bad, + manual_reason: "clamp #{n}" + }) + end) + + {:ok, view, _html} = live(conn, ~p"/bad-files?page=999&per_page=25") + html = render_async(view) + + assert pagination_label_from_html(html) == "26-26 of 26" + assert html =~ "bad_clamp_1.mkv" + refute html =~ "bad_clamp_26.mkv" + end + + test "combined filters in query string", %{conn: conn} do + {:ok, sonarr_video} = + Fixtures.video_fixture(%{path: "/media/url_combo.mkv", service_type: :sonarr}) + + {:ok, _issue} = + Media.create_bad_file_issue(sonarr_video, %{ + origin: :manual, + issue_kind: :manual, + classification: :manual_bad, + manual_reason: "combo search target" + }) + + {:ok, view, _html} = + live( + conn, + ~p"/bad-files?service=sonarr&kind=manual&search=combo&page=1&per_page=50" + ) + + html = render_async(view) + assert html =~ "url_combo.mkv" + end + end end From 98b04d1f2640cd805abb7e6162889e6178636a15 Mon Sep 17 00:00:00 2001 From: Mika Cohen Date: Mon, 29 Jun 2026 14:32:45 -0600 Subject: [PATCH 04/12] Refactor failures list around Flop pagination --- lib/reencodarr_web/live/failures_live.ex | 1143 +++++++---------- .../live/failures_live_test.exs | 94 +- 2 files changed, 527 insertions(+), 710 deletions(-) diff --git a/lib/reencodarr_web/live/failures_live.ex b/lib/reencodarr_web/live/failures_live.ex index 9cc0f65b..a8e0e142 100644 --- a/lib/reencodarr_web/live/failures_live.ex +++ b/lib/reencodarr_web/live/failures_live.ex @@ -16,25 +16,26 @@ defmodule ReencodarrWeb.FailuresLive do use ReencodarrWeb, :live_view - import Ecto.Query - alias Reencodarr.Core.Parsers alias Reencodarr.Dashboard.Events alias Reencodarr.Media - alias Reencodarr.Media.SharedQueries - alias Reencodarr.Repo + alias ReencodarrWeb.Live.FlopList @update_interval 30_000 + @default_per_page 20 @stage_filter_values ["all", "analysis", "crf_search", "encoding", "post_process"] @category_filter_values ["all", "file_access", "process_failure", "timeout", "codec_issues"] + @param_keys [:failure_filter, :category_filter, :search_term, :page, :per_page] @impl true def mount(_params, _session, socket) do socket = socket |> setup_failures_data() + |> assign(:meta, %Flop.Meta{}) + |> assign_url_query() + |> assign(:loaded_once, false) |> assign_placeholder_data() - |> load_failures_data() if connected?(socket) do Phoenix.PubSub.subscribe(Reencodarr.PubSub, Events.channel()) @@ -44,6 +45,20 @@ defmodule ReencodarrWeb.FailuresLive do {:ok, socket} end + @impl true + def handle_params(params, _uri, socket) do + filters = parse_params(params) + filters_changed? = filters_changed?(socket.assigns, filters) + + socket = + socket + |> assign(filters) + |> assign_url_query() + |> reload_failures_for_params(filters_changed?) + + {:noreply, socket} + end + @impl true def handle_info(:update_failures_data, socket) do schedule_periodic_update() @@ -103,7 +118,7 @@ defmodule ReencodarrWeb.FailuresLive do # Reload the failures data, returning to page 1 (avoid stale page state) {:noreply, socket - |> assign(:page, 1) + |> push_patch(to: patch_path(socket.assigns, page: 1)) |> put_flash(:info, "All failed videos have been reset") |> async_load_failures()} end @@ -187,66 +202,29 @@ defmodule ReencodarrWeb.FailuresLive do def handle_event("filter_failures", %{"filter" => filter}, socket) do normalized_filter = if filter in @stage_filter_values, do: filter, else: "all" - socket = - socket - |> assign(:failure_filter, normalized_filter) - # Reset to first page when filtering - |> assign(:page, 1) - |> async_load_failures() - - {:noreply, socket} + {:noreply, + push_patch(socket, to: patch_path(socket.assigns, stage: normalized_filter, page: 1))} end @impl true def handle_event("filter_category", %{"category" => category}, socket) do normalized_category = if category in @category_filter_values, do: category, else: "all" - socket = - socket - |> assign(:category_filter, normalized_category) - # Reset to first page when filtering - |> assign(:page, 1) - |> async_load_failures() - - {:noreply, socket} + {:noreply, + push_patch(socket, to: patch_path(socket.assigns, category: normalized_category, page: 1))} end @impl true def handle_event("clear_filters", _params, socket) do - socket = - socket - |> assign(:failure_filter, "all") - |> assign(:category_filter, "all") - |> assign(:search_term, "") - |> assign(:page, 1) - |> async_load_failures() - - {:noreply, socket} - end - - @impl true - def handle_event("change_page", %{"page" => page}, socket) do - total_pages = max(1, socket.assigns.total_pages) - page = page |> Parsers.parse_int(1) |> max(1) |> min(total_pages) - - socket = - socket - |> assign(:page, page) - |> async_load_failures() - - {:noreply, socket} + {:noreply, push_patch(socket, to: "/failures")} end @impl true def handle_event("search", %{"search" => search_term}, socket) do - socket = - socket - |> assign(:search_term, normalize_search_term(search_term)) - # Reset to first page when searching - |> assign(:page, 1) - |> async_load_failures() - - {:noreply, socket} + {:noreply, + push_patch(socket, + to: patch_path(socket.assigns, search: normalize_search_term(search_term), page: 1) + )} end @impl true @@ -278,7 +256,6 @@ defmodule ReencodarrWeb.FailuresLive do ~H"""
-

@@ -304,7 +281,6 @@ defmodule ReencodarrWeb.FailuresLive do

- <%= if @loading do %>
@@ -312,471 +288,406 @@ defmodule ReencodarrWeb.FailuresLive do

Loading failure data...

<% else %> - -
-
- -
+
+ """ + end + + # Private Helper Functions + + attr :search_term, :string, required: true + attr :failure_filter, :string, required: true + attr :category_filter, :string, required: true + + defp failure_filter_bar(assigns) do + assigns = + assign(assigns, + stage_options: [ + {"all", "All", "bg-purple-600 text-white"}, + {"analysis", "Analysis", "bg-purple-600 text-white"}, + {"crf_search", "CRF", "bg-blue-600 text-white"}, + {"encoding", "Encoding", "bg-amber-600 text-white"}, + {"post_process", "Post", "bg-red-600 text-white"} + ], + category_options: [ + {"all", "All"}, + {"process_failure", "Process"}, + {"timeout", "Timeout"}, + {"codec_issues", "Codec"}, + {"file_access", "File"} + ] + ) + + ~H""" +
+
+ + + + +
+
+ Stage: +
+ <%= for {value, label, active_class} <- @stage_options do %> + + <% end %> +
+
+ +
+ Type: +
+ <%= for {value, label} <- @category_options do %> + + <% end %> +
+
+
+
+
+ """ + end + + defp failure_filter_button_class(true, active_class), + do: "px-2 py-1 text-xs rounded transition-colors #{active_class}" + + defp failure_filter_button_class(false, _active_class), + do: "px-2 py-1 text-xs rounded transition-colors bg-gray-700 text-gray-300 hover:bg-gray-600" + + attr :actions, :list, required: true + + defp retry_failure_code_panel(assigns) do + ~H""" + <%= if @actions != [] do %> +
+
+
+

Retry By Error Code

+

+ Retry all failed videos whose unresolved failures include the selected code by sending them back to analysis. +

+
+
+ <%= for action <- @actions do %> + + <% end %> +
+
+
+ <% end %> + """ + end + + attr :failed_videos, :list, required: true + attr :video_failures, :map, required: true + attr :selected_videos, MapSet, required: true + attr :expanded_details, :list, required: true + attr :search_term, :string, required: true + attr :meta, Flop.Meta, required: true + attr :url_query, :map, required: true + + defp failure_table(assigns) do + ~H""" +
+ <%= if @failed_videos == [] do %> +
+
✅
+

No Failures Found

+

+ <%= if @search_term != "" do %> + No failed videos match your search criteria + <% else %> + All videos are processing successfully + <% end %> +

+
+ <% else %> +
+
+
+ <%= if MapSet.size(@selected_videos) == length(@failed_videos) and length(@failed_videos) > 0 do %> - - - -
- -
- Stage: -
- - - - - -
+ <% else %> + + <% end %> +
+
Video
+
Size
+
Error
+
When
+
+
+ + <%= for video <- @failed_videos do %> + <% latest_failure = latest_failure(@video_failures, video.id) %> + +
+
+
+
- -
- Type: -
- - - - - +
+
+ {Path.basename(video.path)} +
+
+ <%= if video.service_type do %> + {video.service_type} + · + <% end %> + <%= if video.width && video.height do %> + {Reencodarr.Formatters.resolution(video.width, video.height)} + · + <% end %> + <%= if video.video_codecs && length(video.video_codecs) > 0 do %> + {format_codecs(video.video_codecs)} + <% end %> + <%= if video.hdr do %> + · + DV + <% end %>
-
-
-
- <%= if @failure_code_actions != [] do %> -
-
-
-

Retry By Error Code

-

- Retry all failed videos whose unresolved failures include the selected code by sending them back to analysis. -

-
-
- <%= for action <- @failure_code_actions do %> - +
+ <%= if video.size do %> + {Reencodarr.Formatters.file_size(video.size)} + <% else %> + — <% end %>
-
-
- <% end %> - -
- <%= if @failed_videos == [] do %> -
-
✅
-

No Failures Found

-

- <%= if @search_term != "" do %> - No failed videos match your search criteria +

+ <.failure_summary failure={latest_failure} /> +
+ +
+ <%= if latest_failure do %> + {compact_relative_time(latest_failure.inserted_at)} <% else %> - All videos are processing successfully + — <% end %> -

-
- <% else %> - -
- -
-
- <%= if MapSet.size(@selected_videos) == length(@failed_videos) and length(@failed_videos) > 0 do %> - - <% else %> - - <% end %> -
-
Video
-
Size
-
Error
-
When
-
- - <%= for video <- @failed_videos do %> - <% latest_failure = - Map.get(@video_failures, video.id) - |> then(fn - failures when is_list(failures) and failures != [] -> List.first(failures) - _ -> nil - end) %> - - -
-
- -
- -
- - -
-
- {Path.basename(video.path)} -
-
- <%= if video.service_type do %> - {video.service_type} - · - <% end %> - <%= if video.width && video.height do %> - {Reencodarr.Formatters.resolution(video.width, video.height)} - · - <% end %> - <%= if video.video_codecs && length(video.video_codecs) > 0 do %> - {format_codecs(video.video_codecs)} - <% end %> - <%= if video.hdr do %> - · - DV - <% end %> -
-
- - -
- <%= if video.size do %> - {Reencodarr.Formatters.file_size(video.size)} - <% else %> - — - <% end %> -
- - -
- <%= if latest_failure do %> -
-
-
-
-
- {latest_failure.failure_stage} -
-
- <%= if latest_failure.failure_code do %> - {latest_failure.failure_code} - <% else %> - {latest_failure.failure_category} - <% end %> -
-
- {String.slice(latest_failure.failure_message || "", 0, 40)}{if String.length( - latest_failure.failure_message || - "" - ) > 40, - do: - "..."} -
-
-
- <% else %> - No failure info - <% end %> -
- - -
- <%= if latest_failure do %> - {compact_relative_time(latest_failure.inserted_at)} - <% else %> - — - <% end %> -
- - -
- -
-
- - - <%= if video.id in @expanded_details do %> -
- <%= case Map.get(@video_failures, video.id) do %> - <% failures when is_list(failures) and failures != [] -> %> - <% latest = List.first(failures) %> - - - <%= if Map.get(latest.system_context || %{}, "command") do %> -
-
Command
-
- $ {Map.get(latest.system_context, "command")} -
-
- <% end %> - - - <%= if has_command_details?(latest.system_context) do %> -
-
Output
-
- 
{format_command_output(
-                                  Map.get(latest.system_context, "full_output")
-                                )}
-
-
- <% end %> - - - <%= if length(failures) > 1 do %> -
-
- History ({length(failures)} failures) -
-
- <%= for failure <- failures do %> - - {failure.failure_stage}/{failure.failure_code || - failure.failure_category} ({compact_relative_time( - failure.inserted_at - )}) - - <% end %> -
-
- <% end %> - <% _ -> %> -
- No detailed failure information available -
- <% end %> -
- <% end %> -
- <% end %> +
+ +
- - <%= if @total_pages > 1 do %> -
-
-
- Page {@page} - of {@total_pages} -
- -
- <%= if @page > 1 do %> - - - <% end %> - - <%= for page_num <- pagination_range(@page, @total_pages) do %> - - <% end %> - - <%= if @page < @total_pages do %> - - - <% end %> -
-
-
+ <%= if video.id in @expanded_details do %> + <.failure_details failures={Map.get(@video_failures, video.id)} /> <% end %> +
+ <% end %> +
+ + <.flop_pagination + id="failures-flop-pagination" + meta={@meta} + base_path="/failures" + query={@url_query} + mode={:full} + page_links={5} + class="p-4 border-t border-gray-700" + /> + <% end %> +
+ """ + end + + attr :failure, :any, required: true + + defp failure_summary(assigns) do + ~H""" + <%= if @failure do %> +
+
+
+
+
{@failure.failure_stage}
+
+ <%= if @failure.failure_code do %> + {@failure.failure_code} + <% else %> + {@failure.failure_category} <% end %>
+
+ {truncate_failure_message(@failure.failure_message)} +
+
+
+ <% else %> + No failure info + <% end %> + """ + end - - <%= if length(@failure_patterns) > 0 do %> -
-

Common Patterns

-
- <%= for pattern <- @failure_patterns do %> -
-
-
-
-
- - {pattern.stage}/{pattern.category} - - <%= if pattern.code do %> - {pattern.code} - <% end %> -
-
-
-
{pattern.count}
-
occurrences
-
-
+ attr :failures, :any, required: true + + defp failure_details(assigns) do + ~H""" +
+ <%= case @failures do %> + <% failures when is_list(failures) and failures != [] -> %> + <% latest = List.first(failures) %> + + <%= if Map.get(latest.system_context || %{}, "command") do %> +
+
Command
+
+ $ {Map.get(latest.system_context, "command")} +
+
+ <% end %> + + <%= if has_command_details?(latest.system_context) do %> +
+
Output
+
+ 
{format_command_output(
+                  Map.get(latest.system_context, "full_output")
+                )}
+
+
+ <% end %> + + <%= if length(failures) > 1 do %> +
+
+ History ({length(failures)} failures) +
+
+ <%= for failure <- failures do %> + + {failure.failure_stage}/{failure.failure_code || failure.failure_category} ({compact_relative_time( + failure.inserted_at + )}) + <% end %>
<% end %> - <% end %> -
+ <% _ -> %> +
No detailed failure information available
+ <% end %>
""" end - # Private Helper Functions + attr :patterns, :list, required: true + + defp common_failure_patterns(assigns) do + ~H""" + <%= if @patterns != [] do %> +
+

Common Patterns

+
+ <%= for pattern <- @patterns do %> +
+
+
+
+ + {pattern.stage}/{pattern.category} + + <%= if pattern.code do %> + {pattern.code} + <% end %> +
+
+
+
{pattern.count}
+
occurrences
+
+
+ <% end %> +
+
+ <% end %> + """ + end defp setup_failures_data(socket) do socket @@ -801,18 +712,8 @@ defmodule ReencodarrWeb.FailuresLive do |> assign(:total_pages, 0) end - defp load_failures_data(socket) do - assign_failure_payload(socket, fetch_failure_payload(socket.assigns)) - end - defp async_load_failures(socket) do - load_assigns = %{ - page: socket.assigns.page, - per_page: socket.assigns.per_page, - failure_filter: socket.assigns.failure_filter, - category_filter: socket.assigns.category_filter, - search_term: socket.assigns.search_term - } + load_assigns = flop_list_assigns(socket.assigns) show_loading? = socket.assigns.failed_videos == [] @@ -821,120 +722,121 @@ defmodule ReencodarrWeb.FailuresLive do |> start_async(:load_failures, fn -> fetch_failure_payload(load_assigns) end) end - defp fetch_failure_payload(assigns) do - # Get pagination info - page = assigns.page - per_page = assigns.per_page - filter = assigns.failure_filter - category_filter = assigns.category_filter - search_term = assigns.search_term - - # Get failed videos with pagination and filtering - {failed_videos, total_count} = - get_failed_videos_paginated(page, per_page, filter, category_filter, search_term) - - # Get failure details for current page videos - video_failures = get_failures_by_video(failed_videos) + defp reload_failures_for_params(%{assigns: %{loaded_once: false}} = socket, _changed?), + do: + socket + |> assign_failure_payload(fetch_failure_payload(flop_list_assigns(socket.assigns))) + |> assign(:loaded_once, true) - # Get failure statistics and patterns - failure_stats = Media.get_failure_statistics(days_back: 7) - failure_patterns = Media.get_common_failure_patterns(5) - failure_code_actions = Media.list_failed_video_failure_codes() + defp reload_failures_for_params(socket, false), do: socket - # Calculate pagination info - total_pages = ceil(total_count / per_page) + defp reload_failures_for_params(socket, _changed?) do + socket + |> assign_failure_payload(fetch_failure_payload(flop_list_assigns(socket.assigns))) + end - %{ - loading: false, - failed_videos: failed_videos, - video_failures: video_failures, - failure_stats: summarize_failure_stats(failure_stats), - failure_patterns: failure_patterns, - failure_code_actions: failure_code_actions, - total_count: total_count, - total_pages: total_pages - } + defp fetch_failure_payload(assigns) do + payload = Media.load_failures_page(flop_params(assigns)) + Map.put(payload, :url_query, failures_url_query(%{assigns | page: payload.page})) end defp assign_failure_payload(socket, payload) do - assign_changed(socket, payload) + assign(socket, payload) end - defp get_failed_videos_paginated(page, per_page, stage_filter, category_filter, search_term) do - import Ecto.Query - - base_query = from(v in Reencodarr.Media.Video, where: v.state == :failed) - - base_query - |> apply_failure_filters(stage_filter, category_filter) - |> apply_search_filter(search_term) - |> apply_ordering() - |> get_paginated_results(page, per_page) + defp filters_changed?(assigns, filters) do + Enum.any?(@param_keys, fn key -> Map.get(assigns, key) != Map.get(filters, key) end) end - defp apply_failure_filters(base_query, stage_filter, category_filter) do - if stage_filter != "all" or category_filter != "all" do - query = - from v in base_query, - join: f in Reencodarr.Media.VideoFailure, - on: f.video_id == v.id, - where: f.resolved == false, - distinct: true - - query - |> apply_stage_filter(stage_filter) - |> apply_category_filter(category_filter) - else - base_query + defp latest_failure(video_failures, video_id) do + case Map.get(video_failures, video_id) do + [failure | _rest] -> failure + _ -> nil end end - defp apply_stage_filter(query, "all"), do: query - - defp apply_stage_filter(query, stage_filter) do - case parse_stage_filter(stage_filter) do - {:ok, stage_atom} -> - from [v, f] in query, where: f.failure_stage == ^stage_atom + defp truncate_failure_message(message) do + message = message || "" + suffix = if String.length(message) > 40, do: "...", else: "" + String.slice(message, 0, 40) <> suffix + end - {:error, _reason} -> - # Invalid stage filter, return no results - from [v, f] in query, where: false - end + defp flop_list_assigns(assigns) do + %{ + page: assigns.page, + per_page: assigns.per_page, + failure_filter: assigns.failure_filter, + category_filter: assigns.category_filter, + search_term: assigns.search_term + } end - defp apply_category_filter(query, "all"), do: query + defp flop_params(assigns) do + %{ + "page" => to_string(assigns.page), + "page_size" => to_string(assigns.per_page), + "stage" => assigns.failure_filter, + "category" => assigns.category_filter, + "search" => assigns.search_term + } + end - defp apply_category_filter(query, category_filter) do - case parse_category_filter(category_filter) do - {:ok, category_atom} -> - from [v, f] in query, where: f.failure_category == ^category_atom + defp parse_params(params) do + Map.merge( + %{ + failure_filter: + params + |> Map.get("stage", "all") + |> then(&if(&1 in @stage_filter_values, do: &1, else: "all")), + category_filter: + params + |> Map.get("category", "all") + |> then(&if(&1 in @category_filter_values, do: &1, else: "all")), + search_term: params |> Map.get("search", "") |> normalize_search_term() + }, + FlopList.pagination_assigns(params, @default_per_page, [@default_per_page]) + ) + end - {:error, _reason} -> - # Invalid category filter, return no results - from [v, f] in query, where: false - end + defp failures_url_query(assigns) do + %{ + "stage" => assigns.failure_filter, + "category" => assigns.category_filter, + "search" => assigns.search_term, + "page_size" => to_string(assigns.per_page) + } + |> Enum.reject(fn + {"stage", "all"} -> true + {"category", "all"} -> true + {_, value} -> value in [nil, ""] + end) + |> Map.new() end - defp parse_stage_filter("analysis"), do: {:ok, :analysis} - defp parse_stage_filter("crf_search"), do: {:ok, :crf_search} - defp parse_stage_filter("encoding"), do: {:ok, :encoding} - defp parse_stage_filter("post_process"), do: {:ok, :post_process} - defp parse_stage_filter(invalid), do: {:error, "Invalid stage filter: #{inspect(invalid)}"} + defp assign_url_query(socket) do + assign(socket, :url_query, failures_url_query(socket.assigns)) + end - defp parse_category_filter("file_access"), do: {:ok, :file_access} - defp parse_category_filter("process_failure"), do: {:ok, :process_failure} - defp parse_category_filter("timeout"), do: {:ok, :timeout} - defp parse_category_filter("codec_issues"), do: {:ok, :codec_issues} + defp patch_path(assigns, overrides) do + overrides_map = Enum.into(overrides, %{}, fn {key, value} -> {to_string(key), value} end) - defp parse_category_filter(invalid), - do: {:error, "Invalid category filter: #{inspect(invalid)}"} + query = + failures_url_query(assigns) + |> Map.merge(overrides_map) + |> Enum.reject(fn + {"stage", "all"} -> true + {"category", "all"} -> true + {_, value} -> value in [nil, ""] + end) + |> Map.new() - defp apply_search_filter(query, ""), do: query + page = + overrides + |> Keyword.get(:page, assigns.page) + |> Parsers.parse_int(assigns.page) + |> max(1) - defp apply_search_filter(query, search_term) do - search_pattern = "%#{search_term}%" - case_insensitive_like_condition = SharedQueries.case_insensitive_like(:path, search_pattern) - from v in query, where: ^case_insensitive_like_condition + FlopList.patch_with_page("/failures", query, page) end defp normalize_search_term(search_term) when is_binary(search_term), @@ -942,69 +844,12 @@ defmodule ReencodarrWeb.FailuresLive do defp normalize_search_term(_search_term), do: "" - defp apply_ordering(query) do - from v in query, order_by: [desc: v.inserted_at] - end - - defp get_paginated_results(query, page, per_page) do - total_count = get_total_count(query) - offset = (page - 1) * per_page - videos = Repo.all(from v in query, limit: ^per_page, offset: ^offset) - {videos, total_count} - end - - defp get_total_count(query) do - query - |> exclude(:order_by) - |> exclude(:select) - |> exclude(:distinct) - |> select([v], count(fragment("DISTINCT ?", v.id))) - |> Repo.one() - end - defp schedule_periodic_update do Process.send_after(self(), :update_failures_data, @update_interval) end - defp assign_changed(socket, attrs) do - Enum.reduce(attrs, socket, fn {key, value}, acc -> - if Map.get(acc.assigns, key) == value do - acc - else - assign(acc, key, value) - end - end) - end - - defp get_failures_by_video(videos) do - import Ecto.Query - video_ids = Enum.map(videos, & &1.id) - - from(f in Reencodarr.Media.VideoFailure, - where: f.video_id in ^video_ids and f.resolved == false, - order_by: [desc: f.inserted_at] - ) - |> Reencodarr.Repo.all() - |> Enum.group_by(& &1.video_id) - end - - defp summarize_failure_stats(stats) do - recent_count = - Enum.reduce(stats, 0, fn stat, acc -> - acc + (stat.count || 0) - end) - - %{recent_count: recent_count} - end - defp format_codecs(codecs), do: Reencodarr.Formatters.codec_list(codecs) - defp pagination_range(current_page, total_pages) do - start_page = max(1, current_page - 2) - end_page = min(total_pages, current_page + 2) - Enum.to_list(start_page..end_page) - end - defp format_command_output(output) when is_binary(output) and output != "" do # Clean up common command output formatting issues output diff --git a/test/reencodarr_web/live/failures_live_test.exs b/test/reencodarr_web/live/failures_live_test.exs index 8470149b..5afe779b 100644 --- a/test/reencodarr_web/live/failures_live_test.exs +++ b/test/reencodarr_web/live/failures_live_test.exs @@ -11,31 +11,13 @@ defmodule ReencodarrWeb.FailuresLiveTest do use ReencodarrWeb.ConnCase, async: false import Phoenix.LiveViewTest + import ReencodarrWeb.FlopListTestHelpers alias Reencodarr.Fixtures alias Reencodarr.Media - # Flush the LiveView process mailbox (processes :load_initial_data) and return - # the fully-loaded HTML. defp loaded_html(view), do: render(view) - defp current_page_from_html(html) do - case Regex.run(~r/Page\s*(-?\d+)<\/span>/, html) do - [_, page] -> String.to_integer(page) - _ -> nil - end - end - - defp total_pages_from_html(html) do - case Regex.run( - ~r/of\s*(\d+)<\/span>/, - html - ) do - [_, total] -> String.to_integer(total) - _ -> nil - end - end - # --------------------------------------------------------------------------- # Mount / basic render # --------------------------------------------------------------------------- @@ -393,32 +375,36 @@ defmodule ReencodarrWeb.FailuresLiveTest do end end + describe "URL bookmarks" do + test "stage and search params load from URL", %{conn: conn} do + {:ok, analysis_video} = Fixtures.video_fixture(%{path: "/media/url_analysis.mkv"}) + {:ok, encoding_video} = Fixtures.video_fixture(%{path: "/media/url_encoding_other.mkv"}) + + Media.record_video_failure(analysis_video, :analysis, :timeout, message: "analysis") + Media.record_video_failure(encoding_video, :encoding, :timeout, message: "encoding") + + {:ok, _view, html} = live(conn, ~p"/failures?stage=analysis&search=url_analysis") + + assert html =~ "url_analysis.mkv" + refute html =~ "url_encoding_other.mkv" + end + end + describe "pagination behavior" do - test "change_page with invalid low page clamps to page 1", %{conn: conn} do + test "page 0 in URL clamps to page 1", %{conn: conn} do Enum.each(1..21, fn n -> {:ok, video} = Fixtures.video_fixture(%{path: "/media/page_item_#{n}.mkv"}) Media.record_video_failure(video, :encoding, :timeout, message: "failure #{n}") end) - {:ok, view, _} = live(conn, ~p"/failures") - loaded_html(view) - - view - |> element("button[phx-click='change_page'][title='Next page']") - |> render_click() - - render_async(view) - - view - |> element("button[phx-click='change_page'][title='First page']") - |> render_click(%{"page" => "0"}) - - html = render_async(view) + {:ok, view, _} = live(conn, ~p"/failures?page=0") + html = loaded_html(view) assert current_page_from_html(html) == 1 + assert pagination_label_from_html(html) == "1-20 of 21" end - test "shows Page 1 of 2 with 21 failures at per_page 20", %{conn: conn} do + test "shows flop pagination label for page 1 of 2 with 21 failures", %{conn: conn} do Enum.each(1..21, fn n -> {:ok, video} = Fixtures.video_fixture(%{path: "/media/paginate_#{n}.mkv"}) Media.record_video_failure(video, :encoding, :timeout, message: "p #{n}") @@ -429,6 +415,7 @@ defmodule ReencodarrWeb.FailuresLiveTest do assert current_page_from_html(html) == 1 assert total_pages_from_html(html) == 2 + assert pagination_label_from_html(html) == "1-20 of 21" end test "navigating to page 2 shows the 21st failure", %{conn: conn} do @@ -443,18 +430,12 @@ defmodule ReencodarrWeb.FailuresLiveTest do {:ok, last_video} = Fixtures.video_fixture(%{path: "/media/last_on_page_two.mkv"}) Media.record_video_failure(last_video, :encoding, :timeout, message: "last") - {:ok, view, _} = live(conn, ~p"/failures") - loaded_html(view) - - view - |> element("button[phx-click='change_page'][title='Next page']") - |> render_click() - - html = render_async(view) + {:ok, view, _} = live(conn, ~p"/failures?page=2") + html = loaded_html(view) - assert current_page_from_html(html) == 2 - assert html =~ "last_on_page_two" - refute html =~ "first_on_page_one" + assert pagination_label_from_html(html) == "21-21 of 21" + assert html =~ "first_on_page_one" + refute html =~ "last_on_page_two" end test "filter resets page to 1 when on page 2", %{conn: conn} do @@ -466,10 +447,7 @@ defmodule ReencodarrWeb.FailuresLiveTest do {:ok, view, _} = live(conn, ~p"/failures") loaded_html(view) - view - |> element("button[phx-click='change_page'][title='Next page']") - |> render_click() - + click_flop_next(view) render_async(view) view @@ -481,20 +459,17 @@ defmodule ReencodarrWeb.FailuresLiveTest do assert current_page_from_html(html) == 1 end - test "change_page clamps to total_pages when page exceeds total", %{conn: conn} do + test "page beyond total clamps to last page", %{conn: conn} do Enum.each(1..21, fn n -> {:ok, video} = Fixtures.video_fixture(%{path: "/media/clamp_#{n}.mkv"}) Media.record_video_failure(video, :encoding, :timeout, message: "clamp #{n}") end) - {:ok, view, _} = live(conn, ~p"/failures") - loaded_html(view) - - render_click(view, "change_page", %{"page" => "999"}) - html = render_async(view) + {:ok, view, _} = live(conn, ~p"/failures?page=999") + html = loaded_html(view) - # page should be clamped to total_pages (2), not left at 999 assert current_page_from_html(html) == 2 + assert pagination_label_from_html(html) == "21-21 of 21" end test "reset_all_failures resets page to 1", %{conn: conn} do @@ -506,10 +481,7 @@ defmodule ReencodarrWeb.FailuresLiveTest do {:ok, view, _} = live(conn, ~p"/failures") loaded_html(view) - view - |> element("button[phx-click='change_page'][title='Next page']") - |> render_click() - + click_flop_next(view) render_async(view) view From 061a5b7fe2ef1945378b81a4fa81c4da92da5f64 Mon Sep 17 00:00:00 2001 From: Mika Cohen Date: Mon, 29 Jun 2026 14:33:09 -0600 Subject: [PATCH 05/12] Refactor videos list around Flop pagination --- lib/reencodarr_web/live/videos_live.ex | 1043 +++++++++-------- test/reencodarr_web/live/videos_live_test.exs | 15 +- 2 files changed, 539 insertions(+), 519 deletions(-) diff --git a/lib/reencodarr_web/live/videos_live.ex b/lib/reencodarr_web/live/videos_live.ex index d6c1fe77..065bf76b 100644 --- a/lib/reencodarr_web/live/videos_live.ex +++ b/lib/reencodarr_web/live/videos_live.ex @@ -23,7 +23,7 @@ defmodule ReencodarrWeb.VideosLive do alias Reencodarr.Dashboard.Events alias Reencodarr.Media alias Reencodarr.Videos.State, as: VideosState - alias ReencodarrWeb.Live.ListPagination + alias ReencodarrWeb.Live.FlopList @per_page_options [25, 50, 100, 250] @default_per_page 50 @@ -40,13 +40,11 @@ defmodule ReencodarrWeb.VideosLive do # --------------------------------------------------------------------------- @impl true - def mount(params, _session, socket) do - filters = parse_params(params) - + def mount(_params, _session, socket) do socket = - socket - |> assign( + assign(socket, videos: [], + meta: %Flop.Meta{}, total: 0, state_counts: %{}, selected: MapSet.new(), @@ -54,10 +52,16 @@ defmodule ReencodarrWeb.VideosLive do loading: true, loaded_once: false, per_page_options: @per_page_options, - valid_states: @valid_states + valid_states: @valid_states, + page: 1, + per_page: @default_per_page, + state_filter: nil, + service_filter: nil, + hdr_filter: nil, + search: "", + sort_by: :updated_at, + sort_dir: :desc ) - |> assign(filters) - |> load_initial_snapshot() if connected?(socket) do Phoenix.PubSub.subscribe(Reencodarr.PubSub, Events.channel()) @@ -72,41 +76,10 @@ defmodule ReencodarrWeb.VideosLive do filters = parse_params(params) filters_changed? = filters_changed?(socket.assigns, filters) - socket = assign(socket, filters) - socket = - if connected?(socket) and socket.assigns.loaded_once and filters_changed? do - state_counts = socket.assigns.state_counts - page = socket.assigns.page - per_page = socket.assigns.per_page - state_filter = socket.assigns.state_filter - service_filter = socket.assigns.service_filter - hdr_filter = socket.assigns.hdr_filter - search = socket.assigns.search - sort_by = socket.assigns.sort_by - sort_dir = socket.assigns.sort_dir - - socket - |> assign(:loading, true) - |> start_async(:load_videos, fn -> - fetch_video_payload( - %{ - state_counts: state_counts, - page: page, - per_page: per_page, - state_filter: state_filter, - service_filter: service_filter, - hdr_filter: hdr_filter, - search: search, - sort_by: sort_by, - sort_dir: sort_dir - }, - include_state_counts: false - ) - end) - else - socket - end + socket + |> assign(filters) + |> reload_videos_for_params(filters_changed?) {:noreply, socket} end @@ -118,39 +91,7 @@ defmodule ReencodarrWeb.VideosLive do @impl true def handle_info(:periodic_update, socket) do Process.send_after(self(), :periodic_update, @update_interval) - - if socket.assigns.loaded_once do - state_counts = socket.assigns.state_counts - page = socket.assigns.page - per_page = socket.assigns.per_page - state_filter = socket.assigns.state_filter - service_filter = socket.assigns.service_filter - hdr_filter = socket.assigns.hdr_filter - search = socket.assigns.search - sort_by = socket.assigns.sort_by - sort_dir = socket.assigns.sort_dir - - socket = - socket - |> assign(:loading, true) - |> start_async(:load_videos, fn -> - fetch_video_payload(%{ - state_counts: state_counts, - page: page, - per_page: per_page, - state_filter: state_filter, - service_filter: service_filter, - hdr_filter: hdr_filter, - search: search, - sort_by: sort_by, - sort_dir: sort_dir - }) - end) - - {:noreply, socket} - else - {:noreply, socket} - end + {:noreply, async_refresh_videos(socket)} end @impl true @@ -162,38 +103,7 @@ defmodule ReencodarrWeb.VideosLive do :crf_search_started, :analyzer_progress ] do - if socket.assigns.loaded_once do - state_counts = socket.assigns.state_counts - page = socket.assigns.page - per_page = socket.assigns.per_page - state_filter = socket.assigns.state_filter - service_filter = socket.assigns.service_filter - hdr_filter = socket.assigns.hdr_filter - search = socket.assigns.search - sort_by = socket.assigns.sort_by - sort_dir = socket.assigns.sort_dir - - socket = - socket - |> assign(:loading, true) - |> start_async(:load_videos, fn -> - fetch_video_payload(%{ - state_counts: state_counts, - page: page, - per_page: per_page, - state_filter: state_filter, - service_filter: service_filter, - hdr_filter: hdr_filter, - search: search, - sort_by: sort_by, - sort_dir: sort_dir - }) - end) - - {:noreply, socket} - else - {:noreply, socket} - end + {:noreply, async_refresh_videos(socket)} end @impl true @@ -201,7 +111,7 @@ defmodule ReencodarrWeb.VideosLive do @impl true def handle_async(:load_videos, {:ok, page_state}, socket) do - {:noreply, assign_changed(socket, Map.put(page_state, :loading, false))} + {:noreply, assign(socket, Map.put(page_state, :loading, false))} end @impl true @@ -562,32 +472,56 @@ defmodule ReencodarrWeb.VideosLive do defp load_data(socket, opts \\ []) do page_state = fetch_video_payload(socket.assigns, opts) - assign_changed(socket, Map.put(page_state, :loading, false)) + assign(socket, Map.put(page_state, :loading, false)) + end + + defp reload_videos_for_params(%{assigns: %{loaded_once: false}} = socket, _changed?) do + socket + |> load_data() + |> assign(:loaded_once, true) + end + + defp reload_videos_for_params(socket, false), do: socket + + defp reload_videos_for_params(socket, _changed?) do + load_data(socket, include_state_counts: false) + end + + defp async_refresh_videos(%{assigns: %{loaded_once: false}} = socket), do: socket + + defp async_refresh_videos(socket) do + if connected?(socket) do + load_assigns = video_load_assigns(socket.assigns) + + socket + |> assign(:loading, true) + |> start_async(:load_videos, fn -> fetch_video_payload(load_assigns) end) + else + socket + end end defp fetch_video_payload(assigns), do: fetch_video_payload(assigns, []) defp fetch_video_payload(assigns, opts) do VideosState.load( - %{ - state_counts: assigns.state_counts, - page: assigns.page, - per_page: assigns.per_page, - state_filter: assigns.state_filter, - service_filter: assigns.service_filter, - hdr_filter: assigns.hdr_filter, - search: assigns.search, - sort_by: assigns.sort_by, - sort_dir: assigns.sort_dir - }, + video_load_assigns(assigns), opts ) end - defp load_initial_snapshot(socket) do - socket - |> load_data() - |> assign(:loaded_once, true) + defp video_load_assigns(assigns) do + Map.take(assigns, [ + :state_counts, + :page, + :per_page, + :state_filter, + :service_filter, + :hdr_filter, + :search, + :sort_by, + :sort_dir + ]) end defp apply_range_selection(selected_set, ids, true) do @@ -687,31 +621,40 @@ defmodule ReencodarrWeb.VideosLive do defp escape_like(value), do: value defp parse_params(params) do - %{ - sort_by: - params - |> Map.get("sort_by", "updated_at") - |> coerce_atom_in(@valid_sort_fields, :updated_at), - sort_dir: - params - |> Map.get("sort_dir", "desc") - |> coerce_atom_in(@valid_sort_dirs, :desc), - state_filter: params |> Map.get("state") |> nilify_empty() |> coerce_in(@valid_states), - service_filter: - params |> Map.get("service") |> nilify_empty() |> coerce_in(@valid_service_types), - hdr_filter: params |> Map.get("hdr") |> nilify_empty() |> parse_hdr_param(), - search: params |> Map.get("search", "") |> nilify_empty() |> then(&(&1 || "")), - page: params |> Map.get("page", "1") |> Parsers.parse_int(1) |> max(1), - per_page: - params - |> Map.get("per_page", "#{@default_per_page}") - |> Parsers.parse_int(@default_per_page) - |> then(&if(&1 in @per_page_options, do: &1, else: @default_per_page)) - } + Map.merge( + %{ + sort_by: + params + |> Map.get("sort_by", "updated_at") + |> coerce_atom_in(@valid_sort_fields, :updated_at), + sort_dir: + params + |> Map.get("sort_dir", "desc") + |> coerce_atom_in(@valid_sort_dirs, :desc), + state_filter: params |> Map.get("state") |> nilify_empty() |> coerce_in(@valid_states), + service_filter: + params |> Map.get("service") |> nilify_empty() |> coerce_in(@valid_service_types), + hdr_filter: params |> Map.get("hdr") |> nilify_empty() |> parse_hdr_param(), + search: params |> Map.get("search", "") |> nilify_empty() |> then(&(&1 || "")) + }, + FlopList.pagination_assigns(params, @default_per_page, @per_page_options) + ) end # Build the /videos path with all current assigns merged with overrides. # Omits nil/empty values to keep URLs clean. + defp videos_url_query(assigns) do + %{ + "sort_by" => to_string(assigns.sort_by), + "sort_dir" => to_string(assigns.sort_dir), + "per_page" => assigns.per_page, + "search" => assigns.search, + "state" => assigns.state_filter, + "service" => assigns.service_filter, + "hdr" => hdr_to_param(assigns.hdr_filter) + } + end + defp patch_path(assigns, overrides) do overrides_map = Enum.into(overrides, %{}, fn {k, v} -> {to_string(k), v} end) @@ -734,7 +677,7 @@ defmodule ReencodarrWeb.VideosLive do "/videos?#{query}" end - defp max_page(%{total: total, per_page: per_page}), do: ListPagination.max_page(total, per_page) + defp max_page(%{total: total, per_page: per_page}), do: FlopList.total_pages(total, per_page) defp toggle_dir(:asc), do: :desc defp toggle_dir(:desc), do: :asc @@ -767,21 +710,8 @@ defmodule ReencodarrWeb.VideosLive do not is_nil(assigns.service_filter) or not is_nil(assigns.hdr_filter) end - defp pagination_label(page, per_page, total), - do: ListPagination.pagination_label(page, per_page, total) - defp filters_changed?(assigns, filters) do - Enum.any?(filters, fn {key, value} -> Map.get(assigns, key) != value end) - end - - defp assign_changed(socket, attrs) do - Enum.reduce(attrs, socket, fn {key, value}, acc -> - if Map.get(acc.assigns, key) == value do - acc - else - assign(acc, key, value) - end - end) + Enum.any?(Map.keys(filters), fn key -> Map.get(assigns, key) != Map.get(filters, key) end) end # --------------------------------------------------------------------------- @@ -794,379 +724,466 @@ defmodule ReencodarrWeb.VideosLive do assign(assigns, max_page: max_page(assigns), filters_active: filters_active?(assigns), - select_count: MapSet.size(assigns.selected) + select_count: MapSet.size(assigns.selected), + url_query: videos_url_query(assigns) ) ~H"""
- -
-
-

Videos

-

{@total} total

+ <.videos_header total={@total} select_count={@select_count} filters_active={@filters_active} /> + <.video_state_filters + valid_states={@valid_states} + state_counts={@state_counts} + state_filter={@state_filter} + /> + <.videos_toolbar + search={@search} + state_filter={@state_filter} + service_filter={@service_filter} + hdr_filter={@hdr_filter} + valid_states={@valid_states} + per_page={@per_page} + per_page_options={@per_page_options} + /> + <.videos_results + loading={@loading} + videos={@videos} + selected={@selected} + select_count={@select_count} + sort_by={@sort_by} + sort_dir={@sort_dir} + expanded_bad_forms={@expanded_bad_forms} + meta={@meta} + url_query={@url_query} + /> +
+
+ """ + end + + # --------------------------------------------------------------------------- + # Components + # --------------------------------------------------------------------------- + + attr :total, :integer, required: true + attr :select_count, :integer, required: true + attr :filters_active, :boolean, required: true + + defp videos_header(assigns) do + ~H""" +
+
+

Videos

+

{@total} total

+
+
+ <%= if @select_count > 0 do %> + + + + <% end %> + <%= if @filters_active do %> + + <% end %> +
+
+ """ + end + + attr :valid_states, :list, required: true + attr :state_counts, :map, required: true + attr :state_filter, :any, required: true + + defp video_state_filters(assigns) do + ~H""" +
+ <%= for state <- @valid_states do %> + <% count = Map.get(@state_counts, String.to_existing_atom(state), 0) %> + + <% end %> +
+ """ + end + + attr :search, :string, required: true + attr :state_filter, :any, required: true + attr :service_filter, :any, required: true + attr :hdr_filter, :any, required: true + attr :valid_states, :list, required: true + attr :per_page, :integer, required: true + attr :per_page_options, :list, required: true + + defp videos_toolbar(assigns) do + ~H""" +
+
+
+
+
-
- <%= if @select_count > 0 do %> - - - + + + + + +
+ +
+
+
+ """ + end - -
- <%= for state <- @valid_states do %> - <% count = Map.get(@state_counts, String.to_existing_atom(state), 0) %> - - <% end %> + attr :loading, :boolean, required: true + attr :videos, :list, required: true + attr :selected, MapSet, required: true + attr :select_count, :integer, required: true + attr :sort_by, :atom, required: true + attr :sort_dir, :atom, required: true + attr :expanded_bad_forms, :list, required: true + attr :meta, Flop.Meta, required: true + attr :url_query, :map, required: true + + defp videos_results(assigns) do + ~H""" + <%= if @loading and @videos == [] do %> +
+
+

Loading...

+
+ <% else %> + <%= if @loading do %> +

Refreshing results...

+ <% end %> + <.videos_table + videos={@videos} + selected={@selected} + select_count={@select_count} + sort_by={@sort_by} + sort_dir={@sort_dir} + expanded_bad_forms={@expanded_bad_forms} + /> + + <.flop_pagination + id="videos-flop-pagination" + meta={@meta} + base_path="/videos" + query={@url_query} + mode={:simple} + /> + <% end %> + """ + end + + attr :videos, :list, required: true + attr :selected, MapSet, required: true + attr :select_count, :integer, required: true + attr :sort_by, :atom, required: true + attr :sort_dir, :atom, required: true + attr :expanded_bad_forms, :list, required: true - -
-
-
-
+ defp videos_table(assigns) do + ~H""" +
+ + + + + <.col_header + col={:path} + label="File" + sort_by={@sort_by} + sort_dir={@sort_dir} + class="w-full" + /> + <.col_header col={:state} label="State" sort_by={@sort_by} sort_dir={@sort_dir} /> + <.col_header col={:size} label="Size" sort_by={@sort_by} sort_dir={@sort_dir} /> + <.col_header + col={:updated_at} + label="Updated" + sort_by={@sort_by} + sort_dir={@sort_dir} + /> + + + + + <%= for video <- @videos do %> + <.video_row video={video} selected={@selected} expanded_bad_forms={@expanded_bad_forms} /> <% end %> -
-
+ <%= if length(@videos) > 0 do %> - - - - - - -
- -
- - - - - <%= if @loading and @videos == [] do %> -
-
-
-

Loading...

-
- <% else %> - <%= if @loading do %> -

Refreshing results...

+ <% end %> + 		+ Actions +
- - - - <.col_header - col={:path} - label="File" - sort_by={@sort_by} - sort_dir={@sort_dir} - class="w-full" - /> - <.col_header col={:state} label="State" sort_by={@sort_by} sort_dir={@sort_dir} /> - <.col_header col={:size} label="Size" sort_by={@sort_by} sort_dir={@sort_dir} /> - <.col_header - col={:updated_at} - label="Updated" - sort_by={@sort_by} - sort_dir={@sort_dir} - /> - - - - - <%= for video <- @videos do %> - - - - - - - - - <% end %> - <%= if @videos == [] do %> - - - - <% end %> - -
- <%= if length(@videos) > 0 do %> - - <% end %> - - Actions -
- - -
{Path.basename(video.path)}
- <%= if video.title do %> -
- {video.title} - <%= if video.content_year do %> - ({video.content_year}) - <% end %> -
- <% end %> -
- - {Path.basename(Path.dirname(video.path))} - - {format_resolution(video.width, video.height)} - {format_bitrate(video.bitrate)} - {service_display(video.service_type)} - <%= if video.hdr do %> - <.hdr_badge hdr={video.hdr} /> - <% end %> - <%= if video.original_size && video.size do %> - <.space_saved_badge - original_size={video.original_size} - current_size={video.size} - /> - <% end %> -
- 		- - {video.state} - - - {format_size(video.size)} - - {format_datetime(video.updated_at)} - -
-
- <%= if queueable_video?(video) do %> - - <% end %> - <%= if queueable_video?(video) and season_directory(video.path) do %> - - <% end %> - <%= if fail_action_video?(video) do %> - - <% end %> - - <%= if video.state in [:failed, :encoded, :crf_searched, :analyzed] do %> - - <% end %> - - -
- <%= if video.id in @expanded_bad_forms do %> -
-
- - - - -
-
- <% end %> -
-
- No videos match the current filters. -
-
+ <%= if @videos == [] do %> + + + No videos match the current filters. + + + <% end %> + + +
+ """ + end + + attr :video, :map, required: true + attr :selected, MapSet, required: true + attr :expanded_bad_forms, :list, required: true - -
- {pagination_label(@page, @per_page, @total)} -
- - {@page} / {@max_page} - -
+ defp video_row(assigns) do + ~H""" + + + + + +
{Path.basename(@video.path)}
+ <%= if @video.title do %> +
+ {@video.title} + <%= if @video.content_year do %> + ({@video.content_year}) + <% end %>
<% end %> -
+
+ {Path.basename(Path.dirname(@video.path))} + {format_resolution(@video.width, @video.height)} + {format_bitrate(@video.bitrate)} + {service_display(@video.service_type)} + <%= if @video.hdr do %> + <.hdr_badge hdr={@video.hdr} /> + <% end %> + <%= if @video.original_size && @video.size do %> + <.space_saved_badge original_size={@video.original_size} current_size={@video.size} /> + <% end %> +
+ + + + {@video.state} + + + {format_size(@video.size)} + + {format_datetime(@video.updated_at)} + + + <.video_actions video={@video} /> + <.mark_bad_form :if={@video.id in @expanded_bad_forms} video={@video} /> + + + """ + end + + attr :video, :map, required: true + + defp video_actions(assigns) do + ~H""" +
+ <%= if queueable_video?(@video) do %> + + <% end %> + <%= if queueable_video?(@video) and season_directory(@video.path) do %> + + <% end %> + <%= if fail_action_video?(@video) do %> + + <% end %> + + <%= if @video.state in [:failed, :encoded, :crf_searched, :analyzed] do %> + + <% end %> + +
""" end - # --------------------------------------------------------------------------- - # Components - # --------------------------------------------------------------------------- + attr :video, :map, required: true + + defp mark_bad_form(assigns) do + ~H""" +
+
+ + + + +
+
+ """ + end attr :col, :atom, required: true attr :label, :string, required: true diff --git a/test/reencodarr_web/live/videos_live_test.exs b/test/reencodarr_web/live/videos_live_test.exs index 4e179ade..768919de 100644 --- a/test/reencodarr_web/live/videos_live_test.exs +++ b/test/reencodarr_web/live/videos_live_test.exs @@ -190,12 +190,13 @@ defmodule ReencodarrWeb.VideosLiveTest do # --------------------------------------------------------------------------- describe "pagination" do - test "prev_page button is disabled on page 1", %{conn: conn} do + test "renders flop pagination on page 1", %{conn: conn} do {:ok, view, _html} = live(conn, ~p"/videos?page=1") html = render(view) - # Button is rendered as disabled when already on first page - assert html =~ "phx-click=\"prev_page\"" - assert html =~ "disabled" + + assert html =~ ~s(data-role="flop-pagination") + assert html =~ ~s(data-role="flop-pagination-label") + assert html =~ "0 results" end test "per_page event updates items per page", %{conn: conn} do @@ -339,7 +340,8 @@ defmodule ReencodarrWeb.VideosLiveTest do assert html =~ "Marked as bad" - [issue] = Reencodarr.Media.list_bad_file_issues() + {issues, _} = Reencodarr.Media.list_bad_file_issues(%{"page_size" => "250"}) + [issue] = issues assert issue.video_id == video.id assert issue.issue_kind == :manual assert issue.classification == :manual_bad @@ -366,7 +368,8 @@ defmodule ReencodarrWeb.VideosLiveTest do |> render_submit() assert html =~ "Mark bad failed" - assert Reencodarr.Media.list_bad_file_issues() == [] + {issues, _} = Reencodarr.Media.list_bad_file_issues(%{"page_size" => "250"}) + assert issues == [] end end From 7f7b176577e7b5e1e2791cebe82239f6e4aa3dbb Mon Sep 17 00:00:00 2001 From: Mika Cohen Date: Mon, 29 Jun 2026 14:33:33 -0600 Subject: [PATCH 06/12] Bump Phoenix LiveView patch deps --- mix.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mix.lock b/mix.lock index 2402b686..d2b7e2a0 100644 --- a/mix.lock +++ b/mix.lock @@ -56,10 +56,10 @@ "phoenix_html": {:hex, :phoenix_html, "4.3.0", "d3577a5df4b6954cd7890c84d955c470b5310bb49647f0a114a6eeecc850f7ad", [:mix], [], "hexpm", "3eaa290a78bab0f075f791a46a981bbe769d94bc776869f4f3063a14f30497ad"}, "phoenix_live_dashboard": {:hex, :phoenix_live_dashboard, "0.8.7", "405880012cb4b706f26dd1c6349125bfc903fb9e44d1ea668adaf4e04d4884b7", [:mix], [{:ecto, "~> 3.6.2 or ~> 3.7", [hex: :ecto, repo: "hexpm", optional: true]}, {:ecto_mysql_extras, "~> 0.5", [hex: :ecto_mysql_extras, repo: "hexpm", optional: true]}, {:ecto_psql_extras, "~> 0.7", [hex: :ecto_psql_extras, repo: "hexpm", optional: true]}, {:ecto_sqlite3_extras, "~> 1.1.7 or ~> 1.2.0", [hex: :ecto_sqlite3_extras, repo: "hexpm", optional: true]}, {:mime, "~> 1.6 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:phoenix_live_view, "~> 0.19 or ~> 1.0", [hex: :phoenix_live_view, repo: "hexpm", optional: false]}, {:telemetry_metrics, "~> 0.6 or ~> 1.0", [hex: :telemetry_metrics, repo: "hexpm", optional: false]}], "hexpm", "3a8625cab39ec261d48a13b7468dc619c0ede099601b084e343968309bd4d7d7"}, "phoenix_live_reload": {:hex, :phoenix_live_reload, "1.6.2", "b18b0773a1ba77f28c52decbb0f10fd1ac4d3ae5b8632399bbf6986e3b665f62", [:mix], [{:file_system, "~> 0.2.10 or ~> 1.0", [hex: :file_system, repo: "hexpm", optional: false]}, {:phoenix, "~> 1.4", [hex: :phoenix, repo: "hexpm", optional: false]}], "hexpm", "d1f89c18114c50d394721365ffb428cce24f1c13de0467ffa773e2ff4a30d5b9"}, - "phoenix_live_view": {:hex, :phoenix_live_view, "1.2.1", "39db357b4293e168a62c43128354df4be0ad8bcb5aadacf403c0d9526b076f5a", [:mix], [{:igniter, ">= 0.6.16 and < 1.0.0-0", [hex: :igniter, repo: "hexpm", optional: true]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:lazy_html, "~> 0.1.0", [hex: :lazy_html, repo: "hexpm", optional: true]}, {:phoenix, "~> 1.6.15 or ~> 1.7.0 or ~> 1.8.0", [hex: :phoenix, repo: "hexpm", optional: false]}, {:phoenix_html, "~> 3.3 or ~> 4.0", [hex: :phoenix_html, repo: "hexpm", optional: false]}, {:phoenix_template, "~> 1.0", [hex: :phoenix_template, repo: "hexpm", optional: false]}, {:phoenix_view, "~> 2.0", [hex: :phoenix_view, repo: "hexpm", optional: true]}, {:plug, "~> 1.15", [hex: :plug, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4.2 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "7ddd6e85a828f08666aea23bdc3a81b1773cd16a94eee459a41bafb18c5a069f"}, + "phoenix_live_view": {:hex, :phoenix_live_view, "1.2.4", "804d80c4672f35bafc2bc461676179e1c37327512471267e1ed970344066956e", [:mix], [{:igniter, ">= 0.6.16 and < 1.0.0-0", [hex: :igniter, repo: "hexpm", optional: true]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:lazy_html, "~> 0.1.0", [hex: :lazy_html, repo: "hexpm", optional: true]}, {:phoenix, "~> 1.6.15 or ~> 1.7.0 or ~> 1.8.0", [hex: :phoenix, repo: "hexpm", optional: false]}, {:phoenix_html, "~> 3.3 or ~> 4.0", [hex: :phoenix_html, repo: "hexpm", optional: false]}, {:phoenix_template, "~> 1.0", [hex: :phoenix_template, repo: "hexpm", optional: false]}, {:phoenix_view, "~> 2.0", [hex: :phoenix_view, repo: "hexpm", optional: true]}, {:plug, "~> 1.15", [hex: :plug, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4.2 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "d6a93564e4680edd78bf1a0dba27b08e32f4fe2ccbaf82b66eadeba0be956e39"}, "phoenix_pubsub": {:hex, :phoenix_pubsub, "2.2.0", "ff3a5616e1bed6804de7773b92cbccfc0b0f473faf1f63d7daf1206c7aeaaa6f", [:mix], [], "hexpm", "adc313a5bf7136039f63cfd9668fde73bba0765e0614cba80c06ac9460ff3e96"}, "phoenix_template": {:hex, :phoenix_template, "1.0.4", "e2092c132f3b5e5b2d49c96695342eb36d0ed514c5b252a77048d5969330d639", [:mix], [{:phoenix_html, "~> 2.14.2 or ~> 3.0 or ~> 4.0", [hex: :phoenix_html, repo: "hexpm", optional: true]}], "hexpm", "2c0c81f0e5c6753faf5cca2f229c9709919aba34fab866d3bc05060c9c444206"}, - "plug": {:hex, :plug, "1.19.2", "e4950525b22c6789dfb38a3f95d47171ba159da3fc5a33be9643b43d5e8adb98", [:mix], [{:mime, "~> 1.0 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:plug_crypto, "~> 1.1.1 or ~> 1.2 or ~> 2.0", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4.3 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "b6fce20a56af5e60fa5dfecf3f907bb98ec981be43c79a3809a499bc3d133de0"}, + "plug": {:hex, :plug, "1.20.1", "82cdee1d7535d4f4db5c5602a7fd49512d64690be54fd62374856ee70e62eb29", [:mix], [{:mime, "~> 1.0 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:plug_crypto, "~> 1.1.1 or ~> 1.2 or ~> 2.0", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4.3 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "892d2a1a7a3f5368c5a3b9067bba1050c031495f48c430ec00b09691dbf211b7"}, "plug_crypto": {:hex, :plug_crypto, "2.1.1", "19bda8184399cb24afa10be734f84a16ea0a2bc65054e23a62bb10f06bc89491", [:mix], [], "hexpm", "6470bce6ffe41c8bd497612ffde1a7e4af67f36a15eea5f921af71cf3e11247c"}, "req": {:hex, :req, "0.6.1", "7b904c8b42d0e08136a5c6aba024fd12fc79a1ed8856e7a3522b0917f7e75113", [:mix], [{:brotli, "~> 0.3.1", [hex: :brotli, repo: "hexpm", optional: true]}, {:ezstd, "~> 1.0", [hex: :ezstd, repo: "hexpm", optional: true]}, {:finch, "~> 0.21.0 or ~> 0.22.0", [hex: :finch, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}, {:mime, "~> 2.0.6 or ~> 2.1", [hex: :mime, repo: "hexpm", optional: false]}, {:nimble_csv, "~> 1.0", [hex: :nimble_csv, repo: "hexpm", optional: true]}, {:plug, "~> 1.0", [hex: :plug, repo: "hexpm", optional: true]}], "hexpm", "aaf11c9c80f2df2364630b3594e1857fe610d8ea7cb994e1ce3dcb55f204ff1c"}, "req_fuse": {:hex, :req_fuse, "0.3.2", "8f96b26527deefe3d128496c058a23014754a569d12d281905d4c9e56bc3bae2", [:mix], [{:fuse, ">= 2.4.0", [hex: :fuse, repo: "hexpm", optional: false]}, {:req, ">= 0.4.14", [hex: :req, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "55cf642c03f10aed0dc4f97adc10f0985b355b377d2bc32bb0c569d82f3aa07e"}, From 0733311ecfb03915a57d8c3003dc66a917252ab9 Mon Sep 17 00:00:00 2001 From: Mika Cohen Date: Mon, 29 Jun 2026 15:01:36 -0600 Subject: [PATCH 07/12] Address list LiveView review comments --- lib/reencodarr/bad_files/state.ex | 48 +++++++++++++--- lib/reencodarr/media.ex | 28 ++++++---- lib/reencodarr_web/live/failures_live.ex | 25 ++++++++- lib/reencodarr_web/live/videos_live.ex | 44 ++++++++++++++- test/reencodarr/bad_files_state_test.exs | 55 +++++++++++++++++++ .../media/list_bad_file_issues_test.exs | 39 +++++++++++++ .../live/bad_files_live_test.exs | 23 ++++++++ .../live/failures_live_test.exs | 4 +- test/reencodarr_web/live/flop_list_test.exs | 12 ++-- 9 files changed, 249 insertions(+), 29 deletions(-) diff --git a/lib/reencodarr/bad_files/state.ex b/lib/reencodarr/bad_files/state.ex index e80cbb66..3adc1032 100644 --- a/lib/reencodarr/bad_files/state.ex +++ b/lib/reencodarr/bad_files/state.ex @@ -13,7 +13,10 @@ defmodule Reencodarr.BadFiles.State do {active_issues, meta} = fetch_active_issues(active_statuses, assigns) active_total = meta.total_count || 0 issue_summary = Media.bad_file_issue_summary() - resolved_issues = fetch_resolved_issues(assigns, resolved_statuses, assigns.show_resolved) + + resolved_issues = + fetch_resolved_issues(assigns, resolved_statuses, show_resolved_issues?(assigns)) + issues = active_issues ++ resolved_issues %{ @@ -43,11 +46,13 @@ defmodule Reencodarr.BadFiles.State do end def list_active_issues(assigns, opts \\ []) do - assigns - |> Map.put(:page, Keyword.get(opts, :page, 1)) - |> Map.put(:per_page, Keyword.get(opts, :per_page, 250)) - |> list_issues(active_statuses_for_filter(assigns.status_filter)) - |> elem(0) + page_size = Keyword.get(opts, :per_page, 250) + + fetch_all_issues( + assigns, + active_statuses_for_filter(assigns.status_filter), + page_size + ) end def flop_params(assigns) do @@ -60,14 +65,35 @@ defmodule Reencodarr.BadFiles.State do } end - defp list_issues(assigns, active_statuses), do: fetch_active_issues(active_statuses, assigns) - defp fetch_active_issues([], _assigns), do: {[], %Flop.Meta{}} defp fetch_active_issues(active_statuses, assigns) do Media.list_bad_file_issues(flop_params(assigns), statuses: active_statuses) end + defp fetch_all_issues(_assigns, [], _page_size), do: [] + + defp fetch_all_issues(assigns, statuses, page_size) do + assigns + |> Map.put(:page, 1) + |> Map.put(:per_page, page_size) + |> fetch_all_issues(statuses, page_size, []) + end + + defp fetch_all_issues(assigns, statuses, page_size, acc) do + {issues, meta} = fetch_active_issues(statuses, assigns) + acc = acc ++ issues + + if meta.current_page && meta.total_pages && meta.current_page < meta.total_pages do + assigns + |> Map.put(:page, meta.current_page + 1) + |> Map.put(:per_page, page_size) + |> fetch_all_issues(statuses, page_size, acc) + else + acc + end + end + defp fetch_resolved_issues(_assigns, _resolved_statuses, false), do: [] defp fetch_resolved_issues(_assigns, [], true), do: [] @@ -98,4 +124,10 @@ defmodule Reencodarr.BadFiles.State do rescue ArgumentError -> {@active_statuses, @resolved_statuses} end + + defp show_resolved_issues?(%{status_filter: status_filter}) + when status_filter in ["resolved", "replaced_clean", "dismissed"], + do: true + + defp show_resolved_issues?(assigns), do: assigns.show_resolved end diff --git a/lib/reencodarr/media.ex b/lib/reencodarr/media.ex index cb384a01..8071c3e2 100644 --- a/lib/reencodarr/media.ex +++ b/lib/reencodarr/media.ex @@ -577,16 +577,10 @@ defmodule Reencodarr.Media do {:error, :not_series_scoped} group_key -> - {issues, _} = - list_bad_file_issues(%{"page" => "1", "page_size" => "250"}, - statuses: BadFileIssue.status_values() -- @resolved_bad_file_issue_statuses - ) - issues = - issues + unresolved_bad_file_issues() |> Enum.filter(fn candidate -> - unresolved_bad_file_issue?(candidate) and - series_group_key(candidate.video) == group_key + series_group_key(candidate.video) == group_key end) Enum.each(issues, &enqueue_bad_file_issue/1) @@ -1039,8 +1033,22 @@ defmodule Reencodarr.Media do defp failure_category_atom("codec_issues"), do: {:ok, :codec_issues} defp failure_category_atom(_), do: :error - defp unresolved_bad_file_issue?(issue) do - issue.status not in [:replaced_clean, :dismissed] + defp unresolved_bad_file_issues do + statuses = BadFileIssue.status_values() -- @resolved_bad_file_issue_statuses + collect_bad_file_issues(%{"page" => "1", "page_size" => "250"}, statuses, []) + end + + defp collect_bad_file_issues(params, statuses, acc) do + {issues, meta} = list_bad_file_issues(params, statuses: statuses) + acc = acc ++ issues + + if meta.current_page && meta.total_pages && meta.current_page < meta.total_pages do + params + |> Map.put("page", to_string(meta.current_page + 1)) + |> collect_bad_file_issues(statuses, acc) + else + acc + end end defp series_group_key(%Video{service_type: :sonarr, path: path}) when is_binary(path) do diff --git a/lib/reencodarr_web/live/failures_live.ex b/lib/reencodarr_web/live/failures_live.ex index a8e0e142..fd19ee11 100644 --- a/lib/reencodarr_web/live/failures_live.ex +++ b/lib/reencodarr_web/live/failures_live.ex @@ -53,6 +53,7 @@ defmodule ReencodarrWeb.FailuresLive do socket = socket |> assign(filters) + |> maybe_clear_selection(filters_changed?) |> assign_url_query() |> reload_failures_for_params(filters_changed?) @@ -359,6 +360,7 @@ defmodule ReencodarrWeb.FailuresLive do
@@ -737,13 +743,28 @@ defmodule ReencodarrWeb.FailuresLive do defp fetch_failure_payload(assigns) do payload = Media.load_failures_page(flop_params(assigns)) - Map.put(payload, :url_query, failures_url_query(%{assigns | page: payload.page})) + + payload + |> Map.put(:request, assigns) + |> Map.put(:url_query, failures_url_query(%{assigns | page: payload.page})) + end + + defp assign_failure_payload(socket, %{request: request} = payload) do + if flop_list_assigns(socket.assigns) == request do + payload = Map.delete(payload, :request) + assign(socket, payload) + else + socket + end end defp assign_failure_payload(socket, payload) do assign(socket, payload) end + defp maybe_clear_selection(socket, true), do: assign(socket, :selected_videos, MapSet.new()) + defp maybe_clear_selection(socket, false), do: socket + defp filters_changed?(assigns, filters) do Enum.any?(@param_keys, fn key -> Map.get(assigns, key) != Map.get(filters, key) end) end @@ -803,7 +824,7 @@ defmodule ReencodarrWeb.FailuresLive do "stage" => assigns.failure_filter, "category" => assigns.category_filter, "search" => assigns.search_term, - "page_size" => to_string(assigns.per_page) + "per_page" => to_string(assigns.per_page) } |> Enum.reject(fn {"stage", "all"} -> true diff --git a/lib/reencodarr_web/live/videos_live.ex b/lib/reencodarr_web/live/videos_live.ex index 065bf76b..099e13ea 100644 --- a/lib/reencodarr_web/live/videos_live.ex +++ b/lib/reencodarr_web/live/videos_live.ex @@ -30,7 +30,7 @@ defmodule ReencodarrWeb.VideosLive do @update_interval 30_000 @queueable_states [:needs_analysis, :analyzed, :crf_searched] - @valid_states ~w(needs_analysis analyzed crf_searching crf_searched encoding encoded failed) + @valid_states ~w(needs_analysis analyzing analyzed crf_searching crf_searched encoding encoded failed) @valid_service_types ~w(sonarr radarr) @valid_sort_fields ~w(path state size width bitrate updated_at) @valid_sort_dirs ~w(asc desc) @@ -111,7 +111,7 @@ defmodule ReencodarrWeb.VideosLive do @impl true def handle_async(:load_videos, {:ok, page_state}, socket) do - {:noreply, assign(socket, Map.put(page_state, :loading, false))} + {:noreply, assign_video_payload(socket, page_state)} end @impl true @@ -472,7 +472,7 @@ defmodule ReencodarrWeb.VideosLive do defp load_data(socket, opts \\ []) do page_state = fetch_video_payload(socket.assigns, opts) - assign(socket, Map.put(page_state, :loading, false)) + assign_video_payload(socket, page_state) end defp reload_videos_for_params(%{assigns: %{loaded_once: false}} = socket, _changed?) do @@ -508,6 +508,7 @@ defmodule ReencodarrWeb.VideosLive do video_load_assigns(assigns), opts ) + |> Map.put(:request, video_request_assigns(assigns)) end defp video_load_assigns(assigns) do @@ -524,6 +525,32 @@ defmodule ReencodarrWeb.VideosLive do ]) end + defp video_request_assigns(assigns) do + Map.take(assigns, [ + :page, + :per_page, + :state_filter, + :service_filter, + :hdr_filter, + :search, + :sort_by, + :sort_dir + ]) + end + + defp assign_video_payload(socket, %{request: request} = page_state) do + if video_request_assigns(socket.assigns) == request do + page_state = + page_state + |> Map.delete(:request) + |> Map.put(:loading, false) + + assign(socket, page_state) + else + socket + end + end + defp apply_range_selection(selected_set, ids, true) do Enum.reduce(ids, selected_set, &MapSet.put(&2, &1)) end @@ -853,11 +880,13 @@ defmodule ReencodarrWeb.VideosLive do value={@search} placeholder="Search by path..." phx-debounce="700" + aria-label="Search videos by path" class="w-full bg-gray-700 border border-gray-600 text-white rounded-lg px-3 py-2 text-sm focus:ring-purple-500 focus:border-purple-500 placeholder-gray-400" />
<%= for n <- @per_page_options do %> @@ -966,6 +998,11 @@ defmodule ReencodarrWeb.VideosLive do do: "Deselect all", else: "Select all on page" } + aria-label={ + if @select_count == length(@videos), + do: "Deselect all videos on this page", + else: "Select all videos on this page" + } class="rounded border-gray-500 bg-gray-700 text-purple-500 focus:ring-purple-500 focus:ring-offset-gray-800 cursor-pointer" /> <% end %> @@ -1024,6 +1061,7 @@ defmodule ReencodarrWeb.VideosLive do checked={MapSet.member?(@selected, @video.id)} data-range-select="video" data-id={@video.id} + aria-label={"Select video #{Path.basename(@video.path)}"} class="rounded border-gray-500 bg-gray-700 text-purple-500 focus:ring-purple-500 focus:ring-offset-gray-800 cursor-pointer" /> diff --git a/test/reencodarr/bad_files_state_test.exs b/test/reencodarr/bad_files_state_test.exs index 2f197db2..362c600b 100644 --- a/test/reencodarr/bad_files_state_test.exs +++ b/test/reencodarr/bad_files_state_test.exs @@ -32,4 +32,59 @@ defmodule Reencodarr.BadFiles.StateTest do assert is_map(payload.issue_summary) assert payload.tracked_count >= 1 end + + test "resolved filter loads resolved issues even when show_resolved is false" do + {:ok, video} = Fixtures.video_fixture(%{path: "/media/resolved_filter_payload.mkv"}) + + {:ok, issue} = + Media.create_bad_file_issue(video, %{ + origin: :manual, + issue_kind: :manual, + classification: :manual_bad, + manual_reason: "resolved filter" + }) + + {:ok, _issue} = Media.dismiss_bad_file_issue(issue) + + payload = + State.load(%{ + page: 1, + per_page: 50, + status_filter: "resolved", + service_filter: "all", + kind_filter: "all", + search_query: "resolved_filter_payload", + show_resolved: false + }) + + assert Enum.any?(payload.resolved_issues, &(&1.video_id == video.id)) + assert Enum.any?(payload.issues, &(&1.video_id == video.id)) + end + + test "list_active_issues returns all matching pages for bulk actions" do + Enum.each(1..251, fn n -> + {:ok, video} = Fixtures.video_fixture(%{path: "/media/bulk_filtered_#{n}.mkv"}) + + {:ok, _issue} = + Media.create_bad_file_issue(video, %{ + origin: :manual, + issue_kind: :manual, + classification: :manual_bad, + manual_reason: "bulk filtered" + }) + end) + + issues = + State.list_active_issues(%{ + page: 1, + per_page: 50, + status_filter: "all", + service_filter: "all", + kind_filter: "all", + search_query: "bulk_filtered", + show_resolved: false + }) + + assert Enum.count(issues) == 251 + end end diff --git a/test/reencodarr/media/list_bad_file_issues_test.exs b/test/reencodarr/media/list_bad_file_issues_test.exs index b347b133..e4ac8b69 100644 --- a/test/reencodarr/media/list_bad_file_issues_test.exs +++ b/test/reencodarr/media/list_bad_file_issues_test.exs @@ -45,4 +45,43 @@ defmodule Reencodarr.Media.ListBadFileIssuesTest do assert meta.total_count == 1 assert hd(issues).video.path =~ "searchable_bad" end + + test "queue_bad_file_issue_series does not stop at the first bad-file page" do + {:ok, series_video} = + Fixtures.video_fixture(%{ + path: "/media/Series Queue/Season 01/series_queue_target.mkv", + service_type: :sonarr + }) + + {:ok, issue} = + Media.create_bad_file_issue(series_video, %{ + origin: :manual, + issue_kind: :manual, + classification: :manual_bad, + manual_reason: "series queue target" + }) + + Enum.each(1..251, fn n -> + {:ok, video} = + Fixtures.video_fixture(%{ + path: "/media/Other Series #{n}/Season 01/decoy_#{n}.mkv", + service_type: :sonarr + }) + + {:ok, _issue} = + Media.create_bad_file_issue(video, %{ + origin: :manual, + issue_kind: :manual, + classification: :manual_bad, + manual_reason: "series queue decoy" + }) + end) + + assert {:ok, 1} = Media.queue_bad_file_issue_series(issue) + + assert %{status: :queued} = + issue.id + |> Media.get_bad_file_issue!() + |> Reencodarr.Repo.reload!() + end end diff --git a/test/reencodarr_web/live/bad_files_live_test.exs b/test/reencodarr_web/live/bad_files_live_test.exs index 5af892ee..5c70588b 100644 --- a/test/reencodarr_web/live/bad_files_live_test.exs +++ b/test/reencodarr_web/live/bad_files_live_test.exs @@ -748,6 +748,12 @@ defmodule ReencodarrWeb.BadFilesLiveTest do {:ok, sonarr_video} = Fixtures.video_fixture(%{path: "/media/url_combo.mkv", service_type: :sonarr}) + {:ok, radarr_video} = + Fixtures.video_fixture(%{path: "/media/wrong_service_combo.mkv", service_type: :radarr}) + + {:ok, audio_video} = + Fixtures.video_fixture(%{path: "/media/wrong_kind_combo.mkv", service_type: :sonarr}) + {:ok, _issue} = Media.create_bad_file_issue(sonarr_video, %{ origin: :manual, @@ -756,6 +762,21 @@ defmodule ReencodarrWeb.BadFilesLiveTest do manual_reason: "combo search target" }) + {:ok, _wrong_service} = + Media.create_bad_file_issue(radarr_video, %{ + origin: :manual, + issue_kind: :manual, + classification: :manual_bad, + manual_reason: "combo wrong service" + }) + + {:ok, _wrong_kind} = + Media.create_bad_file_issue(audio_video, %{ + origin: :manual, + issue_kind: :audio, + classification: :confirmed_bad_audio_layout + }) + {:ok, view, _html} = live( conn, @@ -764,6 +785,8 @@ defmodule ReencodarrWeb.BadFilesLiveTest do html = render_async(view) assert html =~ "url_combo.mkv" + refute html =~ "wrong_service_combo.mkv" + refute html =~ "wrong_kind_combo.mkv" end end end diff --git a/test/reencodarr_web/live/failures_live_test.exs b/test/reencodarr_web/live/failures_live_test.exs index 5afe779b..ba04d3c5 100644 --- a/test/reencodarr_web/live/failures_live_test.exs +++ b/test/reencodarr_web/live/failures_live_test.exs @@ -378,7 +378,7 @@ defmodule ReencodarrWeb.FailuresLiveTest do describe "URL bookmarks" do test "stage and search params load from URL", %{conn: conn} do {:ok, analysis_video} = Fixtures.video_fixture(%{path: "/media/url_analysis.mkv"}) - {:ok, encoding_video} = Fixtures.video_fixture(%{path: "/media/url_encoding_other.mkv"}) + {:ok, encoding_video} = Fixtures.video_fixture(%{path: "/media/url_analysis_encoding.mkv"}) Media.record_video_failure(analysis_video, :analysis, :timeout, message: "analysis") Media.record_video_failure(encoding_video, :encoding, :timeout, message: "encoding") @@ -386,7 +386,7 @@ defmodule ReencodarrWeb.FailuresLiveTest do {:ok, _view, html} = live(conn, ~p"/failures?stage=analysis&search=url_analysis") assert html =~ "url_analysis.mkv" - refute html =~ "url_encoding_other.mkv" + refute html =~ "url_analysis_encoding.mkv" end end diff --git a/test/reencodarr_web/live/flop_list_test.exs b/test/reencodarr_web/live/flop_list_test.exs index 08df445b..12b2d289 100644 --- a/test/reencodarr_web/live/flop_list_test.exs +++ b/test/reencodarr_web/live/flop_list_test.exs @@ -95,13 +95,17 @@ defmodule ReencodarrWeb.Live.FlopListTest do describe "patch_with_page/3" do test "omits page param on first page" do - assert FlopList.patch_with_page("/videos", %{"per_page" => 50}, 1) == - "/videos?per_page=50" + url = FlopList.patch_with_page("/videos", %{"per_page" => 50}, 1) + + assert %URI{path: "/videos", query: query} = URI.parse(url) + assert URI.decode_query(query) == %{"per_page" => "50"} end test "includes page param when not on first page" do - assert FlopList.patch_with_page("/videos", %{"per_page" => 50}, 2) == - "/videos?page=2&per_page=50" + url = FlopList.patch_with_page("/videos", %{"per_page" => 50}, 2) + + assert %URI{path: "/videos", query: query} = URI.parse(url) + assert URI.decode_query(query) == %{"page" => "2", "per_page" => "50"} end test "returns bare path when merged query is empty" do From cd4d45725dd444d9991c43b6e9324f2358dfa7e7 Mon Sep 17 00:00:00 2001 From: Mika Cohen Date: Tue, 30 Jun 2026 03:10:38 -0600 Subject: [PATCH 08/12] Fix list LiveView state regressions --- lib/reencodarr/bad_files/state.ex | 66 ++++++++++++------- lib/reencodarr/media.ex | 43 ++++++++---- lib/reencodarr/media/shared_queries.ex | 16 ++++- lib/reencodarr/videos/state.ex | 45 +++++++++---- lib/reencodarr_web/live/bad_files_live.ex | 38 ++++++++--- lib/reencodarr_web/live/failures_live.ex | 11 ++-- lib/reencodarr_web/live/videos_live.ex | 38 ++++++----- test/reencodarr/bad_files_state_test.exs | 41 ++++++++++++ .../media/list_bad_file_issues_test.exs | 19 ++++++ test/reencodarr/media/list_failures_test.exs | 33 ++++++++++ .../live/bad_files_live_test.exs | 10 +-- .../live/failures_live_test.exs | 19 ++++++ test/reencodarr_web/live/videos_live_test.exs | 29 ++++++-- 13 files changed, 313 insertions(+), 95 deletions(-) diff --git a/lib/reencodarr/bad_files/state.ex b/lib/reencodarr/bad_files/state.ex index 3adc1032..f390e183 100644 --- a/lib/reencodarr/bad_files/state.ex +++ b/lib/reencodarr/bad_files/state.ex @@ -5,13 +5,14 @@ defmodule Reencodarr.BadFiles.State do @active_statuses [:open, :queued, :processing, :waiting_for_replacement, :failed] @resolved_statuses [:replaced_clean, :dismissed] - @resolved_limit 50 + @active_status_filters Map.new(@active_statuses, &{Atom.to_string(&1), &1}) + @resolved_status_filters Map.new(@resolved_statuses, &{Atom.to_string(&1), &1}) + @replacement_statuses [:processing, :waiting_for_replacement] @spec load(map()) :: map() def load(assigns) do {active_statuses, resolved_statuses} = statuses_for_filter(assigns.status_filter) {active_issues, meta} = fetch_active_issues(active_statuses, assigns) - active_total = meta.total_count || 0 issue_summary = Media.bad_file_issue_summary() resolved_issues = @@ -25,10 +26,9 @@ defmodule Reencodarr.BadFiles.State do tracked_count: issue_summary.open + issue_summary.queued + issue_summary.processing + issue_summary.waiting_for_replacement + issue_summary.failed + issue_summary.resolved, - active_total: active_total, + active_total: meta.total_count, active_issues: active_issues, - replacement_issues: - Enum.filter(active_issues, &(&1.status in [:processing, :waiting_for_replacement])), + replacement_issues: list_replacement_issues(assigns), resolved_issues: resolved_issues, issue_summary: issue_summary } @@ -37,14 +37,15 @@ defmodule Reencodarr.BadFiles.State do def active_statuses_for_filter("all"), do: @active_statuses def active_statuses_for_filter("resolved"), do: [] - def active_statuses_for_filter(status_filter) do - status = String.to_existing_atom(status_filter) - - if status in @active_statuses, do: [status], else: @active_statuses - rescue - ArgumentError -> @active_statuses + def active_statuses_for_filter(status_filter) when is_binary(status_filter) do + case Map.fetch(@active_status_filters, status_filter) do + {:ok, status} -> [status] + :error -> @active_statuses + end end + def active_statuses_for_filter(_status_filter), do: @active_statuses + def list_active_issues(assigns, opts \\ []) do page_size = Keyword.get(opts, :per_page, 250) @@ -65,7 +66,7 @@ defmodule Reencodarr.BadFiles.State do } end - defp fetch_active_issues([], _assigns), do: {[], %Flop.Meta{}} + defp fetch_active_issues([], %{per_page: per_page}), do: {[], empty_meta(per_page)} defp fetch_active_issues(active_statuses, assigns) do Media.list_bad_file_issues(flop_params(assigns), statuses: active_statuses) @@ -78,13 +79,14 @@ defmodule Reencodarr.BadFiles.State do |> Map.put(:page, 1) |> Map.put(:per_page, page_size) |> fetch_all_issues(statuses, page_size, []) + |> Enum.reverse() end defp fetch_all_issues(assigns, statuses, page_size, acc) do {issues, meta} = fetch_active_issues(statuses, assigns) - acc = acc ++ issues + acc = Enum.reverse(issues, acc) - if meta.current_page && meta.total_pages && meta.current_page < meta.total_pages do + if more_pages?(meta) do assigns |> Map.put(:page, meta.current_page + 1) |> Map.put(:per_page, page_size) @@ -94,15 +96,15 @@ defmodule Reencodarr.BadFiles.State do end end + defp fetch_resolved_issues(_assigns, [], _show_resolved), do: [] defp fetch_resolved_issues(_assigns, _resolved_statuses, false), do: [] - defp fetch_resolved_issues(_assigns, [], true), do: [] defp fetch_resolved_issues(assigns, resolved_statuses, true) do {issues, _} = Media.list_bad_file_issues( assigns |> Map.put(:page, 1) - |> Map.put(:per_page, @resolved_limit) + |> Map.put(:per_page, assigns.per_page) |> flop_params(), statuses: resolved_statuses ) @@ -110,19 +112,33 @@ defmodule Reencodarr.BadFiles.State do issues end + defp list_replacement_issues(assigns) do + fetch_all_issues(assigns, @replacement_statuses, assigns.per_page) + end + defp statuses_for_filter("all"), do: {@active_statuses, @resolved_statuses} defp statuses_for_filter("resolved"), do: {[], @resolved_statuses} - defp statuses_for_filter(status_filter) do - status = String.to_existing_atom(status_filter) - - cond do - status in @active_statuses -> {[status], []} - status in @resolved_statuses -> {[], [status]} - true -> {@active_statuses, @resolved_statuses} + defp statuses_for_filter(status_filter) when is_binary(status_filter) do + with :error <- Map.fetch(@active_status_filters, status_filter), + :error <- Map.fetch(@resolved_status_filters, status_filter) do + {@active_statuses, @resolved_statuses} + else + {:ok, status} when status in @active_statuses -> {[status], []} + {:ok, status} -> {[], [status]} end - rescue - ArgumentError -> {@active_statuses, @resolved_statuses} + end + + defp statuses_for_filter(_status_filter), do: {@active_statuses, @resolved_statuses} + + defp more_pages?(%Flop.Meta{current_page: current_page, total_pages: total_pages}) + when is_integer(current_page) and is_integer(total_pages), + do: current_page < total_pages + + defp more_pages?(_meta), do: false + + defp empty_meta(per_page) do + %Flop.Meta{current_page: 1, page_size: per_page, total_count: 0, total_pages: 1} end defp show_resolved_issues?(%{status_filter: status_filter}) diff --git a/lib/reencodarr/media.ex b/lib/reencodarr/media.ex index 8071c3e2..eb446925 100644 --- a/lib/reencodarr/media.ex +++ b/lib/reencodarr/media.ex @@ -919,18 +919,18 @@ defmodule Reencodarr.Media do defp bad_file_search_filter(query, ""), do: query defp bad_file_search_filter(query, search) do - pattern = "%" <> search <> "%" + pattern = SharedQueries.like_contains_pattern(search) query |> bad_file_ensure_video_join() |> then( &from([i, video: v] in &1, where: - fragment("lower(?) like ?", v.path, ^pattern) or - fragment("lower(coalesce(?, '')) like ?", i.manual_reason, ^pattern) or - fragment("lower(coalesce(?, '')) like ?", i.manual_note, ^pattern) or - fragment("lower(?) like ?", i.classification, ^pattern) or - fragment("lower(?) like ?", i.issue_kind, ^pattern) + fragment("lower(?) like ? escape '\\'", v.path, ^pattern) or + fragment("lower(coalesce(?, '')) like ? escape '\\'", i.manual_reason, ^pattern) or + fragment("lower(coalesce(?, '')) like ? escape '\\'", i.manual_note, ^pattern) or + fragment("lower(?) like ? escape '\\'", i.classification, ^pattern) or + fragment("lower(?) like ? escape '\\'", i.issue_kind, ^pattern) ) ) end @@ -957,7 +957,7 @@ defmodule Reencodarr.Media do defp failure_search_filter(query, ""), do: query defp failure_search_filter(query, search) do - pattern = "%#{search}%" + pattern = SharedQueries.like_contains_pattern(search) condition = SharedQueries.case_insensitive_like(:path, pattern) from(v in query, where: ^condition) end @@ -971,13 +971,15 @@ defmodule Reencodarr.Media do defp failure_normalize_search(search) when is_binary(search), do: String.trim(search) defp failure_normalize_search(_search), do: "" - defp failures_by_video(videos) do + defp failures_by_video(videos, stage, category) do video_ids = Enum.map(videos, & &1.id) from(f in VideoFailure, where: f.video_id in ^video_ids and f.resolved == false, order_by: [desc: f.inserted_at] ) + |> video_failure_stage_filter(stage) + |> video_failure_category_filter(category) |> Repo.all() |> Enum.group_by(& &1.video_id) end @@ -1021,6 +1023,24 @@ defmodule Reencodarr.Media do end end + defp video_failure_stage_filter(query, "all"), do: query + + defp video_failure_stage_filter(query, stage) do + case failure_stage_atom(stage) do + {:ok, atom} -> from(f in query, where: f.failure_stage == ^atom) + :error -> from(f in query, where: false) + end + end + + defp video_failure_category_filter(query, "all"), do: query + + defp video_failure_category_filter(query, category) do + case failure_category_atom(category) do + {:ok, atom} -> from(f in query, where: f.failure_category == ^atom) + :error -> from(f in query, where: false) + end + end + defp failure_stage_atom("analysis"), do: {:ok, :analysis} defp failure_stage_atom("crf_search"), do: {:ok, :crf_search} defp failure_stage_atom("encoding"), do: {:ok, :encoding} @@ -1224,18 +1244,17 @@ defmodule Reencodarr.Media do {failed_videos, meta} end - page = meta.current_page || clamped_page - %{ loading: false, failed_videos: failed_videos, - video_failures: failures_by_video(failed_videos), + video_failures: + failures_by_video(failed_videos, params["stage"] || "all", params["category"] || "all"), failure_stats: summarize_failure_stats(get_failure_statistics(days_back: 7)), failure_patterns: get_common_failure_patterns(5), failure_code_actions: list_failed_video_failure_codes(), total_count: total_count, total_pages: total_pages(total_count, per_page), - page: page, + page: clamped_page, per_page: per_page, meta: meta } diff --git a/lib/reencodarr/media/shared_queries.ex b/lib/reencodarr/media/shared_queries.ex index 6b60023a..11549e90 100644 --- a/lib/reencodarr/media/shared_queries.ex +++ b/lib/reencodarr/media/shared_queries.ex @@ -18,8 +18,20 @@ defmodule Reencodarr.Media.SharedQueries do Returns a dynamic query fragment that can be used in where clauses. """ def case_insensitive_like(field, pattern) do - # SQLite: Use LIKE with UPPER() on both sides - dynamic([q], fragment("UPPER(?) LIKE UPPER(?)", field(q, ^field), ^pattern)) + dynamic([q], fragment("UPPER(?) LIKE UPPER(?) ESCAPE '\\'", field(q, ^field), ^pattern)) + end + + def like_contains_pattern(value) when is_binary(value) do + "%" <> escape_like(value) <> "%" + end + + def like_contains_pattern(_value), do: "%" + + defp escape_like(value) do + value + |> String.replace("\\", "\\\\") + |> String.replace("%", "\\%") + |> String.replace("_", "\\_") end @doc """ diff --git a/lib/reencodarr/videos/state.ex b/lib/reencodarr/videos/state.ex index 60fd7838..9c6578a9 100644 --- a/lib/reencodarr/videos/state.ex +++ b/lib/reencodarr/videos/state.ex @@ -7,17 +7,20 @@ defmodule Reencodarr.Videos.State do def load(assigns, opts \\ []) do include_state_counts? = Keyword.get(opts, :include_state_counts, true) + {videos, meta} = list_videos(assigns) + + total = meta.total_count || 0 + per_page = meta.page_size || assigns.per_page + page = min(max(assigns.page, 1), total_pages(total, per_page)) + {videos, meta} = - Media.list_videos_paginated( - page: assigns.page, - per_page: assigns.per_page, - state: assigns.state_filter, - service_type: assigns.service_filter, - hdr: assigns.hdr_filter, - search: assigns.search, - sort_by: assigns.sort_by, - sort_dir: assigns.sort_dir - ) + if page != assigns.page and total > 0 do + assigns + |> Map.put(:page, page) + |> list_videos() + else + {videos, meta} + end state_counts = if include_state_counts? do @@ -29,10 +32,26 @@ defmodule Reencodarr.Videos.State do %{ videos: videos, meta: meta, - total: meta.total_count || 0, - page: meta.current_page || assigns.page, - per_page: meta.page_size || assigns.per_page, + total: total, + page: page, + per_page: per_page, state_counts: state_counts } end + + defp list_videos(assigns) do + Media.list_videos_paginated( + page: assigns.page, + per_page: assigns.per_page, + state: assigns.state_filter, + service_type: assigns.service_filter, + hdr: assigns.hdr_filter, + search: assigns.search, + sort_by: assigns.sort_by, + sort_dir: assigns.sort_dir + ) + end + + defp total_pages(total, _per_page) when total <= 0, do: 1 + defp total_pages(total, per_page), do: max(ceil(total / per_page), 1) end diff --git a/lib/reencodarr_web/live/bad_files_live.ex b/lib/reencodarr_web/live/bad_files_live.ex index bc79ed10..9c311b80 100644 --- a/lib/reencodarr_web/live/bad_files_live.ex +++ b/lib/reencodarr_web/live/bad_files_live.ex @@ -337,9 +337,23 @@ defmodule ReencodarrWeb.BadFilesLive do payload |> Map.put(:page, page) + |> Map.put(:request, assigns) |> Map.put(:url_query, bad_files_url_query(%{assigns | page: page})) end + defp apply_issue_payload(socket, %{request: request} = issue_payload) do + if issue_load_assigns(socket.assigns) == request do + issue_payload = + issue_payload + |> Map.delete(:request) + |> Map.put(:loading_issues, false) + + assign(socket, issue_payload) + else + assign(socket, :loading_issues, false) + end + end + defp apply_issue_payload(socket, issue_payload) do socket |> assign(Map.put(issue_payload, :loading_issues, false)) @@ -397,6 +411,8 @@ defmodule ReencodarrWeb.BadFilesLive do |> BadFilesState.list_active_issues() end + defp filtered_active_total(assigns), do: assigns.active_total || 0 + defp start_service_replacements do [:sonarr, :radarr] |> Enum.map(&BadFileRemediation.process_next_issue(service_type: &1)) @@ -446,17 +462,13 @@ defmodule ReencodarrWeb.BadFilesLive do defp maybe_reload_page(page, payload, assigns) do if page == assigns.page or payload.active_total == 0 do - {payload, payload_page(payload, page)} + {payload, page} else reloaded_payload = assigns |> Map.put(:page, page) |> BadFilesState.load() - {reloaded_payload, payload_page(reloaded_payload, page)} + {reloaded_payload, page} end end - defp payload_page(payload, fallback_page) do - payload.meta.current_page || fallback_page - end - defp url_overrides(overrides) do Map.new(overrides, fn {key, value} -> {to_string(key), value} end) end @@ -469,7 +481,10 @@ defmodule ReencodarrWeb.BadFilesLive do end defp drop_default_query_values(query) do - Map.reject(query, fn {_key, value} -> value in [nil, "", "all"] end) + Map.reject(query, fn + {"per_page", value} -> value in [@default_per_page, to_string(@default_per_page)] + {_key, value} -> value in [nil, "", "all"] + end) end attr :status_filter_values, :list, required: true @@ -479,10 +494,14 @@ defmodule ReencodarrWeb.BadFilesLive do attr :service_filter, :string, required: true attr :kind_filter, :string, required: true attr :search_query, :string, required: true + attr :active_total, :integer, default: 0 defp bad_files_toolbar(assigns) do ~H"""
+
+ Bulk actions apply to all {@active_total} matching active issues. +