fix(api): use correct field for lastReadTime by imnotjames · Pull Request #779 · grimmory-tools/grimmory

imnotjames · 2026-04-23T03:30:16Z

Description

For the sort order lastreadtime we current try to use the field lastReadTime off of BookEntity. this does not exist which leads to an exception and a failure

Instead, we can use progress.lastReadTime

Linked Issue: Fixes #776

Changes

Update the sort order lastreadtime to use progress.lastReadTime

Summary by CodeRabbit

Bug Fixes
- Enhanced 'last read time' sorting to exclusively display your books with accurate filtering based on your reading progress. This improvement ensures that when sorting by this criteria, you see only books from your personal library, with proper timestamps accurately reflecting your reading history and recent access moments for each title.

coderabbitai · 2026-04-23T03:30:24Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 02132680-586c-4a54-9b40-463dff35a805

📥 Commits

Reviewing files that changed from the base of the PR and between 7c466ae and fdc6e89.

📒 Files selected for processing (2)

backend/src/main/java/org/booklore/app/service/AppBookService.java
backend/src/main/java/org/booklore/app/specification/AppBookSpecification.java

📜 Recent review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

GitHub Check: Test Suite / Backend Tests
GitHub Check: Test Suite / Frontend Tests
GitHub Check: Analyze (java-kotlin)

🧰 Additional context used

📓 Path-based instructions (1)

backend/src/**/*.java

📄 CodeRabbit inference engine (AGENTS.md)

backend/src/**/*.java: Use 4-space indentation and match surrounding Java style in backend code
Prefer constructor injection via Lombok patterns already used in the codebase. Do not introduce @Autowired field injection in backend code
Use MapStruct for entity/DTO mapping in backend code
Keep JPA entities on the *Entity suffix in backend code

Files:

backend/src/main/java/org/booklore/app/service/AppBookService.java
backend/src/main/java/org/booklore/app/specification/AppBookSpecification.java

🧠 Learnings (4)

📓 Common learnings

Learnt from: pedronave
Repo: grimmory-tools/grimmory PR: 247
File: booklore-api/src/main/java/org/booklore/service/hardcover/HardcoverSyncService.java:156-159
Timestamp: 2026-04-01T17:50:06.817Z
Learning: In grimmory-tools/grimmory (HardcoverSyncService.java), the Hardcover API already returns `user_book_reads` in a consistent, ordered fashion (ascending by recency), so `getLast()` reliably retrieves the most recent read entry. Adding an explicit `order_by` to the `user_book_reads` query is unnecessary.

📚 Learning: 2026-04-01T17:50:06.817Z

Learnt from: pedronave
Repo: grimmory-tools/grimmory PR: 247
File: booklore-api/src/main/java/org/booklore/service/hardcover/HardcoverSyncService.java:156-159
Timestamp: 2026-04-01T17:50:06.817Z
Learning: In grimmory-tools/grimmory (HardcoverSyncService.java), the Hardcover API already returns `user_book_reads` in a consistent, ordered fashion (ascending by recency), so `getLast()` reliably retrieves the most recent read entry. Adding an explicit `order_by` to the `user_book_reads` query is unnecessary.

Applied to files:

backend/src/main/java/org/booklore/app/service/AppBookService.java

📚 Learning: 2026-04-10T08:15:37.436Z

Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 449
File: booklore-api/src/main/java/org/booklore/service/book/BookDownloadService.java:139-145
Timestamp: 2026-04-10T08:15:37.436Z
Learning: When using Spring `ContentDisposition.builder(...).filename(name, StandardCharsets.UTF_8).build()` (i.e., explicitly providing UTF-8), the resulting header value should include both the quoted `filename="=?UTF-8?..."` and the RFC 5987 `filename*=` parameters. In this case, any extra ASCII fallback computation (e.g., deriving an ASCII `fallbackFilename` via `NON_ASCII_PATTERN` and calling `.filename(fallbackFilename)`) is likely redundant—prefer calling only `.filename(fallbackName?, StandardCharsets.UTF_8)` as appropriate and let Spring handle the UTF-8 header parameters. Verify by comparing the emitted header for `filename` and `filename*` before deciding to keep an ASCII fallback.

Applied to files:

backend/src/main/java/org/booklore/app/service/AppBookService.java
backend/src/main/java/org/booklore/app/specification/AppBookSpecification.java

📚 Learning: 2026-04-14T12:43:08.698Z

Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 502
File: booklore-api/src/main/java/org/booklore/service/reader/ChapterCacheService.java:0-0
Timestamp: 2026-04-14T12:43:08.698Z
Learning: For this codebase (booklore-api), target Java 25 with `--enable-preview`, so `_` is intentionally used as an unnamed/ignored variable (e.g., lambda parameter or pattern variable) per Java’s preview feature JEP 456. Do not flag `_` in those contexts as an invalid/reserved identifier; only flag it if it’s used in a non-supported position (e.g., where an unnamed variable is not applicable for the Java preview rules).

Applied to files:

backend/src/main/java/org/booklore/app/service/AppBookService.java
backend/src/main/java/org/booklore/app/specification/AppBookSpecification.java

🔀 Multi-repo context grimmory-tools/grimmory-docs

Linked repositories findings

grimmory-tools/grimmory-docs

