Add book reading progress tracking#22
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds end-to-end reading progress: shared types and schemas, normalization logic, DB columns and constraints, repository/service methods, a PUT API, display and edit Vue components, library-page optimistic saves with sequencing, and unit tests. ChangesReading Progress Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
app/components/BookReadingProgressModal.vueOops! Something went wrong! :( ESLint: 10.3.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/.nuxt/eslint.config.mjs' imported from /eslint.config.mjs 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
test/unit/reading-progress.test.ts (1)
15-84: ⚡ Quick winAdd a regression test for percent-only updates with existing
currentPage.Please add a case where existing progress has
currentPageset, input only providesprogressPercent, and the explicit percent must be preserved. This protects PATCH semantics and catches silent overwrites.🤖 Prompt for 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. In `@test/unit/reading-progress.test.ts` around lines 15 - 84, Add a new unit test in the same describe block that calls normalizeReadingProgress with an existing readingProgress that has a non-null currentPage (e.g., 120) and then supplies an incoming update that includes only progressPercent (e.g., 75) and no currentPage; assert that the resulting object preserves the explicit progressPercent (75), keeps the existing currentPage unchanged, preserves startedAt, keeps finishedAt null, and leaves status as 'reading'. Reference normalizeReadingProgress, readingProgress, currentPage, progressPercent and startedAt in the test so it guards against silent overwrites of percent-only PATCH updates.
🤖 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 `@app/components/BookReadingProgressModal.vue`:
- Around line 95-104: The parsed numeric helpers are returning raw numbers which
can be fractional or outside allowed ranges; update parseOptionalNumber and the
saveProgress emitter code paths (e.g., the saveProgress method and any parsing
at the 148-157 region) to normalize values by converting to integers (use
Math.round or Math.floor per UX choice), then clamp them into valid boundaries
(minimum 0 and maximum equal to the book’s totalPages or another defined max)
and only emit/save those normalized, bounded numbers; ensure the emitted payload
uses these sanitized values instead of the raw parsed values.
In `@app/pages/library/`[id].vue:
- Around line 195-224: The rollback currently uses previousProgress on a failed
request and can reintroduce stale data when responses arrive out of order;
update the catch block (where currentRequestId, readingRequestId,
previousProgress, book.value, lastConfirmedReadingProgress and
isSavingReadingProgress are referenced) to, on currentRequestId ===
readingRequestId, fetch the authoritative readingProgress from the backend (e.g.
GET `/api/books/${userBookId}` or `/api/books/${userBookId}/reading`), apply
that result to book.value and lastConfirmedReadingProgress, and show an error
toast—do not blindly restore previousProgress; ensure isSavingReadingProgress is
cleared and only ignore stale older-request errors when currentRequestId !==
readingRequestId.
In `@shared/utils/reading-progress.ts`:
- Around line 38-40: The code always recalculates progressPercent from
currentPage when currentPage exists, which overwrites an explicit
input.progressPercent; change the logic in shared/utils/reading-progress.ts so
that if input.progressPercent (or a similarly named incoming param) is
non-null/defined you use that value (clamped/rounded to 0–100) instead of
recomputing, otherwise fall back to computing
Math.round((currentPage/totalPages)*100) when currentPage and totalPages are
available; keep the existing Math.min(100, ...) bound and ensure progressPercent
remains an integer.
---
Nitpick comments:
In `@test/unit/reading-progress.test.ts`:
- Around line 15-84: Add a new unit test in the same describe block that calls
normalizeReadingProgress with an existing readingProgress that has a non-null
currentPage (e.g., 120) and then supplies an incoming update that includes only
progressPercent (e.g., 75) and no currentPage; assert that the resulting object
preserves the explicit progressPercent (75), keeps the existing currentPage
unchanged, preserves startedAt, keeps finishedAt null, and leaves status as
'reading'. Reference normalizeReadingProgress, readingProgress, currentPage,
progressPercent and startedAt in the test so it guards against silent overwrites
of percent-only PATCH updates.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 23803890-699b-47fd-9d4b-529a860c4e4a
⛔ Files ignored due to path filters (2)
server/db/migrations/sqlite/0009_add_reading_progress.sqlis excluded by!server/db/migrations/**server/db/migrations/sqlite/meta/_journal.jsonis excluded by!server/db/migrations/**
📒 Files selected for processing (15)
app/components/BookReadingProgress.vueapp/components/BookReadingProgressModal.vueapp/pages/library/[id].vueserver/api/books/[id]/reading.put.tsserver/db/schema/domain.tsserver/repositories/book.repository.tsserver/services/book.service.tsserver/utils/effect.tsshared/types/book.tsshared/utils/reading-progress.tsshared/utils/schemas.tstest/unit/reading-progress.test.tstest/unit/server/api/_helpers/api-route.tstest/unit/server/api/books/[id]/index.get.test.tstest/unit/server/api/books/[id]/reading.put.test.ts
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
1 participant
Summary
Testing
pnpm lintpnpm typecheckpnpm test:unitSummary by CodeRabbit
New Features
Tests