fix: invalidate shelf cache when book status changes#405
Conversation
When a book's status was changed on /books/:id, the shelf pages at /shelves/:id would show stale status for up to 30 seconds until the React Query cache expired. This was because invalidateBookQueries() invalidated book, session, progress, dashboard, and library queries, but did not invalidate shelf queries. Changes: - Modified invalidateBookQueries() to check React Query cache for the book's shelves and invalidate those shelf caches - Uses cache-based approach: if shelves are cached, invalidates only affected shelves (surgical); otherwise invalidates all shelves (nuclear) to ensure correctness - Removed misleading comment in status route about SessionService handling cache invalidation All 4007 tests pass.
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## develop #405 +/- ##
===========================================
+ Coverage 78.62% 78.66% +0.04%
===========================================
Files 167 167
Lines 7540 7556 +16
Branches 1846 1850 +4
===========================================
+ Hits 5928 5944 +16
Misses 1127 1127
Partials 485 485
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…lidation - Replace type assertion with runtime type guard for safer cache validation - Add debug logging for surgical vs nuclear invalidation paths - Add 6 comprehensive tests covering all cache invalidation scenarios - All 4013 tests passing Addresses @review feedback: test coverage, type safety, and observability
- Add logger for structured logging (replace console.debug) - Fix empty shelves logic to treat [] as no-op - Update test assertions for better verification - Update empty shelves test expectation Fixes suggested by GitHub Copilot code review.
There was a problem hiding this comment.
Pull request overview
This PR fixes a TanStack Query cache invalidation gap so that changing a book’s status on /books/:id also updates any affected /shelves/:id pages immediately, instead of waiting for staleTime to expire.
Changes:
- Extend
invalidateBookQueries()to invalidate shelf queries for shelves containing the updated book (surgical when cache is present; fallback to invalidating all shelves). - Add unit tests covering the new shelf invalidation behaviors.
- Remove a misleading cache invalidation comment from the book status API route.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
hooks/useBookStatus.ts |
Adds shelf-aware cache invalidation logic inside invalidateBookQueries(). |
app/api/books/[id]/status/route.ts |
Removes an inaccurate comment about where cache invalidation occurs. |
__tests__/hooks/useBookStatus.test.ts |
Adds tests validating both surgical and fallback shelf invalidation paths. |
You can also share your feedback on Copilot code review. Take the survey.
| queryClient = new QueryClient({ | ||
| defaultOptions: { | ||
| queries: { retry: false }, | ||
| }, | ||
| }); |
hooks/useBookStatus.ts
Outdated
| export function invalidateBookQueries(queryClient: any, bookId: string): void { | ||
| queryClient.invalidateQueries({ queryKey: queryKeys.book.detail(parseInt(bookId)) }); | ||
| queryClient.invalidateQueries({ queryKey: queryKeys.sessions.byBook(parseInt(bookId)) }); | ||
| queryClient.invalidateQueries({ queryKey: queryKeys.progress.byBook(parseInt(bookId)) }); | ||
| const numericBookId = parseInt(bookId); | ||
|
|
||
| // Invalidate book-related queries | ||
| queryClient.invalidateQueries({ queryKey: queryKeys.book.detail(numericBookId) }); | ||
| queryClient.invalidateQueries({ queryKey: queryKeys.sessions.byBook(numericBookId) }); | ||
| queryClient.invalidateQueries({ queryKey: queryKeys.progress.byBook(numericBookId) }); | ||
| queryClient.invalidateQueries({ queryKey: queryKeys.dashboard.all() }); | ||
| queryClient.invalidateQueries({ queryKey: queryKeys.library.books() }); | ||
| queryClient.invalidateQueries({ queryKey: queryKeys.readNext.base() }); | ||
|
|
||
| // Invalidate shelves containing this book | ||
| // Try to get shelves from cache; if available, invalidate only those shelves (surgical) | ||
| // If not cached, invalidate all shelves (nuclear) to ensure correctness | ||
| const bookShelvesKey = queryKeys.book.shelves(numericBookId); | ||
| const cachedShelves = queryClient.getQueryData(bookShelvesKey); |
- Use createTestQueryClient() for consistent test configuration - Type invalidateBookQueries queryClient parameter as QueryClient Addresses Copilot suggestions: 1. Test configuration consistency - use helper that disables caching 2. Type safety - replace 'any' with proper QueryClient type Co-authored-by: GitHub Copilot <copilot@github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes stale shelf data after changing a book’s status by extending the existing React Query invalidation helper to also invalidate relevant shelf queries, ensuring /shelves/:id reflects status updates immediately.
Changes:
- Extend
invalidateBookQueries()to invalidate shelf caches (surgical when book-shelf membership is cached; otherwise fallback to invalidating all shelves). - Remove a misleading cache invalidation comment from the status API route.
- Add unit tests covering the new shelf invalidation behavior (including fallback paths).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| hooks/useBookStatus.ts | Enhances invalidateBookQueries() to invalidate shelf query caches based on cached book→shelves membership, with a fallback to invalidating all shelves. |
| app/api/books/[id]/status/route.ts | Removes a misleading comment about where cache invalidation occurs. |
| tests/hooks/useBookStatus.test.ts | Adds test coverage for invalidateBookQueries() including shelf invalidation scenarios. |
You can also share your feedback on Copilot code review. Take the survey.
| // Cache unavailable or invalid - invalidate all shelves to be safe | ||
| logger.debug( | ||
| { bookId: numericBookId, cacheState: cachedShelves === null ? 'null' : 'invalid' }, | ||
| 'Nuclear shelf invalidation (cache unavailable)' | ||
| ); | ||
| queryClient.invalidateQueries({ queryKey: queryKeys.shelf.base() }); |
Summary
Fixes a cache invalidation bug where changing a book's status on
/books/:iddoesn't invalidate the cache for/shelves/:id, causing shelves to show stale book status for up to 30 seconds.Problem
When a user changes a book's status on the book detail page, the
invalidateBookQueries()helper function invalidates several React Query caches (book, sessions, progress, dashboard, library, read-next), but does not invalidate shelf queries. This means:staleTimeexpiresSolution
Modified
invalidateBookQueries()inhooks/useBookStatus.tsto also invalidate shelf caches:Cache-Based Approach (Option A)
queryKeys.book.shelves(bookId)queryKeys.shelf.byId(shelfId)queryKeys.shelf.base()to ensure correctnessThis approach balances performance (surgical when possible) with correctness (nuclear when cache unavailable).
Changes
Core Changes
hooks/useBookStatus.ts(lines 42-74):invalidateBookQueries()to check cached book shelvesapp/api/books/[id]/status/route.ts(line 69):Testing
invalidateBookQueries()Implementation Pattern
This follows the same pattern already used in
app/books/[id]/page.tsx(lines 245-261) for theupdateShelves()function, which demonstrates proper shelf invalidation when books are added/removed from shelves.Related Files
docs/plans/fix-shelf-cache-invalidation.mdlib/query-keys.ts(lines 162-172)app/books/[id]/page.tsx(lines 229-242 for shelf caching)