Skip to content

Commit 7b3cf28

Browse files
masonfoxGitHub Copilot
andauthored
fix: invalidate shelf cache when book status changes (#405)
## Summary Fixes a cache invalidation bug where changing a book's status on `/books/:id` doesn'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: - Shelf pages continue showing the old status until the 30-second `staleTime` expires - Users see inconsistent data between the book detail page and shelf pages - Manual refresh or waiting 30s is required to see updated status on shelves ## Solution Modified `invalidateBookQueries()` in `hooks/useBookStatus.ts` to also invalidate shelf caches: ### Cache-Based Approach (Option A) 1. **Check cache first**: Query React Query cache for the book's shelves using `queryKeys.book.shelves(bookId)` 2. **Surgical invalidation**: If cached shelves exist, invalidate only those specific shelf queries using `queryKeys.shelf.byId(shelfId)` 3. **Nuclear fallback**: If no cached shelves, invalidate all shelf queries using `queryKeys.shelf.base()` to ensure correctness This approach balances performance (surgical when possible) with correctness (nuclear when cache unavailable). ## Changes ### Core Changes - **`hooks/useBookStatus.ts`** (lines 42-74): - Enhanced `invalidateBookQueries()` to check cached book shelves - Invalidate affected shelf queries (surgical) or all shelves (nuclear) - Added JSDoc comments documenting the shelf invalidation logic - **`app/api/books/[id]/status/route.ts`** (line 69): - Removed misleading comment about SessionService handling cache invalidation ## Testing - ✅ All 4007 existing tests pass - ✅ No breaking changes to function signature - ✅ Compatible with all existing uses of `invalidateBookQueries()` - ✅ Server running and responsive during testing ## Implementation Pattern This follows the same pattern already used in `app/books/[id]/page.tsx` (lines 245-261) for the `updateShelves()` function, which demonstrates proper shelf invalidation when books are added/removed from shelves. ## Related Files - Investigation details: `docs/plans/fix-shelf-cache-invalidation.md` - Query keys definition: `lib/query-keys.ts` (lines 162-172) - Book detail page: `app/books/[id]/page.tsx` (lines 229-242 for shelf caching) --------- Co-authored-by: GitHub Copilot <copilot@github.com>
1 parent 5c5e0d9 commit 7b3cf28

3 files changed

Lines changed: 172 additions & 9 deletions

File tree

__tests__/hooks/useBookStatus.test.ts

Lines changed: 121 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
import { test, expect, describe, beforeEach, afterEach, vi } from 'vitest';
2-
import { renderHook, waitFor, act } from "../test-utils";
3-
import { useBookStatus } from "@/hooks/useBookStatus";
2+
import { renderHook, waitFor, act, createTestQueryClient } from "../test-utils";
3+
import { useBookStatus, invalidateBookQueries } from "@/hooks/useBookStatus";
44
import type { Book } from "@/hooks/useBookDetail";
5+
import { QueryClient } from '@tanstack/react-query';
6+
import { queryKeys } from '@/lib/query-keys';
57

68
const originalFetch = global.fetch;
79

@@ -387,4 +389,121 @@ describe("useBookStatus", () => {
387389
expect(result.current.selectedStatus).toBe("reading");
388390
});
389391
});
392+
393+
describe("invalidateBookQueries", () => {
394+
let queryClient: QueryClient;
395+
let invalidateSpy: any;
396+
397+
beforeEach(() => {
398+
queryClient = createTestQueryClient();
399+
invalidateSpy = vi.spyOn(queryClient, 'invalidateQueries');
400+
});
401+
402+
afterEach(() => {
403+
invalidateSpy.mockRestore();
404+
});
405+
406+
test("should invalidate all book-related queries", () => {
407+
const bookId = '123';
408+
409+
invalidateBookQueries(queryClient, bookId);
410+
411+
expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.book.detail(123) });
412+
expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.sessions.byBook(123) });
413+
expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.progress.byBook(123) });
414+
expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.dashboard.all() });
415+
expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.library.books() });
416+
expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.readNext.base() });
417+
});
418+
419+
test("should invalidate shelf caches surgically when cached shelves available", () => {
420+
const bookId = '123';
421+
422+
// Mock cached shelves
423+
queryClient.setQueryData(queryKeys.book.shelves(123), {
424+
success: true,
425+
data: [{ id: 1 }, { id: 2 }, { id: 3 }]
426+
});
427+
428+
invalidateBookQueries(queryClient, bookId);
429+
430+
// Verify surgical invalidation for each shelf
431+
expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.shelf.byId(1) });
432+
expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.shelf.byId(2) });
433+
expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.shelf.byId(3) });
434+
435+
// Verify nuclear invalidation was NOT called
436+
expect(invalidateSpy).not.toHaveBeenCalledWith({ queryKey: queryKeys.shelf.base() });
437+
});
438+
439+
test("should invalidate all shelves when cache unavailable", () => {
440+
const bookId = '123';
441+
442+
// No cached shelves - cache unavailable
443+
invalidateBookQueries(queryClient, bookId);
444+
445+
// Verify nuclear invalidation
446+
expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.shelf.base() });
447+
448+
// Verify no surgical (specific shelf ID) invalidations occurred
449+
const surgicalCalls = invalidateSpy.mock.calls.filter(
450+
(call: any) =>
451+
Array.isArray(call[0]?.queryKey) &&
452+
call[0].queryKey[0] === 'shelf' &&
453+
call[0].queryKey.length === 2 &&
454+
typeof call[0].queryKey[1] === 'number'
455+
);
456+
expect(surgicalCalls).toHaveLength(0);
457+
});
458+
459+
test("should NOT invalidate shelves when book is on no shelves", () => {
460+
const bookId = '123';
461+
462+
// Mock cached shelves with empty array (book on zero shelves)
463+
queryClient.setQueryData(queryKeys.book.shelves(123), {
464+
success: true,
465+
data: []
466+
});
467+
468+
invalidateBookQueries(queryClient, bookId);
469+
470+
// Should NOT trigger nuclear invalidation (performance optimization)
471+
expect(invalidateSpy).not.toHaveBeenCalledWith({ queryKey: queryKeys.shelf.base() });
472+
473+
// Should NOT invalidate any specific shelves (none exist)
474+
const surgicalCalls = invalidateSpy.mock.calls.filter(
475+
(call: any) =>
476+
Array.isArray(call[0]?.queryKey) &&
477+
call[0].queryKey[0] === 'shelf' &&
478+
call[0].queryKey.length === 2
479+
);
480+
expect(surgicalCalls).toHaveLength(0);
481+
});
482+
483+
test("should handle invalid cache data gracefully", () => {
484+
const bookId = '123';
485+
486+
// Mock invalid cache structure
487+
queryClient.setQueryData(queryKeys.book.shelves(123), {
488+
invalid: 'structure'
489+
});
490+
491+
invalidateBookQueries(queryClient, bookId);
492+
493+
// Verify nuclear invalidation (fallback for invalid data)
494+
expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.shelf.base() });
495+
});
496+
497+
test("should handle null cache data gracefully", () => {
498+
const bookId = '123';
499+
500+
// Mock null cache
501+
queryClient.setQueryData(queryKeys.book.shelves(123), null);
502+
503+
invalidateBookQueries(queryClient, bookId);
504+
505+
// Verify nuclear invalidation
506+
expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.shelf.base() });
507+
});
508+
});
390509
});

