Refactor list LiveViews around Flop#18
Draft
mjc wants to merge 8 commits into
Draft
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughIntroduces two new utility modules ( ChangesList Utilities and Refactored LiveViews
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/reencodarr_web/live/bad_files_live.ex`:
- Around line 399-400: The page value in bad-files URL state is only
lower-bounded by ListUrl.parse_page/2, so stale bookmarked pages can survive
even after the real total is known. In bad_files_live.ex, update the handling
around the socket assigns and the <.pagination_bar> flow so the requested page
is clamped against the loaded total (`@active_total`) before render or push_patch,
using the existing page/per_page pagination logic in the live view. Keep the
canonical page in sync with the live view state so impossible page numbers
cannot be displayed.
In `@lib/reencodarr_web/live/list_components.ex`:
- Around line 15-25: The shared controls in search_input/1, filter_select/1, and
pagination_bar/1 need accessible names so screen readers can identify them.
Update these components to accept and pass through global attrs or explicit
labeling props, and use them to label the search field, filter select, and the
internal per-page select in pagination_bar/1. Make sure the new BadFilesLive
usages supply a clear accessible name for each control.
In `@lib/reencodarr_web/live/list_pagination.ex`:
- Around line 25-29: Clamp current_page to the valid 1..total_pages range in
pagination_range/3 before calculating start_page and end_page, so oversized
values cannot produce a descending range or huge Enum.to_list/1 result. Use the
existing pagination_range/3 helper in lib/reencodarr_web/live/list_pagination.ex
to apply the clamp, then keep the radius-based window logic unchanged. Add a
regression test covering pagination_range(999999, 3) to verify it returns a
small window near the last page instead of a massive list.
In `@lib/reencodarr_web/live/list_url.ex`:
- Around line 55-63: patch_path/3 in ListUrl always appends a question mark even
when the merged query is empty, which breaks the clean-path behavior. Update
patch_path/3 to inspect the result of build_query/1 and return just base_path
when the query string is empty, otherwise keep appending the query. Use the
existing patch_path/3 and build_query/1 flow so callers clearing the last filter
get the bare path.
In `@test/reencodarr_web/live/bad_files_live_test.exs`:
- Around line 713-719: The test currently only verifies the page renders, so it
does not cover the URL-state normalization contract. Update the bad-files live
test around the existing "invalid filter params coerce to safe defaults" case to
assert the coerced UI state or patched URL after live(conn,
~p"/bad-files?status=not-a-status&service=bogus&page=0"), specifically that
status and service normalize to "all" and the page resets to 1. Use the existing
render_async(view) flow and check the rendered controls or URL state so the
parser behavior is actually enforced.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 01e07a47-932e-40c5-b289-b7a42eb6408a
📒 Files selected for processing (11)
.gitignorelib/reencodarr_web.exlib/reencodarr_web/live/bad_files_live.exlib/reencodarr_web/live/failures_live.exlib/reencodarr_web/live/list_components.exlib/reencodarr_web/live/list_pagination.exlib/reencodarr_web/live/list_url.extest/reencodarr_web/live/bad_files_live_test.exstest/reencodarr_web/live/list_components_test.exstest/reencodarr_web/live/list_pagination_test.exstest/reencodarr_web/live/list_url_test.exs
mjc
changed the title
mjc
commented
Jun 29, 2026
mjc
commented
Jun 29, 2026
mjc
commented
Jun 29, 2026
mjc
commented
Jun 29, 2026
mjc
commented
Jun 29, 2026
mjc
commented
Jun 29, 2026
mjc
commented
Jun 29, 2026
mjc
commented
Jun 29, 2026
mjc
commented
Jun 29, 2026
mjc
commented
Jun 29, 2026
mjc
commented
Jun 29, 2026
mjc
commented
Jun 29, 2026
mjc
changed the title
mjc
force-pushed
the
refactor/unify-list-liveviews
branch
2 times, most recently
from
426f94f to
7f7b176
Compare
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/reencodarr_web/live/failures_live.ex`:
- Around line 121-123: The async failures load flow is applying stale results
unconditionally, which can overwrite newer pagination/filter state after a patch
or refresh. Update the logic around async_load_failures/1 and
assign_failure_payload/2 in failures_live.ex so the loaded payload is only
applied when it still matches the current assigns (for example, by checking the
active page/filter state before assigning). Make sure the state transitions
triggered by handle_params/3 and reset_all_failures do not accept out-of-date
rows/meta from earlier async requests.
- Around line 359-379: The filter buttons in failures_live.ex are only using
color to indicate state, and the select-all plus row checkboxes are missing
accessible names. Update the filter controls around failure_filter_button_class,
filter_failures, and filter_category to expose the current active state to
assistive tech (for example via ARIA state/pressed semantics), and add explicit
accessible labels to the checkbox inputs in the select-all and row selection
markup so screen readers can identify each control.
- Around line 797-806: The pagination query key in failures_url_query/1 does not
match what parse_params/1 expects through FlopList.pagination_assigns/4, so
per-page state cannot round-trip. Update failures_url_query/1 to emit the same
per-page key that FlopList parses, and keep the existing failures_live
pagination flow consistent with FlopList.pagination_assigns/4 and the per-page
assign used by the LiveView.
- Around line 53-57: The current update path in failures_live’s socket handling
keeps selected_videos across URL state changes, which can make retry_selected
act on stale rows. Update the socket flow around assign_url_query and
reload_failures_for_params to clear selection whenever filters_changed? is true,
using a helper like maybe_clear_selection/2 near the existing selection state
logic. Make sure the cleared state resets selected_videos and any related UI
state such as expanded_details so the selection always matches the current
URL-backed failure list.
In `@lib/reencodarr_web/live/videos_live.ex`:
- Around line 113-114: The `handle_async/3` clause for `:load_videos` is
applying `page_state` unconditionally, so stale results from
`async_refresh_videos/1` can overwrite newer filter state. Update
`handle_async(:load_videos, {:ok, page_state}, socket)` in `VideosLive` to
verify the returned payload still matches the current query/filter state in the
socket before assigning it, and ignore the async result when it is outdated.
Apply the same stale-result guard to the related refresh handling around the
other `handle_async` clauses in this module.
- Around line 850-889: Add accessible names to the video toolbar controls in the
VideosLive template: the search input, state/service/HDR selects, and the
per-page select currently rely on placeholder text or unlabeled dropdowns.
Update the relevant form fields in videos_live.ex to include explicit labels or
aria-label/aria-labelledby attributes so assistive technologies can identify
each control; use the existing search/filter/per-page controls as the targets
for these names.
- Line 634: The URL state parsing in VideosLive is rejecting a valid video state
because `@valid_states` does not include analyzing. Update the `@valid_states` list
used by the state_filter parsing in VideosLive to include "analyzing" alongside
the other allowed states, so params passed through coerce_in can successfully
filter or link to videos in that state.
In `@lib/reencodarr/bad_files/state.ex`:
- Around line 71-82: The resolved-issues path in fetch_resolved_issues/3 is
incorrectly gated by show_resolved, which hides results even when the user
explicitly selects the resolved status filter. Update the bad_files LiveView
flow in state.ex so that status_filter == "resolved" always loads resolved
issues via fetch_resolved_issues/3 and renders them, even if show_resolved is
false; keep show_resolved only as a UI toggle for expanding/collapsing, not as
the condition that suppresses data fetching. Ensure the logic around
resolved_statuses, fetch_resolved_issues/3, and the render/state setup for
/bad-files?status=resolved treats resolved as an explicit request.
- Around line 45-50: list_active_issues/2 currently hard-caps bulk operations to
the first 250 results, so it can miss matching active issues beyond page 1.
Update the bad_files/state.ex path by changing list_active_issues/2 to fetch all
pages for the active_statuses_for_filter(assigns.status_filter) query, either by
looping through Flop pages until exhausted or by introducing a dedicated
unpaginated bulk-query helper. Keep the existing list_issues/1 flow intact for
normal paginated use, and make the bulk actions use the new all-results path
instead of the page 1/per_page 250 shortcut.
In `@lib/reencodarr/media.ex`:
- Around line 580-586: The series enqueue flow is only reading the first page of
unresolved issues, so `queue_bad_file_issue_series/1` can miss matching items on
later pages. Update the logic around `list_bad_file_issues/2` so it either
paginates through all unresolved results or uses a dedicated query filtered by
the target series path/group before building the queue. Make sure the full set
of matching bad file issues is collected before enqueueing.
In `@test/reencodarr_web/live/bad_files_live_test.exs`:
- Around line 747-767: The combined filter test in bad_files_live_test only
proves the happy path and can still pass if one of the bookmark query filters is
ignored. Update the test around the live/2 call for the bad-files view to seed
decoy issues for the wrong service and wrong kind that still match the search
term, then refute those entries so the parser contract for service, kind, and
search is exercised independently. Keep the assertions focused on the existing
filter behavior in the bad_files_live_test suite.
In `@test/reencodarr_web/live/failures_live_test.exs`:
- Around line 379-390: The test currently only proves search hydration because
search=url_analysis can isolate the row even if stage is ignored. Update the
failures LiveView test by adding an :encoding failure whose path also includes
url_analysis and then refuting that row, so the `live/2` query to
`failures_live_test.exs` verifies the stage filter from the URL is actually
applied. Use the existing `Media.record_video_failure/4`,
`Fixtures.video_fixture/1`, and the `stage and search params load from URL` test
to locate and extend the assertion.
In `@test/reencodarr_web/live/flop_list_test.exs`:
- Around line 97-105: The tests for FlopList.patch_with_page/3 are asserting an
exact query-string order, which is an implementation detail rather than the
contract. Update the tests in flop_list_test.exs to verify the resulting URL
contains the expected merged params for the first-page and non-first-page cases
without depending on serialization order, using FlopList.patch_with_page/3 as
the target behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 17e8ff2f-7efe-4318-a212-21399385540c
⛔ Files ignored due to path filters (1)
mix.lockis excluded by!**/*.lock
📒 Files selected for processing (23)
.gitignoreAGENTS.mdlib/reencodarr/bad_files/state.exlib/reencodarr/media.exlib/reencodarr/media/bad_file_issue.exlib/reencodarr_web.exlib/reencodarr_web/live/bad_files_live.exlib/reencodarr_web/live/failures_live.exlib/reencodarr_web/live/flop_list.exlib/reencodarr_web/live/list_pagination.exlib/reencodarr_web/live/videos_live.extest/reencodarr/bad_file_issue_test.exstest/reencodarr/flop_fixtures_test.exstest/reencodarr/media/list_bad_file_issues_test.exstest/reencodarr/media/list_failures_test.exstest/reencodarr_web/flop_list_test_helpers_test.exstest/reencodarr_web/live/bad_files_live_test.exstest/reencodarr_web/live/failures_live_test.exstest/reencodarr_web/live/flop_list_test.exstest/reencodarr_web/live/list_pagination_test.exstest/reencodarr_web/live/videos_live_test.exstest/support/flop_fixtures.extest/support/flop_list_test_helpers.ex
💤 Files with no reviewable changes (2)
- test/reencodarr_web/live/list_pagination_test.exs
- lib/reencodarr_web/live/list_pagination.ex
mjc
commented
Jun 29, 2026
mjc
commented
Jun 29, 2026
mjc
commented
Jun 29, 2026
mjc
commented
Jun 29, 2026
mjc
commented
Jun 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Refactors the paginated list LiveViews around Flop for query/pagination semantics and keeps
ReencodarrWeb.Live.FlopListintentionally narrow: themedflop_pagination, pagination assign parsing, clean patch paths, and pagination labels.handle_params/3, context loading stays inVideos.State, periodic/PubSub refreshes usestart_async/3, and the render body is split into page-local function components.Media.load_failures_page/1; URL loads are synchronous fromhandle_params/3; retry-code controls, table rows/details, and common-pattern sections are page-local components.BadFiles.State; bookmarked pages are clamped after loading real totals; URL loads are synchronous fromhandle_params/3; summary, replacement, toolbar, issue-table, and resolved sections are page-local components.ListComponents,ListUrl,ListPagination,assign_changed/2,assign_flop_result/4, orparams_changed?/3helper surface remains.Review follow-up
Testing
nix develop -c mix format --check-formattednix develop -c mix compile --warnings-as-errorsnix develop -c mix credo --strictnix develop -c mix testFull test result:
2751 passed (11 doctests, 71 properties, 2669 tests), 45 excluded.Summary by CodeRabbit
New Features
Documentation
Tests