feat(kobo): implement personal ratings from kobo#1165
feat(kobo): implement personal ratings from kobo#1165imnotjames wants to merge 1 commit intogrimmory-tools:developfrom
Conversation
WalkthroughThis PR implements Kobo rating synchronization, allowing readers' book ratings from Kobo devices to automatically update personal ratings in BookLore. The feature includes a new rating service with access controls, a controller endpoint, and comprehensive test coverage. ChangesKobo Rating Sync Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Comment |
7fcde31 to
137ef3a
Compare
137ef3a to
a4f75bd
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/src/test/java/org/booklore/service/kobo/KoboRatingServiceTest.java`:
- Around line 68-172: Add a unit test that exercises the missing "book not
found" branch when bookRepository.findById(...) returns Optional.empty so
updatePersonalRating(BookLoreUser, long, int) throws the expected
APIException/BOOK_NOT_FOUND; specifically, add a test (e.g.,
updatePersonalRating_throwsForMissingBook) that creates a BookLoreUser (use
getFakeUser), stubs
when(bookRepository.findById(999L)).thenReturn(Optional.empty()), and asserts
that service.updatePersonalRating(user, 999L, 3) throws APIException to cover
the orElseThrow path in updatePersonalRating.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 27bc3392-5843-4003-b0af-96017cddf7bc
📒 Files selected for processing (4)
backend/src/main/java/org/booklore/controller/KoboController.javabackend/src/main/java/org/booklore/service/kobo/KoboInitializationService.javabackend/src/main/java/org/booklore/service/kobo/KoboRatingService.javabackend/src/test/java/org/booklore/service/kobo/KoboRatingServiceTest.java
📜 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). (5)
- GitHub Check: Test Suite / Frontend Tests
- GitHub Check: Test Suite / Backend Tests
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (java-kotlin)
- GitHub Check: Frontend Lint Threshold Check
🧰 Additional context used
📓 Path-based instructions (6)
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@Autowiredfield 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/controller/KoboController.javabackend/src/main/java/org/booklore/service/kobo/KoboInitializationService.javabackend/src/main/java/org/booklore/service/kobo/KoboRatingService.javabackend/src/test/java/org/booklore/service/kobo/KoboRatingServiceTest.java
**/*
⚙️ CodeRabbit configuration file
**/*: This project is being developed using current and future-facing technologies:
- Java 25 with --enable-preview (preview features are INTENTIONAL and encouraged)
- Spring Boot 4 (latest major version, check APIs accordingly)
- Jackson 3 (new package: tools.jackson.* instead of com.fasterxml.jackson.*)
- Hibernate 7.3.x (Jakarta Persistence 3.2, new APIs; avoid deprecated Hibernate 5/6 patterns)
- Angular 21 (signals-based reactivity, no NgModules unless legacy)
Grimmory Internal Tools
- epub4j and pdfium4j are our own internal tools developed by the Grimmory team.
- Always verify behavior and API changes against the upstream repositories:
- If you encounter issues with these libraries, check if a fix exists in the upstream grimmory-tools organization.
Metadata Standards and Compliance
- For all metadata writing and parsing logic, double-check against Dublin Core and ANSI standards to ensure perfect official compliance.
- We strictly follow the widespread and official XML-compliant methods for EPUB2, EPUB3, CBX, and PDF formats.
General Java and Spring rules
- ALWAYS prefer modern, idiomatic Java 25 constructs over legacy patterns.
- Preview features (--enable-preview) are enabled and intentional; do NOT flag them as risky unless there is a concrete runtime issue.
- Prefer: records, sealed classes/interfaces, pattern matching (switch expressions, instanceof), structured concurrency (StructuredTaskScope), scoped values, string templates, unnamed patterns/variables.
- Prefer virtual threads (Thread.ofVirtual(), Executors.newVirtualThreadPerTaskExecutor()) over platform threads for I/O-bound work.
- Prefer the new Sequenced Collections API (SequencedCollection, SequencedMap) where applicable.
- Prefer
varfor local variables when the type is obvious from context.- Use stream().toList() instead of stream().collect(Collectors.toList()) for imm...
Files:
backend/src/main/java/org/booklore/controller/KoboController.javabackend/src/main/java/org/booklore/service/kobo/KoboInitializationService.javabackend/src/main/java/org/booklore/service/kobo/KoboRatingService.javabackend/src/test/java/org/booklore/service/kobo/KoboRatingServiceTest.java
**/controller/**/*.java
⚙️ CodeRabbit configuration file
**/controller/**/*.java: Spring Framework 7 REST controller review:
- Prefer
@RestController+@RequestMappingwith ResponseEntity.- Flag missing input validation (
@Valid/@Validatedon request body/params).- Prefer ProblemDetail (RFC 9457) for error responses.
- Flag
@RequestMappingwithout explicit HTTP method constraint.- Check for missing authorization checks on sensitive endpoints.
- Flag any logging of request body that may contain PII or credentials.
Files:
backend/src/main/java/org/booklore/controller/KoboController.java
**/service/**/*.java
⚙️ CodeRabbit configuration file
**/service/**/*.java: Spring Framework 7 service layer review:
- Flag missing
@Transactionalon methods that perform multiple writes.- Prefer constructor injection over
@Autowiredfield injection. Use@AllArgsConstructor.- Use ApiError enum for throwing exceptions.
- Flag checked exceptions swallowed silently; must log or rethrow.
- Flag Thread.sleep(); prefer Duration-based overloads or ScheduledExecutorService.
- Prefer virtual threads (Thread.ofVirtual()) for I/O-bound operations.
- Flag mutable shared state in singleton beans.
Files:
backend/src/main/java/org/booklore/service/kobo/KoboInitializationService.javabackend/src/main/java/org/booklore/service/kobo/KoboRatingService.javabackend/src/test/java/org/booklore/service/kobo/KoboRatingServiceTest.java
backend/src/test/**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
Prefer focused unit tests; use
@SpringBootTestonly when the Spring context is required in backend code
Files:
backend/src/test/java/org/booklore/service/kobo/KoboRatingServiceTest.java
**/*Test.java
⚙️ CodeRabbit configuration file
**/*Test.java: Java test review:
- Prefer
@ExtendWith(SpringExtension.class) or@SpringBootTestfor integration tests.- Flag tests with no assertions.
- Flag Thread.sleep() in tests; use Awaitility or virtual-thread-friendly alternatives.
- Flag hardcoded ports or file paths.
- Flag missing edge case coverage: null, empty, boundary values.
- Prefer AssertJ over JUnit's built-in assertions for readability.
- Prefer
@Sqlor Testcontainers for database state; not hand-rolled setup/teardown.
Files:
backend/src/test/java/org/booklore/service/kobo/KoboRatingServiceTest.java
🧠 Learnings (5)
📚 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/controller/KoboController.javabackend/src/main/java/org/booklore/service/kobo/KoboInitializationService.javabackend/src/main/java/org/booklore/service/kobo/KoboRatingService.javabackend/src/test/java/org/booklore/service/kobo/KoboRatingServiceTest.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/controller/KoboController.javabackend/src/main/java/org/booklore/service/kobo/KoboInitializationService.javabackend/src/main/java/org/booklore/service/kobo/KoboRatingService.javabackend/src/test/java/org/booklore/service/kobo/KoboRatingServiceTest.java
📚 Learning: 2026-05-04T20:31:11.075Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 1086
File: backend/src/main/java/org/booklore/service/metadata/BookReviewUpdateService.java:63-66
Timestamp: 2026-05-04T20:31:11.075Z
Learning: For this repository, reviewers should treat string truncation done via `String.length()` and `String.substring(0, maxLength)` (UTF-16 code units) as an accepted, consistent convention. Do not flag individual occurrences of this pattern as bugs, even though it is not code-point-aware for surrogate pairs. A separate global effort is already tracked to move toward code-point-aware truncation, so per-site fixes should be avoided during code review.
Applied to files:
backend/src/main/java/org/booklore/controller/KoboController.javabackend/src/main/java/org/booklore/service/kobo/KoboInitializationService.javabackend/src/main/java/org/booklore/service/kobo/KoboRatingService.java
📚 Learning: 2026-04-27T15:25:55.042Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 930
File: backend/src/test/java/org/booklore/service/metadata/parser/ComicvineBookParserTest.java:68-75
Timestamp: 2026-04-27T15:25:55.042Z
Learning: In this repository’s JUnit 5 test sources (e.g., under backend/src/test/java/), do not flag “bare” `assert` statements as a bug. The project test runner is configured to always execute tests with assertions enabled (e.g., `-ea`), so `assert` behavior is consistent. Continue to review for correctness, but don’t treat unguarded `assert` usage in test classes as a static-analysis issue.
Applied to files:
backend/src/test/java/org/booklore/service/kobo/KoboRatingServiceTest.java
📚 Learning: 2026-05-04T05:01:33.919Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 1081
File: backend/src/test/java/org/booklore/service/metadata/parser/AudibleParserTest.java:132-135
Timestamp: 2026-05-04T05:01:33.919Z
Learning: In grimmory-tools/grimmory unit/integration test classes (e.g., files matching **/*Test.java under backend/src/test/java/**), it is acceptable to directly instantiate Jackson mappers (e.g., `new ObjectMapper()` / `new JsonMapper(...)`) and this should NOT be flagged. The Jackson guidance to use Spring bean injection or `JsonMapper.shared()` applies only to production code; tests may construct dependencies directly as standard practice. Production-code mapper instantiation rules should still be enforced outside test sources.
Applied to files:
backend/src/test/java/org/booklore/service/kobo/KoboRatingServiceTest.java
🔀 Multi-repo context grimmory-tools/grimmory-docs
Linked repositories findings
grimmory-tools/grimmory-docs
-
Docs include a dedicated Kobo integration doc and device setup that instructs users to point Kobo api_endpoint to /api/kobo/{token} — relevant to the new Kobo endpoint added in the PR. [::grimmory-tools/grimmory-docs::docs/integration/kobo.md:75-86, 99-104]
-
Magic shelf and other docs describe the personalRating field as a decimal on a 0–10 scale (examples and filters use personalRating values like 8 or 9). The service’s mapping (input 0–5 mapped→0–10 by doubling/clamping) matches the documented internal personalRating scale. [::grimmory-tools/grimmory-docs::docs/magic-shelf.md:101-111, docs/magic-shelf.md:686-748]
-
Many docs mention "Personal Rating" and UI features that surface personalRating (filters, stats, view preferences). If the PR changes how personalRating is persisted or its numeric range, these user-facing docs and examples may need updates — currently the PR’s mapping appears consistent with docs. [::grimmory-tools/grimmory-docs::docs/view-preferences.md; docs/reading-stats.md; docs/metadata/metadata-center.md]
-
I did not find any documentation references to the specific new Kobo rating endpoint path (/v1/products/{bookId}/rating/{rating}) in the docs repository — docs currently describe general Kobo setup but do not list this API route. Consider adding documentation or admin notes about the new endpoint and the expected rating range/behavior. [::grimmory-tools/grimmory-docs::repo-wide search for "products/.*/rating" returned no matches]
🔇 Additional comments (3)
backend/src/main/java/org/booklore/service/kobo/KoboInitializationService.java (1)
37-37: Nice addition ofuser_ratingsinitialization resource.This route registration is consistent with the new Kobo rating sync flow and cleanly integrates with the existing initialization resource map.
backend/src/main/java/org/booklore/controller/KoboController.java (1)
257-274: LGTM — new rating endpoint wired up correctly.The handler follows the existing numeric-bookId → delegate, non-numeric → proxy/404 pattern established by the other endpoints in this controller.
backend/src/main/java/org/booklore/service/kobo/KoboRatingService.java (1)
25-80: LGTM — access guard, find-or-create, and clamping logic are all correct.
@Transactionalcorrectly spans the findOrCreate and thesave. TheMath.clamp((long) rating * 2, 0, 10)call is correct for Java 21+ (longoverload with widenedintliterals).
Description
Adds support for Kobo personal ratings to flow into Grimmory via the Kobo Sync APIs.
Linked Issue
fixes #1164
Changes
KoboRatingService& tests/api/kobo/:token/v1/products/:bookId/rating/:ratingendpointManual Testing Steps
Screenshots (Optional)
Additional Context (Optional)
this only adds the push to Grimmory for a rating, it does not change anything to retrieve ratings
AI Disclosure
None
Checklist
just ui checkandjust api check.Summary by CodeRabbit
Release Notes