"Skip Secondary Series Books" never filters anything - #61
Open
JordanFromIT wants to merge 1 commit into
Open
Conversation
RefreshSeriesService wrote every SeriesBookLink with IsPrimary = true, so the "Skip Secondary Series Books" metadata profile option could never exclude anything and series kept every spin-off and companion entry. The flag already arrives from the API as V5SeriesBook.isPrimary and is carried into SeriesBook.IsPrimary by ConvertV5SeriesToDomain; both link write sites just discarded it. Use it, defaulting null to true as the model documents and as BookInfoProxy already does, so series that have not been refreshed yet keep their current membership. Fixes Chaptarr#60
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
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.
Description
The Skip Secondary Series Books setting currently has no effect. Chaptarr writes every row in
SeriesBookLinkwithIsPrimary = true, so no book is ever considered secondary and the filter has nothing left to remove — a series keeps every spin-off, companion volume, art book and anthology instead of just the main sequence. The metadata API already sends a per-book primary flag, and Chaptarr already carries it all the way to the domain model; the two places that write the link rows simply ignore it and hardcodetrue. This PR uses the value that already arrives. Two lines, plus a regression test.For a concrete sense of the gap: Hardcover describes A Song of Ice and Fire as a 22-book series with 5 primary works. Entries like The Hedge Knight (
#0.4) and The World of Ice & Fire (#0.5) sit at fractional positions precisely because they aren't part of the main sequence. With the option enabled today, all 22 are still kept.The flag was already plumbed most of the way:
isPrimaryper series slotV5SeriesBook.IsPrimary—V5AuthorResponse.cs:423ConvertV5SeriesToDomain—BookInfoProxy.cs:2151IsPrimary = b.IsPrimaryRefreshSeriesService.cs:160,:358trueMetadataProfileService.cs:238Any(y => y.IsPrimary)Both write sites now read
seriesBook.IsPrimary ?? true. The?? truematters:SeriesBook.IsPrimaryis nullable, and its own comment says consumers should treat null as primary so that authors who haven't been refreshed since the field was added don't silently lose series members. That's also exactly whatBookInfoProxy.cs:1171already does — this just brings the other two sites in line with the existing precedent rather than inventing a convention.Blast radius is small.
SeriesBookLink.IsPrimaryhas exactly one functional consumer, theSkipSeriesSecondaryfilter itself. Every otherIsPrimaryin the tree belongs to a different entity (BookNarratorLink,EditionNarratorLink,NarratorCredit), andSeriesVariantService,RefreshEntityCopyandLocalEditiononly copy the value through. So flipping a previously always-true flag to sometimes-false doesn't change behaviour anywhere except the option that was supposed to depend on it.Fixes #60
Database Migration
NO. The
IsPrimarycolumn already exists onSeriesBookLink(001_chaptarr_complete_schema.cs:435, defaulttrue). Only the value written into it changes. Existing rows are corrected on the next author refresh; nothing needs backfilling, and unflagged books keep their current primary status by design.How was this tested?
Built and tested on Linux, .NET 10, from source — not through Docker. Ran the same steps
build.ymluses, including the guards that run before compilation:git grepconflict-marker check +package.jsonJSON parse — clean.python3 build-tools/version_guard.py sync—Version sync OK for working tree: 0.9.929.python3 build-tools/version_guard.py monotonic --compare-ref origin/develop—Version monotonic OK: 0.9.929.python3 build-tools/version_guard.py commit-hygiene --compare-ref origin/develop—OK for range origin/develop..HEAD.dotnet build src/Chaptarr.NoTests.sln --configuration Release— succeeded, 0 warnings, 0 errors (soTreatWarningsAsErrors/EnforceCodeStyleInBuildare satisfied).dotnet test src/Chaptarr.Core.Test/Chaptarr.Core.Test.csproj --configuration Release— 2834 ondevelop→ 2835 on this branch, 0 failed. The one added test is the delta.should_carry_series_book_primary_flag_into_linkstoRefreshSeriesServiceExistingSeriesFixture, covering all three states through the path that actually persists links:isPrimary = true→ linked primary,isPrimary = false→ linked secondary,isPrimary = null→ linked primary (the back-compat case).a book the metadata API flags as secondary should be linked as secondary / Expected: False / But was: True, then passes again with the fix restored.I have not exercised this end-to-end against a live library — the change is two lines with a single consumer, so the unit tests seemed a better proof than a manual refresh. Happy to run a real-instance check if you'd like one before merging.
Screenshots (UI changes only)
n/a — no UI changes.
Two adjacent things I deliberately left out of this PR, mentioned in #60 in case they're worth their own tickets:
WorkCountandPrimaryWorkCountare assigned the same value atBookInfoProxy.cs:4661-4662andSeriesLookupController.cs:222-223, so the({count} Primary)label can never differ from the total; andSeries.TotalBooks/Series.PrimaryBooksare written by some converters but not others, leaving them zero on rows that took the other path. Neither affects filtering, and the first looks like it may be deliberate for search previews, so I didn't want to guess at intent inside a bugfix.