diff --git a/.opencode/command/review-loop.md b/.opencode/command/review-loop.md deleted file mode 100644 index ca017304..00000000 --- a/.opencode/command/review-loop.md +++ /dev/null @@ -1,177 +0,0 @@ ---- -description: Automated PR review loop with @review agent and GitHub Copilot until both approve -agent: build ---- - -# Automated PR Review Loop - -You are orchestrating an automated PR review loop that alternates between the @review agent and GitHub Copilot until both approve the changes. - -## Arguments - -- `$ARGUMENTS` - Optional PR number. If not provided, auto-detect from current branch. - -## Setup Phase - -1. **Get PR context**: - ```bash - # If $ARGUMENTS provided, use that as PR number - # Otherwise, auto-detect from current branch - gh pr view $ARGUMENTS --json number,url,title,headRefName,state -q '{number: .number, url: .url, title: .title, branch: .headRefName, state: .state}' - ``` - -2. **Validate**: - - PR must exist - - PR must be OPEN - - Current branch must match PR branch (if auto-detected) - - All tests must pass: `npm test` - -3. **Announce start**: - ``` - 🔄 Starting review loop for PR # - 📋 Title: - 🔗 URL: <url> - ⚙️ Max iterations: 3 - ``` - -## Review Loop - -Execute up to **3 iterations**. Each iteration: - -### Step 1: @review Agent Review - -Use the Task tool to invoke the `review` subagent with this prompt: - -``` -Review the current changes in this PR. - -PR Context: -- Number: #<number> -- Branch: <branch> -- Title: <title> - -Focus on: -1. Constitution compliance (.specify/memory/constitution.md) -2. Pattern adherence (.specify/memory/patterns.md) -3. Code quality and maintainability -4. Security vulnerabilities -5. Test coverage -6. Performance implications - -Review the diff: -!`git diff develop...HEAD` - -End your review with EXACTLY one of: -- "APPROVED: All changes look good" (if no issues found) -- "CHANGES REQUESTED: [specific issues]" (if issues found) -``` - -**Parse the response**: -- If contains "APPROVED: All changes look good" → `review_approved = true` -- If contains "CHANGES REQUESTED:" → `review_approved = false`, extract issues - -### Step 2: Implement @review Recommendations - -**Only if** `review_approved = false`: - -1. Analyze the requested changes -2. Implement the fixes using Edit/Write tools -3. Run tests: `npm test` (must pass) -4. Commit: `git commit -am "fix: implement @review feedback (iteration <N>)"` -5. Push: `git push` - -**If** `review_approved = true`: -- Skip to Step 3 - -### Step 3: GitHub Copilot Review - -Request Copilot review of the PR: - -```bash -# Get Copilot's review -gh pr review <number> --comment --body "Please review this PR for code quality, best practices, and potential issues." -``` - -Wait 5 seconds, then check for Copilot comments: - -```bash -gh pr view <number> --comments --json comments -q '.comments[] | select(.author.login == "github-copilot") | {body: .body, createdAt: .createdAt}' | tail -1 -``` - -**Parse Copilot's response**: -- If no new comments or contains "LGTM" or "looks good" → `copilot_approved = true` -- Otherwise → `copilot_approved = false`, extract suggestions - -### Step 4: @review Re-evaluates Copilot Suggestions - -**Only if** `copilot_approved = false`: - -Use Task tool to invoke `review` subagent: - -``` -GitHub Copilot provided these suggestions: - -<copilot_feedback> - -As the senior reviewer: -1. Do you agree with these suggestions? -2. Which ones should be implemented? -3. Are any suggestions unnecessary or incorrect? - -If changes should be made, specify exactly what to change and why. -End with "IMPLEMENT: <specific changes>" or "IGNORE: <reasoning>". -``` - -If response contains "IMPLEMENT:": -1. Make the agreed-upon changes -2. Run tests: `npm test` -3. Commit: `git commit -am "fix: address copilot suggestions (iteration <N>)"` -4. Push: `git push` - -### Step 5: Check Exit Conditions - -**Exit loop if ANY of**: -- ✅ `review_approved = true` AND `copilot_approved = true` -- ⚠️ Iteration count >= 3 -- ❌ Tests failed and cannot be fixed - -Otherwise, continue to next iteration. - -## Final Summary - -Report: -``` -━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ -📊 Review Loop Complete - -Total iterations: <N> -Final status: <status> - -@review agent: <approved/not approved> -GitHub Copilot: <approved/not approved> - -Changes made: <N> commits -━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - -🔗 PR: <url> -``` - -Status can be: -- ✅ "Both agents approved - ready to merge!" -- ⚠️ "Max iterations reached - manual review needed" -- ❌ "Tests failed - manual intervention required" - -## Error Handling - -- If PR not found: "❌ No PR found for current branch. Create a PR first or pass PR number as argument." -- If tests fail: Try to fix once, then report error -- If git operations fail: Report exact error and exit -- If agent invocation fails: Report error and continue to next step - -## Important Rules - -- NEVER skip test runs after making changes -- ALWAYS push changes immediately after committing -- ALWAYS verify PR is still OPEN before each iteration -- Log progress clearly at each step for user visibility -- Save iteration state in comments for debugging diff --git a/.opencode/command/review-status.md b/.opencode/command/review-status.md deleted file mode 100644 index 08b337b3..00000000 --- a/.opencode/command/review-status.md +++ /dev/null @@ -1,39 +0,0 @@ ---- -description: Check review status of current PR -agent: build ---- - -# Review Status Check - -Quick status check for the current PR's review state. - -## Process - -1. **Get PR info**: - ```bash - gh pr view --json number,url,title,reviewDecision,statusCheckRollup,commits -q '{number: .number, url: .url, title: .title, reviewDecision: .reviewDecision, checks: .statusCheckRollup, commitCount: (.commits | length)}' - ``` - -2. **Get recent comments** (last 5): - ```bash - gh pr view --comments --json comments -q '.comments[-5:] | .[] | {author: .author.login, body: .body, createdAt: .createdAt}' - ``` - -3. **Get recent commits** (last 5): - ```bash - git log -5 --oneline HEAD - ``` - -4. **Summary**: - ``` - 📊 PR #<number> Review Status - ━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - Title: <title> - Review Decision: <decision> - Status Checks: <passing/failing> - Recent Commits: <count> - - 🔗 <url> - ``` - -This gives you a quick snapshot without triggering any reviews or changes. diff --git a/__tests__/e2e/api/books/dnf-transitions.test.ts b/__tests__/e2e/api/books/dnf-transitions.test.ts new file mode 100644 index 00000000..dc3e6bdf --- /dev/null +++ b/__tests__/e2e/api/books/dnf-transitions.test.ts @@ -0,0 +1,309 @@ +import { describe, test, expect, beforeAll, beforeEach, afterAll, vi } from "vitest"; +import { setupTestDatabase, teardownTestDatabase, clearTestDatabase } from "@/__tests__/helpers/db-setup"; +import { bookRepository, sessionRepository } from "@/lib/repositories"; +import { createTestBook, createTestSession, createMockRequest } from "@/__tests__/fixtures/test-data"; +import { POST } from "@/app/api/books/[id]/status/route"; +import type { Book } from "@/lib/db/schema/books"; +import type { NextRequest } from "next/server"; + +/** + * Mock Rationale: Prevent Next.js cache revalidation side effects during tests. + */ +vi.mock("next/cache", () => ({ + revalidatePath: () => {}, +})); + +/** + * Mock Rationale: Prevent Calibre sync during tests (file system I/O). + */ +vi.mock("@/lib/services/calibre.service", () => ({ + calibreService: { + updateRating: () => {}, + readRating: () => null, + updateTags: () => {}, + readTags: () => [], + }, + CalibreService: class {}, +})); + +/** + * Mock Rationale: Prevent streak rebuilding side effects during tests. + */ +vi.mock("@/lib/services/streak.service", () => ({ + streakService: { + rebuildStreak: vi.fn(() => Promise.resolve()), + }, +})); + +describe("DNF Status Transitions (E2E API)", () => { + let testBook: Book; + + beforeAll(async () => { + await setupTestDatabase(__filename); + }); + + afterAll(async () => { + await teardownTestDatabase(__filename); + }); + + beforeEach(async () => { + await clearTestDatabase(__filename); + + // Create test book + testBook = await bookRepository.create(createTestBook({ + calibreId: 888, + title: "DNF E2E Test", + authors: ["Test Author"], + path: "/test/dnf-e2e", + totalPages: 400, + })); + }); + + test("POST /api/books/[id]/status - DNF to read-next creates new session", async () => { + // ARRANGE: Book with DNF session + const dnfSession = await sessionRepository.create(createTestSession({ + bookId: testBook.id, + sessionNumber: 1, + status: "dnf", + isActive: false, + startedDate: "2026-01-01", + completedDate: "2026-01-10", + })); + + // ACT: Call status update API + const request = createMockRequest("POST", `/api/books/${testBook.id}/status`, { + status: "read-next" + }) as NextRequest; + const response = await POST(request, { params: Promise.resolve({ id: testBook.id.toString() }) }); + + // ASSERT: API response (session properties spread at top level when archived) + expect(response.status).toBe(200); + const data = await response.json(); + + expect(data.id).not.toBe(dnfSession.id); + expect(data.sessionNumber).toBe(2); + expect(data.status).toBe("read-next"); + expect(data.sessionArchived).toBe(true); + expect(data.archivedSessionNumber).toBe(1); + + // Verify database state + const allSessions = await sessionRepository.findAllByBookId(testBook.id); + expect(allSessions).toHaveLength(2); + + const archived = allSessions.find(s => s.sessionNumber === 1); + expect(archived?.status).toBe("dnf"); + expect(archived?.isActive).toBe(false); + + const active = allSessions.find(s => s.sessionNumber === 2); + expect(active?.status).toBe("read-next"); + expect(active?.isActive).toBe(true); + }); + + test("POST /api/books/[id]/status - DNF to to-read creates new session", async () => { + // ARRANGE: Book with DNF session + const dnfSession = await sessionRepository.create(createTestSession({ + bookId: testBook.id, + sessionNumber: 1, + status: "dnf", + isActive: false, + startedDate: "2026-01-01", + completedDate: "2026-01-10", + })); + + // ACT: Call status update API + const request = createMockRequest("POST", `/api/books/${testBook.id}/status`, { + status: "to-read" + }) as NextRequest; + const response = await POST(request, { params: Promise.resolve({ id: testBook.id.toString() }) }); + + // ASSERT: API response (session properties spread at top level when archived) + expect(response.status).toBe(200); + const data = await response.json(); + + expect(data.id).not.toBe(dnfSession.id); + expect(data.sessionNumber).toBe(2); + expect(data.status).toBe("to-read"); + expect(data.sessionArchived).toBe(true); + + // Verify database state + const allSessions = await sessionRepository.findAllByBookId(testBook.id); + expect(allSessions).toHaveLength(2); + }); + + test("POST /api/books/[id]/status - DNF to reading creates new session", async () => { + // ARRANGE: Book with DNF session + const dnfSession = await sessionRepository.create(createTestSession({ + bookId: testBook.id, + sessionNumber: 1, + status: "dnf", + isActive: false, + startedDate: "2026-01-01", + completedDate: "2026-01-10", + })); + + // ACT: Call status update API + const request = createMockRequest("POST", `/api/books/${testBook.id}/status`, { + status: "reading" + }) as NextRequest; + const response = await POST(request, { params: Promise.resolve({ id: testBook.id.toString() }) }); + + // ASSERT: API response (session properties spread at top level when archived) + expect(response.status).toBe(200); + const data = await response.json(); + + expect(data.id).not.toBe(dnfSession.id); + expect(data.sessionNumber).toBe(2); + expect(data.status).toBe("reading"); + expect(data.startedDate).toBeTruthy(); // Auto-set today + expect(data.sessionArchived).toBe(true); + + // Verify DNF session preserved + const archived = await sessionRepository.findById(dnfSession.id); + expect(archived?.status).toBe("dnf"); + expect(archived?.completedDate).toBe("2026-01-10"); + expect(archived?.isActive).toBe(false); + }); + + test("POST /api/books/[id]/status - DNF to read returns 400 error", async () => { + // ARRANGE: Book with DNF session + await sessionRepository.create(createTestSession({ + bookId: testBook.id, + sessionNumber: 1, + status: "dnf", + isActive: false, + startedDate: "2026-01-01", + completedDate: "2026-01-10", + })); + + // ACT: Try to mark as read + const request = createMockRequest("POST", `/api/books/${testBook.id}/status`, { + status: "read" + }) as NextRequest; + const response = await POST(request, { params: Promise.resolve({ id: testBook.id.toString() }) }); + + // ASSERT: Should return 400 error + expect(response.status).toBe(400); + const data = await response.json(); + expect(data.error).toContain("Cannot mark DNF book as read directly"); + }); + + test("POST /api/books/[id]/status - DNF completedDate is preserved when archiving", async () => { + // ARRANGE: Book with DNF session with specific completedDate + const originalCompletedDate = "2026-01-20"; + const dnfSession = await sessionRepository.create(createTestSession({ + bookId: testBook.id, + sessionNumber: 1, + status: "dnf", + isActive: false, + startedDate: "2026-01-10", + completedDate: originalCompletedDate, + })); + + // ACT: Transition to read-next via API + const request = createMockRequest("POST", `/api/books/${testBook.id}/status`, { + status: "read-next" + }) as NextRequest; + const response = await POST(request, { params: Promise.resolve({ id: testBook.id.toString() }) }); + + // ASSERT: Success response + expect(response.status).toBe(200); + + // Verify original completedDate preserved in database + const archived = await sessionRepository.findById(dnfSession.id); + expect(archived?.completedDate).toBe(originalCompletedDate); + expect(archived?.status).toBe("dnf"); + }); + + describe("terminal state behavior (E2E)", () => { + test("should set isActive=false when transitioning to 'read' status", async () => { + // ARRANGE: Active reading session + const activeSession = await sessionRepository.create(createTestSession({ + bookId: testBook.id, + sessionNumber: 1, + status: "reading", + isActive: true, + startedDate: "2026-01-01", + })); + + // ACT: Mark as read via API + const request = createMockRequest("POST", `/api/books/${testBook.id}/status`, { + status: "read", + completedDate: "2026-01-15", + }) as NextRequest; + const response = await POST(request, { params: Promise.resolve({ id: testBook.id.toString() }) }); + const data = await response.json(); + + // ASSERT: Response successful + expect(response.status).toBe(200); + expect(data.status).toBe("read"); + expect(data.isActive).toBe(false); // Terminal "read" status archives session (ADR-004/008) + + // ASSERT: Database reflects change + const updated = await sessionRepository.findById(activeSession.id); + expect(updated?.isActive).toBe(false); + expect(updated?.status).toBe("read"); + expect(updated?.completedDate).toBe("2026-01-15"); + }); + + test("should handle DNF → reading transition with archived DNF session", async () => { + // ARRANGE: Archived DNF session (correct post-terminal-state state) + const dnfSession = await sessionRepository.create(createTestSession({ + bookId: testBook.id, + sessionNumber: 1, + status: "dnf", + isActive: false, + startedDate: "2026-01-01", + completedDate: "2026-01-10", + })); + + // ACT: Restart reading via API + const request = createMockRequest("POST", `/api/books/${testBook.id}/status`, { + status: "reading", + }) as NextRequest; + const response = await POST(request, { params: Promise.resolve({ id: testBook.id.toString() }) }); + const data = await response.json(); + + // ASSERT: New session created + expect(response.status).toBe(200); + expect(data.sessionArchived).toBe(true); + expect(data.id).toBeDefined(); // Flattened response structure + expect(data.sessionNumber).toBe(2); + expect(data.status).toBe("reading"); + expect(data.isActive).toBe(true); + + // ASSERT: DNF session still archived + const archived = await sessionRepository.findById(dnfSession.id); + expect(archived?.isActive).toBe(false); + }); + + test("should assign proper readNextOrder for DNF → read-next transition", async () => { + // ARRANGE: Archived DNF session + const dnfSession = await sessionRepository.create(createTestSession({ + bookId: testBook.id, + sessionNumber: 1, + status: "dnf", + isActive: false, + startedDate: "2026-01-01", + completedDate: "2026-01-10", + })); + + // ACT: Transition to read-next via API + const request = createMockRequest("POST", `/api/books/${testBook.id}/status`, { + status: "read-next", + }) as NextRequest; + const response = await POST(request, { params: Promise.resolve({ id: testBook.id.toString() }) }); + const data = await response.json(); + + // ASSERT: New session created with proper ordering + expect(response.status).toBe(200); + expect(data.sessionArchived).toBe(true); + expect(data.sessionNumber).toBe(2); + expect(data.status).toBe("read-next"); + expect(data.readNextOrder).toBeGreaterThanOrEqual(0); // Verifies readNextOrder is assigned (0 for first read-next book) + + // ASSERT: DNF session still archived + const archived = await sessionRepository.findById(dnfSession.id); + expect(archived?.isActive).toBe(false); + }); + }); +}); diff --git a/__tests__/e2e/api/progress/progress-edit.test.ts b/__tests__/e2e/api/progress/progress-edit.test.ts index e1841026..921aaae0 100644 --- a/__tests__/e2e/api/progress/progress-edit.test.ts +++ b/__tests__/e2e/api/progress/progress-edit.test.ts @@ -444,4 +444,84 @@ describe("Progress Edit API", () => { expect(data.error).toBe("Progress entry does not belong to this book"); }); }); + + describe("PATCH - stable sort with same-date entries (issue #399)", () => { + test("should calculate pagesRead correctly when moving entry with same-date siblings", async () => { + // Reproduce the exact scenario from issue #399 + const book = await bookRepository.create(testBook); + const session = await sessionRepository.create({ + bookId: book.id, + sessionNumber: 1, + status: "reading", + isActive: true, + startedDate: "2026-03-08", + }); + + // Create the exact entries from the bug report + // Entry 1: Mar 8, page 43 + const entry1 = await progressRepository.create({ + bookId: book.id, + sessionId: session.id, + currentPage: 43, + currentPercentage: 9, + pagesRead: 43, + progressDate: "2026-03-08", + }); + + // Entry 2: Mar 8, page 57 (stable sort uses ID as tiebreaker) + const entry2 = await progressRepository.create({ + bookId: book.id, + sessionId: session.id, + currentPage: 57, + currentPercentage: 13, + pagesRead: 14, + progressDate: "2026-03-08", + }); + + // Entry 3: Mar 9, page 122 (should read 65 pages: 122 - 57) + const entry3 = await progressRepository.create({ + bookId: book.id, + sessionId: session.id, + currentPage: 122, + currentPercentage: 28, + pagesRead: 65, + progressDate: "2026-03-09", + }); + + // Step 1: Move entry3 from Mar 9 to Mar 8 + const request1 = createMockRequest( + "PATCH", + `/api/books/${book.id}/progress/${entry3.id}`, + { progressDate: "2026-03-08" } + ) as any; + + const response1 = await PATCH(request1, { + params: { id: book.id.toString(), progressId: entry3.id.toString() }, + }); + + expect(response1.status).toBe(200); + const data1 = await response1.json(); + expect(data1.progressDate).toBe("2026-03-08"); + // pagesRead should be calculated consistently + + // Step 2: Move entry3 back from Mar 8 to Mar 9 + const request2 = createMockRequest( + "PATCH", + `/api/books/${book.id}/progress/${entry3.id}`, + { progressDate: "2026-03-09" } + ) as any; + + const response2 = await PATCH(request2, { + params: { id: book.id.toString(), progressId: entry3.id.toString() }, + }); + + expect(response2.status).toBe(200); + const data2 = await response2.json(); + + // Critical assertion: Should be 65 pages (122 - 57), not 79 (122 - 43) + expect(data2.progressDate).toBe("2026-03-09"); + expect(data2.currentPage).toBe(122); + expect(data2.pagesRead).toBe(65); // This was the bug - it returned 79 + }); + }); }); diff --git a/__tests__/e2e/api/status-rating-sync.test.ts b/__tests__/e2e/api/status-rating-sync.test.ts index 6b496a0b..01513cac 100644 --- a/__tests__/e2e/api/status-rating-sync.test.ts +++ b/__tests__/e2e/api/status-rating-sync.test.ts @@ -265,7 +265,7 @@ describe("POST /api/books/[id]/status - Rating Sync to Calibre", () => { // Assert - Status update still succeeded expect(response.status).toBe(200); expect(data.status).toBe("read"); - expect(data.isActive).toBe(true); // Terminal states stay active + expect(data.isActive).toBe(false); // Terminal "read" status archives session (ADR-004/008) // Assert - Tome DB was still updated with rating const updatedBook = await bookRepository.findById(book.id); diff --git a/__tests__/e2e/api/status.test.ts b/__tests__/e2e/api/status.test.ts index aba39c23..80066304 100644 --- a/__tests__/e2e/api/status.test.ts +++ b/__tests__/e2e/api/status.test.ts @@ -193,7 +193,7 @@ describe("POST /api/books/[id]/status - Backward Movement with Session Archival" expect(data.sessionArchived).toBeUndefined(); // No archival for terminal states expect(data.sessionNumber).toBe(1); // Same session expect(data.status).toBe("read"); - expect(data.isActive).toBe(true); // Terminal states stay active + expect(data.isActive).toBe(false); // Terminal "read" status archives session (ADR-004/008) expect(data.completedDate).toBeDefined(); }); @@ -869,7 +869,7 @@ describe("POST /api/books/[id]/status - Rating Sync to Calibre", () => { // Assert - Status update still succeeded expect(response.status).toBe(200); expect(data.status).toBe("read"); - expect(data.isActive).toBe(true); // Terminal states stay active + expect(data.isActive).toBe(false); // Terminal "read" status archives session (ADR-004/008) // Assert - Tome DB was still updated const updatedBook = await bookRepository.findById(book.id); diff --git a/__tests__/hooks/useBookDetail.test.ts b/__tests__/hooks/useBookDetail.test.ts index 4636e134..11b289ce 100644 --- a/__tests__/hooks/useBookDetail.test.ts +++ b/__tests__/hooks/useBookDetail.test.ts @@ -385,4 +385,94 @@ describe("useBookDetail", () => { }); }); }); + + describe("updateBookPartial", () => { + test("should optimistically update book data with partial updates", async () => { + const mockBook = { + id: 123, + calibreId: 1, + title: "Test Book", + authors: ["Test Author"], + tags: ["old-tag"], + totalPages: 300, + }; + + vi.mocked(bookApi.getDetail).mockResolvedValue(mockBook); + + const { result } = renderHook(() => useBookDetail("123")); + + await waitFor(() => { + expect(result.current.loading).toBe(false); + }); + + expect(result.current.book?.title).toBe("Test Book"); + expect(result.current.book?.totalPages).toBe(300); + + // Update partial fields + act(() => { + result.current.updateBookPartial({ totalPages: 400, title: "Updated Book" }); + }); + + // Check optimistic update applied + await waitFor(() => { + expect(result.current.book?.totalPages).toBe(400); + expect(result.current.book?.title).toBe("Updated Book"); + }); + + // Other fields should remain unchanged + expect(result.current.book?.authors).toEqual(["Test Author"]); + expect(result.current.book?.tags).toEqual(["old-tag"]); + }); + + test("should handle updates when book is null", async () => { + // Set up to never resolve the book fetch + vi.mocked(bookApi.getDetail).mockImplementation(() => new Promise(() => {})); + + const { result } = renderHook(() => useBookDetail("123")); + + // Book is still loading/null + expect(result.current.book).toBeNull(); + + // updateBookPartial should not crash when book is null + act(() => { + result.current.updateBookPartial({ totalPages: 400 }); + }); + + // Book should still be null + expect(result.current.book).toBeNull(); + }); + + test("should apply multiple fields in a single partial update", async () => { + const mockBook = { + id: 123, + calibreId: 1, + title: "Test Book", + authors: ["Test Author"], + tags: [], + totalPages: 300, + }; + + vi.mocked(bookApi.getDetail).mockResolvedValue(mockBook); + + const { result } = renderHook(() => useBookDetail("123")); + + await waitFor(() => { + expect(result.current.loading).toBe(false); + }); + + // Update multiple fields at once + act(() => { + result.current.updateBookPartial({ totalPages: 350, title: "Updated Title" }); + }); + + // Both updates should be present + await waitFor(() => { + expect(result.current.book?.totalPages).toBe(350); + expect(result.current.book?.title).toBe("Updated Title"); + }); + + // Other fields should remain unchanged + expect(result.current.book?.authors).toEqual(["Test Author"]); + }); + }); }); diff --git a/__tests__/hooks/useBookStatus.test.ts b/__tests__/hooks/useBookStatus.test.ts index 19a533f2..52a3cb51 100644 --- a/__tests__/hooks/useBookStatus.test.ts +++ b/__tests__/hooks/useBookStatus.test.ts @@ -1,7 +1,9 @@ import { test, expect, describe, beforeEach, afterEach, vi } from 'vitest'; -import { renderHook, waitFor, act } from "../test-utils"; -import { useBookStatus } from "@/hooks/useBookStatus"; +import { renderHook, waitFor, act, createTestQueryClient } from "../test-utils"; +import { useBookStatus, invalidateBookQueries } from "@/hooks/useBookStatus"; import type { Book } from "@/hooks/useBookDetail"; +import { QueryClient } from '@tanstack/react-query'; +import { queryKeys } from '@/lib/query-keys'; const originalFetch = global.fetch; @@ -387,4 +389,121 @@ describe("useBookStatus", () => { expect(result.current.selectedStatus).toBe("reading"); }); }); + + describe("invalidateBookQueries", () => { + let queryClient: QueryClient; + let invalidateSpy: any; + + beforeEach(() => { + queryClient = createTestQueryClient(); + invalidateSpy = vi.spyOn(queryClient, 'invalidateQueries'); + }); + + afterEach(() => { + invalidateSpy.mockRestore(); + }); + + test("should invalidate all book-related queries", () => { + const bookId = '123'; + + invalidateBookQueries(queryClient, bookId); + + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.book.detail(123) }); + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.sessions.byBook(123) }); + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.progress.byBook(123) }); + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.dashboard.all() }); + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.library.books() }); + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.readNext.base() }); + }); + + test("should invalidate shelf caches surgically when cached shelves available", () => { + const bookId = '123'; + + // Mock cached shelves + queryClient.setQueryData(queryKeys.book.shelves(123), { + success: true, + data: [{ id: 1 }, { id: 2 }, { id: 3 }] + }); + + invalidateBookQueries(queryClient, bookId); + + // Verify surgical invalidation for each shelf + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.shelf.byId(1) }); + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.shelf.byId(2) }); + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.shelf.byId(3) }); + + // Verify nuclear invalidation was NOT called + expect(invalidateSpy).not.toHaveBeenCalledWith({ queryKey: queryKeys.shelf.base() }); + }); + + test("should invalidate all shelves when cache unavailable", () => { + const bookId = '123'; + + // No cached shelves - cache unavailable + invalidateBookQueries(queryClient, bookId); + + // Verify nuclear invalidation + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.shelf.base() }); + + // Verify no surgical (specific shelf ID) invalidations occurred + const surgicalCalls = invalidateSpy.mock.calls.filter( + (call: any) => + Array.isArray(call[0]?.queryKey) && + call[0].queryKey[0] === 'shelf' && + call[0].queryKey.length === 2 && + typeof call[0].queryKey[1] === 'number' + ); + expect(surgicalCalls).toHaveLength(0); + }); + + test("should NOT invalidate shelves when book is on no shelves", () => { + const bookId = '123'; + + // Mock cached shelves with empty array (book on zero shelves) + queryClient.setQueryData(queryKeys.book.shelves(123), { + success: true, + data: [] + }); + + invalidateBookQueries(queryClient, bookId); + + // Should NOT trigger nuclear invalidation (performance optimization) + expect(invalidateSpy).not.toHaveBeenCalledWith({ queryKey: queryKeys.shelf.base() }); + + // Should NOT invalidate any specific shelves (none exist) + const surgicalCalls = invalidateSpy.mock.calls.filter( + (call: any) => + Array.isArray(call[0]?.queryKey) && + call[0].queryKey[0] === 'shelf' && + call[0].queryKey.length === 2 + ); + expect(surgicalCalls).toHaveLength(0); + }); + + test("should handle invalid cache data gracefully", () => { + const bookId = '123'; + + // Mock invalid cache structure + queryClient.setQueryData(queryKeys.book.shelves(123), { + invalid: 'structure' + }); + + invalidateBookQueries(queryClient, bookId); + + // Verify nuclear invalidation (fallback for invalid data) + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.shelf.base() }); + }); + + test("should handle null cache data gracefully", () => { + const bookId = '123'; + + // Mock null cache + queryClient.setQueryData(queryKeys.book.shelves(123), null); + + invalidateBookQueries(queryClient, bookId); + + // Verify nuclear invalidation + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.shelf.base() }); + }); + }); }); diff --git a/__tests__/hooks/usePageTitle.test.ts b/__tests__/hooks/usePageTitle.test.ts new file mode 100644 index 00000000..c3a79609 --- /dev/null +++ b/__tests__/hooks/usePageTitle.test.ts @@ -0,0 +1,205 @@ +import { renderHook } from "@testing-library/react"; +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { usePageTitle } from "@/lib/hooks/usePageTitle"; + +describe("usePageTitle", () => { + let originalTitle: string; + + beforeEach(() => { + // Save original title + originalTitle = document.title; + }); + + afterEach(() => { + // Restore original title + document.title = originalTitle; + }); + + describe("basic functionality", () => { + it("should set document title with 'Tome - ' prefix when title is provided", () => { + renderHook(() => usePageTitle("Dashboard")); + + expect(document.title).toBe("Tome - Dashboard"); + }); + + it("should set document title to 'Tome' when title is undefined", () => { + renderHook(() => usePageTitle(undefined)); + + expect(document.title).toBe("Tome"); + }); + + it("should set document title to 'Tome' when title is empty string", () => { + renderHook(() => usePageTitle("")); + + expect(document.title).toBe("Tome"); + }); + + it("should handle complex titles with special characters", () => { + renderHook(() => usePageTitle("The Lord of the Rings by J.R.R. Tolkien")); + + expect(document.title).toBe("Tome - The Lord of the Rings by J.R.R. Tolkien"); + }); + }); + + describe("title updates", () => { + it("should update document title when title prop changes", () => { + const { rerender } = renderHook(({ title }: { title?: string }) => usePageTitle(title), { + initialProps: { title: "Dashboard" as string | undefined }, + }); + + expect(document.title).toBe("Tome - Dashboard"); + + rerender({ title: "Library" }); + + expect(document.title).toBe("Tome - Library"); + }); + + it("should update from defined to undefined", () => { + const { rerender } = renderHook(({ title }: { title?: string }) => usePageTitle(title), { + initialProps: { title: "Dashboard" as string | undefined }, + }); + + expect(document.title).toBe("Tome - Dashboard"); + + rerender({ title: undefined }); + + expect(document.title).toBe("Tome"); + }); + + it("should update from undefined to defined", () => { + const { rerender } = renderHook(({ title }: { title?: string }) => usePageTitle(title), { + initialProps: { title: undefined as string | undefined }, + }); + + expect(document.title).toBe("Tome"); + + rerender({ title: "Dashboard" }); + + expect(document.title).toBe("Tome - Dashboard"); + }); + }); + + describe("cleanup behavior", () => { + it("should reset document title to 'Tome' on unmount", () => { + const { unmount } = renderHook(() => usePageTitle("Dashboard")); + + expect(document.title).toBe("Tome - Dashboard"); + + unmount(); + + expect(document.title).toBe("Tome"); + }); + + it("should reset to 'Tome' even when title was undefined", () => { + const { unmount } = renderHook(() => usePageTitle(undefined)); + + expect(document.title).toBe("Tome"); + + unmount(); + + expect(document.title).toBe("Tome"); + }); + + it("should not interfere with subsequent renders", () => { + // First hook + const { unmount: unmount1 } = renderHook(() => usePageTitle("Dashboard")); + expect(document.title).toBe("Tome - Dashboard"); + unmount1(); + expect(document.title).toBe("Tome"); + + // Second hook + const { unmount: unmount2 } = renderHook(() => usePageTitle("Library")); + expect(document.title).toBe("Tome - Library"); + unmount2(); + expect(document.title).toBe("Tome"); + }); + }); + + describe("edge cases", () => { + it("should handle titles with 'Tome - ' already in them", () => { + renderHook(() => usePageTitle("Tome - Dashboard")); + + // Should add prefix anyway (resulting in double prefix) + // This is expected behavior - the hook doesn't check for existing prefix + expect(document.title).toBe("Tome - Tome - Dashboard"); + }); + + it("should handle very long titles", () => { + const longTitle = "A".repeat(500); + renderHook(() => usePageTitle(longTitle)); + + expect(document.title).toBe(`Tome - ${longTitle}`); + }); + + it("should handle titles with newlines and whitespace", () => { + renderHook(() => usePageTitle("Dashboard\nwith\nnewlines")); + + expect(document.title).toBe("Tome - Dashboard\nwith\nnewlines"); + }); + + it("should handle titles with unicode characters", () => { + renderHook(() => usePageTitle("📚 Dashboard 📖")); + + expect(document.title).toBe("Tome - 📚 Dashboard 📖"); + }); + }); + + describe("multiple instances", () => { + it("should handle multiple hooks updating the same document title", () => { + const { rerender: rerender1 } = renderHook( + ({ title }: { title?: string }) => usePageTitle(title), + { initialProps: { title: "Dashboard" as string | undefined } } + ); + + expect(document.title).toBe("Tome - Dashboard"); + + // Second hook overrides the first + const { rerender: rerender2 } = renderHook( + ({ title }: { title?: string }) => usePageTitle(title), + { initialProps: { title: "Library" as string | undefined } } + ); + + expect(document.title).toBe("Tome - Library"); + + // Update first hook - title gets overridden again + rerender1({ title: "Settings" }); + + // Last effect to run wins (this is expected React behavior) + expect(document.title).toBe("Tome - Settings"); + }); + }); + + describe("loading states", () => { + it("should simulate loading state (undefined -> defined)", () => { + // Simulate a component that loads data + const { rerender } = renderHook(({ title }: { title?: string }) => usePageTitle(title), { + initialProps: { title: undefined as string | undefined }, + }); + + // Initially shows "Tome" (loading) + expect(document.title).toBe("Tome"); + + // After data loads, shows full title + rerender({ title: "The Fellowship of the Ring by J.R.R. Tolkien" }); + expect(document.title).toBe("Tome - The Fellowship of the Ring by J.R.R. Tolkien"); + }); + + it("should handle error state (defined -> undefined)", () => { + const { rerender } = renderHook(({ title }: { title?: string }) => usePageTitle(title), { + initialProps: { title: "Dashboard" as string | undefined }, + }); + + expect(document.title).toBe("Tome - Dashboard"); + + // Error occurs, falls back to undefined + rerender({ title: undefined }); + expect(document.title).toBe("Tome"); + }); + }); + + // NOTE: MutationObserver tests are difficult to implement in happy-dom test environment + // because MutationObserver callbacks don't fire consistently. The MutationObserver + // in usePageTitle.ts (lines 45-50) handles React hydration edge cases where Next.js + // server-rendered titles get overwritten by client-side rendering. This is tested + // manually and works correctly in production browsers. +}); diff --git a/__tests__/hooks/useShelfBooks.test.ts b/__tests__/hooks/useShelfBooks.test.ts index 67abbb5f..125ad822 100644 --- a/__tests__/hooks/useShelfBooks.test.ts +++ b/__tests__/hooks/useShelfBooks.test.ts @@ -719,4 +719,370 @@ describe('useShelfBooks', () => { // Should not crash when previousShelf is undefined }); }); + + describe('moveToTop mutation', () => { + test('should call the correct API endpoint', async () => { + (shelfApi.get as any).mockResolvedValue(mockShelf); + global.fetch = vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({}), + }); + + const { result } = renderHook(() => useShelfBooks(1), { wrapper: createWrapper() }); + await waitFor(() => expect(result.current.shelf).toEqual(mockShelf)); + + await result.current.moveToTop(10); + + expect(global.fetch).toHaveBeenCalledWith('/api/shelves/1/books/10/move-to-top', { + method: 'POST', + }); + }); + + test('should throw when shelfId is null', async () => { + const { result } = renderHook(() => useShelfBooks(null), { wrapper: createWrapper() }); + + await expect(result.current.moveToTop(10)).rejects.toThrow('No shelf ID provided'); + }); + + test('should handle API error response', async () => { + (shelfApi.get as any).mockResolvedValue(mockShelf); + global.fetch = vi.fn().mockResolvedValue({ + ok: false, + json: async () => ({ error: { message: 'Book not found' } }), + }); + + const { toast } = await import('@/utils/toast'); + const { result } = renderHook(() => useShelfBooks(1), { wrapper: createWrapper() }); + await waitFor(() => expect(result.current.shelf).toEqual(mockShelf)); + + await expect(result.current.moveToTop(10)).rejects.toThrow('Book not found'); + expect(toast.error).toHaveBeenCalledWith(expect.stringContaining('Failed to move to top')); + }); + + test('should handle API error response without message', async () => { + (shelfApi.get as any).mockResolvedValue(mockShelf); + global.fetch = vi.fn().mockResolvedValue({ + ok: false, + json: async () => ({}), + }); + + const { result } = renderHook(() => useShelfBooks(1), { wrapper: createWrapper() }); + await waitFor(() => expect(result.current.shelf).toEqual(mockShelf)); + + await expect(result.current.moveToTop(10)).rejects.toThrow('Failed to move to top'); + }); + + test('should optimistically move book to top', async () => { + (shelfApi.get as any).mockResolvedValue(mockShelf); + global.fetch = vi.fn().mockImplementation(() => + new Promise(resolve => setTimeout(() => resolve({ ok: true, json: async () => ({}) }), 100)) + ); + + const { result } = renderHook(() => useShelfBooks(1), { wrapper: createWrapper() }); + await waitFor(() => expect(result.current.books.length).toBe(2)); + + // Book 20 should be at index 1 + expect(result.current.books[1].id).toBe(20); + + // Start mutation (don't await) + result.current.moveToTop(20); + + // Should optimistically move book 20 to top + await waitFor(() => { + expect(result.current.books[0].id).toBe(20); + expect(result.current.books[0].sortOrder).toBe(0); + expect(result.current.books[1].id).toBe(10); + expect(result.current.books[1].sortOrder).toBe(1); + }); + }); + + test('should show success toast', async () => { + (shelfApi.get as any).mockResolvedValue(mockShelf); + global.fetch = vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({}), + }); + + const { toast } = await import('@/utils/toast'); + const { result } = renderHook(() => useShelfBooks(1), { wrapper: createWrapper() }); + await waitFor(() => expect(result.current.shelf).toEqual(mockShelf)); + + await result.current.moveToTop(10); + + expect(toast.success).toHaveBeenCalledWith('Moved to top of shelf'); + }); + + test('should rollback on error', async () => { + (shelfApi.get as any).mockResolvedValue(mockShelf); + global.fetch = vi.fn().mockResolvedValue({ + ok: false, + json: async () => ({ error: { message: 'Failed' } }), + }); + + const { result } = renderHook(() => useShelfBooks(1), { wrapper: createWrapper() }); + await waitFor(() => expect(result.current.books.length).toBe(2)); + + const originalOrder = result.current.books.map(b => b.id); + + await expect(result.current.moveToTop(20)).rejects.toThrow(); + + // Should rollback to original order + await waitFor(() => { + expect(result.current.books.map(b => b.id)).toEqual(originalOrder); + }); + }); + + test('should handle book not found in shelf', async () => { + (shelfApi.get as any).mockResolvedValue(mockShelf); + global.fetch = vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({}), + }); + + const { result } = renderHook(() => useShelfBooks(1), { wrapper: createWrapper() }); + await waitFor(() => expect(result.current.shelf).toEqual(mockShelf)); + + // Try to move a book that doesn't exist + await result.current.moveToTop(999); + + // Should not crash - optimistic update just won't find the book + expect(result.current.books.length).toBe(2); + }); + + test('should track isMovingToTop state', async () => { + (shelfApi.get as any).mockResolvedValue(mockShelf); + global.fetch = vi.fn().mockImplementation(() => + new Promise(resolve => setTimeout(() => resolve({ ok: true, json: async () => ({}) }), 100)) + ); + + const { result } = renderHook(() => useShelfBooks(1), { wrapper: createWrapper() }); + await waitFor(() => expect(result.current.shelf).toEqual(mockShelf)); + + expect(result.current.isMovingToTop).toBe(false); + + // Start mutation + result.current.moveToTop(10); + + await waitFor(() => { + expect(result.current.isMovingToTop).toBe(true); + }); + + await waitFor(() => { + expect(result.current.isMovingToTop).toBe(false); + }); + }); + }); + + describe('moveToBottom mutation', () => { + test('should call the correct API endpoint', async () => { + (shelfApi.get as any).mockResolvedValue(mockShelf); + global.fetch = vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({}), + }); + + const { result } = renderHook(() => useShelfBooks(1), { wrapper: createWrapper() }); + await waitFor(() => expect(result.current.shelf).toEqual(mockShelf)); + + await result.current.moveToBottom(10); + + expect(global.fetch).toHaveBeenCalledWith('/api/shelves/1/books/10/move-to-bottom', { + method: 'POST', + }); + }); + + test('should throw when shelfId is null', async () => { + const { result } = renderHook(() => useShelfBooks(null), { wrapper: createWrapper() }); + + await expect(result.current.moveToBottom(10)).rejects.toThrow('No shelf ID provided'); + }); + + test('should handle API error response', async () => { + (shelfApi.get as any).mockResolvedValue(mockShelf); + global.fetch = vi.fn().mockResolvedValue({ + ok: false, + json: async () => ({ error: { message: 'Book not found' } }), + }); + + const { toast } = await import('@/utils/toast'); + const { result } = renderHook(() => useShelfBooks(1), { wrapper: createWrapper() }); + await waitFor(() => expect(result.current.shelf).toEqual(mockShelf)); + + await expect(result.current.moveToBottom(10)).rejects.toThrow('Book not found'); + expect(toast.error).toHaveBeenCalledWith(expect.stringContaining('Failed to move to bottom')); + }); + + test('should handle API error response without message', async () => { + (shelfApi.get as any).mockResolvedValue(mockShelf); + global.fetch = vi.fn().mockResolvedValue({ + ok: false, + json: async () => ({}), + }); + + const { result } = renderHook(() => useShelfBooks(1), { wrapper: createWrapper() }); + await waitFor(() => expect(result.current.shelf).toEqual(mockShelf)); + + await expect(result.current.moveToBottom(10)).rejects.toThrow('Failed to move to bottom'); + }); + + test('should optimistically move book to bottom', async () => { + (shelfApi.get as any).mockResolvedValue(mockShelf); + global.fetch = vi.fn().mockImplementation(() => + new Promise(resolve => setTimeout(() => resolve({ ok: true, json: async () => ({}) }), 100)) + ); + + const { result } = renderHook(() => useShelfBooks(1), { wrapper: createWrapper() }); + await waitFor(() => expect(result.current.books.length).toBe(2)); + + // Book 10 should be at index 0 + expect(result.current.books[0].id).toBe(10); + + // Start mutation (don't await) + result.current.moveToBottom(10); + + // Should optimistically move book 10 to bottom + await waitFor(() => { + expect(result.current.books[1].id).toBe(10); + expect(result.current.books[1].sortOrder).toBe(1); + expect(result.current.books[0].id).toBe(20); + }); + }); + + test('should handle sortOrder sorting with asc direction', async () => { + const shelfWithSortOrder = { + ...mockShelf, + books: [ + { ...mockShelf.books[0], sortOrder: 0 }, + { ...mockShelf.books[1], sortOrder: 1 }, + ], + }; + + (shelfApi.get as any).mockResolvedValue(shelfWithSortOrder); + global.fetch = vi.fn().mockImplementation(() => + new Promise(resolve => setTimeout(() => resolve({ ok: true, json: async () => ({}) }), 100)) + ); + + const { result } = renderHook(() => useShelfBooks(1, 'sortOrder', 'asc'), { + wrapper: createWrapper() + }); + + await waitFor(() => expect(result.current.books.length).toBe(2)); + + // Start mutation + result.current.moveToBottom(10); + + await waitFor(() => { + // Should sort by sortOrder ascending + const sortOrders = result.current.books.map(b => b.sortOrder); + expect(sortOrders[0]).toBeLessThanOrEqual(sortOrders[1]!); + }); + }); + + test('should handle sortOrder sorting with desc direction', async () => { + const shelfWithSortOrder = { + ...mockShelf, + books: [ + { ...mockShelf.books[0], sortOrder: 1 }, + { ...mockShelf.books[1], sortOrder: 0 }, + ], + }; + + (shelfApi.get as any).mockResolvedValue(shelfWithSortOrder); + global.fetch = vi.fn().mockImplementation(() => + new Promise(resolve => setTimeout(() => resolve({ ok: true, json: async () => ({}) }), 100)) + ); + + const { result } = renderHook(() => useShelfBooks(1, 'sortOrder', 'desc'), { + wrapper: createWrapper() + }); + + await waitFor(() => expect(result.current.books.length).toBe(2)); + + // Start mutation + result.current.moveToBottom(20); + + await waitFor(() => { + // Should sort by sortOrder descending + const sortOrders = result.current.books.map(b => b.sortOrder); + expect(sortOrders[0]).toBeGreaterThanOrEqual(sortOrders[1]!); + }); + }); + + test('should show success toast', async () => { + (shelfApi.get as any).mockResolvedValue(mockShelf); + global.fetch = vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({}), + }); + + const { toast } = await import('@/utils/toast'); + const { result } = renderHook(() => useShelfBooks(1), { wrapper: createWrapper() }); + await waitFor(() => expect(result.current.shelf).toEqual(mockShelf)); + + await result.current.moveToBottom(10); + + expect(toast.success).toHaveBeenCalledWith('Moved to bottom of shelf'); + }); + + test('should rollback on error', async () => { + (shelfApi.get as any).mockResolvedValue(mockShelf); + global.fetch = vi.fn().mockResolvedValue({ + ok: false, + json: async () => ({ error: { message: 'Failed' } }), + }); + + const { result } = renderHook(() => useShelfBooks(1), { wrapper: createWrapper() }); + await waitFor(() => expect(result.current.books.length).toBe(2)); + + const originalOrder = result.current.books.map(b => b.id); + + await expect(result.current.moveToBottom(10)).rejects.toThrow(); + + // Should rollback to original order + await waitFor(() => { + expect(result.current.books.map(b => b.id)).toEqual(originalOrder); + }); + }); + + test('should handle book not found in shelf', async () => { + (shelfApi.get as any).mockResolvedValue(mockShelf); + global.fetch = vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({}), + }); + + const { result } = renderHook(() => useShelfBooks(1), { wrapper: createWrapper() }); + await waitFor(() => expect(result.current.shelf).toEqual(mockShelf)); + + // Try to move a book that doesn't exist + await result.current.moveToBottom(999); + + // Should not crash - optimistic update just won't find the book + expect(result.current.books.length).toBe(2); + }); + + test('should handle books with undefined sortOrder', async () => { + const shelfWithUndefinedSort = { + ...mockShelf, + books: [ + { ...mockShelf.books[0], sortOrder: undefined }, + { ...mockShelf.books[1], sortOrder: undefined }, + ], + }; + + (shelfApi.get as any).mockResolvedValue(shelfWithUndefinedSort); + global.fetch = vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({}), + }); + + const { result } = renderHook(() => useShelfBooks(1), { wrapper: createWrapper() }); + await waitFor(() => expect(result.current.shelf).toEqual(shelfWithUndefinedSort)); + + // Should not crash when sortOrder is undefined + await result.current.moveToBottom(10); + + expect(result.current.books.length).toBe(2); + }); + }); }); diff --git a/__tests__/integration/api/read-filter-lifecycle.test.ts b/__tests__/integration/api/read-filter-lifecycle.test.ts index bf240e08..12985136 100644 --- a/__tests__/integration/api/read-filter-lifecycle.test.ts +++ b/__tests__/integration/api/read-filter-lifecycle.test.ts @@ -107,7 +107,7 @@ describe("Integration: Read Filter Lifecycle", () => { const sessions = await sessionRepository.findAllByBookId(book.id); const archivedSession = sessions.find(s => s.status === "read"); expect(archivedSession).not.toBeNull(); - expect(archivedSession?.isActive).toBe(true); // Terminal states stay active + expect(archivedSession?.isActive).toBe(false); // Terminal "read" status archives session (ADR-004/008) expect(archivedSession?.completedDate).not.toBeNull(); // Verify rating is on the book (not the session) @@ -229,18 +229,14 @@ describe("Integration: Read Filter Lifecycle", () => { // ======================================================================== // ======================================================================== - // STEP 5: Verify sessions - 1 archived, 1 active (both "read" status) + // STEP 5: Verify sessions - BOTH archived (terminal "read" state per ADR-004/008) // ======================================================================== const allSessions = await sessionRepository.findAllByBookId(book.id); const readSessions = allSessions.filter(s => s.status === "read"); expect(readSessions).toHaveLength(2); // Both completed reads const archivedReadSessions = readSessions.filter(s => s.isActive === false); - expect(archivedReadSessions).toHaveLength(1); // First read (archived when re-read started) - - const currentActiveReadSession = readSessions.find(s => s.isActive === true); - expect(currentActiveReadSession).not.toBeNull(); // Second read (currently active) - expect(archivedReadSessions).toHaveLength(1); // First read (archived when re-read started) + expect(archivedReadSessions).toHaveLength(2); // Both reads archived (terminal state per ADR-004/008) // ======================================================================== diff --git a/__tests__/integration/page-count-status-change.test.ts b/__tests__/integration/page-count-status-change.test.ts index a0938551..975a690f 100644 --- a/__tests__/integration/page-count-status-change.test.ts +++ b/__tests__/integration/page-count-status-change.test.ts @@ -145,7 +145,7 @@ describe("Integration: Page Count Update + Status Change", () => { const completedSession = await sessionRepository.findById(session.id); expect(completedSession?.status).toBe("read"); expect(completedSession?.completedDate).toBeTruthy(); - expect(completedSession?.isActive).toBe(true); // Terminal states stay active + expect(completedSession?.isActive).toBe(false); // Terminal "read" status archives session (ADR-004/008) // Assert: Rating synced to Tome DB const updatedBook = await bookRepository.findById(book.id); diff --git a/__tests__/integration/services/progress.service.test.ts b/__tests__/integration/services/progress.service.test.ts index c02d7790..2cec9f82 100644 --- a/__tests__/integration/services/progress.service.test.ts +++ b/__tests__/integration/services/progress.service.test.ts @@ -674,4 +674,331 @@ describe("ProgressService", () => { expect(result).toBe(false); }); }); + + describe("updateProgress - stable sort with same-date entries (issue #399)", () => { + test("should calculate pagesRead correctly with multiple entries on same date", async () => { + // Setup: Create 3 entries, 2 on the same date (Mar 8) and 1 on the next day (Mar 9) + // This reproduces the bug from issue #399 + const mar8 = "2026-03-08"; + const mar9 = "2026-03-09"; + + // Entry 1: Mar 8, page 43 + const entry1 = await progressRepository.create(createTestProgress({ + bookId: book1.id, + sessionId: session.id, + currentPage: 43, + progressDate: mar8, + pagesRead: 43, + })); + + // Entry 2: Mar 8, page 57 (later in the day) + // Stable sort uses ID as tiebreaker for same-date entries + const entry2 = await progressRepository.create(createTestProgress({ + bookId: book1.id, + sessionId: session.id, + currentPage: 57, + progressDate: mar8, + pagesRead: 14, // 57 - 43 + })); + + // Entry 3: Mar 9, page 122 (next day) + const entry3 = await progressRepository.create(createTestProgress({ + bookId: book1.id, + sessionId: session.id, + currentPage: 122, + progressDate: mar9, + pagesRead: 65, // 122 - 57 (should use page 57, not page 43) + })); + + // The bug: when we move entry3 to mar8, then back to mar9, + // it should still calculate 122 - 57 = 65 pages + // But without stable sort, it might use page 43 instead (122 - 43 = 79) + + // Action 1: Move entry3 from Mar 9 to Mar 8 + const updated1 = await progressService.updateProgress(entry3.id, { + progressDate: mar8, + }); + + // Should be 122 - 57 = 65 (using the most recent entry on mar8) + // OR 122 - 0 = 122 if it considers itself the first entry + // Either way, pagesRead should be calculated consistently + expect(updated1.currentPage).toBe(122); + expect(updated1.progressDate).toBe(mar8); + + // Action 2: Move entry3 back from Mar 8 to Mar 9 + const updated2 = await progressService.updateProgress(entry3.id, { + progressDate: mar9, + }); + + // Critical assertion: Should be 122 - 57 = 65 pages (idempotent operation) + // Without stable sort, it might incorrectly calculate 122 - 43 = 79 + expect(updated2.currentPage).toBe(122); + expect(updated2.progressDate).toBe(mar9); + expect(updated2.pagesRead).toBe(65); // Must use page 57, not page 43! + }); + + test("should maintain stable order when creating new entries on same date", async () => { + // Test that logProgress also respects stable sort + const today = "2026-03-10"; + + // Create two entries on the same date + await progressService.logProgress(book1.id, { + currentPage: 50, + progressDate: today, + }); + + const result = await progressService.logProgress(book1.id, { + currentPage: 100, + progressDate: today, + }); + + // Should calculate from the most recent entry (page 50) + expect(result.progressLog.pagesRead).toBe(50); // 100 - 50 + }); + + test("should use stable sort with 5+ entries on same date", async () => { + const testDate = "2026-03-11"; + const pages = [10, 25, 40, 60, 85, 100]; + + // Create multiple entries on the same date + // Stable sort uses ID as tiebreaker for same-date entries + for (const page of pages) { + await progressService.logProgress(book1.id, { + currentPage: page, + progressDate: testDate, + }); + } + + // Now create an entry the next day + const result = await progressService.logProgress(book1.id, { + currentPage: 150, + progressDate: "2026-03-12", + }); + + // Should calculate from the last entry on testDate (page 100) + expect(result.progressLog.pagesRead).toBe(50); // 150 - 100 + }); + + test("should use createdAt for ordering when same progressDate", async () => { + // This test specifically exercises the createdAt comparison path (line 360) + const date1 = "2026-03-13"; + const date2 = "2026-03-14"; + const date3 = "2026-03-15"; + + // Create first entry on date1 + const entry1 = await progressRepository.create(createTestProgress({ + bookId: book1.id, + sessionId: session.id, + currentPage: 30, + progressDate: date1, + pagesRead: 30, + })); + + // Create second entry on date1 (same date, higher ID due to insertion order) + const entry2 = await progressRepository.create(createTestProgress({ + bookId: book1.id, + sessionId: session.id, + currentPage: 50, + progressDate: date1, + pagesRead: 20, + })); + + // Create entry on date2 + const entry3 = await progressRepository.create(createTestProgress({ + bookId: book1.id, + sessionId: session.id, + currentPage: 80, + progressDate: date2, + pagesRead: 30, + })); + + // Create entry on date3 + const entry4 = await progressRepository.create(createTestProgress({ + bookId: book1.id, + sessionId: session.id, + currentPage: 120, + progressDate: date3, + pagesRead: 40, + })); + + // Now update entry4 back to date2 + // This exercises the createdAt sorting when querying for "previous" entry + // Previous should be entry3 (last entry before date2 is from date1, which is entry2) + // Wait, previous means p.progressDate < requestedDateString + // So for date2, previous is last entry from date1, which should be entry2 (page 50) + // Actually entry3 is ON date2, so when moving entry4 to date2, previous is entry2 (page 50) + // But wait, we're finding entries with progressDate < date2, so we get date1 entries + // The stable sort ensures entry2 (with higher createdAt/ID) comes after entry1 + // So findLast will get entry2 as the previous + const updated = await progressService.updateProgress(entry4.id, { + progressDate: date2, + currentPage: 120, + }); + + // Should calculate from the LAST entry before date2, which is entry2 on date1 (page 50) + // 120 - 50 = 70 + expect(updated.pagesRead).toBe(70); + }); + + test("should fall back to ID when progressDate and createdAt are equal", async () => { + // Edge case: Same progressDate and same createdAt (exercises line 365) + const testDate = "2026-03-15"; + + // Create entries with same timestamp (simulating batch inserts) + const now = new Date(); + const entry1 = await progressRepository.create(createTestProgress({ + bookId: book1.id, + sessionId: session.id, + currentPage: 20, + progressDate: testDate, + pagesRead: 20, + })); + + const entry2 = await progressRepository.create(createTestProgress({ + bookId: book1.id, + sessionId: session.id, + currentPage: 45, + progressDate: testDate, + pagesRead: 25, + })); + + // Create entry the next day + const entry3 = await progressRepository.create(createTestProgress({ + bookId: book1.id, + sessionId: session.id, + currentPage: 90, + progressDate: "2026-03-16", + pagesRead: 45, + })); + + // Update entry3 back to testDate to force sort logic + const updated = await progressService.updateProgress(entry3.id, { + progressDate: testDate, + }); + + // Should use entry2 as previous (highest ID on same date) + // When updating entry3 to testDate, it should calculate: 90 - 45 = 45 + expect(updated.currentPage).toBe(90); + expect(updated.pagesRead).toBeGreaterThanOrEqual(0); // Valid calculation + }); + + test("should handle entries with identical timestamps but different IDs", async () => { + // This test ensures ID is used as final tiebreaker (line 365) + const date1 = "2026-03-17"; + const date2 = "2026-03-18"; + const date3 = "2026-03-19"; + + // Create 3 entries on date1 in succession + // Stable sort uses ID as tiebreaker when timestamps are identical + const entry1 = await progressRepository.create(createTestProgress({ + bookId: book1.id, + sessionId: session.id, + currentPage: 15, + progressDate: date1, + pagesRead: 15, + })); + + const entry2 = await progressRepository.create(createTestProgress({ + bookId: book1.id, + sessionId: session.id, + currentPage: 35, + progressDate: date1, + pagesRead: 20, + })); + + const entry3 = await progressRepository.create(createTestProgress({ + bookId: book1.id, + sessionId: session.id, + currentPage: 60, + progressDate: date1, + pagesRead: 25, + })); + + // Create an entry on date2 + const entry4 = await progressRepository.create(createTestProgress({ + bookId: book1.id, + sessionId: session.id, + currentPage: 85, + progressDate: date2, + pagesRead: 25, + })); + + // Create an entry on date3 + const entry5 = await progressRepository.create(createTestProgress({ + bookId: book1.id, + sessionId: session.id, + currentPage: 110, + progressDate: date3, + pagesRead: 25, + })); + + // Update entry5 back to date2 + // Previous entry should be entry4 from date2? No, previous is < date2 + // So previous is the last entry from date1, which should be entry3 (highest ID on date1) + // This tests that the stable sort (with ID tiebreaker) ensures entry3 comes last + const updated = await progressService.updateProgress(entry5.id, { + progressDate: date2, + currentPage: 110, + }); + + // Previous is entry3 (page 60), so 110 - 60 = 50 + expect(updated.currentPage).toBe(110); + expect(updated.pagesRead).toBe(50); + }); + + test("should handle update with same progressDate as existing entries", async () => { + // Test updating an entry to have the same date as other entries + const date1 = "2026-03-20"; + const date2 = "2026-03-21"; + const date3 = "2026-03-22"; + + // Create entry on date1 + const entry1 = await progressRepository.create(createTestProgress({ + bookId: book1.id, + sessionId: session.id, + currentPage: 40, + progressDate: date1, + pagesRead: 40, + })); + + // Create another entry on date1 (same date, higher ID) + const entry2 = await progressRepository.create(createTestProgress({ + bookId: book1.id, + sessionId: session.id, + currentPage: 50, + progressDate: date1, + pagesRead: 10, + })); + + // Create entry on date2 + const entry3 = await progressRepository.create(createTestProgress({ + bookId: book1.id, + sessionId: session.id, + currentPage: 75, + progressDate: date2, + pagesRead: 25, + })); + + // Create another entry on date3 + const entry4 = await progressRepository.create(createTestProgress({ + bookId: book1.id, + sessionId: session.id, + currentPage: 100, + progressDate: date3, + pagesRead: 25, + })); + + // Update entry4 back to date2 - this exercises the sort with mixed dates + // When updating to date2, previous entry is < date2, so it's the last entry from date1 + // That should be entry2 (page 50) because of stable sort (higher createdAt/ID) + const updated = await progressService.updateProgress(entry4.id, { + progressDate: date2, + currentPage: 100, + }); + + // Previous is entry2 (page 50), so 100 - 50 = 50 + expect(updated.progressDate).toBe(date2); + expect(updated.pagesRead).toBe(50); + }); + }); }); diff --git a/__tests__/integration/services/sessions/session/core.test.ts b/__tests__/integration/services/sessions/session/core.test.ts index 1b1a96dd..af6bf77e 100644 --- a/__tests__/integration/services/sessions/session/core.test.ts +++ b/__tests__/integration/services/sessions/session/core.test.ts @@ -216,7 +216,7 @@ describe("SessionService", () => { expect(result.session.status).toBe("read"); expect(result.session.completedDate).toBeDefined(); - expect(result.session.isActive).toBe(true); // Kept active for terminal state + expect(result.session.isActive).toBe(false); // Terminal "read" status archives session (ADR-004/008) }); test("should use custom completedDate", async () => { @@ -331,6 +331,137 @@ describe("SessionService", () => { }); }); + describe("updateStatus - DNF transitions", () => { + test("should archive DNF session when transitioning to read-next", async () => { + // ARRANGE: Book with DNF session + const dnfSession = await sessionRepository.create(createTestSession({ + bookId: book1.id, + sessionNumber: 1, + status: "dnf", + isActive: false, + startedDate: "2026-01-01", + completedDate: "2026-01-15", + })); + + // ACT: Transition to read-next + const result = await sessionService.updateStatus(book1.id, { + status: "read-next", + }); + + // ASSERT: New session created + expect(result.session.id).not.toBe(dnfSession.id); + expect(result.session.sessionNumber).toBe(2); + expect(result.session.status).toBe("read-next"); + expect(result.session.isActive).toBe(true); + expect(result.sessionArchived).toBe(true); + expect(result.archivedSessionNumber).toBe(1); + + // Verify DNF session preserved + const archived = await sessionRepository.findById(dnfSession.id); + expect(archived?.status).toBe("dnf"); + expect(archived?.completedDate).toBe("2026-01-15"); + expect(archived?.isActive).toBe(false); + }); + + test("should archive DNF session when transitioning to to-read", async () => { + // ARRANGE: Book with DNF session + const dnfSession = await sessionRepository.create(createTestSession({ + bookId: book1.id, + sessionNumber: 1, + status: "dnf", + isActive: false, + startedDate: "2026-01-01", + completedDate: "2026-01-15", + })); + + // ACT: Transition to to-read + const result = await sessionService.updateStatus(book1.id, { + status: "to-read", + }); + + // ASSERT: New session created + expect(result.session.id).not.toBe(dnfSession.id); + expect(result.session.sessionNumber).toBe(2); + expect(result.session.status).toBe("to-read"); + expect(result.session.isActive).toBe(true); + expect(result.sessionArchived).toBe(true); + expect(result.archivedSessionNumber).toBe(1); + + // Verify DNF session preserved + const archived = await sessionRepository.findById(dnfSession.id); + expect(archived?.status).toBe("dnf"); + expect(archived?.isActive).toBe(false); + }); + + test("should archive DNF session when transitioning to reading (like re-read)", async () => { + // ARRANGE: Book with DNF session + const dnfSession = await sessionRepository.create(createTestSession({ + bookId: book1.id, + sessionNumber: 1, + status: "dnf", + isActive: false, + startedDate: "2026-01-01", + completedDate: "2026-01-10", + })); + + // ACT: Transition to reading + const result = await sessionService.updateStatus(book1.id, { + status: "reading", + }); + + // ASSERT: New session created (like re-read) + expect(result.session.id).not.toBe(dnfSession.id); + expect(result.session.sessionNumber).toBe(2); + expect(result.session.status).toBe("reading"); + expect(result.session.isActive).toBe(true); + expect(result.session.startedDate).toBeTruthy(); // Auto-set today + expect(result.sessionArchived).toBe(true); + + // Verify DNF session preserved + const archived = await sessionRepository.findById(dnfSession.id); + expect(archived?.status).toBe("dnf"); + expect(archived?.isActive).toBe(false); + }); + + test("should throw error when transitioning DNF to read directly", async () => { + // ARRANGE: Book with DNF session + await sessionRepository.create(createTestSession({ + bookId: book1.id, + sessionNumber: 1, + status: "dnf", + isActive: false, + startedDate: "2026-01-01", + completedDate: "2026-01-15", + })); + + // ACT & ASSERT: Should throw validation error + await expect( + sessionService.updateStatus(book1.id, { status: "read" }) + ).rejects.toThrow("Cannot mark DNF book as read directly"); + }); + + test("should preserve DNF completedDate when archiving", async () => { + // ARRANGE: Book with DNF session with specific completedDate + const originalCompletedDate = "2026-01-20"; + const dnfSession = await sessionRepository.create(createTestSession({ + bookId: book1.id, + sessionNumber: 1, + status: "dnf", + isActive: false, + startedDate: "2026-01-10", + completedDate: originalCompletedDate, + })); + + // ACT: Transition to read-next + await sessionService.updateStatus(book1.id, { status: "read-next" }); + + // ASSERT: Original completedDate preserved + const archived = await sessionRepository.findById(dnfSession.id); + expect(archived?.completedDate).toBe(originalCompletedDate); + expect(archived?.status).toBe("dnf"); + }); + }); + describe("updateStatus - with rating", () => { test("should update book rating when provided", async () => { await sessionRepository.create(createTestSession({ @@ -485,4 +616,140 @@ describe("SessionService", () => { ).rejects.toThrow("Invalid status"); }); }); + + describe("terminal state behavior", () => { + test("should set isActive=false when marking session as DNF", async () => { + // ARRANGE: Active reading session with progress + const session = await sessionRepository.create(createTestSession({ + bookId: book1.id, + sessionNumber: 1, + status: "reading", + isActive: true, + startedDate: "2026-01-01", + })); + + await progressRepository.create({ + bookId: book1.id, + sessionId: session.id, + currentPage: 50, + currentPercentage: 50, + progressDate: "2026-01-10", + }); + + // ACT: Mark as DNF + await sessionService.markAsDNF({ + bookId: book1.id, + completedDate: "2026-01-15", + }); + + // ASSERT: Session should be archived (isActive=false) + const updated = await sessionRepository.findById(session.id); + expect(updated?.status).toBe("dnf"); + expect(updated?.isActive).toBe(false); + expect(updated?.completedDate).toBe("2026-01-15"); + }); + + test("should set isActive=false when transitioning to 'read' status", async () => { + // ARRANGE: Active reading session + const session = await sessionRepository.create(createTestSession({ + bookId: book1.id, + sessionNumber: 1, + status: "reading", + isActive: true, + startedDate: "2026-01-01", + })); + + // ACT: Mark as read + await sessionService.updateStatus(book1.id, { + status: "read", + completedDate: "2026-01-15", + }); + + // ASSERT: Session should be archived (isActive=false) + const updated = await sessionRepository.findById(session.id); + expect(updated?.status).toBe("read"); + expect(updated?.isActive).toBe(false); + expect(updated?.completedDate).toBe("2026-01-15"); + }); + + test("should handle DNF → read-next with archived DNF session and proper readNextOrder", async () => { + // ARRANGE: Archived DNF session (correct state after terminal state changes) + const dnfSession = await sessionRepository.create(createTestSession({ + bookId: book1.id, + sessionNumber: 1, + status: "dnf", + isActive: false, + startedDate: "2026-01-01", + completedDate: "2026-01-15", + })); + + // ACT: Transition to read-next + const result = await sessionService.updateStatus(book1.id, { + status: "read-next", + }); + + // ASSERT: New session created with proper ordering + expect(result.sessionArchived).toBe(true); + expect(result.session.sessionNumber).toBe(2); + expect(result.session.status).toBe("read-next"); + expect(result.session.readNextOrder).toBeGreaterThanOrEqual(0); // Verifies readNextOrder is assigned (0 for first read-next book) + expect(result.session.isActive).toBe(true); + + // Verify DNF session still archived + const archived = await sessionRepository.findById(dnfSession.id); + expect(archived?.isActive).toBe(false); + }); + + test("should handle DNF → reading with archived DNF session", async () => { + // ARRANGE: Archived DNF session + const dnfSession = await sessionRepository.create(createTestSession({ + bookId: book1.id, + sessionNumber: 1, + status: "dnf", + isActive: false, + startedDate: "2026-01-01", + completedDate: "2026-01-15", + })); + + // ACT: Restart reading + const result = await sessionService.updateStatus(book1.id, { + status: "reading", + }); + + // ASSERT: New session created + expect(result.sessionArchived).toBe(true); + expect(result.session.sessionNumber).toBe(2); + expect(result.session.status).toBe("reading"); + expect(result.session.isActive).toBe(true); + expect(result.session.startedDate).toBeDefined(); + + // Verify DNF session still archived + const archived = await sessionRepository.findById(dnfSession.id); + expect(archived?.isActive).toBe(false); + }); + + test("should handle read → reading transition (re-read)", async () => { + // ARRANGE: Archived read session + const readSession = await sessionRepository.create(createTestSession({ + bookId: book1.id, + sessionNumber: 1, + status: "read", + isActive: false, + startedDate: "2026-01-01", + completedDate: "2026-01-20", + })); + + // ACT: Start re-read + const result = await sessionService.startReread(book1.id); + + // ASSERT: New reading session created + expect(result.sessionNumber).toBe(2); + expect(result.status).toBe("reading"); + expect(result.isActive).toBe(true); + + // Verify original read session still archived + const archived = await sessionRepository.findById(readSession.id); + expect(archived?.isActive).toBe(false); + }); + }); }); diff --git a/__tests__/integration/services/sessions/session/mark-as-read.strategies.test.ts b/__tests__/integration/services/sessions/session/mark-as-read.strategies.test.ts index a9cecc54..36199269 100644 --- a/__tests__/integration/services/sessions/session/mark-as-read.strategies.test.ts +++ b/__tests__/integration/services/sessions/session/mark-as-read.strategies.test.ts @@ -233,7 +233,7 @@ describe("SessionService - Mark as Read Strategies", () => { // Verify session was updated to "read" const updatedSession = await sessionRepository.findById(result.sessionId!); expect(updatedSession?.status).toBe("read"); - expect(updatedSession?.isActive).toBe(true); // Terminal state stays active + expect(updatedSession?.isActive).toBe(false); // Terminal "read" status archives session (ADR-004/008) }); test("uses provided completedDate", async () => { diff --git a/__tests__/lib/query-keys.test.ts b/__tests__/lib/query-keys.test.ts new file mode 100644 index 00000000..18b5e3bf --- /dev/null +++ b/__tests__/lib/query-keys.test.ts @@ -0,0 +1,324 @@ +import { describe, test, expect } from 'vitest'; +import { queryKeys, type AnalyticsDays } from '@/lib/query-keys'; + +describe('queryKeys', () => { + describe('book keys', () => { + test('should generate base key for all books', () => { + expect(queryKeys.book.base()).toEqual(['book']); + }); + + test('should generate detail key with book ID', () => { + expect(queryKeys.book.detail(123)).toEqual(['book', 123]); + }); + + test('should generate available tags key', () => { + expect(queryKeys.book.availableTags()).toEqual(['availableTags']); + }); + + test('should generate available shelves key', () => { + expect(queryKeys.book.availableShelves()).toEqual(['availableShelves']); + }); + + test('should generate book shelves key with book ID', () => { + expect(queryKeys.book.shelves(456)).toEqual(['bookShelves', 456]); + }); + }); + + describe('library keys', () => { + test('should generate library books key', () => { + expect(queryKeys.library.books()).toEqual(['library-books']); + }); + }); + + describe('sessions keys', () => { + test('should generate sessions by book key', () => { + expect(queryKeys.sessions.byBook(123)).toEqual(['sessions', 123]); + }); + + test('should generate session progress key', () => { + expect(queryKeys.sessions.progress(789)).toEqual(['session-progress', 789]); + }); + }); + + describe('progress keys', () => { + test('should generate progress by book key', () => { + expect(queryKeys.progress.byBook(123)).toEqual(['progress', 123]); + }); + }); + + describe('streak keys', () => { + test('should generate base streak key', () => { + expect(queryKeys.streak.base()).toEqual(['streak']); + }); + + test('should generate streak settings key', () => { + expect(queryKeys.streak.settings()).toEqual(['streak', 'settings']); + }); + + test('should generate streak analytics key with numeric days', () => { + expect(queryKeys.streak.analytics(7)).toEqual(['streak', 'analytics', 7]); + expect(queryKeys.streak.analytics(30)).toEqual(['streak', 'analytics', 30]); + expect(queryKeys.streak.analytics(90)).toEqual(['streak', 'analytics', 90]); + expect(queryKeys.streak.analytics(180)).toEqual(['streak', 'analytics', 180]); + expect(queryKeys.streak.analytics(365)).toEqual(['streak', 'analytics', 365]); + }); + + test('should generate streak analytics key with string periods', () => { + expect(queryKeys.streak.analytics('this-year')).toEqual(['streak', 'analytics', 'this-year']); + expect(queryKeys.streak.analytics('all-time')).toEqual(['streak', 'analytics', 'all-time']); + }); + + test('should generate streak heatmap key', () => { + expect(queryKeys.streak.heatmap(7)).toEqual(['streak', 'analytics', 'heatmap', 7]); + expect(queryKeys.streak.heatmap(30)).toEqual(['streak', 'analytics', 'heatmap', 30]); + expect(queryKeys.streak.heatmap(90)).toEqual(['streak', 'analytics', 'heatmap', 90]); + expect(queryKeys.streak.heatmap(180)).toEqual(['streak', 'analytics', 'heatmap', 180]); + expect(queryKeys.streak.heatmap(365)).toEqual(['streak', 'analytics', 'heatmap', 365]); + }); + }); + + describe('goals keys', () => { + test('should generate base goals key', () => { + expect(queryKeys.goals.base()).toEqual(['goals']); + }); + + test('should generate reading goal by year key', () => { + expect(queryKeys.goals.byYear(2024)).toEqual(['reading-goal', 2024]); + }); + + test('should generate monthly breakdown key', () => { + expect(queryKeys.goals.monthlyBreakdown(2024)).toEqual(['monthly-breakdown', 2024]); + }); + + test('should generate completed books key', () => { + expect(queryKeys.goals.completedBooks(2024)).toEqual(['completed-books', 2024]); + }); + }); + + describe('shelf keys', () => { + test('should generate base shelf key', () => { + expect(queryKeys.shelf.base()).toEqual(['shelf']); + }); + + test('should generate shelf by ID key', () => { + expect(queryKeys.shelf.byId(42)).toEqual(['shelf', 42]); + }); + + test('should generate shelf detail with default options', () => { + expect(queryKeys.shelf.detail(42, {})).toEqual([ + 'shelf', + 42, + 'books', + {}, + ]); + }); + + test('should generate shelf detail with sorting options', () => { + expect(queryKeys.shelf.detail(42, { orderBy: 'title', direction: 'asc' })).toEqual([ + 'shelf', + 42, + 'books', + { orderBy: 'title', direction: 'asc' }, + ]); + + expect(queryKeys.shelf.detail(42, { orderBy: 'authors', direction: 'desc' })).toEqual([ + 'shelf', + 42, + 'books', + { orderBy: 'authors', direction: 'desc' }, + ]); + }); + }); + + describe('series keys', () => { + test('should generate all series key', () => { + expect(queryKeys.series.all()).toEqual(['series']); + }); + + test('should generate series detail by name', () => { + expect(queryKeys.series.detail('The Lord of the Rings')).toEqual([ + 'series', + 'The Lord of the Rings', + ]); + }); + }); + + describe('readNext keys', () => { + test('should generate base read next key', () => { + expect(queryKeys.readNext.base()).toEqual(['read-next-books']); + }); + + test('should generate read next books key without search', () => { + expect(queryKeys.readNext.books()).toEqual(['read-next-books', undefined]); + }); + + test('should generate read next books key with search', () => { + expect(queryKeys.readNext.books('tolkien')).toEqual(['read-next-books', 'tolkien']); + }); + }); + + describe('journal keys', () => { + test('should generate journal entries key with timezone', () => { + expect(queryKeys.journal.entries('America/New_York')).toEqual([ + 'journal-entries', + 'America/New_York', + ]); + }); + + test('should generate journal archive key with timezone', () => { + expect(queryKeys.journal.archive('Europe/London')).toEqual([ + 'journal-archive', + 'Europe/London', + ]); + }); + }); + + describe('tags keys', () => { + test('should generate base tags key', () => { + expect(queryKeys.tags.base()).toEqual(['tags']); + }); + + test('should generate tag books key', () => { + expect(queryKeys.tags.books(123)).toEqual(['tag-books', 123]); + }); + }); + + describe('dashboard keys', () => { + test('should generate dashboard all key', () => { + expect(queryKeys.dashboard.all()).toEqual(['dashboard']); + }); + }); + + describe('stats keys', () => { + test('should generate stats all key', () => { + expect(queryKeys.stats.all()).toEqual(['stats']); + }); + }); + + describe('version keys', () => { + test('should generate version info key', () => { + expect(queryKeys.version.info()).toEqual(['version']); + }); + }); + + describe('type safety', () => { + test('should maintain const types for all keys', () => { + // Ensure keys are readonly arrays (const assertions) + const bookBase = queryKeys.book.base(); + const bookDetail = queryKeys.book.detail(1); + const streakAnalytics = queryKeys.streak.analytics(7); + + // TypeScript will enforce these are readonly at compile time + // At runtime, we can verify they're arrays + expect(Array.isArray(bookBase)).toBe(true); + expect(Array.isArray(bookDetail)).toBe(true); + expect(Array.isArray(streakAnalytics)).toBe(true); + }); + + test('should handle all valid AnalyticsDays values', () => { + const validDays: AnalyticsDays[] = [7, 30, 90, 180, 365, 'this-year', 'all-time']; + + validDays.forEach((days) => { + const key = queryKeys.streak.analytics(days); + expect(key).toEqual(['streak', 'analytics', days]); + }); + }); + }); + + describe('key collision prevention', () => { + test('should generate unique keys for different entities', () => { + // Book detail vs sessions by book should be different + expect(queryKeys.book.detail(123)).not.toEqual(queryKeys.sessions.byBook(123)); + + // Streak analytics vs heatmap should be different + expect(queryKeys.streak.analytics(7)).not.toEqual(queryKeys.streak.heatmap(7)); + + // Different search terms should generate different keys + expect(queryKeys.readNext.books('search1')).not.toEqual( + queryKeys.readNext.books('search2') + ); + }); + + test('should allow wildcard invalidation via base keys', () => { + // Base keys should be prefixes of specific keys + const bookBase = queryKeys.book.base(); + const bookDetail = queryKeys.book.detail(123); + + expect(bookDetail[0]).toBe(bookBase[0]); + + const streakBase = queryKeys.streak.base(); + const streakAnalytics = queryKeys.streak.analytics(7); + + expect(streakAnalytics[0]).toBe(streakBase[0]); + }); + }); + + describe('hierarchical structure', () => { + test('should maintain consistent hierarchy for book queries', () => { + // All book queries should start with 'book' + expect(queryKeys.book.base()[0]).toBe('book'); + expect(queryKeys.book.detail(1)[0]).toBe('book'); + }); + + test('should maintain consistent hierarchy for streak queries', () => { + // All streak queries should start with 'streak' + expect(queryKeys.streak.base()[0]).toBe('streak'); + expect(queryKeys.streak.settings()[0]).toBe('streak'); + expect(queryKeys.streak.analytics(7)[0]).toBe('streak'); + expect(queryKeys.streak.heatmap(7)[0]).toBe('streak'); + }); + + test('should maintain consistent hierarchy for shelf queries', () => { + // All shelf queries should start with 'shelf' + expect(queryKeys.shelf.base()[0]).toBe('shelf'); + expect(queryKeys.shelf.byId(1)[0]).toBe('shelf'); + expect(queryKeys.shelf.detail(1, {})[0]).toBe('shelf'); + }); + + test('should differentiate between analytics and heatmap queries', () => { + const analytics = queryKeys.streak.analytics(7); + const heatmap = queryKeys.streak.heatmap(7); + + // Both should start with 'streak' and 'analytics' + expect(analytics[0]).toBe('streak'); + expect(analytics[1]).toBe('analytics'); + + expect(heatmap[0]).toBe('streak'); + expect(heatmap[1]).toBe('analytics'); + + // But heatmap should have 'heatmap' as third element + expect(heatmap[2]).toBe('heatmap'); + }); + }); + + describe('edge cases', () => { + test('should handle zero as valid ID', () => { + expect(queryKeys.book.detail(0)).toEqual(['book', 0]); + expect(queryKeys.shelf.byId(0)).toEqual(['shelf', 0]); + }); + + test('should handle negative IDs', () => { + expect(queryKeys.book.detail(-1)).toEqual(['book', -1]); + expect(queryKeys.sessions.byBook(-999)).toEqual(['sessions', -999]); + }); + + test('should handle empty strings', () => { + expect(queryKeys.series.detail('')).toEqual(['series', '']); + expect(queryKeys.readNext.books('')).toEqual(['read-next-books', '']); + }); + + test('should handle special characters in series names', () => { + expect(queryKeys.series.detail('The Hitchhiker\'s Guide')).toEqual([ + 'series', + 'The Hitchhiker\'s Guide', + ]); + + expect(queryKeys.series.detail('Series (2024)')).toEqual(['series', 'Series (2024)']); + }); + + test('should handle large year values', () => { + expect(queryKeys.goals.byYear(9999)).toEqual(['reading-goal', 9999]); + expect(queryKeys.goals.monthlyBreakdown(2100)).toEqual(['monthly-breakdown', 2100]); + }); + }); +}); diff --git a/app/api/books/[id]/status/route.ts b/app/api/books/[id]/status/route.ts index d69127c9..095fcc5f 100644 --- a/app/api/books/[id]/status/route.ts +++ b/app/api/books/[id]/status/route.ts @@ -1,6 +1,6 @@ import { getLogger } from "@/lib/logger"; import { NextRequest, NextResponse } from "next/server"; -import { SessionService } from "@/lib/services/session.service"; +import { SessionService, SessionValidationError } from "@/lib/services/session.service"; import { validateDateString } from "@/lib/utils/date-validation"; export const dynamic = 'force-dynamic'; @@ -66,8 +66,6 @@ export async function POST(request: NextRequest, props: { params: Promise<{ id: const result = await sessionService.updateStatus(bookId, statusData); - // Note: Cache invalidation handled by SessionService.invalidateCache() - // Return full result if session was archived, otherwise just the session if (result.sessionArchived) { return NextResponse.json({ @@ -81,17 +79,16 @@ export async function POST(request: NextRequest, props: { params: Promise<{ id: } catch (error) { getLogger().error({ err: error }, "Error updating status"); - // Handle specific errors + // Handle SessionValidationError with typed error codes + if (error instanceof SessionValidationError) { + return NextResponse.json({ + error: error.message, + code: error.code + }, { status: error.httpStatus }); + } + + // Handle other specific errors if (error instanceof Error) { - // Check for error code first - const errorWithCode = error as any; - if (errorWithCode.code === "PAGES_REQUIRED") { - return NextResponse.json({ - error: error.message, - code: "PAGES_REQUIRED" - }, { status: 400 }); - } - if (error.message.includes("not found")) { return NextResponse.json({ error: error.message }, { status: 404 }); } diff --git a/app/books/[id]/page.tsx b/app/books/[id]/page.tsx index c7a12ff4..aecccd4d 100644 --- a/app/books/[id]/page.tsx +++ b/app/books/[id]/page.tsx @@ -1,10 +1,11 @@ "use client"; import { useEffect, useState, useRef } from "react"; -import { useParams } from "next/navigation"; +import { useParams, useRouter } from "next/navigation"; import { useQueryClient, useQuery } from "@tanstack/react-query"; +import { queryKeys } from "@/lib/query-keys"; import Link from "next/link"; -import { BookOpen, BookCheck, Pencil } from "lucide-react"; +import { BookOpen, BookCheck, Pencil, ArrowLeft } from "lucide-react"; import { getShelfIcon } from "@/components/ShelfManagement/ShelfIconPicker"; import { ShelfAvatar } from "@/components/ShelfManagement/ShelfAvatar"; import ReadingHistoryTab from "@/components/CurrentlyReading/ReadingHistoryTab"; @@ -34,6 +35,7 @@ import { useBookRating } from "@/hooks/useBookRating"; import { useSessionDetails } from "@/hooks/useSessionDetails"; import { useDraftNote } from "@/hooks/useDraftNote"; import { Spinner } from "@/components/Utilities/Spinner"; +import { usePageTitle } from "@/lib/hooks/usePageTitle"; const logger = getLogger().child({ component: "BookDetailPage" }); @@ -41,6 +43,7 @@ export default function BookDetailPage() { const params = useParams(); const bookId = params?.id as string; const queryClient = useQueryClient(); + const router = useRouter(); // Custom hooks encapsulate all business logic const { @@ -53,7 +56,7 @@ export default function BookDetailPage() { const bookProgressHook = useBookProgress(bookId, book, async () => { // Invalidate relevant queries to refetch fresh data - await queryClient.invalidateQueries({ queryKey: ['book', bookId] }); + await queryClient.invalidateQueries({ queryKey: queryKeys.book.detail(parseInt(bookId)) }); }); const { @@ -72,6 +75,12 @@ export default function BookDetailPage() { handleMarkAsDNF, } = useBookStatus(book, bookProgressHook.progress, bookId); + // Set dynamic page title + const bookTitle = book + ? `${book.title}${book.authors.length > 0 ? ` by ${book.authors.join(", ")}` : ""}` + : undefined; + usePageTitle(bookTitle); + // Handle finishing book from auto-completion modal (when progress reaches 100%) // Note: Book status is already "read" at this point (auto-completed by progress service) // We only need to update rating/review, not status @@ -112,10 +121,10 @@ export default function BookDetailPage() { clearDraft(); // Refresh data - await queryClient.invalidateQueries({ queryKey: ['dashboard'] }); - await queryClient.invalidateQueries({ queryKey: ['book', bookId] }); - await queryClient.invalidateQueries({ queryKey: ['sessions', bookId] }); - await queryClient.invalidateQueries({ queryKey: ['library-books'] }); + await queryClient.invalidateQueries({ queryKey: queryKeys.dashboard.all() }); + await queryClient.invalidateQueries({ queryKey: queryKeys.book.detail(parseInt(bookId)) }); + await queryClient.invalidateQueries({ queryKey: queryKeys.sessions.byBook(parseInt(bookId)) }); + await queryClient.invalidateQueries({ queryKey: queryKeys.library.books() }); } catch (error) { logger.error({ error }, "Failed to finish book"); throw error; @@ -131,7 +140,7 @@ export default function BookDetailPage() { const sessionDetailsHook = useSessionDetails(bookId, book?.activeSession, async () => { // Invalidate relevant queries to refetch fresh data - await queryClient.invalidateQueries({ queryKey: ['book', bookId] }); + await queryClient.invalidateQueries({ queryKey: queryKeys.book.detail(parseInt(bookId)) }); }); // Draft note management with localStorage autosave @@ -172,9 +181,22 @@ export default function BookDetailPage() { return () => window.removeEventListener('resize', checkMobile); }, []); + // Back button logic: Show on mobile when user has navigation history + const [hasHistory, setHasHistory] = useState(false); + + useEffect(() => { + setHasHistory(window.history.length > 1); + }, []); + + const shouldShowBackButton = isMobile && hasHistory; + + const handleBack = () => { + router.back(); + }; + // Fetch available tags for the tag editor const { data: availableTagsData } = useQuery<{ tags: string[] }>({ - queryKey: ['availableTags'], + queryKey: queryKeys.book.availableTags(), queryFn: async () => { const response = await fetch('/api/tags'); if (!response.ok) { @@ -191,7 +213,7 @@ export default function BookDetailPage() { // Fetch available shelves for the shelf editor const { data: availableShelvesData } = useQuery<{ success: boolean; data: Array<{ id: number; name: string; description: string | null; color: string | null; icon: string | null }> }>({ - queryKey: ['availableShelves'], + queryKey: queryKeys.book.availableShelves(), queryFn: async () => { const response = await fetch('/api/shelves'); if (!response.ok) { @@ -205,7 +227,7 @@ export default function BookDetailPage() { // Fetch current shelves for this book const { data: bookShelvesData, refetch: refetchBookShelves } = useQuery<{ success: boolean; data: Array<{ id: number; name: string; description: string | null; color: string | null; icon: string | null }> }>({ - queryKey: ['bookShelves', bookId], + queryKey: queryKeys.book.shelves(parseInt(bookId)), queryFn: async () => { const response = await fetch(`/api/books/${bookId}/shelves`); if (!response.ok) { @@ -233,9 +255,9 @@ export default function BookDetailPage() { const affectedShelfIds = [...new Set([...addedShelfIds, ...removedShelfIds])]; // Invalidate all affected shelf queries - // Use broad invalidation without orderBy/direction to catch all variants + // Use byId() to invalidate all variants (different orderBy/direction) affectedShelfIds.forEach(shelfId => { - queryClient.invalidateQueries({ queryKey: ['shelf', shelfId] }); + queryClient.invalidateQueries({ queryKey: queryKeys.shelf.byId(shelfId) }); }); // Refetch book's shelf data @@ -388,6 +410,19 @@ export default function BookDetailPage() { return ( <div className="max-w-6xl mx-auto"> + {/* Mobile back button - only shown when navigating from another page */} + {shouldShowBackButton && ( + <button + onClick={handleBack} + className="flex items-center gap-2 text-[var(--accent)] hover:text-[var(--light-accent)] mb-4 font-medium transition-colors" + > + <span className="flex items-center justify-center w-8 h-8 rounded-full bg-[var(--card-bg)] transition-colors flex-shrink-0"> + <ArrowLeft strokeWidth={3} className="w-4 h-4" /> + </span> + <span>Back</span> + </button> + )} + {/* Single responsive grid - no duplicates! */} <div className="grid grid-cols-1 md:grid-cols-[250px_1fr] gap-6 md:gap-8"> {/* Sidebar: Cover, Status, Rating, Re-read - responsive width */} @@ -411,7 +446,7 @@ export default function BookDetailPage() { {/* Main Content Area */} <div className="space-y-6 min-w-0"> {/* Title and Author */} - <div className="mt-3 md:mt-0 text-center md:text-left"> + <div className="text-center md:text-left"> {/* Series Information */} {book.series && ( <Link diff --git a/app/goals/page.tsx b/app/goals/page.tsx index 68c88695..21ded4b8 100644 --- a/app/goals/page.tsx +++ b/app/goals/page.tsx @@ -1,21 +1,17 @@ +"use client"; + import { PageHeader } from "@/components/Layout/PageHeader"; import { GoalsPagePanel } from "@/components/ReadingGoals/GoalsPagePanel"; import { Target } from "lucide-react"; -import { readingGoalsService } from "@/lib/services"; - -export const dynamic = "force-dynamic"; -export const revalidate = 0; // Disable all caching including router cache +import { usePageTitle } from "@/lib/hooks/usePageTitle"; +import { useReadingGoals } from "@/hooks/useReadingGoals"; -export default async function GoalsPage() { - // Get the most appropriate goal for initial display (most recent year with a goal) - // Falls back to null if no goals exist (triggers onboarding) - const defaultGoal = await readingGoalsService.getDefaultGoal(null); - - // Get all goals for year selector - const allGoals = await readingGoalsService.getAllGoals(null); +export default function GoalsPage() { + usePageTitle("Goals"); + const { goals, isLoading } = useReadingGoals(); // Check if user has any goals at all - const hasNoGoals = allGoals.length === 0; + const hasNoGoals = !isLoading && goals.length === 0; return ( <div className="space-y-10"> @@ -27,10 +23,7 @@ export default async function GoalsPage() { /> )} - <GoalsPagePanel - initialGoalData={defaultGoal} - allGoals={allGoals} - /> + <GoalsPagePanel /> </div> ); } diff --git a/app/journal/page.tsx b/app/journal/page.tsx index c16ee1b4..d0d52ec8 100644 --- a/app/journal/page.tsx +++ b/app/journal/page.tsx @@ -3,6 +3,7 @@ import { useState, useEffect, useRef, useCallback, useMemo } from "react"; import { createPortal } from "react-dom"; import { useInfiniteQuery, useQuery } from "@tanstack/react-query"; +import { queryKeys } from "@/lib/query-keys"; import { PageHeader } from "@/components/Layout/PageHeader"; import { BookOpen, Calendar, ChevronRight, Archive } from "lucide-react"; import { formatDate, formatDayOfWeek } from '@/utils/dateHelpers'; @@ -16,8 +17,10 @@ import type { ArchiveNode } from "@/lib/utils/archive-builder"; import { matchesDateKey } from "@/lib/utils/archive-builder"; import { journalApi, type GroupedJournalEntry, type JournalEntry } from "@/lib/api"; import { getCoverUrl } from "@/lib/utils/cover-url"; +import { usePageTitle } from "@/lib/hooks/usePageTitle"; export default function JournalPage() { + usePageTitle("Journal"); const [collapsedDates, setCollapsedDates] = useState<Set<string>>(new Set()); const observerTarget = useRef<HTMLDivElement>(null); @@ -48,7 +51,7 @@ export default function JournalPage() { fetchNextPage, hasNextPage, } = useInfiniteQuery({ - queryKey: ['journal-entries', timezone], + queryKey: queryKeys.journal.entries(timezone), queryFn: async ({ pageParam = 0 }) => { return journalApi.listEntries({ timezone, @@ -67,7 +70,7 @@ export default function JournalPage() { // Fetch archive data const { data: archiveData = [], isLoading: archiveLoading } = useQuery({ - queryKey: ['journal-archive', timezone], + queryKey: queryKeys.journal.archive(timezone), queryFn: () => journalApi.getArchive({ timezone }), staleTime: 60000, // 1 minute }); diff --git a/app/library/page.tsx b/app/library/page.tsx index 564c4933..59f58bbc 100644 --- a/app/library/page.tsx +++ b/app/library/page.tsx @@ -9,8 +9,10 @@ import { LibraryFilters } from "@/components/Library/LibraryFilters"; import { BookGrid } from "@/components/Books/BookGrid"; import { ScrollToTopButton } from "@/components/Layout/ScrollToTopButton"; import { toast } from "@/utils/toast"; +import { usePageTitle } from "@/lib/hooks/usePageTitle"; function LibraryPageContent() { + usePageTitle("Library"); const searchParams = useSearchParams(); const router = useRouter(); const [isReady, setIsReady] = useState(false); diff --git a/app/login/page.tsx b/app/login/page.tsx index 1ddb4c56..9f0a6d6e 100644 --- a/app/login/page.tsx +++ b/app/login/page.tsx @@ -3,8 +3,10 @@ import { useState, FormEvent } from "react"; import { useRouter } from "next/navigation"; import Image from "next/image"; +import { usePageTitle } from "@/lib/hooks/usePageTitle"; export default function LoginPage() { + usePageTitle("Login"); const [password, setPassword] = useState(""); const [error, setError] = useState(""); const [loading, setLoading] = useState(false); diff --git a/app/page.tsx b/app/page.tsx index b7a3f3c2..76e51b83 100644 --- a/app/page.tsx +++ b/app/page.tsx @@ -11,8 +11,10 @@ import CurrentlyReadingSection from "@/components/CurrentlyReading/CurrentlyRead import Link from "next/link"; import { useDashboard } from "@/hooks/useDashboard"; import { Button } from "@/components/Utilities/Button"; +import { usePageTitle } from "@/lib/hooks/usePageTitle"; export default function Dashboard() { + usePageTitle("Dashboard"); const { stats, streak, currentlyReading, currentlyReadingTotal, readNext, readNextTotal, isLoading } = useDashboard(); return ( diff --git a/app/read-next/page.tsx b/app/read-next/page.tsx index e6c9882a..2c95e7a0 100644 --- a/app/read-next/page.tsx +++ b/app/read-next/page.tsx @@ -25,8 +25,10 @@ import BaseModal from "@/components/Modals/BaseModal"; import { PageHeader } from "@/components/Layout/PageHeader"; import { Button } from "@/components/Utilities/Button"; import type { Book } from "@/lib/db/schema/books"; +import { usePageTitle } from "@/lib/hooks/usePageTitle"; export default function ReadNextPage() { + usePageTitle("Read Next"); const { sessions, loading, reorderBooks, removeBooks, moveToTop, moveToBottom } = useReadNextBooks(); const [removingBook, setRemovingBook] = useState<{ id: number; title: string } | null>(null); const [removeLoading, setRemoveLoading] = useState(false); diff --git a/app/series/[name]/page.tsx b/app/series/[name]/page.tsx index 3ce836c2..45778bc1 100644 --- a/app/series/[name]/page.tsx +++ b/app/series/[name]/page.tsx @@ -2,6 +2,7 @@ import { useState } from "react"; import { useQuery } from "@tanstack/react-query"; +import { queryKeys } from "@/lib/query-keys"; import { useParams } from "next/navigation"; import Link from "next/link"; import Image from "next/image"; @@ -12,6 +13,7 @@ import { StatusBadge } from "@/components/Utilities/StatusBadge"; import { StarRating } from "@/components/Utilities/StarRating"; import { type BookStatus } from "@/utils/statusConfig"; import { getCoverUrl } from "@/lib/utils/cover-url"; +import { usePageTitle } from "@/lib/hooks/usePageTitle"; interface SeriesBook { id: number; @@ -43,7 +45,7 @@ export default function SeriesDetailPage() { const [imageErrors, setImageErrors] = useState<Record<number, boolean>>({}); const { data, isLoading, error } = useQuery({ - queryKey: ['series', seriesName], + queryKey: queryKeys.series.detail(seriesName), queryFn: async () => { const response = await fetch(`/api/series/${encodeURIComponent(seriesName)}`); @@ -60,6 +62,10 @@ export default function SeriesDetailPage() { enabled: !!seriesName, }); + // Set dynamic page title (show just "Tome" during loading for clean UX) + const pageTitle = data?.series?.name ? `Series / ${data.series.name}` : undefined; + usePageTitle(pageTitle); + const handleImageError = (calibreId: number) => { setImageErrors(prev => ({ ...prev, [calibreId]: true })); }; diff --git a/app/series/page.tsx b/app/series/page.tsx index 5e7aaa3d..b8cad3c9 100644 --- a/app/series/page.tsx +++ b/app/series/page.tsx @@ -2,11 +2,13 @@ import { useState, useMemo } from "react"; import { useQuery } from "@tanstack/react-query"; +import { queryKeys } from "@/lib/query-keys"; import { Library, BookMarked, Search, X } from "lucide-react"; import SeriesCard from "@/components/Books/SeriesCard"; import SeriesCardSkeleton from "@/components/Books/SeriesCardSkeleton"; import { PageHeader } from "@/components/Layout/PageHeader"; import { ScrollToTopButton } from "@/components/Layout/ScrollToTopButton"; +import { usePageTitle } from "@/lib/hooks/usePageTitle"; interface SeriesInfo { name: string; @@ -15,10 +17,11 @@ interface SeriesInfo { } export default function SeriesPage() { + usePageTitle("Series"); const [searchQuery, setSearchQuery] = useState(""); const { data: series = [], isLoading, error } = useQuery({ - queryKey: ['series'], + queryKey: queryKeys.series.all(), queryFn: async () => { const response = await fetch("/api/series"); diff --git a/app/settings/page.tsx b/app/settings/page.tsx index 89d08c48..4ad9ea4e 100644 --- a/app/settings/page.tsx +++ b/app/settings/page.tsx @@ -1,16 +1,14 @@ +"use client"; + import { Settings as SettingsIcon, Github, Bug, BookOpen } from "lucide-react"; import { ThemeSettings } from "@/components/Settings/ThemeSettings"; import { TimezoneSettings } from "@/components/Settings/TimezoneSettings"; import { PageHeader } from "@/components/Layout/PageHeader"; import { VersionSettings } from "@/components/Settings/VersionSettings"; -import { streakService } from "@/lib/services/streak.service"; - -export const dynamic = 'force-dynamic'; +import { usePageTitle } from "@/lib/hooks/usePageTitle"; -export default async function SettingsPage() { - // Get current streak to fetch timezone (auto-creates if doesn't exist) - const streak = await streakService.getStreak(null); - const initialTimezone = streak.userTimezone; +export default function SettingsPage() { + usePageTitle("Settings"); return ( <div className="space-y-10"> @@ -24,7 +22,7 @@ export default async function SettingsPage() { <ThemeSettings /> {/* Timezone Settings */} - <TimezoneSettings initialTimezone={initialTimezone} /> + <TimezoneSettings /> {/* Version Information */} <VersionSettings /> diff --git a/app/shelves/[id]/page.tsx b/app/shelves/[id]/page.tsx index 551e8c3c..4619e93f 100644 --- a/app/shelves/[id]/page.tsx +++ b/app/shelves/[id]/page.tsx @@ -32,6 +32,7 @@ import { cn } from "@/utils/cn"; import { ShelfAvatar } from "@/components/ShelfManagement/ShelfAvatar"; import { Button } from "@/components/Utilities/Button"; import type { ShelfOrderBy, ShelfSortDirection } from "@/lib/repositories/shelf.repository"; +import { usePageTitle } from "@/lib/hooks/usePageTitle"; type SortOption = ShelfOrderBy; type SortDirection = ShelfSortDirection; @@ -72,6 +73,10 @@ export default function ShelfDetailPage() { // Use shared book list view hook for filtering and selection const listView = useBookListView({ books }); + // Set dynamic page title (show just "Tome" during loading for clean UX) + const pageTitle = shelf?.name ? `Shelf / ${shelf.name}` : undefined; + usePageTitle(pageTitle); + // Calculate canonical shelf position (min/max sortOrder) for Move to Top/Bottom buttons // This ensures buttons reflect actual shelf order, not filtered/sorted display order const minSortOrder = books.length > 0 ? Math.min(...books.map(b => b.sortOrder ?? Infinity)) : Infinity; diff --git a/app/shelves/page.tsx b/app/shelves/page.tsx index ad690c3b..5b3fe04b 100644 --- a/app/shelves/page.tsx +++ b/app/shelves/page.tsx @@ -11,8 +11,10 @@ import { PageHeader } from "@/components/Layout/PageHeader"; import BaseModal from "@/components/Modals/BaseModal"; import { NewShelfFAB } from "@/components/ShelfManagement/NewShelfFAB"; import { Button } from "@/components/Utilities/Button"; +import { usePageTitle } from "@/lib/hooks/usePageTitle"; export default function ShelvesPage() { + usePageTitle("Shelves"); const { shelves, loading, diff --git a/app/stats/page.tsx b/app/stats/page.tsx index 6f4e388d..ebf039bb 100644 --- a/app/stats/page.tsx +++ b/app/stats/page.tsx @@ -11,8 +11,10 @@ import { BarChart3, } from "lucide-react"; import { useStats } from "@/hooks/useStats"; +import { usePageTitle } from "@/lib/hooks/usePageTitle"; export default function StatsPage() { + usePageTitle("Stats"); const { stats, streak, isLoading } = useStats(); return ( diff --git a/app/streak/page.tsx b/app/streak/page.tsx index 8652a902..a4b9cec2 100644 --- a/app/streak/page.tsx +++ b/app/streak/page.tsx @@ -1,35 +1,12 @@ +"use client"; + import { StreakPagePanel } from "@/components/Streaks/StreakPagePanel"; import { PageHeader } from "@/components/Layout/PageHeader"; -import { getLogger } from "@/lib/logger"; import { Flame } from "lucide-react"; -import { streakService } from "@/lib/services/streak.service"; - -const logger = getLogger(); - -// Force dynamic rendering for this page -export const dynamic = "force-dynamic"; +import { usePageTitle } from "@/lib/hooks/usePageTitle"; -export default async function StreakPage() { - // Check if streak tracking is enabled - const currentStreak = await streakService.getStreak(null); - const streakEnabled = currentStreak.streakEnabled; - - // If streak is not enabled, show onboarding (no header, no analytics) - if (!streakEnabled) { - return <StreakPagePanel streakEnabled={false} />; - } - - // Fetch 7 days by default to match initial client state (StreakChartSection) - // This avoids wasting bandwidth and server resources on 365 days that get immediately discarded - // Call service directly instead of making HTTP request to API route - let analyticsData; - try { - analyticsData = await streakService.getAnalytics(7, null); - } catch (error) { - logger.error({ error }, "Failed to fetch analytics"); - // Pass undefined analyticsData to show error state - analyticsData = undefined; - } +export default function StreakPage() { + usePageTitle("Streak"); return ( <div className="space-y-10"> @@ -39,10 +16,7 @@ export default async function StreakPage() { icon={Flame} /> - <StreakPagePanel - streakEnabled={true} - analyticsData={analyticsData} - /> + <StreakPagePanel /> </div> ); } diff --git a/app/tags/page.tsx b/app/tags/page.tsx index e8f032e8..7d7d1751 100644 --- a/app/tags/page.tsx +++ b/app/tags/page.tsx @@ -15,8 +15,10 @@ import { useTagBooks } from "@/hooks/useTagBooks"; import { useTagModals } from "@/hooks/useTagModals"; import { toast } from "@/utils/toast"; import type { TagOperationResult } from "@/types/tag-operations"; +import { usePageTitle } from "@/lib/hooks/usePageTitle"; function TagsPageContent() { + usePageTitle("Tags"); const [selectedTag, setSelectedTag] = useState<string | null>(null); const tagListRef = useRef<TagListRef>(null); diff --git a/components/CurrentlyReading/CurrentlyReadingList.tsx b/components/CurrentlyReading/CurrentlyReadingList.tsx index 8ae85bfa..4c5cab6d 100644 --- a/components/CurrentlyReading/CurrentlyReadingList.tsx +++ b/components/CurrentlyReading/CurrentlyReadingList.tsx @@ -5,6 +5,7 @@ import Image from "next/image"; import Link from "next/link"; import { BookOpen, Pencil } from "lucide-react"; import { useQueryClient } from "@tanstack/react-query"; +import { queryKeys } from "@/lib/query-keys"; import { useRouter } from "next/navigation"; import LogProgressModal from "@/components/Modals/LogProgressModal"; import FinishBookModal from "@/components/Modals/FinishBookModal"; @@ -97,10 +98,10 @@ export default function CurrentlyReadingList({ } setShowCompletionModal(false); - await queryClient.invalidateQueries({ queryKey: ['dashboard'] }); - await queryClient.invalidateQueries({ queryKey: ['book', completedBookId] }); - await queryClient.invalidateQueries({ queryKey: ['sessions', completedBookId] }); - await queryClient.invalidateQueries({ queryKey: ['library-books'] }); + await queryClient.invalidateQueries({ queryKey: queryKeys.dashboard.all() }); + await queryClient.invalidateQueries({ queryKey: queryKeys.book.detail(completedBookId) }); + await queryClient.invalidateQueries({ queryKey: queryKeys.sessions.byBook(completedBookId) }); + await queryClient.invalidateQueries({ queryKey: queryKeys.library.books() }); router.refresh(); toast.success("Book completed!"); } catch (error) { diff --git a/components/CurrentlyReading/ReadingHistoryTab.tsx b/components/CurrentlyReading/ReadingHistoryTab.tsx index 27fcdb43..485d4b48 100644 --- a/components/CurrentlyReading/ReadingHistoryTab.tsx +++ b/components/CurrentlyReading/ReadingHistoryTab.tsx @@ -2,6 +2,7 @@ import { useState } from "react"; import { useQuery, useQueryClient } from "@tanstack/react-query"; +import { queryKeys } from "@/lib/query-keys"; import { Calendar, BookOpen, ChevronRight } from "lucide-react"; import SessionEditModal from "@/components/Modals/SessionEditModal"; import SessionProgressModal from "@/components/Modals/SessionProgressModal"; @@ -50,7 +51,7 @@ export default function ReadingHistoryTab({ bookId, bookTitle = "this book" }: R // Fetch sessions using TanStack Query - automatic caching and background refetching const { data: allSessions = [], isLoading: loading } = useQuery<ReadingSession[]>({ - queryKey: ['sessions', bookId], + queryKey: queryKeys.sessions.byBook(parseInt(bookId)), queryFn: async () => { const response = await fetch(`/api/books/${bookId}/sessions`, { cache: 'no-store', @@ -112,8 +113,8 @@ export default function ReadingHistoryTab({ bookId, bookTitle = "this book" }: R } // Invalidate queries to refetch fresh data - await queryClient.invalidateQueries({ queryKey: ['sessions', bookId] }); - await queryClient.invalidateQueries({ queryKey: ['book', bookId] }); + await queryClient.invalidateQueries({ queryKey: queryKeys.sessions.byBook(parseInt(bookId)) }); + await queryClient.invalidateQueries({ queryKey: queryKeys.book.detail(parseInt(bookId)) }); handleCloseEditModal(); toast.success("Session updated successfully"); @@ -136,8 +137,8 @@ export default function ReadingHistoryTab({ bookId, bookTitle = "this book" }: R } // Invalidate queries to refetch fresh data - await queryClient.invalidateQueries({ queryKey: ['sessions', bookId] }); - await queryClient.invalidateQueries({ queryKey: ['book', bookId] }); + await queryClient.invalidateQueries({ queryKey: queryKeys.sessions.byBook(parseInt(bookId)) }); + await queryClient.invalidateQueries({ queryKey: queryKeys.book.detail(parseInt(bookId)) }); handleCloseDeleteModal(); toast.success("Session deleted"); diff --git a/components/Modals/LogProgressModal.tsx b/components/Modals/LogProgressModal.tsx index c671e579..ddfec252 100644 --- a/components/Modals/LogProgressModal.tsx +++ b/components/Modals/LogProgressModal.tsx @@ -2,6 +2,7 @@ import { useState, useRef, useEffect } from "react"; import { useQueryClient } from "@tanstack/react-query"; +import { queryKeys } from "@/lib/query-keys"; import { useRouter } from "next/navigation"; import { BottomSheet } from "@/components/Layout/BottomSheet"; import BaseModal from "./BaseModal"; @@ -58,8 +59,8 @@ export default function LogProgressModal({ const bookProgressHook = useBookProgress(book.id.toString(), book as any, async () => { // Invalidate dashboard queries to refresh data - await queryClient.invalidateQueries({ queryKey: ['dashboard'] }); - await queryClient.invalidateQueries({ queryKey: ['book', book.id] }); + await queryClient.invalidateQueries({ queryKey: queryKeys.dashboard.all() }); + await queryClient.invalidateQueries({ queryKey: queryKeys.book.detail(book.id) }); // Refresh server components (dashboard page) router.refresh(); }); @@ -164,10 +165,10 @@ export default function LogProgressModal({ onClose(); // Refresh data - await queryClient.invalidateQueries({ queryKey: ['dashboard'] }); - await queryClient.invalidateQueries({ queryKey: ['book', book.id] }); - await queryClient.invalidateQueries({ queryKey: ['sessions', book.id] }); - await queryClient.invalidateQueries({ queryKey: ['library-books'] }); // Invalidate library + await queryClient.invalidateQueries({ queryKey: queryKeys.dashboard.all() }); + await queryClient.invalidateQueries({ queryKey: queryKeys.book.detail(book.id) }); + await queryClient.invalidateQueries({ queryKey: queryKeys.sessions.byBook(book.id) }); + await queryClient.invalidateQueries({ queryKey: queryKeys.library.books() }); // Invalidate library router.refresh(); // Refresh server components toast.success("Book completed!"); diff --git a/components/ReadingGoals/GoalsPagePanel.tsx b/components/ReadingGoals/GoalsPagePanel.tsx index 44108b3a..29a1d32f 100644 --- a/components/ReadingGoals/GoalsPagePanel.tsx +++ b/components/ReadingGoals/GoalsPagePanel.tsx @@ -2,6 +2,7 @@ import { useState, useEffect, useMemo } from "react"; import { useQuery, useMutation, useQueryClient } from "@tanstack/react-query"; +import { queryKeys } from "@/lib/query-keys"; import { ReadingGoalWidget } from "./ReadingGoalWidget"; import { ReadingGoalWidgetSkeleton } from "./ReadingGoalWidgetSkeleton"; import { ReadingGoalForm } from "./ReadingGoalForm"; @@ -20,17 +21,12 @@ import { useReadingGoals } from "@/hooks/useReadingGoals"; import type { ReadingGoalWithProgress, MonthlyBreakdown } from "@/lib/services/reading-goals.service"; import type { ReadingGoal } from "@/lib/db/schema"; -interface GoalsPagePanelProps { - initialGoalData: ReadingGoalWithProgress | null; - allGoals: ReadingGoal[]; -} - -export function GoalsPagePanel({ initialGoalData, allGoals }: GoalsPagePanelProps) { +export function GoalsPagePanel() { const queryClient = useQueryClient(); - const { createGoalAsync, updateGoalAsync } = useReadingGoals(); + const { goals: allGoals, createGoalAsync, updateGoalAsync } = useReadingGoals(); const [isModalOpen, setIsModalOpen] = useState(false); const [modalMode, setModalMode] = useState<"create" | "edit">("create"); - const [selectedYear, setSelectedYear] = useState(initialGoalData?.goal.year || new Date().getFullYear()); + const [selectedYear, setSelectedYear] = useState(new Date().getFullYear()); const [selectedMonth, setSelectedMonth] = useState<number | null>(null); const [booksGoal, setBooksGoal] = useState<number | "">(""); const [saving, setSaving] = useState(false); @@ -43,7 +39,7 @@ export function GoalsPagePanel({ initialGoalData, allGoals }: GoalsPagePanelProp // Goal data query const { data: currentGoalData, isPending: goalLoading, error: goalError } = useQuery({ - queryKey: ['reading-goal', selectedYear], + queryKey: queryKeys.goals.byYear(selectedYear), queryFn: async () => { const response = await fetch(`/api/reading-goals?year=${selectedYear}`); // 404 is expected when no goal exists for the year - return null instead of throwing @@ -56,13 +52,12 @@ export function GoalsPagePanel({ initialGoalData, allGoals }: GoalsPagePanelProp const data = await response.json(); return data.success ? data.data as ReadingGoalWithProgress : null; }, - initialData: selectedYear === initialGoalData?.goal.year ? initialGoalData : undefined, staleTime: 30000, // 30 seconds }); // Monthly breakdown query const { data: monthlyData = [], isPending: monthlyLoading } = useQuery({ - queryKey: ['monthly-breakdown', selectedYear], + queryKey: queryKeys.goals.monthlyBreakdown(selectedYear), queryFn: async () => { const response = await fetch(`/api/reading-goals/monthly?year=${selectedYear}`); if (!response.ok) { @@ -71,6 +66,7 @@ export function GoalsPagePanel({ initialGoalData, allGoals }: GoalsPagePanelProp const data = await response.json(); return data.success ? data.data.monthlyData as MonthlyBreakdown[] : []; }, + placeholderData: (previousData) => previousData, // Keep previous data visible while fetching staleTime: 30000, // 30 seconds }); @@ -79,7 +75,7 @@ export function GoalsPagePanel({ initialGoalData, allGoals }: GoalsPagePanelProp data: booksData, isPending: booksLoading } = useQuery({ - queryKey: ['completed-books', selectedYear], + queryKey: queryKeys.goals.completedBooks(selectedYear), queryFn: async () => { const response = await fetch(`/api/reading-goals/books?year=${selectedYear}`); if (!response.ok) { @@ -120,9 +116,9 @@ export function GoalsPagePanel({ initialGoalData, allGoals }: GoalsPagePanelProp return data.data; }, onSuccess: (_data, variables) => { - queryClient.invalidateQueries({ queryKey: ['reading-goal', variables.year] }); - queryClient.invalidateQueries({ queryKey: ['monthly-breakdown', variables.year] }); - queryClient.invalidateQueries({ queryKey: ['completed-books', variables.year] }); + queryClient.invalidateQueries({ queryKey: queryKeys.goals.byYear(variables.year) }); + queryClient.invalidateQueries({ queryKey: queryKeys.goals.monthlyBreakdown(variables.year) }); + queryClient.invalidateQueries({ queryKey: queryKeys.goals.completedBooks(variables.year) }); setSelectedYear(variables.year); }, }); @@ -180,9 +176,9 @@ export function GoalsPagePanel({ initialGoalData, allGoals }: GoalsPagePanelProp await updateGoalAsync({ id: currentGoalData.goal.id, data: { booksGoal } }); } // Invalidate all queries for the current year - queryClient.invalidateQueries({ queryKey: ['reading-goal', selectedYear] }); - queryClient.invalidateQueries({ queryKey: ['monthly-breakdown', selectedYear] }); - queryClient.invalidateQueries({ queryKey: ['completed-books', selectedYear] }); + queryClient.invalidateQueries({ queryKey: queryKeys.goals.byYear(selectedYear) }); + queryClient.invalidateQueries({ queryKey: queryKeys.goals.monthlyBreakdown(selectedYear) }); + queryClient.invalidateQueries({ queryKey: queryKeys.goals.completedBooks(selectedYear) }); setIsModalOpen(false); setBooksGoal(""); } catch (error) { diff --git a/components/Settings/TimezoneSettings.tsx b/components/Settings/TimezoneSettings.tsx index 21b98abf..aa6129e0 100644 --- a/components/Settings/TimezoneSettings.tsx +++ b/components/Settings/TimezoneSettings.tsx @@ -1,20 +1,25 @@ "use client"; -import { useState } from "react"; +import { useState, useEffect } from "react"; import { Globe } from "lucide-react"; import { useStreak } from "@/hooks/useStreak"; +import { useStreakQuery } from "@/hooks/useStreakQuery"; -interface TimezoneSettingsProps { - initialTimezone: string; -} +export function TimezoneSettings() { + const { streak, isLoadingStreak } = useStreakQuery(); + const [timezone, setTimezone] = useState<string | null>(null); + const { updateTimezoneAsync, isUpdatingTimezone } = useStreak(); -export function TimezoneSettings({ initialTimezone }: TimezoneSettingsProps) { - const [timezone, setTimezone] = useState(initialTimezone); - const { updateTimezone, isUpdatingTimezone } = useStreak(); + // Initialize local state when streak data loads + useEffect(() => { + if (streak?.userTimezone && timezone === null) { + setTimezone(streak.userTimezone); + } + }, [streak?.userTimezone, timezone]); async function handleTimezoneChange(newTimezone: string) { try { - await updateTimezone(newTimezone); + await updateTimezoneAsync(newTimezone); setTimezone(newTimezone); } catch (error) { // Error handling is done in the hook, reset timezone on failure @@ -22,6 +27,22 @@ export function TimezoneSettings({ initialTimezone }: TimezoneSettingsProps) { } } + // Show skeleton loader while loading or timezone not yet set + if (isLoadingStreak || timezone === null) { + return ( + <div className="bg-[var(--card-bg)] border border-[var(--border-color)] rounded-md p-6"> + <div className="animate-pulse"> + <div className="flex items-center gap-3 mb-4"> + <div className="w-6 h-6 bg-[var(--border-color)] rounded" /> + <div className="h-6 w-32 bg-[var(--border-color)] rounded" /> + </div> + <div className="h-4 w-full bg-[var(--border-color)] rounded mb-4" /> + <div className="h-12 w-full bg-[var(--border-color)] rounded" /> + </div> + </div> + ); + } + return ( <div className="bg-[var(--card-bg)] border border-[var(--border-color)] rounded-md p-6"> <div className="flex items-center gap-3 mb-4"> diff --git a/components/Streaks/StreakAnalytics.tsx b/components/Streaks/StreakAnalytics.tsx index c796010c..1e14cdac 100644 --- a/components/Streaks/StreakAnalytics.tsx +++ b/components/Streaks/StreakAnalytics.tsx @@ -26,11 +26,6 @@ export function StreakAnalytics({ // Show encouraging message for new users with < 7 days of data const showEncouragingMessage = daysOfData < 7; - const handleEditSuccess = () => { - // Refresh the page to get updated data - window.location.reload(); - }; - return ( <div className="space-y-6"> {/* Encouraging message for new users */} @@ -185,7 +180,7 @@ export function StreakAnalytics({ isOpen={isEditModalOpen} onClose={() => setIsEditModalOpen(false)} initialThreshold={dailyThreshold} - onSuccess={handleEditSuccess} + onSuccess={() => setIsEditModalOpen(false)} /> </div> ); diff --git a/components/Streaks/StreakChart.tsx b/components/Streaks/StreakChart.tsx index 4f5d8863..0716d9b9 100644 --- a/components/Streaks/StreakChart.tsx +++ b/components/Streaks/StreakChart.tsx @@ -113,7 +113,7 @@ export function StreakChart({ data, threshold }: StreakChartProps) { <ResponsiveContainer width="100%" height="100%"> <ComposedChart data={dataWithAverage} - margin={{ top: 20, right: 10, left: 0, bottom: 40 }} + margin={{ top: 20, right: 10, left: 0, bottom: 5 }} > <defs> <linearGradient id="colorGradient" x1="0" y1="0" x2="0" y2="1"> @@ -128,7 +128,7 @@ export function StreakChart({ data, threshold }: StreakChartProps) { angle={-45} textAnchor="end" interval={tickInterval} - height={60} + height={50} tick={{ fontSize: 10 }} /> <YAxis diff --git a/components/Streaks/StreakChartSection.tsx b/components/Streaks/StreakChartSection.tsx index ecffa246..564acf17 100644 --- a/components/Streaks/StreakChartSection.tsx +++ b/components/Streaks/StreakChartSection.tsx @@ -1,14 +1,12 @@ "use client"; -import { useState, useCallback, useMemo } from "react"; +import { useCallback } from "react"; import { useQuery } from "@tanstack/react-query"; +import { queryKeys } from "@/lib/query-keys"; import { StreakChart } from "@/components/Streaks/StreakChart"; import { StreakHeatmap } from "@/components/Streaks/StreakHeatmap"; import { TimePeriodFilter, TimePeriod } from "@/components/Utilities/TimePeriodFilter"; import { TrendingUp } from "lucide-react"; -import { getLogger } from "@/lib/logger"; - -const logger = getLogger(); interface DailyReading { date: string; @@ -17,11 +15,15 @@ interface DailyReading { } interface StreakChartSectionProps { - initialData: DailyReading[]; + data: DailyReading[]; threshold: number; + selectedPeriod: TimePeriod; + onPeriodChange: (period: TimePeriod) => void; + isLoading?: boolean; + error?: Error | null; } -async function fetchAnalytics(days: TimePeriod): Promise<DailyReading[]> { +async function fetchHeatmapAnalytics(days: number): Promise<DailyReading[]> { const response = await fetch(`/api/streak/analytics?days=${days}`, { cache: "no-store", }); @@ -40,33 +42,26 @@ async function fetchAnalytics(days: TimePeriod): Promise<DailyReading[]> { } export function StreakChartSection({ - initialData, + data, threshold, + selectedPeriod, + onPeriodChange, + isLoading = false, + error = null, }: StreakChartSectionProps) { - const [selectedPeriod, setSelectedPeriod] = useState<TimePeriod>(7); - - // Use TanStack Query for fetching analytics (for area chart) - const { data, isLoading, error } = useQuery<DailyReading[]>({ - queryKey: ['streak-analytics', selectedPeriod], - queryFn: () => fetchAnalytics(selectedPeriod), - initialData: selectedPeriod === 7 ? initialData : undefined, - staleTime: 60000, // 1 minute - streak data changes slowly - refetchOnWindowFocus: true, - }); - // Always fetch 365 days for heatmap (like GitHub's contribution graph) const { data: heatmapData, isLoading: isHeatmapLoading } = useQuery<DailyReading[]>({ - queryKey: ['streak-analytics-heatmap', 365], - queryFn: () => fetchAnalytics(365 as TimePeriod), + queryKey: queryKeys.streak.heatmap(365), + queryFn: () => fetchHeatmapAnalytics(365), staleTime: 60000, refetchOnWindowFocus: true, }); const handlePeriodChange = useCallback( (period: TimePeriod) => { - setSelectedPeriod(period); + onPeriodChange(period); }, - [] + [onPeriodChange] ); return ( diff --git a/components/Streaks/StreakPagePanel.tsx b/components/Streaks/StreakPagePanel.tsx index d63941f4..ad8db596 100644 --- a/components/Streaks/StreakPagePanel.tsx +++ b/components/Streaks/StreakPagePanel.tsx @@ -1,36 +1,132 @@ "use client"; +import { useState } from "react"; +import { useQuery } from "@tanstack/react-query"; +import { queryKeys } from "@/lib/query-keys"; import { StreakAnalytics } from "./StreakAnalytics"; import { StreakChartSection } from "./StreakChartSection"; import { StreakRebuildSection } from "./StreakRebuildSection"; import { StreakOnboarding } from "./StreakOnboarding"; +import { useStreakQuery } from "@/hooks/useStreakQuery"; +import type { TimePeriod } from "@/components/Utilities/TimePeriodFilter"; -interface StreakPagePanelProps { - streakEnabled: boolean; - analyticsData?: { - streak: { - currentStreak: number; - longestStreak: number; - dailyThreshold: number; - totalDaysActive: number; - }; - dailyReadingHistory: { - date: string; - pagesRead: number; - thresholdMet: boolean; - }[]; - booksAheadOrBehind?: number; +interface StreakAnalyticsData { + streak: { + currentStreak: number; + longestStreak: number; + dailyThreshold: number; + totalDaysActive: number; }; + dailyReadingHistory: { + date: string; + pagesRead: number; + thresholdMet: boolean; + }[]; + booksAheadOrBehind?: number; } -export function StreakPagePanel({ streakEnabled, analyticsData }: StreakPagePanelProps) { +export function StreakPagePanel() { + const { streak, analytics, isLoading, streakError } = useStreakQuery(); + + // Manage selected period state + const [selectedPeriod, setSelectedPeriod] = useState<TimePeriod>(7); + + // Query for selected period analytics (for chart) + const { + data: selectedPeriodAnalytics, + isLoading: isSelectedPeriodLoading, + error: selectedPeriodError, + } = useQuery<StreakAnalyticsData>({ + queryKey: queryKeys.streak.analytics(selectedPeriod), + queryFn: async () => { + const response = await fetch(`/api/streak/analytics?days=${String(selectedPeriod)}`); + if (!response.ok) throw new Error('Failed to fetch analytics'); + const json = await response.json(); + if (!json.success) throw new Error(json.error?.message || 'Failed to load analytics'); + return json.data; + }, + placeholderData: (previousData) => previousData, // Keep previous data visible while fetching + staleTime: 60000, + refetchOnWindowFocus: true, + enabled: streak?.streakEnabled ?? false, // Only fetch if streak is enabled + }); + + // Loading state - Match the actual page layout + if (isLoading) { + return ( + <div className="space-y-10"> + {/* Streak Stats Skeleton */} + <div className="space-y-6"> + <div className="h-8 w-48 bg-[var(--border-color)] rounded animate-pulse" /> + <div className="grid grid-cols-2 sm:grid-cols-4 gap-6"> + {/* Current Streak - Orange gradient skeleton */} + <div className="bg-gradient-to-br from-orange-400/30 to-orange-500/30 rounded-md p-6 animate-pulse"> + <div className="h-3 w-24 bg-white/30 rounded mb-4" /> + <div className="h-10 w-16 bg-white/40 rounded mx-auto mb-2" /> + <div className="h-3 w-20 bg-white/30 rounded mx-auto" /> + </div> + + {/* Longest Streak - Green gradient skeleton */} + <div className="bg-gradient-to-br from-emerald-400/30 to-emerald-500/30 rounded-md p-6 animate-pulse"> + <div className="h-3 w-24 bg-white/30 rounded mb-4" /> + <div className="h-10 w-16 bg-white/40 rounded mx-auto mb-2" /> + <div className="h-3 w-20 bg-white/30 rounded mx-auto" /> + </div> + + {/* Total Days Active - Purple gradient skeleton */} + <div className="bg-gradient-to-br from-purple-400/30 to-purple-500/30 rounded-md p-6 animate-pulse"> + <div className="h-3 w-24 bg-white/30 rounded mb-4" /> + <div className="h-10 w-16 bg-white/40 rounded mx-auto mb-2" /> + <div className="h-3 w-20 bg-white/30 rounded mx-auto" /> + </div> + + {/* Daily Goal - Blue gradient skeleton */} + <div className="bg-gradient-to-br from-blue-400/30 to-blue-500/30 rounded-md p-6 animate-pulse"> + <div className="h-3 w-24 bg-white/30 rounded mb-4" /> + <div className="h-10 w-16 bg-white/40 rounded mx-auto mb-2" /> + <div className="h-3 w-20 bg-white/30 rounded mx-auto" /> + </div> + </div> + </div> + + {/* Chart Section Skeleton */} + <div className="space-y-4"> + <div className="flex justify-between items-center"> + <div className="h-8 w-56 bg-[var(--border-color)] rounded animate-pulse" /> + <div className="h-10 w-32 bg-[var(--border-color)] rounded animate-pulse" /> + </div> + <div className="bg-[var(--card-bg)] border border-[var(--border-color)] rounded-md p-6"> + <div className="h-64 bg-[var(--border-color)] rounded animate-pulse" /> + </div> + </div> + + {/* Heatmap Section Skeleton */} + <div className="space-y-4"> + <div className="h-8 w-64 bg-[var(--border-color)] rounded animate-pulse" /> + <div className="bg-[var(--card-bg)] border border-[var(--border-color)] rounded-md p-6"> + <div className="h-32 bg-[var(--border-color)] rounded animate-pulse" /> + </div> + </div> + </div> + ); + } + + // Error state + if (streakError) { + return ( + <div className="bg-red-50 border border-red-200 text-red-800 px-4 py-3 rounded-md"> + <p className="text-sm font-medium">Failed to load streak data. Please try again later.</p> + </div> + ); + } + // Show onboarding if streak tracking is not enabled - if (!streakEnabled) { + if (!streak?.streakEnabled) { return <StreakOnboarding />; } // Show analytics if streak is enabled - if (!analyticsData) { + if (!analytics) { return ( <div className="bg-[var(--card-bg)] border border-[var(--border-color)] p-8 text-center rounded-md"> <p className="text-[var(--foreground)]/70 font-medium"> @@ -40,24 +136,31 @@ export function StreakPagePanel({ streakEnabled, analyticsData }: StreakPagePane ); } - const { streak, dailyReadingHistory, booksAheadOrBehind } = analyticsData; + const { streak: streakData, dailyReadingHistory, booksAheadOrBehind } = analytics; + + // Use selected period analytics for chart, or fall back to 7-day analytics + const chartData = selectedPeriodAnalytics?.dailyReadingHistory || dailyReadingHistory; return ( <div className="space-y-10"> {/* Analytics Stats */} <StreakAnalytics - currentStreak={streak.currentStreak} - longestStreak={streak.longestStreak} - totalDaysActive={streak.totalDaysActive} - dailyThreshold={streak.dailyThreshold} + currentStreak={streakData.currentStreak} + longestStreak={streakData.longestStreak} + totalDaysActive={streakData.totalDaysActive} + dailyThreshold={streakData.dailyThreshold} booksAheadOrBehind={booksAheadOrBehind} daysOfData={dailyReadingHistory.length} /> {/* Chart Section */} <StreakChartSection - initialData={dailyReadingHistory} - threshold={streak.dailyThreshold} + data={chartData} + threshold={streakData.dailyThreshold} + selectedPeriod={selectedPeriod} + onPeriodChange={setSelectedPeriod} + isLoading={isSelectedPeriodLoading} + error={selectedPeriodError} /> {/* Rebuild Section */} diff --git a/docs/AI_CODING_PATTERNS.md b/docs/AI_CODING_PATTERNS.md index f345a1f4..24d2c778 100644 --- a/docs/AI_CODING_PATTERNS.md +++ b/docs/AI_CODING_PATTERNS.md @@ -427,6 +427,189 @@ test("should handle pagination correctly", async () => { --- +## 🔑 Query Key Factory Pattern + +**Location:** `lib/query-keys.ts` + +### The Pattern + +Use a **centralized query key factory** for all TanStack Query (React Query) keys to prevent collisions and enable type-safe invalidations. + +**Why This Matters:** +- **Prevents key collisions**: PR #395 found two components using `'streak-analytics'` with different data structures +- **Fixes invalidation bugs**: `useStreak.ts` was invalidating `['streak-analytics']` but actual key was `['streak-analytics-full', 7]` +- **Type safety**: Catch typos at compile time +- **Easier refactoring**: Change key structure in one place +- **Consistent patterns**: Hierarchical keys enable wildcard invalidation + +### Rules + +✅ **DO: Use the query key factory for ALL queries:** + +```typescript +import { queryKeys } from '@/lib/query-keys'; +import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query'; + +// In queries +const { data } = useQuery({ + queryKey: queryKeys.book.detail(bookId), + queryFn: () => bookApi.get(bookId) +}); + +// In queries with parameters +const { data } = useQuery({ + queryKey: queryKeys.streak.analytics(7), + queryFn: () => fetch('/api/streak/analytics?days=7') +}); + +// In mutations - invalidate specific query +const queryClient = useQueryClient(); +const mutation = useMutation({ + mutationFn: updateBook, + onSuccess: () => { + queryClient.invalidateQueries({ + queryKey: queryKeys.book.detail(bookId) + }); + } +}); + +// Wildcard invalidation - invalidate ALL book queries +queryClient.invalidateQueries({ + queryKey: queryKeys.book.base() +}); + +// Wildcard invalidation - invalidate ALL streak queries +queryClient.invalidateQueries({ + queryKey: queryKeys.streak.base() +}); +``` + +❌ **DON'T: Use hardcoded query key strings:** + +```typescript +// ❌ WRONG - Hardcoded strings (collision risk, no type safety) +useQuery({ + queryKey: ['book', bookId], + queryFn: () => bookApi.get(bookId) +}); + +// ❌ WRONG - Will cause invalidation bugs +queryClient.invalidateQueries({ queryKey: ['streak-analytics'] }); +// Actual key might be ['streak-analytics-full', 7] - invalidation fails! + +// ❌ WRONG - Collision risk (two components, different data structures) +useQuery({ queryKey: ['streak-analytics', 7], ... }); +useQuery({ queryKey: ['streak-analytics', 365], ... }); +``` + +### Key Structure + +Keys follow a **hierarchical pattern** for collision avoidance: + +```typescript +// Base keys (for wildcard invalidation) +queryKeys.book.base() // ['book'] +queryKeys.streak.base() // ['streak'] + +// Specific queries +queryKeys.book.detail(123) // ['book', 123] +queryKeys.sessions.byBook(456) // ['sessions', 456] + +// Nested hierarchies (prevents collisions) +queryKeys.streak.analytics(7) // ['streak', 'analytics', 7] +queryKeys.streak.heatmap(365) // ['streak', 'analytics', 'heatmap', 365] +``` + +### When to Add New Keys + +When adding new TanStack Query queries: + +1. **Check if key exists** in `lib/query-keys.ts` +2. **Add to appropriate domain** (book, streak, session, etc.) +3. **Use hierarchical structure** to avoid collisions +4. **Add JSDoc comments** describing what the key represents +5. **Use `as const`** for type safety + +**Example - Adding a new key:** + +```typescript +export const queryKeys = { + // ... existing keys ... + + book: { + base: () => ['book'] as const, + detail: (id: number) => ['book', id] as const, + + // NEW: Add book reviews key + /** Book reviews: ['book', bookId, 'reviews'] */ + reviews: (bookId: number) => ['book', bookId, 'reviews'] as const, + }, +}; +``` + +### Handling Nullable IDs + +When query parameters can be `null` (disabled queries), use conditional keys: + +```typescript +// Handle nullable sessionId +export function useSessionProgress(sessionId: number | null) { + return useQuery({ + queryKey: sessionId + ? queryKeys.sessions.progress(sessionId) + : ['session-progress-empty'], // Placeholder when disabled + queryFn: async () => { + if (!sessionId) return []; + return fetchProgress(sessionId); + }, + enabled: !!sessionId, // Only fetch when sessionId exists + }); +} + +// For mutations with nullable IDs, guard invalidations +const mutation = useMutation({ + onSuccess: () => { + if (shelfId !== null) { + queryClient.invalidateQueries({ + queryKey: queryKeys.shelf.detail(shelfId) + }); + } + } +}); +``` + +### Critical Bugs Fixed + +This pattern fixes two critical bugs found during implementation: + +**Bug 1: Invalidation Failure** (`hooks/useStreak.ts`) +```typescript +// ❌ BEFORE: Invalidation did nothing +queryClient.invalidateQueries({ queryKey: ['streak-analytics'] }); +// Actual key was ['streak-analytics-full', 7] - mismatch! + +// ✅ AFTER: Correct wildcard invalidation +queryClient.invalidateQueries({ queryKey: queryKeys.streak.base() }); +// Invalidates ALL streak queries: analytics, heatmap, settings +``` + +**Bug 2: Key Collision** (PR #395) +```typescript +// ❌ BEFORE: Two components, same key, different data +// Component A: useStreakQuery.ts +queryKey: ['streak-analytics', 7] // Returns full analytics + +// Component B: StreakChartSection.tsx +queryKey: ['streak-analytics', 365] // Returns heatmap data +// Collision! Same prefix, different data structures + +// ✅ AFTER: Hierarchical keys prevent collision +queryKey: queryKeys.streak.analytics(7) // ['streak', 'analytics', 7] +queryKey: queryKeys.streak.heatmap(365) // ['streak', 'analytics', 'heatmap', 365] +``` + +--- + ## 🗄️ Database Patterns ### Repository Pattern (PRIMARY PATTERN) diff --git a/docs/REVIEW_LOOP.md b/docs/REVIEW_LOOP.md index dd7c295e..f15aeb78 100644 --- a/docs/REVIEW_LOOP.md +++ b/docs/REVIEW_LOOP.md @@ -25,7 +25,7 @@ An automated review system that uses two AI agents to iteratively review and imp **What it does**: 1. @review agent reviews the PR 2. Implements recommended changes (if any) -3. Requests GitHub Copilot review +3. Waits for GitHub Copilot review (auto-triggered on push) 4. @review re-evaluates Copilot suggestions 5. Implements agreed-upon changes 6. Repeats until both approve or max iterations reached @@ -80,7 +80,7 @@ gh pr create --base develop --title "Add new feature" # The system will: # - @review checks constitution, patterns, code quality # - Fixes issues automatically -# - Gets Copilot's perspective +# - Waits for Copilot review (auto-triggered on push) # - Iterates until both approve # 3. Check final status @@ -155,8 +155,9 @@ The @review agent structures feedback as: - ❌ Changes requested: Response contains "CHANGES REQUESTED:" ### GitHub Copilot -- ✅ Approved: No critical suggestions or contains "LGTM"/"looks good" -- ❌ Changes requested: Provides specific suggestions +- ✅ Approved: Check completes with no issues, or contains "LGTM"/"looks good" +- ❌ Changes requested: Provides specific suggestions in PR comments +- ⚠️ Timeout: If check doesn't complete in 3 minutes, treated as approved (continue anyway) ## Safety Features @@ -165,6 +166,7 @@ The @review agent structures feedback as: 3. **Read-only @review**: Agent cannot directly modify code 4. **Explicit approval**: Both agents must explicitly approve 5. **Push after commit**: Changes immediately pushed to remote +6. **Copilot timeout**: 3-minute limit prevents indefinite waiting (60s initial + 2min polling) ## Troubleshooting @@ -176,6 +178,11 @@ The @review agent structures feedback as: - Fix test failures manually - Re-run `/review-loop` to continue +### "Copilot review timeout" +- Copilot may still be processing +- Check PR manually for Copilot feedback +- Loop continues anyway (treats as approved) + ### "Max iterations reached" - Review the suggested changes - Some issues may need manual intervention diff --git a/hooks/useBookDetail.ts b/hooks/useBookDetail.ts index e9c4123c..d1bba343 100644 --- a/hooks/useBookDetail.ts +++ b/hooks/useBookDetail.ts @@ -2,6 +2,7 @@ import { useState } from "react"; import { useQuery, useMutation, useQueryClient } from "@tanstack/react-query"; import { toast } from "@/utils/toast"; import { bookApi } from "@/lib/api"; +import { queryKeys } from "@/lib/query-keys"; export interface Book { id: number; @@ -63,7 +64,7 @@ export function useBookDetail(bookId: string): UseBookDetailReturn { error, refetch, } = useQuery<Book>({ - queryKey: ['book', bookId], + queryKey: queryKeys.book.detail(parseInt(bookId)), queryFn: () => bookApi.getDetail(bookId), staleTime: 5000, // Data is fresh for 5 seconds }); @@ -78,13 +79,14 @@ export function useBookDetail(bookId: string): UseBookDetailReturn { bookApi.updateBook(bookId, { totalPages }), onMutate: async (totalPages) => { // Cancel outgoing queries to prevent them from overwriting our optimistic update - await queryClient.cancelQueries({ queryKey: ['book', bookId] }); + const bookKey = queryKeys.book.detail(parseInt(bookId)); + await queryClient.cancelQueries({ queryKey: bookKey }); // Snapshot the previous value - const previousBook = queryClient.getQueryData<Book>(['book', bookId]); + const previousBook = queryClient.getQueryData<Book>(bookKey); // Optimistically update to the new value - queryClient.setQueryData<Book>(['book', bookId], (old) => + queryClient.setQueryData<Book>(bookKey, (old) => old ? { ...old, totalPages } : old ); @@ -94,13 +96,13 @@ export function useBookDetail(bookId: string): UseBookDetailReturn { onError: (_err, _totalPages, context) => { // Rollback on error if (context?.previousBook) { - queryClient.setQueryData(['book', bookId], context.previousBook); + queryClient.setQueryData(queryKeys.book.detail(parseInt(bookId)), context.previousBook); } toast.error('Failed to update page count'); }, onSuccess: () => { // Invalidate and refetch book data - queryClient.invalidateQueries({ queryKey: ['book', bookId] }); + queryClient.invalidateQueries({ queryKey: queryKeys.book.detail(parseInt(bookId)) }); toast.success('Page count updated'); }, }); @@ -111,13 +113,14 @@ export function useBookDetail(bookId: string): UseBookDetailReturn { bookApi.updateTags(bookId, { tags }), onMutate: async (tags) => { // Cancel outgoing queries to prevent them from overwriting our optimistic update - await queryClient.cancelQueries({ queryKey: ['book', bookId] }); + const bookKey = queryKeys.book.detail(parseInt(bookId)); + await queryClient.cancelQueries({ queryKey: bookKey }); // Snapshot the previous value - const previousBook = queryClient.getQueryData<Book>(['book', bookId]); + const previousBook = queryClient.getQueryData<Book>(bookKey); // Optimistically update to the new value - queryClient.setQueryData<Book>(['book', bookId], (old) => + queryClient.setQueryData<Book>(bookKey, (old) => old ? { ...old, tags } : old ); @@ -127,15 +130,15 @@ export function useBookDetail(bookId: string): UseBookDetailReturn { onError: (_err, _tags, context) => { // Rollback on error if (context?.previousBook) { - queryClient.setQueryData(['book', bookId], context.previousBook); + queryClient.setQueryData(queryKeys.book.detail(parseInt(bookId)), context.previousBook); } toast.error('Failed to update tags'); }, onSuccess: () => { // Invalidate and refetch book data - queryClient.invalidateQueries({ queryKey: ['book', bookId] }); + queryClient.invalidateQueries({ queryKey: queryKeys.book.detail(parseInt(bookId)) }); // Invalidate available tags cache since tags may have been added/removed - queryClient.invalidateQueries({ queryKey: ['availableTags'] }); + queryClient.invalidateQueries({ queryKey: queryKeys.book.availableTags() }); toast.success('Tags updated'); }, }); @@ -155,7 +158,7 @@ export function useBookDetail(bookId: string): UseBookDetailReturn { // Partial update method for optimistic updates from other hooks const updateBookPartial = (updates: Partial<Book>) => { - queryClient.setQueryData<Book>(['book', bookId], (old) => + queryClient.setQueryData<Book>(queryKeys.book.detail(parseInt(bookId)), (old) => old ? { ...old, ...updates } : old ); }; diff --git a/hooks/useBookProgress.ts b/hooks/useBookProgress.ts index c6159404..eabe9344 100644 --- a/hooks/useBookProgress.ts +++ b/hooks/useBookProgress.ts @@ -1,5 +1,6 @@ import { useState, useEffect, useCallback } from "react"; import { useQuery, useMutation, useQueryClient } from "@tanstack/react-query"; +import { queryKeys } from "@/lib/query-keys"; import type { Book } from "./useBookDetail"; import { toast } from "@/utils/toast"; import { getTodayLocalDate } from '@/utils/dateHelpers'; @@ -79,7 +80,7 @@ export function useBookProgress( // Fetch progress entries with TanStack Query const { data: progress = [], isLoading } = useQuery({ - queryKey: ['progress', bookId], + queryKey: queryKeys.progress.byBook(parseInt(bookId)), queryFn: async () => { return bookApi.listProgress(bookId); }, @@ -98,12 +99,13 @@ export function useBookProgress( }, onSuccess: () => { // Invalidate relevant queries - queryClient.invalidateQueries({ queryKey: ['progress', bookId] }); - queryClient.invalidateQueries({ queryKey: ['book', bookId] }); - queryClient.invalidateQueries({ queryKey: ['sessions', bookId] }); // Refresh reading history - queryClient.invalidateQueries({ queryKey: ['dashboard'] }); - queryClient.invalidateQueries({ queryKey: ['streak-analytics'] }); - queryClient.invalidateQueries({ queryKey: ['stats'] }); + const bookIdNum = parseInt(bookId); + queryClient.invalidateQueries({ queryKey: queryKeys.progress.byBook(bookIdNum) }); + queryClient.invalidateQueries({ queryKey: queryKeys.book.detail(bookIdNum) }); + queryClient.invalidateQueries({ queryKey: queryKeys.sessions.byBook(bookIdNum) }); // Refresh reading history + queryClient.invalidateQueries({ queryKey: queryKeys.dashboard.all() }); + queryClient.invalidateQueries({ queryKey: queryKeys.streak.base() }); // Invalidates all streak queries + queryClient.invalidateQueries({ queryKey: queryKeys.stats.all() }); // Clear form state setNotes(""); @@ -135,12 +137,13 @@ export function useBookProgress( }, onSuccess: () => { // Invalidate relevant queries - queryClient.invalidateQueries({ queryKey: ['progress', bookId] }); - queryClient.invalidateQueries({ queryKey: ['book', bookId] }); - queryClient.invalidateQueries({ queryKey: ['sessions', bookId] }); // Refresh reading history - queryClient.invalidateQueries({ queryKey: ['dashboard'] }); - queryClient.invalidateQueries({ queryKey: ['streak-analytics'] }); - queryClient.invalidateQueries({ queryKey: ['stats'] }); + const bookIdNum = parseInt(bookId); + queryClient.invalidateQueries({ queryKey: queryKeys.progress.byBook(bookIdNum) }); + queryClient.invalidateQueries({ queryKey: queryKeys.book.detail(bookIdNum) }); + queryClient.invalidateQueries({ queryKey: queryKeys.sessions.byBook(bookIdNum) }); // Refresh reading history + queryClient.invalidateQueries({ queryKey: queryKeys.dashboard.all() }); + queryClient.invalidateQueries({ queryKey: queryKeys.streak.base() }); // Invalidates all streak queries + queryClient.invalidateQueries({ queryKey: queryKeys.stats.all() }); // Close modal setShowEditProgressModal(false); @@ -163,12 +166,13 @@ export function useBookProgress( }, onSuccess: () => { // Invalidate relevant queries - queryClient.invalidateQueries({ queryKey: ['progress', bookId] }); - queryClient.invalidateQueries({ queryKey: ['book', bookId] }); - queryClient.invalidateQueries({ queryKey: ['sessions', bookId] }); // Refresh reading history - queryClient.invalidateQueries({ queryKey: ['dashboard'] }); - queryClient.invalidateQueries({ queryKey: ['streak-analytics'] }); - queryClient.invalidateQueries({ queryKey: ['stats'] }); + const bookIdNum = parseInt(bookId); + queryClient.invalidateQueries({ queryKey: queryKeys.progress.byBook(bookIdNum) }); + queryClient.invalidateQueries({ queryKey: queryKeys.book.detail(bookIdNum) }); + queryClient.invalidateQueries({ queryKey: queryKeys.sessions.byBook(bookIdNum) }); // Refresh reading history + queryClient.invalidateQueries({ queryKey: queryKeys.dashboard.all() }); + queryClient.invalidateQueries({ queryKey: queryKeys.streak.base() }); // Invalidates all streak queries + queryClient.invalidateQueries({ queryKey: queryKeys.stats.all() }); // Close modal setShowEditProgressModal(false); @@ -351,7 +355,7 @@ export function useBookProgress( }, []); const refetchProgress = useCallback(async () => { - await queryClient.invalidateQueries({ queryKey: ['progress', bookId] }); + await queryClient.invalidateQueries({ queryKey: queryKeys.progress.byBook(parseInt(bookId)) }); }, [queryClient, bookId]); const clearFormState = useCallback(() => { diff --git a/hooks/useBookRating.ts b/hooks/useBookRating.ts index 42edfec7..266cd359 100644 --- a/hooks/useBookRating.ts +++ b/hooks/useBookRating.ts @@ -1,5 +1,6 @@ import { useState, useCallback } from "react"; import { useMutation, useQueryClient } from "@tanstack/react-query"; +import { queryKeys } from "@/lib/query-keys"; import type { Book } from "./useBookDetail"; import { toast } from "@/utils/toast"; import { bookApi } from "@/lib/api"; @@ -46,13 +47,13 @@ export function useBookRating( }, onMutate: async (newRating) => { // Cancel outgoing queries - await queryClient.cancelQueries({ queryKey: ['book', bookId] }); + await queryClient.cancelQueries({ queryKey: queryKeys.book.detail(parseInt(bookId)) }); // Snapshot previous value - const previousBook = queryClient.getQueryData<Book>(['book', bookId]); + const previousBook = queryClient.getQueryData<Book>(queryKeys.book.detail(parseInt(bookId))); // Optimistic update - queryClient.setQueryData<Book>(['book', bookId], (old) => + queryClient.setQueryData<Book>(queryKeys.book.detail(parseInt(bookId)), (old) => old ? { ...old, rating: newRating } : old ); @@ -64,7 +65,7 @@ export function useBookRating( onError: (_err, _newRating, context) => { // Rollback on error if (context?.previousBook) { - queryClient.setQueryData(['book', bookId], context.previousBook); + queryClient.setQueryData(queryKeys.book.detail(parseInt(bookId)), context.previousBook); // Legacy support updateBookPartial?.({ rating: context.previousBook.rating }); } @@ -72,7 +73,7 @@ export function useBookRating( }, onSuccess: (data) => { // Invalidate related queries - queryClient.invalidateQueries({ queryKey: ['book', bookId] }); + queryClient.invalidateQueries({ queryKey: queryKeys.book.detail(parseInt(bookId)) }); // Show success message if (data.rating === null) { diff --git a/hooks/useBookStatus.ts b/hooks/useBookStatus.ts index 8bba1619..9b3b1a45 100644 --- a/hooks/useBookStatus.ts +++ b/hooks/useBookStatus.ts @@ -1,11 +1,14 @@ import { useState, useEffect, useCallback, useMemo } from "react"; -import { useMutation, useQueryClient } from "@tanstack/react-query"; +import { useMutation, useQueryClient, QueryClient } from "@tanstack/react-query"; +import { queryKeys } from "@/lib/query-keys"; import type { Book } from "./useBookDetail"; import { toast } from "@/utils/toast"; import { getLogger } from "@/lib/logger"; import { bookApi, ApiError } from "@/lib/api"; import { libraryService } from "@/lib/library-service"; +const logger = getLogger().child({ hook: 'useBookStatus' }); + interface ProgressEntry { id: number; currentPage: number; @@ -37,14 +40,61 @@ function requiresArchiveConfirmation( /** * Invalidates all queries related to a book * Exported to allow reuse in components that make direct API calls + * + * Also invalidates shelf caches for shelves containing this book to ensure + * shelf pages reflect updated book status immediately. */ -export function invalidateBookQueries(queryClient: any, bookId: string): void { - queryClient.invalidateQueries({ queryKey: ['book', bookId] }); - queryClient.invalidateQueries({ queryKey: ['sessions', bookId] }); - queryClient.invalidateQueries({ queryKey: ['progress', bookId] }); - queryClient.invalidateQueries({ queryKey: ['dashboard'] }); - queryClient.invalidateQueries({ queryKey: ['library-books'] }); - queryClient.invalidateQueries({ queryKey: ['read-next-books'] }); +export function invalidateBookQueries(queryClient: QueryClient, bookId: string): void { + 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 series pages (if book belongs to a series) + queryClient.invalidateQueries({ queryKey: queryKeys.series.all() }); + + // 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); + + // Type guard to validate cached shelves structure + const isValidShelvesData = (data: unknown): data is { success: boolean; data: Array<{ id: number }> } => { + return ( + data !== null && + typeof data === 'object' && + 'data' in data && + Array.isArray((data as any).data) + ); + }; + + if (isValidShelvesData(cachedShelves)) { + // Cache is valid - use surgical approach + if (cachedShelves.data.length > 0) { + // Book is on some shelves - invalidate only those + logger.debug( + { bookId: numericBookId, shelfIds: cachedShelves.data.map(s => s.id) }, + 'Surgical shelf invalidation' + ); + cachedShelves.data.forEach((shelf: { id: number }) => { + queryClient.invalidateQueries({ queryKey: queryKeys.shelf.byId(shelf.id) }); + }); + } + // else: Book is on zero shelves (data.length === 0) - no-op, nothing to invalidate + } else { + // 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() }); + } // Clear entire LibraryService cache to ensure status changes reflect across all filters libraryService.clearCache(); @@ -131,7 +181,7 @@ export function useBookStatus( }, onMutate: async ({ status }) => { // Cancel outgoing queries - await queryClient.cancelQueries({ queryKey: ['book', bookId] }); + await queryClient.cancelQueries({ queryKey: queryKeys.book.detail(parseInt(bookId)) }); // Snapshot previous value const previousOptimistic = optimisticStatus; @@ -188,7 +238,7 @@ export function useBookStatus( }, onMutate: async () => { // Cancel outgoing queries and snapshot state - await queryClient.cancelQueries({ queryKey: ['book', bookId] }); + await queryClient.cancelQueries({ queryKey: queryKeys.book.detail(parseInt(bookId)) }); const previousOptimistic = optimisticStatus; // Optimistic update @@ -251,7 +301,7 @@ export function useBookStatus( }, onMutate: async () => { // Cancel outgoing queries and snapshot state - await queryClient.cancelQueries({ queryKey: ['book', bookId] }); + await queryClient.cancelQueries({ queryKey: queryKeys.book.detail(parseInt(bookId)) }); const previousOptimistic = optimisticStatus; // Optimistic update @@ -309,7 +359,7 @@ export function useBookStatus( }, onMutate: async () => { // Cancel outgoing queries and snapshot state - await queryClient.cancelQueries({ queryKey: ['book', bookId] }); + await queryClient.cancelQueries({ queryKey: queryKeys.book.detail(parseInt(bookId)) }); const previousOptimistic = optimisticStatus; // Optimistic update diff --git a/hooks/useDashboard.ts b/hooks/useDashboard.ts index a065e89c..39d77195 100644 --- a/hooks/useDashboard.ts +++ b/hooks/useDashboard.ts @@ -1,4 +1,5 @@ import { useQuery } from "@tanstack/react-query"; +import { queryKeys } from "@/lib/query-keys"; import { dashboardApi, type DashboardData } from "@/lib/api"; export type { @@ -14,7 +15,7 @@ async function fetchDashboardData(): Promise<DashboardData> { export function useDashboard() { const { data, isLoading, error, refetch } = useQuery<DashboardData>({ - queryKey: ["dashboard"], + queryKey: queryKeys.dashboard.all(), queryFn: fetchDashboardData, staleTime: 30000, // 30 seconds - dashboard updates fairly frequently refetchOnWindowFocus: true, // Refetch when user returns to tab diff --git a/hooks/useLibraryData.ts b/hooks/useLibraryData.ts index a8bf6915..7633f08e 100644 --- a/hooks/useLibraryData.ts +++ b/hooks/useLibraryData.ts @@ -1,5 +1,6 @@ import { useState, useCallback, useMemo } from "react"; import { useInfiniteQuery, useQueryClient } from "@tanstack/react-query"; +import { queryKeys } from "@/lib/query-keys"; import { libraryService, LibraryFilters } from "@/lib/library-service"; const BOOKS_PER_PAGE = 50; @@ -19,8 +20,9 @@ export function useLibraryData(initialFilters?: Partial<LibraryFilters>) { const service = useMemo(() => libraryService, []); // Create a stable query key based on filter values (excluding pagination.skip) + // Use queryKeys.library.books() as base to ensure consistency with refresh/invalidation const queryKey = useMemo(() => [ - 'library-books', + ...queryKeys.library.books(), filters.status, filters.search, filters.tags, @@ -131,7 +133,7 @@ export function useLibraryData(initialFilters?: Partial<LibraryFilters>) { // Clear cache and refresh const refresh = useCallback(async () => { service.clearCache(); - await queryClient.refetchQueries({ queryKey: ['library-books'] }); + await queryClient.refetchQueries({ queryKey: queryKeys.library.books() }); }, [service, queryClient]); // Search function with debouncing logic diff --git a/hooks/usePullToRefreshLogic.ts b/hooks/usePullToRefreshLogic.ts index 7c7c33cd..d0860772 100644 --- a/hooks/usePullToRefreshLogic.ts +++ b/hooks/usePullToRefreshLogic.ts @@ -2,6 +2,7 @@ import { useCallback } from "react"; import { useQueryClient } from "@tanstack/react-query"; +import { queryKeys } from "@/lib/query-keys"; import { usePathname } from "next/navigation"; /** @@ -13,52 +14,52 @@ export function usePullToRefreshLogic() { const pathname = usePathname(); const handleRefresh = useCallback(async () => { - // Map pages to their query keys - const pageQueryMap: Record<string, string[]> = { - "/": ["dashboard"], - "/library": ["library", "libraryStats"], - "/read-next": ["readNext"], - "/series": ["series"], - "/stats": ["stats"], - "/goals": ["goals"], - "/streak": ["streak"], - "/journal": ["sessions"], - "/shelves": ["shelves"], - "/tags": ["tags"], - "/settings": ["settings"], - }; - - // Check if we're on a book detail page + // Check if we're on a specific detail page const isBookDetail = pathname.startsWith("/books/"); const isSeriesDetail = pathname.startsWith("/series/") && pathname !== "/series"; const isShelfDetail = pathname.startsWith("/shelves/") && pathname !== "/shelves"; - let queryKeys: string[] = []; - if (isBookDetail) { - // For book details, invalidate book-related queries - queryKeys = ["book", "bookProgress", "bookSessions", "bookTags", "bookShelves"]; + // For book details, invalidate all book-related queries using base keys + await Promise.all([ + queryClient.invalidateQueries({ queryKey: queryKeys.book.base() }), + // Note: Can't invalidate specific book progress/sessions without bookId + // These will be invalidated by the specific mutations that need them + ]); } else if (isSeriesDetail) { // For series detail pages - queryKeys = ["series", "seriesBooks"]; + await queryClient.invalidateQueries({ queryKey: queryKeys.series.all() }); } else if (isShelfDetail) { // For shelf detail pages - queryKeys = ["shelf", "shelfBooks"]; + await queryClient.invalidateQueries({ queryKey: queryKeys.shelf.base() }); } else { - // Find matching page in the map - queryKeys = pageQueryMap[pathname] || []; - } + // Map pages to their query key invalidation + const invalidationsByPath: Record<string, readonly unknown[]> = { + "/": queryKeys.dashboard.all(), + "/library": queryKeys.library.books(), + "/read-next": queryKeys.readNext.base(), + "/series": queryKeys.series.all(), + "/stats": queryKeys.stats.all(), + "/goals": queryKeys.goals.base(), + "/streak": queryKeys.streak.base(), + "/shelves": queryKeys.shelf.base(), + "/tags": queryKeys.tags.base(), + }; - // Invalidate queries - if (queryKeys.length > 0) { - await Promise.all( - queryKeys.map((key) => - queryClient.invalidateQueries({ queryKey: [key] }) - ) - ); - } else { - // Fallback: invalidate all queries if we don't have specific keys - await queryClient.invalidateQueries(); + const queryKey = invalidationsByPath[pathname]; + + if (queryKey) { + await queryClient.invalidateQueries({ queryKey }); + } else if (pathname === "/journal") { + // Special case: Invalidate both journal entries and archive using prefix matching + await Promise.all([ + queryClient.invalidateQueries({ queryKey: queryKeys.journal.entriesBase() }), + queryClient.invalidateQueries({ queryKey: queryKeys.journal.archiveBase() }), + ]); + } else { + // Fallback: invalidate all queries if we don't have specific keys + await queryClient.invalidateQueries(); + } } // Return a promise that resolves after invalidation diff --git a/hooks/useReadNextBooks.ts b/hooks/useReadNextBooks.ts index 6a921476..2a9298c7 100644 --- a/hooks/useReadNextBooks.ts +++ b/hooks/useReadNextBooks.ts @@ -6,6 +6,7 @@ */ import { useQuery, useMutation, useQueryClient } from "@tanstack/react-query"; +import { queryKeys } from "@/lib/query-keys"; import { toast } from "@/utils/toast"; import { invalidateBookQueries } from "@/hooks/useBookStatus"; import type { SessionWithBook } from "@/lib/repositories/session.repository"; @@ -19,7 +20,7 @@ export function useReadNextBooks(search?: string) { isLoading, error, } = useQuery({ - queryKey: ["read-next-books", search], + queryKey: queryKeys.readNext.books(search), queryFn: async () => { const params = new URLSearchParams(); if (search) { @@ -59,13 +60,12 @@ export function useReadNextBooks(search?: string) { }, onMutate: async (updates) => { // Cancel outgoing refetches - await queryClient.cancelQueries({ queryKey: ["read-next-books"] }); + await queryClient.cancelQueries({ queryKey: queryKeys.readNext.base() }); // Snapshot previous value - const previousSessions = queryClient.getQueryData<SessionWithBook[]>([ - "read-next-books", - search, - ]); + const previousSessions = queryClient.getQueryData<SessionWithBook[]>( + queryKeys.readNext.books(search) + ); // Optimistically update cache if (previousSessions) { @@ -81,7 +81,7 @@ export function useReadNextBooks(search?: string) { }) .sort((a, b) => (a.readNextOrder || 0) - (b.readNextOrder || 0)); - queryClient.setQueryData(["read-next-books", search], optimisticSessions); + queryClient.setQueryData(queryKeys.readNext.books(search), optimisticSessions); } // Return context for rollback @@ -91,7 +91,7 @@ export function useReadNextBooks(search?: string) { // Rollback to previous state if (context?.previousSessions) { queryClient.setQueryData( - ["read-next-books", search], + queryKeys.readNext.books(search), context.previousSessions ); } @@ -100,7 +100,7 @@ export function useReadNextBooks(search?: string) { }, onSettled: () => { // Refetch after error or success to sync with server - queryClient.invalidateQueries({ queryKey: ["read-next-books"] }); + queryClient.invalidateQueries({ queryKey: queryKeys.readNext.base() }); }, // No success toast - visual feedback of reordering is confirmation enough }); @@ -138,7 +138,7 @@ export function useReadNextBooks(search?: string) { }); // Also explicitly invalidate read-next query (redundant but clear) - queryClient.invalidateQueries({ queryKey: ["read-next-books"] }); + queryClient.invalidateQueries({ queryKey: queryKeys.readNext.base() }); const count = bookIds.length; toast.success( @@ -170,13 +170,12 @@ export function useReadNextBooks(search?: string) { }, onMutate: async (sessionId) => { // Cancel outgoing refetches - await queryClient.cancelQueries({ queryKey: ["read-next-books"] }); + await queryClient.cancelQueries({ queryKey: queryKeys.readNext.base() }); // Snapshot previous value - const previousSessions = queryClient.getQueryData<SessionWithBook[]>([ - "read-next-books", - search, - ]); + const previousSessions = queryClient.getQueryData<SessionWithBook[]>( + queryKeys.readNext.books(search) + ); // Optimistically update cache if (previousSessions) { @@ -192,7 +191,7 @@ export function useReadNextBooks(search?: string) { })), ]; - queryClient.setQueryData(["read-next-books", search], optimisticSessions); + queryClient.setQueryData(queryKeys.readNext.books(search), optimisticSessions); } } @@ -203,7 +202,7 @@ export function useReadNextBooks(search?: string) { // Rollback to previous state if (context?.previousSessions) { queryClient.setQueryData( - ["read-next-books", search], + queryKeys.readNext.books(search), context.previousSessions ); } @@ -215,7 +214,7 @@ export function useReadNextBooks(search?: string) { }, onSettled: () => { // Refetch after error or success to sync with server - queryClient.invalidateQueries({ queryKey: ["read-next-books"] }); + queryClient.invalidateQueries({ queryKey: queryKeys.readNext.base() }); }, }); @@ -234,13 +233,12 @@ export function useReadNextBooks(search?: string) { }, onMutate: async (sessionId) => { // Cancel outgoing refetches - await queryClient.cancelQueries({ queryKey: ["read-next-books"] }); + await queryClient.cancelQueries({ queryKey: queryKeys.readNext.base() }); // Snapshot previous value - const previousSessions = queryClient.getQueryData<SessionWithBook[]>([ - "read-next-books", - search, - ]); + const previousSessions = queryClient.getQueryData<SessionWithBook[]>( + queryKeys.readNext.books(search) + ); // Optimistically update cache if (previousSessions) { @@ -278,7 +276,7 @@ export function useReadNextBooks(search?: string) { return a.id - b.id; }); - queryClient.setQueryData(["read-next-books", search], sortedOptimisticSessions); + queryClient.setQueryData(queryKeys.readNext.books(search), sortedOptimisticSessions); } } @@ -289,7 +287,7 @@ export function useReadNextBooks(search?: string) { // Rollback to previous state if (context?.previousSessions) { queryClient.setQueryData( - ["read-next-books", search], + queryKeys.readNext.books(search), context.previousSessions ); } @@ -301,7 +299,7 @@ export function useReadNextBooks(search?: string) { }, onSettled: () => { // Refetch after error or success to sync with server - queryClient.invalidateQueries({ queryKey: ["read-next-books"] }); + queryClient.invalidateQueries({ queryKey: queryKeys.readNext.base() }); }, }); diff --git a/hooks/useReadingGoals.ts b/hooks/useReadingGoals.ts index 7bb2ec68..390277bd 100644 --- a/hooks/useReadingGoals.ts +++ b/hooks/useReadingGoals.ts @@ -1,4 +1,5 @@ import { useQuery, useMutation, useQueryClient } from "@tanstack/react-query"; +import { queryKeys } from "@/lib/query-keys"; import { toast } from "sonner"; import type { ReadingGoal } from "@/lib/db/schema"; import { goalsApi } from "@/lib/api"; @@ -22,7 +23,7 @@ export function useReadingGoals() { // Query: Fetch all reading goals const { data: goals = [], isLoading, error } = useQuery({ - queryKey: ['reading-goals'], + queryKey: queryKeys.goals.base(), queryFn: async () => { const response = await goalsApi.list(); return response.success ? response.data : []; @@ -36,7 +37,7 @@ export function useReadingGoals() { return goalsApi.create(payload); }, onSuccess: () => { - queryClient.invalidateQueries({ queryKey: ['reading-goals'] }); + queryClient.invalidateQueries({ queryKey: queryKeys.goals.base() }); toast.success('Reading goal created!'); }, onError: (error: Error) => { @@ -50,7 +51,7 @@ export function useReadingGoals() { return goalsApi.update(params.id, params.data); }, onSuccess: () => { - queryClient.invalidateQueries({ queryKey: ['reading-goals'] }); + queryClient.invalidateQueries({ queryKey: queryKeys.goals.base() }); toast.success('Reading goal updated!'); }, onError: (error: Error) => { @@ -64,7 +65,7 @@ export function useReadingGoals() { return goalsApi.delete(goalId); }, onSuccess: () => { - queryClient.invalidateQueries({ queryKey: ['reading-goals'] }); + queryClient.invalidateQueries({ queryKey: queryKeys.goals.base() }); toast.success('Reading goal deleted!'); }, onError: (error: Error) => { diff --git a/hooks/useSessionProgress.ts b/hooks/useSessionProgress.ts index 0984a569..a30e93d7 100644 --- a/hooks/useSessionProgress.ts +++ b/hooks/useSessionProgress.ts @@ -1,4 +1,5 @@ import { useQuery } from "@tanstack/react-query"; +import { queryKeys } from "@/lib/query-keys"; import { bookApi } from "@/lib/api"; export interface SessionProgressEntry { @@ -20,7 +21,7 @@ export interface SessionProgressEntry { */ export function useSessionProgress(bookId: string, sessionId: number | null) { return useQuery<SessionProgressEntry[]>({ - queryKey: ['session-progress', sessionId], + queryKey: sessionId ? queryKeys.sessions.progress(sessionId) : ['session-progress-empty'], queryFn: async () => { if (!sessionId) return []; diff --git a/hooks/useShelfBooks.ts b/hooks/useShelfBooks.ts index 764176cd..96616433 100644 --- a/hooks/useShelfBooks.ts +++ b/hooks/useShelfBooks.ts @@ -1,4 +1,5 @@ import { useQuery, useMutation, useQueryClient } from "@tanstack/react-query"; +import { queryKeys } from "@/lib/query-keys"; import { toast } from "@/utils/toast"; import { shelfApi, ApiError } from "@/lib/api"; import type { ShelfWithBooks } from "@/lib/api"; @@ -21,9 +22,11 @@ export function useShelfBooks( isLoading, error, } = useQuery({ - queryKey: ["shelf", shelfId, { orderBy, direction }], + queryKey: shelfId !== null + ? queryKeys.shelf.detail(shelfId, { orderBy, direction }) + : ['shelf-empty'], queryFn: async () => { - if (!shelfId) return null; + if (shelfId === null) return null; const shelf = await shelfApi.get(shelfId, { withBooks: true, @@ -50,13 +53,13 @@ export function useShelfBooks( */ const addBooksMutation = useMutation({ mutationFn: async (bookIds: number[]) => { - if (!shelfId || bookIds.length === 0) { + if (shelfId === null || bookIds.length === 0) { throw new Error("No shelf ID or book IDs provided"); } return shelfApi.addBooks(shelfId, { bookIds }); }, onSuccess: (result) => { - queryClient.invalidateQueries({ queryKey: ["shelf", shelfId] }); + if (shelfId !== null) queryClient.invalidateQueries({ queryKey: queryKeys.shelf.byId(shelfId) }); const bookWord = result.count === 1 ? 'book' : 'books'; toast.success(`${result.count} ${bookWord} added to shelf`); }, @@ -70,21 +73,19 @@ export function useShelfBooks( */ const removeBookMutation = useMutation({ mutationFn: async (bookId: number) => { - if (!shelfId) { + if (shelfId === null) { throw new Error("No shelf ID provided"); } return shelfApi.removeBook(shelfId, bookId); }, onMutate: async (bookId) => { // Cancel outgoing refetches - await queryClient.cancelQueries({ queryKey: ["shelf", shelfId] }); + if (shelfId !== null) await queryClient.cancelQueries({ queryKey: queryKeys.shelf.byId(shelfId) }); // Snapshot previous value - const previousShelf = queryClient.getQueryData<ShelfWithBooksExtended>([ - "shelf", - shelfId, - { orderBy, direction }, - ]); + const previousShelf = queryClient.getQueryData<ShelfWithBooksExtended>( + queryKeys.shelf.detail(shelfId!, { orderBy, direction }) + ); // Optimistically update: filter out removed book and reindex sortOrder if (previousShelf) { @@ -93,7 +94,7 @@ export function useShelfBooks( .map((book, index) => ({ ...book, sortOrder: index } as BookWithStatus)); queryClient.setQueryData( - ["shelf", shelfId, { orderBy, direction }], + queryKeys.shelf.detail(shelfId!, { orderBy, direction }), { ...previousShelf, books: remainingBooks, @@ -110,14 +111,14 @@ export function useShelfBooks( // Rollback on error if (context?.previousShelf) { queryClient.setQueryData( - ["shelf", shelfId, { orderBy, direction }], + queryKeys.shelf.detail(shelfId!, { orderBy, direction }), context.previousShelf ); } toast.error(`Failed to remove book from shelf: ${getErrorMessage(err)}`); }, onSettled: () => { - queryClient.invalidateQueries({ queryKey: ["shelf", shelfId] }); + if (shelfId !== null) queryClient.invalidateQueries({ queryKey: queryKeys.shelf.byId(shelfId) }); }, }); @@ -126,21 +127,19 @@ export function useShelfBooks( */ const removeBooksMutation = useMutation({ mutationFn: async (bookIds: number[]) => { - if (!shelfId || bookIds.length === 0) { + if (shelfId === null || bookIds.length === 0) { throw new Error("No shelf ID or book IDs provided"); } return shelfApi.removeBooks(shelfId, { bookIds }); }, onMutate: async (bookIds) => { // Cancel outgoing refetches - await queryClient.cancelQueries({ queryKey: ["shelf", shelfId] }); + if (shelfId !== null) await queryClient.cancelQueries({ queryKey: queryKeys.shelf.byId(shelfId) }); // Snapshot previous value - const previousShelf = queryClient.getQueryData<ShelfWithBooksExtended>([ - "shelf", - shelfId, - { orderBy, direction }, - ]); + const previousShelf = queryClient.getQueryData<ShelfWithBooksExtended>( + queryKeys.shelf.detail(shelfId!, { orderBy, direction }) + ); // Optimistically update: remove books from local state and reindex if (previousShelf) { @@ -149,7 +148,7 @@ export function useShelfBooks( .map((book, index) => ({ ...book, sortOrder: index } as BookWithStatus)); queryClient.setQueryData( - ["shelf", shelfId, { orderBy, direction }], + queryKeys.shelf.detail(shelfId!, { orderBy, direction }), { ...previousShelf, books: remainingBooks, @@ -167,14 +166,14 @@ export function useShelfBooks( // Rollback on error if (context?.previousShelf) { queryClient.setQueryData( - ["shelf", shelfId, { orderBy, direction }], + queryKeys.shelf.detail(shelfId!, { orderBy, direction }), context.previousShelf ); } toast.error(`Failed to remove books: ${getErrorMessage(err)}`); }, onSettled: () => { - queryClient.invalidateQueries({ queryKey: ["shelf", shelfId] }); + if (shelfId !== null) queryClient.invalidateQueries({ queryKey: queryKeys.shelf.byId(shelfId) }); }, }); @@ -183,21 +182,19 @@ export function useShelfBooks( */ const reorderBooksMutation = useMutation({ mutationFn: async (bookIds: number[]) => { - if (!shelfId) { + if (shelfId === null) { throw new Error("No shelf ID provided"); } return shelfApi.reorderBooks(shelfId, { bookIds }); }, onMutate: async (bookIds) => { // Cancel outgoing refetches - await queryClient.cancelQueries({ queryKey: ["shelf", shelfId] }); + if (shelfId !== null) await queryClient.cancelQueries({ queryKey: queryKeys.shelf.byId(shelfId) }); // Snapshot previous value - const previousShelf = queryClient.getQueryData<ShelfWithBooksExtended>([ - "shelf", - shelfId, - { orderBy, direction }, - ]); + const previousShelf = queryClient.getQueryData<ShelfWithBooksExtended>( + queryKeys.shelf.detail(shelfId!, { orderBy, direction }) + ); // Optimistically update: reorder books and update sortOrder if (previousShelf) { @@ -210,7 +207,7 @@ export function useShelfBooks( .filter((book): book is BookWithStatus => book !== undefined); queryClient.setQueryData( - ["shelf", shelfId, { orderBy, direction }], + queryKeys.shelf.detail(shelfId!, { orderBy, direction }), { ...previousShelf, books: reorderedBooks, @@ -224,14 +221,14 @@ export function useShelfBooks( // Rollback on error if (context?.previousShelf) { queryClient.setQueryData( - ["shelf", shelfId, { orderBy, direction }], + queryKeys.shelf.detail(shelfId!, { orderBy, direction }), context.previousShelf ); } toast.error(`Failed to reorder books: ${getErrorMessage(err)}`); }, onSettled: () => { - queryClient.invalidateQueries({ queryKey: ["shelf", shelfId] }); + if (shelfId !== null) queryClient.invalidateQueries({ queryKey: queryKeys.shelf.byId(shelfId) }); }, // No success toast - visual feedback of reordering is confirmation enough }); @@ -248,21 +245,19 @@ export function useShelfBooks( bookIds: number[]; targetShelfName?: string; }) => { - if (!shelfId || bookIds.length === 0) { + if (shelfId === null || bookIds.length === 0) { throw new Error("No shelf ID or book IDs provided"); } return shelfApi.moveBooks(shelfId, targetShelfId, { bookIds }); }, onMutate: async ({ bookIds }) => { // Cancel outgoing refetches - await queryClient.cancelQueries({ queryKey: ["shelf", shelfId] }); + if (shelfId !== null) await queryClient.cancelQueries({ queryKey: queryKeys.shelf.byId(shelfId) }); // Snapshot previous value - const previousShelf = queryClient.getQueryData<ShelfWithBooksExtended>([ - "shelf", - shelfId, - { orderBy, direction }, - ]); + const previousShelf = queryClient.getQueryData<ShelfWithBooksExtended>( + queryKeys.shelf.detail(shelfId!, { orderBy, direction }) + ); // Optimistically update: remove books from local state if (previousShelf) { @@ -271,7 +266,7 @@ export function useShelfBooks( .map((book, index) => ({ ...book, sortOrder: index } as BookWithStatus)); queryClient.setQueryData( - ["shelf", shelfId, { orderBy, direction }], + queryKeys.shelf.detail(shelfId!, { orderBy, direction }), { ...previousShelf, books: remainingBooks, @@ -290,14 +285,14 @@ export function useShelfBooks( // Rollback on error if (context?.previousShelf) { queryClient.setQueryData( - ["shelf", shelfId, { orderBy, direction }], + queryKeys.shelf.detail(shelfId!, { orderBy, direction }), context.previousShelf ); } toast.error(`Failed to move books: ${getErrorMessage(err)}`); }, onSettled: () => { - queryClient.invalidateQueries({ queryKey: ["shelf", shelfId] }); + if (shelfId !== null) queryClient.invalidateQueries({ queryKey: queryKeys.shelf.byId(shelfId) }); }, }); @@ -334,7 +329,7 @@ export function useShelfBooks( */ const moveToTopMutation = useMutation({ mutationFn: async (bookId: number) => { - if (!shelfId) { + if (shelfId === null) { throw new Error("No shelf ID provided"); } @@ -351,14 +346,12 @@ export function useShelfBooks( }, onMutate: async (bookId) => { // Cancel outgoing refetches - await queryClient.cancelQueries({ queryKey: ["shelf", shelfId] }); + if (shelfId !== null) await queryClient.cancelQueries({ queryKey: queryKeys.shelf.byId(shelfId) }); // Snapshot previous value - const previousShelf = queryClient.getQueryData<ShelfWithBooksExtended>([ - "shelf", - shelfId, - { orderBy, direction }, - ]); + const previousShelf = queryClient.getQueryData<ShelfWithBooksExtended>( + queryKeys.shelf.detail(shelfId!, { orderBy, direction }) + ); // Optimistically update: move book to top if (previousShelf) { @@ -375,7 +368,7 @@ export function useShelfBooks( ] as BookWithStatus[]; queryClient.setQueryData( - ["shelf", shelfId, { orderBy, direction }], + queryKeys.shelf.detail(shelfId!, { orderBy, direction }), { ...previousShelf, books: optimisticBooks, @@ -390,7 +383,7 @@ export function useShelfBooks( // Rollback on error if (context?.previousShelf) { queryClient.setQueryData( - ["shelf", shelfId, { orderBy, direction }], + queryKeys.shelf.detail(shelfId!, { orderBy, direction }), context.previousShelf ); } @@ -400,13 +393,13 @@ export function useShelfBooks( toast.success("Moved to top of shelf"); }, onSettled: () => { - queryClient.invalidateQueries({ queryKey: ["shelf", shelfId] }); + if (shelfId !== null) queryClient.invalidateQueries({ queryKey: queryKeys.shelf.byId(shelfId) }); }, }); const moveToBottomMutation = useMutation({ mutationFn: async (bookId: number) => { - if (!shelfId) { + if (shelfId === null) { throw new Error("No shelf ID provided"); } @@ -423,14 +416,12 @@ export function useShelfBooks( }, onMutate: async (bookId) => { // Cancel outgoing refetches - await queryClient.cancelQueries({ queryKey: ["shelf", shelfId] }); + if (shelfId !== null) await queryClient.cancelQueries({ queryKey: queryKeys.shelf.byId(shelfId) }); // Snapshot previous value - const previousShelf = queryClient.getQueryData<ShelfWithBooksExtended>([ - "shelf", - shelfId, - { orderBy, direction }, - ]); + const previousShelf = queryClient.getQueryData<ShelfWithBooksExtended>( + queryKeys.shelf.detail(shelfId!, { orderBy, direction }) + ); // Optimistically update: move book to bottom if (previousShelf) { @@ -469,7 +460,7 @@ export function useShelfBooks( } queryClient.setQueryData( - ["shelf", shelfId, { orderBy, direction }], + queryKeys.shelf.detail(shelfId!, { orderBy, direction }), { ...previousShelf, books: optimisticBooks, @@ -484,7 +475,7 @@ export function useShelfBooks( // Rollback on error if (context?.previousShelf) { queryClient.setQueryData( - ["shelf", shelfId, { orderBy, direction }], + queryKeys.shelf.detail(shelfId!, { orderBy, direction }), context.previousShelf ); } @@ -494,7 +485,7 @@ export function useShelfBooks( toast.success("Moved to bottom of shelf"); }, onSettled: () => { - queryClient.invalidateQueries({ queryKey: ["shelf", shelfId] }); + if (shelfId !== null) queryClient.invalidateQueries({ queryKey: queryKeys.shelf.byId(shelfId) }); }, }); diff --git a/hooks/useStats.ts b/hooks/useStats.ts index 0c17cf35..dd92f31b 100644 --- a/hooks/useStats.ts +++ b/hooks/useStats.ts @@ -1,4 +1,5 @@ import { useQuery } from "@tanstack/react-query"; +import { queryKeys } from "@/lib/query-keys"; import { statsApi, type StatsOverview, type StreakData } from "@/lib/api"; async function fetchStatsOverview(): Promise<StatsOverview> { @@ -11,14 +12,14 @@ async function fetchStreak(): Promise<StreakData> { export function useStats() { const { data: stats, isLoading: isLoadingStats, error: statsError } = useQuery<StatsOverview>({ - queryKey: ['stats'], + queryKey: queryKeys.stats.all(), queryFn: fetchStatsOverview, staleTime: 30000, // 30 seconds refetchOnWindowFocus: true, }); const { data: streak, isLoading: isLoadingStreak, error: streakError } = useQuery<StreakData>({ - queryKey: ['streaks'], + queryKey: queryKeys.streak.settings(), queryFn: fetchStreak, staleTime: 30000, // 30 seconds refetchOnWindowFocus: true, diff --git a/hooks/useStreak.ts b/hooks/useStreak.ts index eac2728c..d26067b7 100644 --- a/hooks/useStreak.ts +++ b/hooks/useStreak.ts @@ -1,6 +1,7 @@ import { useMutation, useQueryClient } from "@tanstack/react-query"; import { toast } from "sonner"; import { streakApi } from "@/lib/api"; +import { queryKeys } from "@/lib/query-keys"; /** * Custom hook for managing streak operations @@ -15,10 +16,9 @@ export function useStreak() { return streakApi.rebuild(); }, onSuccess: () => { - // Invalidate all streak-related queries - queryClient.invalidateQueries({ queryKey: ['streak-analytics'] }); - queryClient.invalidateQueries({ queryKey: ['streaks'] }); - queryClient.invalidateQueries({ queryKey: ['dashboard'] }); + // Invalidate all streak-related queries (base key invalidates all children) + queryClient.invalidateQueries({ queryKey: queryKeys.streak.base() }); + queryClient.invalidateQueries({ queryKey: queryKeys.dashboard.all() }); toast.success("Streak recalculated successfully!"); }, onError: (error) => { @@ -34,9 +34,8 @@ export function useStreak() { }, onSuccess: () => { // Invalidate streak-related queries - queryClient.invalidateQueries({ queryKey: ['streak-analytics'] }); - queryClient.invalidateQueries({ queryKey: ['streaks'] }); - queryClient.invalidateQueries({ queryKey: ['dashboard'] }); + queryClient.invalidateQueries({ queryKey: queryKeys.streak.base() }); + queryClient.invalidateQueries({ queryKey: queryKeys.dashboard.all() }); toast.success("Daily reading goal updated!"); }, onError: (error: any) => { @@ -52,10 +51,9 @@ export function useStreak() { }, onSuccess: () => { // Invalidate all queries since timezone affects date boundaries - queryClient.invalidateQueries({ queryKey: ['streak-analytics'] }); - queryClient.invalidateQueries({ queryKey: ['streaks'] }); - queryClient.invalidateQueries({ queryKey: ['dashboard'] }); - queryClient.invalidateQueries({ queryKey: ['stats'] }); // Stats page also uses timezone + queryClient.invalidateQueries({ queryKey: queryKeys.streak.base() }); + queryClient.invalidateQueries({ queryKey: queryKeys.dashboard.all() }); + queryClient.invalidateQueries({ queryKey: queryKeys.stats.all() }); // Stats page also uses timezone toast.success("Timezone updated! Streak recalculated with new day boundaries."); }, onError: (error: any) => { @@ -71,9 +69,8 @@ export function useStreak() { }, onSuccess: (data) => { // Invalidate streak-related queries - queryClient.invalidateQueries({ queryKey: ['streak-analytics'] }); - queryClient.invalidateQueries({ queryKey: ['streaks'] }); - queryClient.invalidateQueries({ queryKey: ['dashboard'] }); + queryClient.invalidateQueries({ queryKey: queryKeys.streak.base() }); + queryClient.invalidateQueries({ queryKey: queryKeys.dashboard.all() }); const message = data.data.streakEnabled ? "Streak tracking enabled!" diff --git a/hooks/useStreakQuery.ts b/hooks/useStreakQuery.ts new file mode 100644 index 00000000..bc6e7cd1 --- /dev/null +++ b/hooks/useStreakQuery.ts @@ -0,0 +1,93 @@ +import { useQuery } from "@tanstack/react-query"; +import { queryKeys } from "@/lib/query-keys"; + +interface StreakData { + id: number; + userId: number | null; + streakEnabled: boolean; + currentStreak: number; + longestStreak: number; + lastActivityDate: string | null; + streakStartDate: string | null; + totalDaysActive: number; + dailyThreshold: number; + userTimezone: string; + lastCheckedDate: string | null; + updatedAt: string; + hoursRemainingToday: number; +} + +interface StreakAnalyticsData { + streak: { + currentStreak: number; + longestStreak: number; + dailyThreshold: number; + totalDaysActive: number; + }; + dailyReadingHistory: { + date: string; + pagesRead: number; + thresholdMet: boolean; + }[]; + booksAheadOrBehind?: number; +} + +/** + * Custom hook for querying streak data + * + * Provides read-only access to streak settings and analytics. + * For mutations (updates), use the useStreak() hook. + * + * @example + * const { streak, analytics, isLoading } = useStreakQuery(); + * + * if (isLoading) return <Skeleton />; + * if (streak?.streakEnabled) { + * // Show streak analytics + * } + */ +export function useStreakQuery() { + // Query: Fetch streak status and settings + const streakQuery = useQuery({ + queryKey: queryKeys.streak.settings(), + queryFn: async () => { + const response = await fetch('/api/streak'); + if (!response.ok) throw new Error('Failed to fetch streak'); + const json = await response.json(); + if (!json.success) throw new Error(json.error?.message || 'Failed to load streak'); + return json.data as StreakData; + }, + staleTime: 60000, // 1 minute + refetchOnWindowFocus: false, // Disabled: /api/streak performs writes (checkAndResetStreakIfNeeded) + }); + + // Query: Fetch streak analytics (only if streak is enabled) + const analyticsQuery = useQuery({ + queryKey: queryKeys.streak.analytics(7), + queryFn: async () => { + const response = await fetch('/api/streak/analytics?days=7'); + if (!response.ok) throw new Error('Failed to fetch analytics'); + const json = await response.json(); + if (!json.success) throw new Error(json.error?.message || 'Failed to load analytics'); + return json.data as StreakAnalyticsData; + }, + staleTime: 60000, + refetchOnWindowFocus: false, // Consistent with streak query; staleTime provides sufficient freshness + enabled: streakQuery.data?.streakEnabled ?? false, // Only fetch if streak is enabled + }); + + return { + // Streak data + streak: streakQuery.data, + isLoadingStreak: streakQuery.isLoading, + streakError: streakQuery.error, + + // Analytics data + analytics: analyticsQuery.data, + isLoadingAnalytics: analyticsQuery.isLoading, + analyticsError: analyticsQuery.error, + + // Combined loading state + isLoading: streakQuery.isLoading || (streakQuery.data?.streakEnabled && analyticsQuery.isLoading), + }; +} diff --git a/hooks/useTagBooks.ts b/hooks/useTagBooks.ts index b2643acd..4167afa0 100644 --- a/hooks/useTagBooks.ts +++ b/hooks/useTagBooks.ts @@ -1,5 +1,6 @@ import { useState, useEffect, useCallback, useRef } from "react"; import { useQueryClient } from "@tanstack/react-query"; +import { queryKeys } from "@/lib/query-keys"; import type { Book } from "@/lib/db/schema/books"; import { tagApi } from "@/lib/api"; @@ -150,8 +151,8 @@ export function useTagBooks(tagName: string | null) { await fetchInitialBooks(); // Invalidate tags cache since book count changed - queryClient.invalidateQueries({ queryKey: ['tags-stats'] }); - queryClient.invalidateQueries({ queryKey: ['availableTags'] }); + queryClient.invalidateQueries({ queryKey: queryKeys.tags.base() }); + queryClient.invalidateQueries({ queryKey: queryKeys.book.availableTags() }); } catch (err) { throw err instanceof Error ? err : new Error("Failed to remove tag"); } @@ -167,8 +168,8 @@ export function useTagBooks(tagName: string | null) { await fetchInitialBooks(); // Invalidate tags cache since book count changed - queryClient.invalidateQueries({ queryKey: ['tags-stats'] }); - queryClient.invalidateQueries({ queryKey: ['availableTags'] }); + queryClient.invalidateQueries({ queryKey: queryKeys.tags.base() }); + queryClient.invalidateQueries({ queryKey: queryKeys.book.availableTags() }); } catch (err) { throw err instanceof Error ? err : new Error("Failed to add tag"); } @@ -184,8 +185,8 @@ export function useTagBooks(tagName: string | null) { await fetchInitialBooks(); // Invalidate tags cache since book count changed - queryClient.invalidateQueries({ queryKey: ['tags-stats'] }); - queryClient.invalidateQueries({ queryKey: ['availableTags'] }); + queryClient.invalidateQueries({ queryKey: queryKeys.tags.base() }); + queryClient.invalidateQueries({ queryKey: queryKeys.book.availableTags() }); return data; } catch (err) { diff --git a/hooks/useTagManagement.ts b/hooks/useTagManagement.ts index 40ffc64a..c48f2950 100644 --- a/hooks/useTagManagement.ts +++ b/hooks/useTagManagement.ts @@ -1,5 +1,6 @@ import { useCallback, useRef } from "react"; import { useQuery, useMutation, useQueryClient } from "@tanstack/react-query"; +import { queryKeys } from "@/lib/query-keys"; import type { TagOperationResult } from "@/types/tag-operations"; import { tagApi } from "@/lib/api"; @@ -20,7 +21,7 @@ export function useTagManagement() { error: queryError, refetch: refetchQuery } = useQuery({ - queryKey: ['tags-stats'], + queryKey: queryKeys.tags.base(), queryFn: () => tagApi.getStats(), staleTime: 30000, // 30 seconds - data stays fresh for 30s gcTime: 300000, // 5 minutes - cache stays in memory for 5 min @@ -44,9 +45,9 @@ export function useTagManagement() { tagApi.rename(oldName, newName), onSuccess: () => { // Invalidate tags query to refetch - queryClient.invalidateQueries({ queryKey: ['tags-stats'] }); + queryClient.invalidateQueries({ queryKey: queryKeys.tags.base() }); // Invalidate available tags cache for book detail pages - queryClient.invalidateQueries({ queryKey: ['availableTags'] }); + queryClient.invalidateQueries({ queryKey: queryKeys.book.availableTags() }); }, }); @@ -55,9 +56,9 @@ export function useTagManagement() { mutationFn: (tagName: string) => tagApi.delete(tagName), onSuccess: () => { // Invalidate tags query to refetch - queryClient.invalidateQueries({ queryKey: ['tags-stats'] }); + queryClient.invalidateQueries({ queryKey: queryKeys.tags.base() }); // Invalidate available tags cache for book detail pages - queryClient.invalidateQueries({ queryKey: ['availableTags'] }); + queryClient.invalidateQueries({ queryKey: queryKeys.book.availableTags() }); }, }); @@ -67,9 +68,9 @@ export function useTagManagement() { tagApi.merge(sourceTags, targetTag), onSuccess: () => { // Invalidate tags query to refetch - queryClient.invalidateQueries({ queryKey: ['tags-stats'] }); + queryClient.invalidateQueries({ queryKey: queryKeys.tags.base() }); // Invalidate available tags cache for book detail pages - queryClient.invalidateQueries({ queryKey: ['availableTags'] }); + queryClient.invalidateQueries({ queryKey: queryKeys.book.availableTags() }); }, }); diff --git a/hooks/useVersion.ts b/hooks/useVersion.ts index 3dcf4311..c0a2c243 100644 --- a/hooks/useVersion.ts +++ b/hooks/useVersion.ts @@ -1,4 +1,5 @@ import { useQuery } from "@tanstack/react-query"; +import { queryKeys } from "@/lib/query-keys"; /** * Version data returned from the API @@ -24,7 +25,7 @@ export interface VersionData { */ export function useVersion() { return useQuery<VersionData>({ - queryKey: ['version'], + queryKey: queryKeys.version.info(), queryFn: async () => { const response = await fetch('/api/version'); if (!response.ok) { diff --git a/lib/hooks/usePageTitle.ts b/lib/hooks/usePageTitle.ts new file mode 100644 index 00000000..7c66a0fb --- /dev/null +++ b/lib/hooks/usePageTitle.ts @@ -0,0 +1,76 @@ +import { useLayoutEffect, useEffect } from "react"; + +/** + * Custom hook to set the page title + * + * IMPLEMENTATION NOTE - Render-time side effect: + * This hook intentionally sets document.title during render (outside useLayoutEffect) + * because Next.js 16 client components don't properly update titles on initial page + * load when using effects alone. Without this immediate setting, titles don't render + * at all on page refresh. + * + * This violates React best practices (no side effects during render), but is a + * necessary tradeoff because: + * 1. Without it, titles don't work on initial load (functional requirement) + * 2. document.title assignment is idempotent and safe + * 3. Client-only guard prevents SSR issues + * 4. Worst case in concurrent rendering: title set to correct value on next render + * + * We also use useLayoutEffect for updates and cleanup to follow React patterns + * where possible. + * + * HYDRATION FIX: + * Next.js 16 server-renders <title> from metadata, then React hydration overwrites + * any client-side title changes to match the server HTML. We use a MutationObserver + * to detect and correct this hydration overwrite, ensuring client-side titles persist. + * + * @param title - The title to display after "Tome - ". + * If undefined or empty string, shows just "Tome" + */ +export function usePageTitle(title?: string) { + // Compute title once to avoid duplication + const newTitle = title ? `Tome - ${title}` : "Tome"; + + // Set title immediately on client-side to handle initial load + // This is necessary because useLayoutEffect alone doesn't work for initial render + if (typeof window !== 'undefined' && document.title !== newTitle) { + document.title = newTitle; + } + + // Watch for external title changes (primarily React hydration overwriting our title) + // This fixes the issue where hard refresh causes title to revert to "Tome" + useEffect(() => { + if (typeof window === 'undefined') return; + + const observer = new MutationObserver(() => { + if (document.title !== newTitle) { + // React hydration or another process changed the title - restore it + document.title = newTitle; + } + }); + + const titleElement = document.querySelector('title'); + if (titleElement) { + observer.observe(titleElement, { + childList: true, + characterData: true, + subtree: true, + }); + } + + return () => observer.disconnect(); + }, [newTitle]); + + // Also use useLayoutEffect to handle updates and ensure it persists + useLayoutEffect(() => { + document.title = newTitle; + + // Cleanup: Only reset if title is still what this hook set + // This prevents clobbering titles set by other components during route transitions + return () => { + if (document.title === newTitle) { + document.title = "Tome"; + } + }; + }, [newTitle]); +} diff --git a/lib/query-keys.ts b/lib/query-keys.ts new file mode 100644 index 00000000..658ae618 --- /dev/null +++ b/lib/query-keys.ts @@ -0,0 +1,247 @@ +/** + * Centralized Query Key Factory + * + * This file provides type-safe, collision-resistant query keys for React Query. + * + * ## Benefits + * + * - **Prevents key collisions**: Hierarchical organization ensures unique keys + * - **Type safety**: Compile-time checks catch typos and invalid parameters + * - **Centralized management**: Easier refactoring and maintenance + * - **Consistent invalidation**: Base keys enable wildcard invalidation + * + * ## Usage Examples + * + * ### In Queries + * ```typescript + * import { queryKeys } from '@/lib/query-keys'; + * + * // Fetch a specific book + * useQuery({ + * queryKey: queryKeys.book.detail(bookId), + * queryFn: () => bookApi.get(bookId) + * }); + * + * // Fetch streak analytics + * useQuery({ + * queryKey: queryKeys.streak.analytics(7), + * queryFn: () => fetch('/api/streak/analytics?days=7') + * }); + * ``` + * + * ### In Invalidations + * ```typescript + * // Invalidate a specific book + * queryClient.invalidateQueries({ + * queryKey: queryKeys.book.detail(bookId) + * }); + * + * // Invalidate ALL book queries (wildcard) + * queryClient.invalidateQueries({ + * queryKey: queryKeys.book.base() + * }); + * + * // Invalidate all streak queries (fixes critical bug in useStreak.ts) + * queryClient.invalidateQueries({ + * queryKey: queryKeys.streak.base() + * }); + * ``` + * + * ## Bug Fixes + * + * This factory addresses critical bugs: + * + * 1. **Invalidation Bug** (useStreak.ts): Was invalidating `['streak-analytics']` + * but actual key was `['streak-analytics-full', 7]`. Now uses `queryKeys.streak.base()`. + * + * 2. **Collision Bug** (PR #395): Two components used `'streak-analytics'` with different + * data structures. Now uses hierarchical keys: `queryKeys.streak.analytics(7)` vs + * `queryKeys.streak.heatmap(365)`. + * + * ## Key Structure + * + * Keys follow a hierarchical pattern: + * - `['book']` - Base key for all books (wildcard invalidation) + * - `['book', id]` - Specific book detail + * - `['streak', 'analytics', days]` - Streak analytics with specific period + * - `['streak', 'analytics', 'heatmap', days]` - Heatmap (distinct from analytics) + * + * @see {@link https://tkdodo.eu/blog/effective-react-query-keys} - TanStack Query key best practices + */ + +/** + * Valid time periods for analytics queries + */ +export type AnalyticsDays = 7 | 30 | 90 | 180 | 365 | "this-year" | "all-time"; + +export const queryKeys = { + // ============================================================================ + // BOOKS + // ============================================================================ + book: { + /** Base key for all book queries: ['book'] */ + base: () => ['book'] as const, + + /** Book detail by ID: ['book', id] */ + detail: (id: number) => ['book', id] as const, + + /** Available tags for books: ['availableTags'] */ + availableTags: () => ['availableTags'] as const, + + /** Available shelves for books: ['availableShelves'] */ + availableShelves: () => ['availableShelves'] as const, + + /** Shelves for a specific book: ['bookShelves', bookId] */ + shelves: (bookId: number) => ['bookShelves', bookId] as const, + }, + + // ============================================================================ + // LIBRARY + // ============================================================================ + library: { + /** Library books list: ['library-books'] */ + books: () => ['library-books'] as const, + }, + + // ============================================================================ + // SESSIONS + // ============================================================================ + sessions: { + /** Sessions for a specific book: ['sessions', bookId] */ + byBook: (bookId: number) => ['sessions', bookId] as const, + + /** Progress entries for a specific session: ['session-progress', sessionId] */ + progress: (sessionId: number) => ['session-progress', sessionId] as const, + }, + + // ============================================================================ + // PROGRESS + // ============================================================================ + progress: { + /** Progress for a specific book: ['progress', bookId] */ + byBook: (bookId: number) => ['progress', bookId] as const, + }, + + // ============================================================================ + // STREAK + // ============================================================================ + streak: { + /** Base key for all streak queries: ['streak'] */ + base: () => ['streak'] as const, + + /** Streak settings and status: ['streak', 'settings'] */ + settings: () => ['streak', 'settings'] as const, + + /** Streak analytics for daily chart: ['streak', 'analytics', days] */ + analytics: (days: AnalyticsDays) => ['streak', 'analytics', days] as const, + + /** Streak heatmap data: ['streak', 'analytics', 'heatmap', days] */ + heatmap: (days: 7 | 30 | 90 | 180 | 365) => ['streak', 'analytics', 'heatmap', days] as const, + }, + + // ============================================================================ + // GOALS + // ============================================================================ + goals: { + /** Base key for all goal queries: ['goals'] */ + base: () => ['goals'] as const, + + /** Reading goal for specific year: ['reading-goal', year] */ + byYear: (year: number) => ['reading-goal', year] as const, + + /** Monthly breakdown for specific year: ['monthly-breakdown', year] */ + monthlyBreakdown: (year: number) => ['monthly-breakdown', year] as const, + + /** Completed books for specific year: ['completed-books', year] */ + completedBooks: (year: number) => ['completed-books', year] as const, + }, + + // ============================================================================ + // SHELVES + // ============================================================================ + shelf: { + /** Base key for all shelf queries: ['shelf'] */ + base: () => ['shelf'] as const, + + /** Shelf by ID (for invalidation/cancellation): ['shelf', id] */ + byId: (id: number) => ['shelf', id] as const, + + /** Shelf detail with books and sorting: ['shelf', id, 'books', options] */ + detail: (id: number, options: { orderBy?: string; direction?: string }) => + ['shelf', id, 'books', options] as const, + }, + + // ============================================================================ + // SERIES + // ============================================================================ + series: { + /** All series: ['series'] */ + all: () => ['series'] as const, + + /** Series detail by name: ['series', name] */ + detail: (name: string) => ['series', name] as const, + }, + + // ============================================================================ + // READ NEXT + // ============================================================================ + readNext: { + /** Base key for read next queries: ['read-next-books'] */ + base: () => ['read-next-books'] as const, + + /** Read next books with optional search: ['read-next-books', search] */ + books: (search?: string) => ['read-next-books', search] as const, + }, + + // ============================================================================ + // JOURNAL + // ============================================================================ + journal: { + /** Base key for journal entries (prefix matching): ['journal-entries'] */ + entriesBase: () => ['journal-entries'] as const, + + /** Base key for journal archive (prefix matching): ['journal-archive'] */ + archiveBase: () => ['journal-archive'] as const, + + /** Journal entries for timezone: ['journal-entries', timezone] */ + entries: (timezone: string) => ['journal-entries', timezone] as const, + + /** Journal archive for timezone: ['journal-archive', timezone] */ + archive: (timezone: string) => ['journal-archive', timezone] as const, + }, + + // ============================================================================ + // TAGS + // ============================================================================ + tags: { + /** Base key for all tag queries: ['tags'] */ + base: () => ['tags'] as const, + + /** Books for a specific tag: ['tag-books', tagId] */ + books: (tagId: number) => ['tag-books', tagId] as const, + }, + + // ============================================================================ + // DASHBOARD + // ============================================================================ + dashboard: { + /** Dashboard data: ['dashboard'] */ + all: () => ['dashboard'] as const, + }, + + // ============================================================================ + // STATS + // ============================================================================ + stats: { + /** Stats page data: ['stats'] */ + all: () => ['stats'] as const, + }, + + // ============================================================================ + // VERSION + // ============================================================================ + version: { + /** App version and update info: ['version'] */ + info: () => ['version'] as const, + }, +} as const; diff --git a/lib/repositories/progress.repository.ts b/lib/repositories/progress.repository.ts index 71db3937..cc4c0fe3 100644 --- a/lib/repositories/progress.repository.ts +++ b/lib/repositories/progress.repository.ts @@ -19,18 +19,24 @@ export class ProgressRepository extends BaseRepository< /** * Find all progress logs for a book, sorted by date (descending) + * Uses stable multi-column sort to handle multiple entries on same date (fix for #399) */ async findByBookId(bookId: number): Promise<ProgressLog[]> { return this.getDatabase() .select() .from(progressLogs) .where(eq(progressLogs.bookId, bookId)) - .orderBy(desc(progressLogs.progressDate)) + .orderBy( + desc(progressLogs.progressDate), + desc(progressLogs.createdAt), + desc(progressLogs.id) + ) .all(); } /** * Find progress logs for a specific session + * Uses stable multi-column sort to handle multiple entries on same date (fix for #399) */ async findBySessionId(sessionId: number, tx?: any): Promise<ProgressLog[]> { const database = tx || this.getDatabase(); @@ -38,25 +44,35 @@ export class ProgressRepository extends BaseRepository< .select() .from(progressLogs) .where(eq(progressLogs.sessionId, sessionId)) - .orderBy(desc(progressLogs.progressDate)) + .orderBy( + desc(progressLogs.progressDate), + desc(progressLogs.createdAt), + desc(progressLogs.id) + ) .all(); } /** * Find latest progress for a book + * Uses stable multi-column sort to handle multiple entries on same date (fix for #399) */ async findLatestByBookId(bookId: number): Promise<ProgressLog | undefined> { return this.getDatabase() .select() .from(progressLogs) .where(eq(progressLogs.bookId, bookId)) - .orderBy(desc(progressLogs.progressDate)) + .orderBy( + desc(progressLogs.progressDate), + desc(progressLogs.createdAt), + desc(progressLogs.id) + ) .limit(1) .get(); } /** * Find latest progress for a session + * Uses stable multi-column sort to handle multiple entries on same date (fix for #399) */ async findLatestBySessionId(sessionId: number, tx?: any): Promise<ProgressLog | undefined> { const database = tx || this.getDatabase(); @@ -64,13 +80,18 @@ export class ProgressRepository extends BaseRepository< .select() .from(progressLogs) .where(eq(progressLogs.sessionId, sessionId)) - .orderBy(desc(progressLogs.progressDate)) + .orderBy( + desc(progressLogs.progressDate), + desc(progressLogs.createdAt), + desc(progressLogs.id) + ) .limit(1) .get(); } /** * Find progress logs for a book and session + * Uses stable multi-column sort to handle multiple entries on same date (fix for #399) */ async findByBookIdAndSessionId(bookId: number, sessionId: number): Promise<ProgressLog[]> { return this.getDatabase() @@ -79,12 +100,17 @@ export class ProgressRepository extends BaseRepository< .where( and(eq(progressLogs.bookId, bookId), eq(progressLogs.sessionId, sessionId)) ) - .orderBy(desc(progressLogs.progressDate)) + .orderBy( + desc(progressLogs.progressDate), + desc(progressLogs.createdAt), + desc(progressLogs.id) + ) .all(); } /** * Find latest progress for a book and session + * Uses stable multi-column sort to handle multiple entries on same date (fix for #399) */ async findLatestByBookIdAndSessionId( bookId: number, @@ -96,13 +122,18 @@ export class ProgressRepository extends BaseRepository< .where( and(eq(progressLogs.bookId, bookId), eq(progressLogs.sessionId, sessionId)) ) - .orderBy(desc(progressLogs.progressDate)) + .orderBy( + desc(progressLogs.progressDate), + desc(progressLogs.createdAt), + desc(progressLogs.id) + ) .limit(1) .get(); } /** * Find progress logs after a certain date + * Uses stable multi-column sort to handle multiple entries on same date (fix for #399) * * @param dateString - Date in YYYY-MM-DD format (UTC calendar day) * @returns Progress logs on or after the specified date @@ -117,13 +148,18 @@ export class ProgressRepository extends BaseRepository< .select() .from(progressLogs) .where(gte(progressLogs.progressDate, dateString)) - .orderBy(asc(progressLogs.progressDate)) + .orderBy( + asc(progressLogs.progressDate), + asc(progressLogs.createdAt), + asc(progressLogs.id) + ) .all(); } /** * Find progress entries BEFORE a specific date for a session * Used to validate that new entry has progress ≥ all previous entries + * Uses stable multi-column sort to handle multiple entries on same date (fix for #399) * @param date - Date string in YYYY-MM-DD format */ async findBeforeDateForSession( @@ -139,13 +175,18 @@ export class ProgressRepository extends BaseRepository< lt(progressLogs.progressDate, date) ) ) - .orderBy(desc(progressLogs.progressDate)) + .orderBy( + desc(progressLogs.progressDate), + desc(progressLogs.createdAt), + desc(progressLogs.id) + ) .all(); } /** * Find progress entries AFTER a specific date for a session * Used to validate that new entry has progress ≤ all future entries + * Uses stable multi-column sort to handle multiple entries on same date (fix for #399) * @param date - Date string in YYYY-MM-DD format */ async findAfterDateForSession( @@ -161,12 +202,17 @@ export class ProgressRepository extends BaseRepository< gt(progressLogs.progressDate, date) ) ) - .orderBy(asc(progressLogs.progressDate)) + .orderBy( + asc(progressLogs.progressDate), + asc(progressLogs.createdAt), + asc(progressLogs.id) + ) .all(); } /** * Find the closest progress entry before a date + * Uses stable multi-column sort to handle multiple entries on same date (fix for #399) * * @param sessionId - Reading session ID * @param dateString - Date in YYYY-MM-DD format (UTC calendar day) @@ -190,13 +236,18 @@ export class ProgressRepository extends BaseRepository< lt(progressLogs.progressDate, dateString) ) ) - .orderBy(desc(progressLogs.progressDate)) + .orderBy( + desc(progressLogs.progressDate), + desc(progressLogs.createdAt), + desc(progressLogs.id) + ) .limit(1) .get(); } /** * Find the closest progress entry after a date + * Uses stable multi-column sort to handle multiple entries on same date (fix for #399) * * @param sessionId - Reading session ID * @param dateString - Date in YYYY-MM-DD format (UTC calendar day) @@ -220,7 +271,11 @@ export class ProgressRepository extends BaseRepository< gt(progressLogs.progressDate, dateString) ) ) - .orderBy(asc(progressLogs.progressDate)) + .orderBy( + asc(progressLogs.progressDate), + asc(progressLogs.createdAt), + asc(progressLogs.id) + ) .limit(1) .get(); } @@ -511,12 +566,17 @@ export class ProgressRepository extends BaseRepository< /** * Get all progress logs ordered by date + * Uses stable multi-column sort to handle multiple entries on same date (fix for #399) */ async getAllProgressOrdered(): Promise<ProgressLog[]> { return this.getDatabase() .select() .from(progressLogs) - .orderBy(asc(progressLogs.progressDate)) + .orderBy( + asc(progressLogs.progressDate), + asc(progressLogs.createdAt), + asc(progressLogs.id) + ) .all(); } diff --git a/lib/repositories/session.repository.ts b/lib/repositories/session.repository.ts index 8146acac..e9ef7e83 100644 --- a/lib/repositories/session.repository.ts +++ b/lib/repositories/session.repository.ts @@ -532,8 +532,9 @@ export class SessionRepository extends BaseRepository< * Get the next available read_next_order value * Returns max(read_next_order) + 1, or 0 if no read-next books exist */ - async getNextReadNextOrder(): Promise<number> { - const result = this.getDatabase() + async getNextReadNextOrder(tx?: any): Promise<number> { + const database = tx || this.getDatabase(); + const result = database .select({ maxOrder: sql<number>`COALESCE(MAX(${readingSessions.readNextOrder}), -1)` }) .from(readingSessions) .where(eq(readingSessions.status, 'read-next')) diff --git a/lib/services/journal.service.ts b/lib/services/journal.service.ts index 5c3328f5..1bb67fa4 100644 --- a/lib/services/journal.service.ts +++ b/lib/services/journal.service.ts @@ -81,7 +81,11 @@ export class JournalService { }) .from(progressLogs) .innerJoin(books, eq(progressLogs.bookId, books.id)) - .orderBy(desc(progressLogs.progressDate), desc(progressLogs.createdAt)) + .orderBy( + desc(progressLogs.progressDate), + desc(progressLogs.createdAt), + desc(progressLogs.id) + ) .limit(limit) .offset(skip) .all(); diff --git a/lib/services/progress.service.ts b/lib/services/progress.service.ts index 39fbaa01..49f66034 100644 --- a/lib/services/progress.service.ts +++ b/lib/services/progress.service.ts @@ -349,7 +349,21 @@ export class ProgressService { const allProgress = await progressRepository.findBySessionId(session.id); const sortedProgress = allProgress .filter(p => p.id !== progressId) // Exclude the entry being edited - .sort((a, b) => a.progressDate.localeCompare(b.progressDate)); // ADR-014: Lexicographic string comparison + .sort((a, b) => { + // Stable multi-column sort to handle multiple entries on same date (fix for #399) + // Primary: Sort by progressDate (YYYY-MM-DD string) + const dateCompare = a.progressDate.localeCompare(b.progressDate); + if (dateCompare !== 0) return dateCompare; + + // Secondary: Sort by createdAt timestamp (chronological order within same date) + if (a.createdAt && b.createdAt) { + const timeCompare = a.createdAt.getTime() - b.createdAt.getTime(); + if (timeCompare !== 0) return timeCompare; + } + + // Tertiary: Sort by ID (database insertion order as final tiebreaker) + return a.id - b.id; + }); // Find the entry immediately before this one by date (use findLast to get the closest previous entry) // ADR-014: Use lexicographic string comparison for dates @@ -507,6 +521,7 @@ export class ProgressService { revalidatePath("/stats"); // Stats page revalidatePath("/journal"); // Journal page revalidatePath(`/books/${bookId}`); // Book detail page + revalidatePath("/series"); // Series pages (list and detail) } catch (error) { getLogger().error({ err: error }, "[ProgressService] Failed to invalidate cache"); // Don't fail the request if cache invalidation fails diff --git a/lib/services/session.service.ts b/lib/services/session.service.ts index cf0d8a6a..946a14b8 100644 --- a/lib/services/session.service.ts +++ b/lib/services/session.service.ts @@ -5,6 +5,21 @@ import { calibreService } from "@/lib/services/calibre.service"; import { progressService } from "@/lib/services/progress.service"; import { getLogger } from "@/lib/logger"; +/** + * Custom error class for session validation errors + */ +export class SessionValidationError extends Error { + public readonly code: string; + public readonly httpStatus: number; + + constructor(message: string, code: string, httpStatus: number = 400) { + super(message); + this.name = 'SessionValidationError'; + this.code = code; + this.httpStatus = httpStatus; + } +} + /** * Status update data structure */ @@ -259,50 +274,109 @@ export class SessionService { // Validate page count requirement for reading/read status if ((status === "reading" || status === "read") && !book.totalPages) { - const error = new Error("Page count required. Please set the total number of pages before starting to read."); - (error as any).code = "PAGES_REQUIRED"; - throw error; + throw new SessionValidationError( + "Page count required. Please set the total number of pages before starting to read.", + "PAGES_REQUIRED", + 400 + ); } - // Find active reading session or prepare to create new one + // Find active or most recent session + // Terminal sessions ("read", "dnf") are archived (isActive = false per ADR-004/008) + // Re-reading a book reactivates via startReread() which creates a new active session + // Fall back to latest session for legacy data or edge cases let readingSession = await sessionRepository.findActiveByBookId(bookId, tx); + const mostRecentSession = !readingSession ? await sessionRepository.findLatestByBookId(bookId, tx) : null; + const oldStatus = readingSession?.status; - // Detect "backward movement" from "reading" to planning statuses - const isBackwardMovement = - readingSession && - readingSession.status === "reading" && - (status === "read-next" || status === "to-read"); + // Block direct DNF → read transition (must go through "reading" with progress) + const sessionToCheck = readingSession || mostRecentSession; + if (sessionToCheck?.status === "dnf" && status === "read") { + throw new SessionValidationError( + "Cannot mark DNF book as read directly. Please restart reading first.", + "DNF_TO_READ_BLOCKED", + 400 + ); + } - // Check if current session has progress - let hasProgress = false; - if (isBackwardMovement && readingSession) { - hasProgress = await progressRepository.hasProgressForSession(readingSession.id, tx); + // Detect "backward movement" from terminal states to planning/reading statuses + // Case 1: From "reading" to planning statuses (existing logic) + // Case 2: From "dnf" to any non-DNF status (new logic - DNF is terminal) + // Use mostRecentSession as fallback for edge cases where no active session exists + // After terminal state changes, both "read" and "dnf" sessions have isActive=false + const sessionForBackwardCheck = readingSession || mostRecentSession; + + const isReadingToPlanning = + sessionForBackwardCheck?.status === "reading" && + (status === "read-next" || status === "to-read"); + + const isDnfToAnyOther = + sessionForBackwardCheck?.status === "dnf" && + status !== "dnf"; + + const isBackwardMovement = + sessionForBackwardCheck && (isReadingToPlanning || isDnfToAnyOther); + + // Check if current session should be archived + // DNF sessions: Always archive (already terminal) + // Reading sessions: Only archive if they have progress + let shouldArchive = false; + if (isBackwardMovement && sessionForBackwardCheck) { + if (sessionForBackwardCheck.status === "dnf") { + shouldArchive = true; + } else if (sessionForBackwardCheck.status === "reading") { + shouldArchive = await progressRepository.hasProgressForSession(sessionForBackwardCheck.id, tx); + } } - // If moving backward with progress, archive current session and create new one - if (isBackwardMovement && hasProgress && readingSession) { - getLogger().info(`[SessionService] Archiving session #${readingSession.sessionNumber} and creating new session for backward movement`); + // If moving backward with progress (or from DNF), archive current session and create new one + if (isBackwardMovement && shouldArchive && sessionForBackwardCheck) { + getLogger().info(`[SessionService] Archiving session #${sessionForBackwardCheck.sessionNumber} and creating new session for backward movement`); + + // Helper to determine completedDate for archiving + // DNF sessions: Preserve existing completedDate (already set by markAsDNF) + // Reading sessions: Use last progress date or current date + const getArchiveCompletedDate = async (session: ReadingSession): Promise<string> => { + if (session.completedDate) return session.completedDate; + + if (session.status === "reading") { + const latestProgress = await progressRepository.findLatestBySessionId(session.id, tx); + return latestProgress?.progressDate || await this.getTodayDateString(); + } + + return await this.getTodayDateString(); + }; - // Get last progress date for completedDate (use last activity or current date) - const latestProgress = await progressRepository.findLatestBySessionId(readingSession.id, tx); - const archiveCompletedDate = latestProgress?.progressDate || await this.getTodayDateString(); + const archiveCompletedDate = await getArchiveCompletedDate(sessionForBackwardCheck); // Archive current session WITH completedDate - await sessionRepository.update(readingSession.id, { + await sessionRepository.update(sessionForBackwardCheck.id, { isActive: false, completedDate: archiveCompletedDate, } as any, tx); // Create new session with new status - const newSessionNumber = readingSession.sessionNumber + 1; - const newSession = await sessionRepository.create({ - userId: readingSession.userId, + const newSessionNumber = sessionForBackwardCheck.sessionNumber + 1; + const newSessionData: any = { + userId: sessionForBackwardCheck.userId, bookId, sessionNumber: newSessionNumber, status: status as any, isActive: true, - }, tx); + }; + + // Auto-set startedDate for "reading" status + if (status === "reading") { + newSessionData.startedDate = startedDate || await this.getTodayDateString(); + } + + // Ensure proper ordering for "read-next" status (prevents duplicate order=0) + if (status === "read-next") { + newSessionData.readNextOrder = await sessionRepository.getNextReadNextOrder(tx); + } + + const newSession = await sessionRepository.create(newSessionData, tx); // Rebuild streak to ensure consistency (only if not in transaction) if (!tx) { @@ -317,7 +391,7 @@ export class SessionService { return { session: newSession, sessionArchived: true, - archivedSessionNumber: readingSession.sessionNumber, + archivedSessionNumber: sessionForBackwardCheck.sessionNumber, }; } @@ -345,7 +419,7 @@ export class SessionService { updateData.startedDate = startedDate || await this.getTodayDateString(); } updateData.completedDate = completedDate || await this.getTodayDateString(); - // Keep session active for terminal "read" state (archived only on re-read) + updateData.isActive = false; // Archive read session (terminal state per ADR-004/008) } if (review !== undefined) { @@ -1078,7 +1152,7 @@ export class SessionService { }; } - // Mark session as DNF (keep active - archived only on re-read) + // Mark session as DNF (archives session with isActive=false per ADR-004/008) const finalCompletedDate = completedDate || lastProgress?.progressDate || await this.getTodayDateString(); logger.info({ bookId, sessionId: activeSession.id, completedDate: finalCompletedDate }, "Marking session as DNF"); @@ -1086,6 +1160,7 @@ export class SessionService { await sessionRepository.update(activeSession.id, { status: "dnf", completedDate: finalCompletedDate, + isActive: false, // Archive DNF session (terminal state) } as any); // Best-effort: Update rating @@ -1287,6 +1362,7 @@ export class SessionService { revalidatePath("/stats"); // Stats page revalidatePath("/journal"); // Journal page revalidatePath(`/books/${bookId}`); // Book detail page + revalidatePath("/series"); // Series pages (list and detail) } catch (error) { getLogger().error({ err: error }, "[SessionService] Failed to invalidate cache"); // Don't fail the request if cache invalidation fails diff --git a/package.json b/package.json index af01a1f2..6dea0f35 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "tome", - "version": "0.6.7", + "version": "0.6.8", "description": "A Calibre-integrated book tracker", "homepage": "https://github.com/masonfox/tome", "author": "Mason Fox <masonfox22@gmail.com> (https://github.com/masonfox)",