Add MUSIC_LIBRARIES checkbox UI to setup wizard and migration assistant#459
Conversation
After a successful test connection, the setup wizard and migration assistant now fetch the provider's actual library list and render a checkbox per library. All checked = scan everything. Selection persists to the same MUSIC_LIBRARIES app_config key, replacing the easy-to-mistype free-text advanced field. The migration write also wipes the source provider's old filter value automatically (single shared key). Sustainable Navidrome migration fix: rather than the implicit "skip filter when user_creds is set" heuristic, threads user_creds through _get_target_music_folder_ids and adds an explicit apply_filter parameter on get_all_songs. Migration probes pass apply_filter=False so the source provider's library names don't falsely zero out the target catalog during dry-run; live-provider scans default to apply_filter=True so the saved selection is still honored. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements a library selection feature for the setup wizard and migration assistant, allowing users to filter which music folders are scanned. New API endpoints and UI components were added to support listing and persisting these selections across various media server providers. Review feedback points out that the filtering logic is currently only active for Navidrome, rendering the UI selection ineffective for other providers. It was also suggested that the migration dry-run should incorporate the user's target library selection for accurate track matching, and that the Navidrome implementation should more robustly handle API responses that return single objects instead of lists.
Subsonic-compatible servers may return musicFolder as a single dict (not a list) when only one folder exists, depending on the JSON parser configuration. Coerce to a list in both list_libraries and _get_target_music_folder_ids so iteration is consistent. Adds a regression test covering the single-folder response shape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Just some observation, for the migration:
For the setup wizard:
In both the scenario you keep MUSIC_LIBRARY parameter in the config table as a comma separated list and you please avoid to change the logic to avoid regression.
Off course if you have better idea let me know. |
Currently provider_probe.fetch_all_tracks passes apply_filter=False, so dry-run fetches the entire target catalog regardless of selection. So it's not used for step 3. For migration all tracks are used because starting point is existing library. Regarding the reload, that needs to be fixed
It is removed from advanced fields, no duplication.
Will add the disclaimer
Already what I have. |
Lyrion's scan-time filter (_get_target_paths_for_filtering) treats MUSIC_LIBRARIES as filesystem paths and substring-matches them against album file URLs. Prior to this change list_libraries returned the folder name, so a user who picked "MyMusic" in the checkbox UI wrote MUSIC_LIBRARIES="MyMusic" — which only matched if the folder name happened to be a substring of the file path (a brittle coincidence). list_libraries now returns the path reported by the musicfolders JSON-RPC command as the display/persisted name, falling back to the folder name when the server omits the path. The saved MUSIC_LIBRARIES value now matches what _get_target_paths_for_filtering expects, so the Cleaning task and live-provider scans correctly include/exclude the selected libraries. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Migration: /api/migration/libraries now also returns
state.selected_libraries; renderMigLibraries pre-checks the matching
names so a page reload in step 2 shows the user's prior selection
instead of resetting to all-checked. The maintainer's UX expectation
("if you want to change selection after a refresh, click discard") now
holds: refresh preserves state, discard is the only way to reset.
Setup wizard: added a small disclaimer below the music-libraries
checkbox list explaining that changes only affect new analyses, and
that Cleaning is the way to remove already-analyzed tracks from
libraries the user unchecks.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Ok I think it make sense: if you already analyzed them you already want them without overcomplicating the logic. So you select the library of the target server to find all of your song on the target server. |
Two fixes for the library checkbox UI:
* The global ``.field-row input { width: 100% }`` rule was stretching
each checkbox across the row, pushing the library names to the far
right. Override the checkbox width/margin inline so it sits flush
against its label text.
* When the saved MUSIC_LIBRARIES value has no overlap with the current
provider's library names (e.g. user pointed the wizard at a
different server, or library names changed), the rendered list was
showing every box unchecked plus the "select at least one" hint —
looked like a bug to the user. Detect "stale selection has zero
matches" and fall back to all-checked, matching the maintainer's
"empty MUSIC_LIBRARIES means scan everything" semantics.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When Navidrome's "Report Real Path" setting is disabled, the
``getAlbum`` and ``search3`` Subsonic responses report the file path
under ``url`` instead of ``path``. ``navidrome_test_connection`` (the
step-2 probe) already handled this with ``s.get('path') or s.get('url')``,
but ``get_all_songs`` (the step-3 dry-run probe) and
``get_tracks_from_album`` only read ``path``, leaving the migration
matcher with no path data.
Pre-existed but was masked: prior to the apply_filter=False path, the
migration probe usually fell through ``_get_target_music_folder_ids``'s
empty-set branch and returned 0 songs (the bug commit b426682 was
trying to fix). Now that the dry-run actually returns songs, the
missing url fallback breaks path-based matching.
Apply the same ``path or url`` fallback in both helpers to restore
path detection in the dry-run when Report Real Path is off.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ename
Two interconnected bugs that prevented the migration assistant from
ever showing checkboxes for a Lyrion target:
* Wrong CLI command. The JSON-RPC command is ``musicfolder`` (singular).
``musicfolders`` (plural) is undocumented on Lyrion 9.0.x and the
server drops the connection without responding ("Connection
aborted, RemoteDisconnected"). The pre-existing
``_get_target_music_folder_ids`` had the same plural bug but it's
dead code — ``get_recent_albums`` uses ``_get_target_paths_for_filtering``
for live filtering, so nobody actually exercised
``_get_target_music_folder_ids``. ``list_libraries`` was the first
caller to actually invoke the broken command and surface the issue.
* Wrong field name on Lyrion 9.0.x. The folder loop entries report the
display name under ``filename``, not ``name`` or ``folder`` (which
older Lyrion versions used). The previous code skipped every folder
because it required ``name``/``folder``, so even after fixing the
command the list would have come back empty.
list_libraries now calls ``musicfolder`` and reads
``filename or name or folder`` so it works against both 9.0.x and
older releases. Adds a regression test covering the 9.0.x response
shape.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@NeptuneHub tested every provider |
|
@NeptuneHub just make sure you didn't miss. Think it's done. UI improvement for setup wizard, adding library filter to migration and fix for #449 |
…matched cap - Add 'No restriction' pseudo-checkbox in setup + migration library pickers (empty list = scan all); preserve checkbox state across test-connection re-renders. - Cap unmatched-albums payload at MIGRATION_UNMATCHED_ALBUMS_PAYLOAD_LIMIT (env/DB-overridable, default 200); show truncation warning with full count. - Step-4 'matched albums' view now lists only manually-corrected albums and labels them as such; refresh list after every match (not just rematch). - Make Prev/Next pagination buttons use btn-primary.
|
Hi rendy here my review:
Then I directly push some fix:
Please do a re-check of this change to be sure I didn't introduced any bug (better have a doublecheck): Thanks! |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces music library filtering for both the setup wizard and the provider migration process. It adds backend logic to list libraries from various media servers, frontend checkbox UI for selection, and persistence of these settings in the application configuration. Additionally, it optimizes the migration review process by capping the number of unmatched albums and refining song-fetching filters. Feedback was provided to improve error handling consistency by using standard HTTP error codes for failed library listing requests.
2 participants
STILL TESTING
Replaces the "type comma-separated library names" advanced field with a checkbox list rendered after a successful test connection.
Setup wizard (/setup)
Migration assistant (/provider-migration)
Library listers — new sibling functions (one per provider) that return the raw [{id, name}] list with optional user_creds. Existing get_target* filter helpers untouched.
Sustainable Navidrome migration fix
The contract is now visible in the function signature instead of inferred from another argument's presence.