Repository navigation
feat(series): fix one book's series, and keep removed books out (#2554) - #3090
Conversation
…avallee#2554) A refresh re-adds whatever series a provider reports and never removes one, so a book taken out of a series came back with the next refresh. Removing a book from a series now records it in book_series_exclusions, keyed by the series' foreign id so it holds through a merge, and LinkBookIfMissing, which every automatic path goes through, skips an excluded pair inside its INSERT. Adding the book back by hand clears it. The table is migration 102. UpsertBookLink, the manual add, now also demotes the book's other series when it files it as primary, so a book keeps one primary series (vavallee#2525). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Francis Rath <fr@ncis.no>
…e#2554) Edit metadata gets a Series row: a dropdown of the library's series, New series… to create one, and a position. It edits the book's primary series: picking another files the book there as primary and takes it out of the old one, which the server remembers so a refresh does not put it back; clearing it takes the book out; changing only the position updates it in place. Nothing is sent for the series unless it changed, and the book page reloads its series after a save. If the series cannot be loaded the row stays hidden and the other fields still save. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Francis Rath <fr@ncis.no>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Francis Rath <fr@ncis.no>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Francis Rath <fr@ncis.no>
The Save button's condition is now canSave, which the name field's Enter shares, so Enter does nothing while the name or title is empty. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Francis Rath <fr@ncis.no>
…avallee#2554) A book the user takes out of a series stays out on refresh, but nothing said so: the only way back was knowing to pick the series again in Edit metadata. The book page now lists those series under Series, named as they are now (following merge aliases), with Restore, which files the book back at the position it had (now kept with the exclusion) and as primary only when it has none. GET /book/{id}/series-exclusions serves the list. Unlock all fields hands the whole book back to metadata refresh, so it now forgets the series exclusions too, and is offered when there are any even without locked fields. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Francis Rath <fr@ncis.no>
The author page loaded the series grouping once per author, so after a Refresh it reloaded the books but grouped them by the series from before it: a book the refresh filed under a series stayed under Standalone until a full page reload. The grouping now loads again with the reload a finished Refresh triggers, keeping the current grouping on screen while it fetches. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Francis Rath <fr@ncis.no>
UpsertBookLink and RemoveBookFromSeries roll back as a whole when a step fails; a failed exclusion read or clear is an error; Unlock all and Remove answer 500 when their write fails; the exclusions endpoint 404s for an unknown book. On the web side: a book in no series filed under one, a single non-primary membership preselected, and a failed Restore shown. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Francis Rath <fr@ncis.no>
…lee#2554) The server links a book to a series without a position, and unnumbered series and omnibus volumes have none, but the Add Book dialog refused to save without one and showed a "1" placeholder that looked like a value. The field is now labelled optional and Add only needs a book. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Francis Rath <fr@ncis.no>
Merging a duplicate into a series a book is already in left the book's exclusion from the duplicate behind. The duplicate's id then resolves to the kept series, so the book page listed the book as kept out of a series it is in. A merge now drops exclusions that resolve to the target for books in it, and those of a hand-made source whose id is not kept as an alias. A book kept out of the duplicate only still stays out. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Francis Rath <fr@ncis.no>
…ee#2554) No series in Edit metadata only removed the series the dialog showed, so a book in several series kept the others. It now removes the book from each of them, and each removal is remembered as before. The dialog also showed No series for a book in several series with none primary, so saving any other field would have taken the book out of them. It now preselects the series the server names the book under, in the same order as GetPrimarySeriesForBook. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Francis Rath <fr@ncis.no>
…dpoint (vavallee#2554) API.md gains GET /book/{id}/series-exclusions and the series book endpoints it relates to, with how a removal is remembered and forgotten. The series wiki gets a section on Edit metadata's Series field, Restore and Unlock all, and the changelog an entry for this half of vavallee#2554. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Francis Rath <fr@ncis.no>
Every series change is admin only on the server, but the series page, the book page, Edit metadata and the authors toolbar offered them to every user, who got a 403. They now show for admins only; other users still see every series, its books and what is missing. useIsAdmin answers true outside the auth provider, as useIsRequester answers false there, so components tested in isolation keep the full controls they were written for. Unlock all fields forgets the book's series exclusions only for an admin, so another user's unlock cannot change series membership either. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Francis Rath <fr@ncis.no>
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Adds the series.merge strings from #3057 to both locales. The keys from #3090 and #3116 wait until those land in en.json. Translations by francisrath. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9 Signed-off-by: vavallee <vavallee@protonmail.com>
The book_series_exclusions table is created with IF NOT EXISTS. The migration runs on custom builds ahead of the merge; should it be renumbered, the renumbered copy must not stop those databases from starting. A test runs it over an existing table. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Francis Rath <fr@ncis.no>
Signed-off-by: Francis Rath <fr@ncis.no>
vavallee
left a comment
There was a problem hiding this comment.
Thanks Francis, this is a really careful piece of work. The exclusion check sitting inside the INSERT in LinkBookIfMissing means every automatic writer (author sync, ABS and Calibre imports, rebind, list sync) honours it without any caller changing, and the merge handling through aliases is nice. Migration 102 looks safe to me, including running it again over its own table. I checked an upgrade from a database that has 106 but not 102, and a database that already has the table, and both come up fine. Mutating each guard makes your tests fail, which is what I wanted to see.
One thing I would like changed before merge. If you take a book out of a series and then click Add on that same book in the Hardcover missing list, the response says queued 1 but the book is not linked, because ensureHardcoverCatalogBook goes through LinkBookPreservingPrimary and the exclusion skips the link silently. The series page then keeps listing it as missing and Add can never fix it. For the explicit single book Add, which is the user asking for it, I think it should clear the exclusion first, the same way adding by hand does. Bulk fill should keep honouring it.
Smaller things, none blocking. If Edit metadata saves the series and then the book update fails, a retry with New series selected would create the series a second time. The Restore button is the same size as its neighbours but still under 44px on touch. Thanks again for splitting this out and for the tests.
Clicking Add on a book the user took out of a Hardcover linked series answered queued and left the book out: ensureHardcoverCatalogBook links through the exclusion guarded LinkBookIfMissing and ignored its false return, so the series page kept listing the book as missing. The single book path is the user asking for the book, so it now clears that series' exclusion before linking, as adding by hand does. Bulk fill still honours the exclusion. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9 Signed-off-by: vavallee <vavallee@protonmail.com>
…ee#2554) Edit metadata saved the series before the book update, but left its form state as loaded, so a retry after a failed book update sent the series change again and New series created a second series. The new series is now selected as soon as it exists and the saved membership is recorded. The Restore button in the kept out list gets the touch hit area. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9 Signed-off-by: vavallee <vavallee@protonmail.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9 Signed-off-by: vavallee <vavallee@protonmail.com>
…requires (vavallee#2554) Main added a required covered field to the Hardcover diff (vavallee#2524), which this branch's test fixture predates. Types and the test failed only once the two met. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9 Signed-off-by: vavallee <vavallee@protonmail.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9 Signed-off-by: vavallee <vavallee@protonmail.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9 Signed-off-by: vavallee <vavallee@protonmail.com>
vavallee
left a comment
There was a problem hiding this comment.
Thanks for the careful work on this, Francis. I pushed a few commits to your branch to cover the review. Add on a book you removed from a series now clears the removal and puts it back, so it no longer sits in the missing list forever. Fill all still leaves removed books out, and both cases have tests. In Edit metadata, a retry after a failed save no longer creates the new series a second time. I also gave Restore the touch hit area and merged main in, and a test fixture needed a small tweak after #2915 added the covered list. CI is green, so I am merging this.
Provider lists in the README, the roadmap Delivered and Next up sections, the In Library meaning for partly owned dual format books, ratings, the Authors list have / total count and the author page sort and filters, the series field in the book edit table, the availableBookCount and rating fields, the rating sort values and the primary provider values in the API reference, and the one time dedup key recompute on first boot. Documents #3125, #3095, #3088, #3134, #3090, #3127, #3150, #3158, #3116, #2915 and #2994. No new environment variables, dependencies or breaking behaviour shipped, so DEPLOYMENT.md env tables, THIRD_PARTY_LICENSES.md and docs/upgrade-v1.md are unchanged. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9 Signed-off-by: vavallee <vavallee@protonmail.com>
Provider lists in the README, the roadmap Delivered and Next up sections, the In Library meaning for partly owned dual format books, ratings, the Authors list have / total count and the author page sort and filters, the series field in the book edit table, the availableBookCount and rating fields, the rating sort values and the primary provider values in the API reference, and the one time dedup key recompute on first boot. Documents #3125, #3095, #3088, #3134, #3090, #3127, #3150, #3158, #3116, #2915 and #2994. No new environment variables, dependencies or breaking behaviour shipped, so DEPLOYMENT.md env tables, THIRD_PARTY_LICENSES.md and docs/upgrade-v1.md are unchanged. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9 Signed-off-by: vavallee <vavallee@protonmail.com>
Summary
Fixing one book's series from the book, and keeping a book out of a series it was taken out of. This is the second half of #2554, split from #3057 at review (now merged); it builds on its
series_aliases.Refs #2554.
What changes
POST /seriesandPOST /series/{id}/books, no new write endpointUpsertBookLinkdemotes the book's other series in the same transaction when it files a book as primary (#2525)RemoveBookFromSeriesrecords the removal inbook_series_exclusions(migration 102), keyed by the series' provider id so it follows a merge through the aliases.LinkBookIfMissing, which every automatic path goes through, skips an excluded pair inside its INSERT. Adding the book back by hand clears it, and so does Unlock all fields by an adminGET /book/{id}/series-exclusionsSchema
Suggested review order
102_book_series_exclusions.sql, thenRemoveBookFromSeries,LinkBookIfMissing,UpsertBookLinkand the exclusion list/clear ininternal/db/series.go, withseries_exclusions_test.gointernal/db/series_merge.gointernal/api/book_series_exclusions.goand the Unlock all change ininternal/api/books.goEditBookModal.tsx, thenBookDetailPage.tsxuseIsAdminand the gating on the series, book and authors pagesdocs/API.md, "Fixing One Book's Series" indocs/Hardcover-Series-Wiki.md,changelog.d/2554-book-series.mdChecklist
Signed-off-bythat matches its author (git commit -s)changelog.d/2554-book-series.mddocs/updatedTest plan
go vet ./..., golangci-lint v2.11.4 (0 issues)go test ./cmd/... ./internal/...cd web && npm run typecheck && npm run build && npm test(1861 passed)🤖 Generated with Claude Code