Skip to content

fix(hardcover): add series position decimals and use primary books count#87

Open
pedronave wants to merge 2 commits intogrimmory-tools:developfrom
pedronave:fix/missing-halves-hardcover-series
Open

fix(hardcover): add series position decimals and use primary books count#87
pedronave wants to merge 2 commits intogrimmory-tools:developfrom
pedronave:fix/missing-halves-hardcover-series

Conversation

@pedronave
Copy link
Contributor

@pedronave pedronave commented Mar 20, 2026

Description

  • Changes the Hardcover Series position to float instead of int, to allow for decimal positions (books between two main volumes)
  • Uses primary books count instead of total books count for series

Since both these changes are quite small I included them in the same PR, but can separate them if needed

Linked Issue: Fixes #86 also fixes #61

Summary by CodeRabbit

  • Bug Fixes
    • Fixed inaccurate series total counts so book metadata now reflects the correct number of primary books in a series.
    • Improved numeric precision for series positioning to ensure consistent ordering where applicable.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 20, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: fc2ac1e6-464f-44d0-9394-814299faf2e4

📥 Commits

Reviewing files that changed from the base of the PR and between ac272b0 and 4937283.

📒 Files selected for processing (4)
  • booklore-api/src/main/java/org/booklore/service/metadata/parser/HardcoverParser.java
  • booklore-api/src/main/java/org/booklore/service/metadata/parser/hardcover/GraphQLResponse.java
  • booklore-api/src/main/java/org/booklore/service/metadata/parser/hardcover/HardcoverBookSearchService.java
  • booklore-api/src/test/java/org/booklore/service/metadata/parser/HardcoverParserTest.java
✅ Files skipped from review due to trivial changes (1)
  • booklore-api/src/main/java/org/booklore/service/metadata/parser/hardcover/HardcoverBookSearchService.java
🚧 Files skipped from review as they are similar to previous changes (3)
  • booklore-api/src/main/java/org/booklore/service/metadata/parser/hardcover/GraphQLResponse.java
  • booklore-api/src/test/java/org/booklore/service/metadata/parser/HardcoverParserTest.java
  • booklore-api/src/main/java/org/booklore/service/metadata/parser/HardcoverParser.java

📝 Walkthrough

Walkthrough

Updated Hardcover parsing to use the GraphQL primary_books_count for series totals and to deserialize series positions as floats; GraphQL query, response model, parser, and tests were adjusted accordingly.

Changes

Cohort / File(s) Summary
GraphQL Response Model
booklore-api/src/main/java/org/booklore/service/metadata/parser/hardcover/GraphQLResponse.java
Added Series.primaryBooksCount mapped to primary_books_count; changed FeaturedSeries.position type from Integer to Float.
Parser Logic
booklore-api/src/main/java/org/booklore/service/metadata/parser/HardcoverParser.java
Switched assignments to populate BookMetadata.seriesTotal from getPrimaryBooksCount() instead of getBooksCount() in both series-mapping paths.
GraphQL Query
booklore-api/src/main/java/org/booklore/service/metadata/parser/hardcover/HardcoverBookSearchService.java
Extended the featured_book_series.series selection to request primary_books_count.
Tests / Fixtures
booklore-api/src/test/java/org/booklore/service/metadata/parser/HardcoverParserTest.java
Updated fixtures to set primaryBooksCount = 3, changed FeaturedSeries.position to float (2f), and asserted metadata.getSeriesTotal() equals 3.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I nibble lines of code tonight,
swapping counts to make things right.
Floats for positions, primary for total,
a small hop in logic — now it's vocal.
Happy hops — metadata shines bright! ✨📚

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and specifically describes the main changes: updating Hardcover series position to accept decimals and switching to primary books count.
Description check ✅ Passed The description includes all required sections from the template and explains both changes with clear context and linked issue references.
Linked Issues check ✅ Passed All code changes directly address the linked issues: float type for series position (#61) and primary_books_count usage for series totals (#86).
Out of Scope Changes check ✅ Passed All changes are tightly scoped to the two specific issues: position type conversion and field substitution; no unrelated modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@imajes imajes force-pushed the fix/missing-halves-hardcover-series branch from ac272b0 to 4937283 Compare March 21, 2026 01:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Hardcover series count should use the primary count instead of total count [Bug] Series Position is pulled from Hardcover incorrectly

1 participant