feat: show audiobooks under all their series and fix the Primary-series toggle (#658)#659
Conversation
…le (Listenarrs#658) - Library grouping and series collections list a book under every series membership, not just the metadata provider's primary - Library list payload now carries seriesMemberships (batched query, no blanket Include on the shared GetAllAsync) - Fix EditAudiobookModal save payload that re-flagged the first series as primary, reverting a non-default Primary selection - Preserve the user's chosen primary across a manual metadata rescan - Cards/modals list all series; {Series} folder naming follows the chosen primary Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
therobbiedavis
left a comment
There was a problem hiding this comment.
Thanks, this is a nicely focused fix and the regression coverage is good. I left two comments: one backend concurrency risk around starting multiple EF queries on the same scoped DbContext, and one UI gap where the series collection view resolves the per-series number but never renders it. Also the changelog is no longer needed.
- LibraryListService: await the three shared-scoped-context reads (file summaries, file counts, series memberships) sequentially. They all run on the scoped ListenArrDbContext, so firing them concurrently risked EF's "a second operation was started on this context instance" under real DB latency (only survived on SQLite because its async path runs sync). The download read uses IDbContextFactory (own context) and stays parallel. - CollectionView: render the resolved per-collection series position (#N) in the series collection list and grid item details — previously resolveSeriesForCollection computed it but nothing displayed it. - Drop the CHANGELOG entries (maintainer feedback: changelog no longer needed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A book in multiple series lists every series it belongs to, but the series catalog mapped each book via book.Series.FirstOrDefault() — its primary series. So opening "Series A" showed not-added (catalog) suggestions with their primary series number (e.g. "Series B Listenarrs#5") instead of their position in the series being viewed ("Series A Listenarrs#3"). Library items were already correct; only the remote catalog "Not Added" items were wrong. SeriesCatalogService now reorders each fetched book's series list so the catalogued series (matched by ASIN, else name) comes first, before the books are cached and returned. Both the cache write and the controller response mapping (both FirstOrDefault-based) then reflect the requested series. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
GetSeriesMembershipsByAudiobookIdsAsync implied filtering by audiobook IDs, but it takes no IDs and returns the entire AudiobookSeriesMemberships table grouped by AudiobookId. The sole caller (LibraryListService, the full library list) wants all rows, so the behavior is correct — only the name was misleading (a footgun for future callers who might pass IDs and silently get every row). Renamed to GetAllSeriesMembershipsGroupedByAudiobookIdAsync across the interface, implementation, and caller. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Fixed the points from the review, also pushed two follow-ups that I noticed while testing:
I also called out the LibraryListService sequential-await change in the description as the latent DbContext "second operation" fix, per the review note. These are separate commits for easier review — can be squashed before merge. |
therobbiedavis
left a comment
There was a problem hiding this comment.
This one LGTM as well!
… from PATCH_BRANCHES 658-multi-series-display-and-primary-toggle (PR Listenarrs#659) and 626-sort-series-by-position (PR Listenarrs#660) merged into upstream/canary, so they now arrive via the base when my-canary is recreated.
Catch-up merge bringing upstream canary up to v1.0.11. Net-new from canary: System Storage available-space (Listenarrs#656), Title-folder persistence (Listenarrs#646), sort series by reading order (Listenarrs#626/Listenarrs#660), audiobooks-under-all-series + Primary-series toggle (Listenarrs#658/Listenarrs#659), dependency/vulnerability + lint-config updates (Listenarrs#636). Conflict resolutions of note (series work overlapped kevin/live's series suite): - LibraryListService: kept kevin's importedAt (Recently-Imported) AND adopted canary's series memberships, using canary's sequential shared-DbContext awaits. - SeriesCatalogService: kept kevin's richer GetSeriesCandidates/owned-book ResolveSeriesAsync; added canary's PrioritizeCatalogSeries/FindCatalogSeries. - AudiobooksView: merged kevin's narrator grouping with canary's multi-series grouping (getBookSeriesNames) + formatSeriesMemberships display. - CollectionView: adopted canary's proper series-position sort + multi-series membership matching, superseding kevin's interim "Series Order" relabel, but kept kevin's article/apostrophe-aware normalizeSeriesName for slug matching. - SeriesMonitoringServiceTests: kept both kevin's (old path, +enhancements) and canary's relocated copy (different namespaces, no collision). - Test files: unioned both sides' added cases; dropped kevin's now-superseded "Series Order" option test, adapted the default-sort test to series-position. - Fixed pre-existing unused-var lint now surfaced by canary's stricter config. Validated on the throwaway branch: 1093 backend + 501 frontend tests pass; backend + frontend build, FE lint, and type-check all clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
) - Remove the two Listenarrs#571 CHANGELOG entries — changelog entries are no longer wanted on upstream PRs (maintainer confirmed on Listenarrs#659/Listenarrs#660); they only cause [Unreleased] conflicts. Keeps the existing Authentication entries. - Unify LibraryController.ComputeAudiobookBaseDirectoryFromPattern's no-pattern fallback from {Author}/{Title} to {Author}/{Series}/{Title}, matching the orchestrator's legacy default (review follow-up). Reachable only when no folder pattern is configured at all; empty {Series} collapses away, so non-series books are unaffected. 719 backend tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… from PATCH_BRANCHES 658-multi-series-display-and-primary-toggle (PR Listenarrs#659) and 626-sort-series-by-position (PR Listenarrs#660) merged into upstream/canary, so they now arrive via the base when my-canary is recreated.
Summary
The backend already models books that belong to multiple series (
AudiobookSeriesMembershipjoin withIsPrimary/SortOrder, populated by Audible/Audnexus), but the UI and one list query still treated each book as single-series. As a result a book that's in two or more series only showed under its provider-chosen primary, and the per-book "Primary" series toggle silently reverted on save.This PR makes a book appear under every series it belongs to and makes the Primary selection actually stick — which also makes
{Series}folder naming follow the chosen series.Motivating example: a series catalogued in both publication order and chronological order (e.g. Jack Ryan) previously split across two half-empty series with no way to pick the canonical one.
Closes #658.
Changes
Added
LibraryAudiobookListItemnow carriesSeriesMemberships, batch-loaded inLibraryListServicevia a newGetAllSeriesMembershipsGroupedByAudiobookIdAsyncrepository query (mirrors the existing file-summary batching — no per-rowInclude, no change to the sharedGetAllAsync).Fixed
isPrimary: Boolean(membership.isPrimary || index === 0), which re-flagged the first series as primary alongside the user's pick; the backend keeps the first primary it finds, so the choice reverted to the provider default. The payload now sends only the user's selection.AudiobookSeriesMembershipHelper.ApplyToAudiobookPreservingPrimaryre-applies the user's chosen primary when the provider still returns that series (falling back to the provider default otherwise).LibraryListServicestarted the file-summary, file-count, and series-membership reads concurrently on the shared scopedListenArrDbContext; under real database latency that risks EF Core's "a second operation was started on this context instance" error (SQLite masks it today by running the async path synchronously). Those same-context reads now run sequentially, while the download read — which uses its ownIDbContextFactorycontext — stays parallel.Testing
dotnet test: full suite green, including new unit tests forApplyToAudiobookPreservingPrimaryand theNormalizenon-first-primary case, a library-list payload test assertingseriesMemberships, and aRenameServiceregression asserting{Series}folders under the chosen primary.vitest: full suite green, including a newEditAudiobookModaltest asserting the save payload keeps a non-first primary (index 0 =isPrimary:false) andseriesUtilsunit tests.vue-tsctype-check, ESLint, Vue template-handler check, Prettier, anddotnet formatall clean.Notes
AudiobookSeriesMembershipstable already exists; this is display + persistence wiring only.canary— the two compose (this PR gives [Feature Request] Sort Series by the order of the book in the series #626 correct per-series numbers for multi-series books) but neither depends on the other, and they can merge in any order.RenameServiceTeststest is placed at the end of the class specifically so it no longer collides with Implicit patterns added to configured naming patterns #571's additions — the two now auto-merge inRenameServiceTests.csregardless of merge order.