app/api/books/[id]/status/route.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,6 @@ export async function POST(request: NextRequest, props: { params: Promise<{ id:
6666

6767
const result = await sessionService.updateStatus(bookId, statusData);
6868

69-
// Note: Cache invalidation handled by SessionService.invalidateCache()
70-
7169
// Return full result if session was archived, otherwise just the session
7270
if (result.sessionArchived) {
7371
return NextResponse.json({

hooks/useBookStatus.ts

Lines changed: 51 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
import { useState, useEffect, useCallback, useMemo } from "react";
2-
import { useMutation, useQueryClient } from "@tanstack/react-query";
2+
import { useMutation, useQueryClient, QueryClient } from "@tanstack/react-query";
33
import { queryKeys } from "@/lib/query-keys";
44
import type { Book } from "./useBookDetail";
55
import { toast } from "@/utils/toast";
66
import { getLogger } from "@/lib/logger";
77
import { bookApi, ApiError } from "@/lib/api";
88
import { libraryService } from "@/lib/library-service";
99

10+
const logger = getLogger().child({ hook: 'useBookStatus' });
11+
1012
interface ProgressEntry {
1113
id: number;
1214
currentPage: number;
@@ -38,15 +40,59 @@ function requiresArchiveConfirmation(
3840
/**
3941
* Invalidates all queries related to a book
4042
* Exported to allow reuse in components that make direct API calls
43+
*
44+
* Also invalidates shelf caches for shelves containing this book to ensure
45+
* shelf pages reflect updated book status immediately.
4146
*/
42-
export function invalidateBookQueries(queryClient: any, bookId: string): void {
43-
queryClient.invalidateQueries({ queryKey: queryKeys.book.detail(parseInt(bookId)) });
44-
queryClient.invalidateQueries({ queryKey: queryKeys.sessions.byBook(parseInt(bookId)) });
45-
queryClient.invalidateQueries({ queryKey: queryKeys.progress.byBook(parseInt(bookId)) });
47+
export function invalidateBookQueries(queryClient: QueryClient, bookId: string): void {
48+
const numericBookId = parseInt(bookId);
49+
50+
// Invalidate book-related queries
51+
queryClient.invalidateQueries({ queryKey: queryKeys.book.detail(numericBookId) });
52+
queryClient.invalidateQueries({ queryKey: queryKeys.sessions.byBook(numericBookId) });
53+
queryClient.invalidateQueries({ queryKey: queryKeys.progress.byBook(numericBookId) });
4654
queryClient.invalidateQueries({ queryKey: queryKeys.dashboard.all() });
4755
queryClient.invalidateQueries({ queryKey: queryKeys.library.books() });
4856
queryClient.invalidateQueries({ queryKey: queryKeys.readNext.base() });
4957

58+
// Invalidate shelves containing this book
59+
// Try to get shelves from cache; if available, invalidate only those shelves (surgical)
60+
// If not cached, invalidate all shelves (nuclear) to ensure correctness
61+
const bookShelvesKey = queryKeys.book.shelves(numericBookId);
62+
const cachedShelves = queryClient.getQueryData(bookShelvesKey);
63+
64+
// Type guard to validate cached shelves structure
65+
const isValidShelvesData = (data: unknown): data is { success: boolean; data: Array<{ id: number }> } => {
66+
return (
67+
data !== null &&
68+
typeof data === 'object' &&
69+
'data' in data &&
70+
Array.isArray((data as any).data)
71+
);
72+
};
73+
74+
if (isValidShelvesData(cachedShelves)) {
75+
// Cache is valid - use surgical approach
76+
if (cachedShelves.data.length > 0) {
77+
// Book is on some shelves - invalidate only those
78+
logger.debug(
79+
{ bookId: numericBookId, shelfIds: cachedShelves.data.map(s => s.id) },
80+
'Surgical shelf invalidation'
81+
);
82+
cachedShelves.data.forEach((shelf: { id: number }) => {
83+
queryClient.invalidateQueries({ queryKey: queryKeys.shelf.byId(shelf.id) });
84+
});
85+
}
86+
// else: Book is on zero shelves (data.length === 0) - no-op, nothing to invalidate
87+
} else {
88+
// Cache unavailable or invalid - invalidate all shelves to be safe
89+
logger.debug(
90+
{ bookId: numericBookId, cacheState: cachedShelves === null ? 'null' : 'invalid' },
91+
'Nuclear shelf invalidation (cache unavailable)'
92+
);
93+
queryClient.invalidateQueries({ queryKey: queryKeys.shelf.base() });
94+
}
95+
5096
// Clear entire LibraryService cache to ensure status changes reflect across all filters
5197
libraryService.clearCache();
5298
}

0 commit comments

Comments
 (0)