docs/magic-shelf.md: lines showing "Last Read" uses field name lastReadTime in examples and filters (e.g., table of fields, examples at ~lines 87, 184, 402, 1158). These are documentation references to the sort/filter key the backend maps. [::grimmory-tools/grimmory-docs::docs/magic-shelf.md:87,184,402,1158]
docs/* (multiple): general docs reference "Last Read" / "reading progress" and the progress feature (e.g., docs/integration/koreader.md, docs/integration/kobo.md, docs/reading-stats.md, docs/dashboard.md). No code-level consumers found referencing the new Specification method names (withProgress) or the repository classes (AppBookSpecification, AppBookService, BookEntity). [::grimmory-tools/grimmory-docs::docs/integration/koreader.md, docs/integration/kobo.md, docs/reading-stats.md, docs/dashboard.md]

Quality / relevance: Documentation contains explicit examples and field names using lastReadTime. The backend change shifts sorting to use a joined user-progress path (userBookProgress.lastReadTime) and enforces per-user progress filtering; documentation examples that instruct users to sort/filter by lastReadTime remain valid conceptually but may need clarification that "Last Read" is user-specific.

🔇 Additional comments (3)

backend/src/main/java/org/booklore/app/specification/AppBookSpecification.java (1)

134-154: LGTM — the filtered LEFT join matches the intended sort behavior.

The ON predicate keeps the joined progress scoped to the current user while optional=true still allows unread books to remain in the result set.

backend/src/main/java/org/booklore/app/service/AppBookService.java (2)

1047-1050: LGTM — the progress join is scoped to the affected sort only.

Adding withProgress(userId, true) only for lastreadtime avoids broadening the query shape for unrelated list requests.

1069-1069: Add explicit null ordering for unread books to align with codebase patterns.

The property path is correct, but userBookProgress.lastReadTime is nullable for books without progress records. The codebase pattern in AppSeriesService (line 338) applies explicit null handling (e.g., NULLS LAST) when sorting by lastReadTime. Apply the same pattern here to ensure unread books consistently appear after recently read books, independent of database/JPA provider defaults.

📝 Walkthrough

Walkthrough

Fixes a sorting exception when filtering books by "Last Read" by introducing user-specific book progress filtering and correcting the property path for the sort field from a non-existent property to the joined userBookProgress.lastReadTime path.

Changes

Cohort / File(s)	Summary
User-Specific Progress Filtering `backend/src/main/java/org/booklore/app/specification/AppBookSpecification.java`	Adds new `withProgress(userId, optional)` JPA Specification that conditionally joins `userBookProgress` to `BookEntity` with user-specific filtering. Returns neutral predicate when `userId` is null or `optional` is true; otherwise restricts results to books with matching user progress.
Last Read Sort Logic `backend/src/main/java/org/booklore/app/service/AppBookService.java`	Updates `lastreadtime` sort behavior to apply user-specific progress filtering via `withProgress(userId, true)` and corrects sort field path from `"lastReadTime"` to `"userBookProgress.lastReadTime"` to match the joined entity structure.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

backend, enhancement

Poem

🐰 A rabbit's cheer for sorted reads,
No more exceptions, just book deeds!
User progress joins with graceful flair,
Last read times sorted, accurate and fair! 📚✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.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 PR title follows the conventional commit format with the 'fix' type prefix and a clear, descriptive subject.
Description check	✅ Passed	The PR description includes all required sections from the template: a clear description of the problem, the linked issue, and documented changes.
Linked Issues check	✅ Passed	The PR implementation correctly addresses issue `#776` by fixing the sort field reference to use the correct user-specific progress data path.
Out of Scope Changes check	✅ Passed	All changes are directly aligned with the objective to fix the lastReadTime sort field reference and ensure user-specific progress handling.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

✨ Simplify code

Create PR with simplified code

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

imnotjames · 2026-04-23T03:45:25Z

Tested by manually manipulating the database before and after & observing the result in the UI.

imnotjames added 2 commits April 22, 2026 22:08

fix(api): use correct field for lastReadTime

8b78282

fix(api): ensure we only handle the current user's progress

fdc6e89

imnotjames marked this pull request as ready for review April 23, 2026 03:31

coderabbitai Bot added backend enhancement labels Apr 23, 2026

imnotjames requested a review from balazs-szucs April 23, 2026 03:45

zachyale approved these changes Apr 23, 2026

View reviewed changes

imnotjames merged commit bfca35b into grimmory-tools:develop Apr 23, 2026
22 of 23 checks passed

coderabbitai Bot mentioned this pull request Apr 24, 2026

fix(api): improve sort feature parity of AppBookService #828

Merged

github-merge-queue Bot pushed a commit that referenced this pull request Apr 25, 2026

fix(api): use correct field for lastReadTime (#779)

a3616c9

coderabbitai Bot mentioned this pull request May 1, 2026

fix(author): implement author sorting and add sort name field to AuthorEntity #970

Closed

dsmouse pushed a commit to dsmouse/grimmory that referenced this pull request May 11, 2026

fix(api): use correct field for lastReadTime (grimmory-tools#779)

d2d9fdf

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(api): use correct field for lastReadTime#779

fix(api): use correct field for lastReadTime#779
imnotjames merged 2 commits into
grimmory-tools:developfrom
imnotjames:fix/776/sort-last-read

imnotjames commented Apr 23, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Apr 23, 2026 •

edited

Loading

Linked repositories findings

Walkthrough

Changes

Estimated code review effort

Suggested labels

Poem

❌ Failed checks (1 warning)

Uh oh!

imnotjames commented Apr 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

imnotjames commented Apr 23, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Apr 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Linked repositories findings

Walkthrough

Changes

Estimated code review effort

Suggested labels

Poem

❌ Failed checks (1 warning)

Uh oh!

imnotjames commented Apr 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

imnotjames commented Apr 23, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Apr 23, 2026 •

edited

Loading