diff --git a/__tests__/integration/issues/issue-413-session-number-after-deletion.test.ts b/__tests__/integration/issues/issue-413-session-number-after-deletion.test.ts new file mode 100644 index 00000000..d605e64b --- /dev/null +++ b/__tests__/integration/issues/issue-413-session-number-after-deletion.test.ts @@ -0,0 +1,341 @@ +/** + * Integration test for Issue #413: + * Session numbers should display correctly after deletion + * + * Bug: When a session is deleted and a new one is created, the session number + * increments incorrectly (shows "Read #2" when it should be "Read #1") + * + * Fix: + * 1. Use getNextSessionNumber() instead of hardcoded 1 in deleteSession() + * 2. Calculate displayNumber based on array index from chronologically ordered sessions + */ + +import { describe, test, expect, beforeAll, beforeEach, afterAll } from "vitest"; +import { setupTestDatabase, clearTestDatabase, teardownTestDatabase, type TestDatabaseInstance } from "@/__tests__/helpers/db-setup"; +import { sessionService } from "@/lib/services/session.service"; +import { sessionRepository, bookRepository } from "@/lib/repositories"; + +const TEST_FILE_PATH = __filename; +let testDbInstance: TestDatabaseInstance; +let bookId: number; + +beforeAll(async () => { + testDbInstance = await setupTestDatabase(TEST_FILE_PATH); +}); + +beforeEach(async () => { + await clearTestDatabase(testDbInstance); + + // Create a test book + const book = await bookRepository.create({ + calibreId: 1, + title: "Test Book", + authors: ["Test Author"], + path: "/test/path", + totalPages: 100, + }); + bookId = book.id; +}); + +afterAll(async () => { + await teardownTestDatabase(testDbInstance); +}); + +describe("Issue #413 - Session Number After Deletion", () => { + + test("should use getNextSessionNumber instead of hardcoded 1 when creating session after deletion", async () => { + // Step 1: Create a "read" session + const session1 = await sessionRepository.create({ + bookId, + sessionNumber: 1, + status: "read", + isActive: false, + startedDate: "2024-01-01", + completedDate: "2024-01-15", + }); + + // Verify session exists + let sessions = await sessionRepository.findAllByBookId(bookId); + expect(sessions).toHaveLength(1); + expect(sessions[0].sessionNumber).toBe(1); + + // Step 2: Delete the session - deleteSession should create new "to-read" session + await sessionService.deleteSession(bookId, session1.id); + + // Verify deleteSession created a new "to-read" session + sessions = await sessionRepository.findAllByBookId(bookId); + expect(sessions).toHaveLength(1); + expect(sessions[0].status).toBe("to-read"); + + // CRITICAL FIX: After deleting session #1, the new "to-read" session gets the NEXT number + // Since we deleted session #1, getNextSessionNumber() looks at remaining sessions (none), + // so it returns 1. Before the fix, it was hardcoded to 1, now it's calculated. + // The key is that it's using getNextSessionNumber() to avoid conflicts. + expect(sessions[0].sessionNumber).toBe(1); + + // Step 3: Archive the "to-read" session and create another "read" session + await sessionRepository.update(sessions[0].id, { isActive: false }); + + const session2 = await sessionRepository.create({ + bookId, + sessionNumber: await sessionRepository.getNextSessionNumber(bookId), + status: "read", + isActive: true, + startedDate: "2024-01-20", + completedDate: "2024-02-01", + }); + + // The new session should get sessionNumber 2 (getNextSessionNumber finds 1, returns 2) + // This is the correct behavior - continuous numbering + expect(session2.sessionNumber).toBe(2); + }); + + test("should assign next session number correctly when other sessions exist after deletion", async () => { + // Create multiple sessions + const session1 = await sessionRepository.create({ + bookId, + sessionNumber: 1, + status: "read", + isActive: false, + startedDate: "2024-01-01", + completedDate: "2024-01-15", + }); + + const session2 = await sessionRepository.create({ + bookId, + sessionNumber: 2, + status: "read", + isActive: false, + startedDate: "2024-02-01", + completedDate: "2024-02-15", + }); + + // Verify both exist + let sessions = await sessionRepository.findAllByBookId(bookId); + expect(sessions).toHaveLength(2); + + // Delete session #1 + await sessionService.deleteSession(bookId, session1.id); + + // Verify to-read session was created with sessionNumber=3 (not 1!) + // This proves getNextSessionNumber() is being used, not hardcoded 1 + sessions = await sessionRepository.findAllByBookId(bookId); + const toReadSession = sessions.find(s => s.status === "to-read"); + expect(toReadSession).toBeDefined(); + expect(toReadSession!.sessionNumber).toBe(3); // CRITICAL: Must be 3, not 1 + + // Should still have session2 with sessionNumber=2 + const readSession = sessions.find(s => s.sessionNumber === 2); + expect(readSession).toBeDefined(); + }); + + test("should calculate display numbers based on chronological order", async () => { + // Create 3 sessions at different times + const session1 = await sessionRepository.create({ + bookId, + sessionNumber: 1, + status: "read", + isActive: false, + startedDate: "2024-01-01", + completedDate: "2024-01-15", + }); + + const session2 = await sessionRepository.create({ + bookId, + sessionNumber: 2, + status: "read", + isActive: false, + startedDate: "2024-02-01", + completedDate: "2024-02-15", + }); + + const session3 = await sessionRepository.create({ + bookId, + sessionNumber: 3, + status: "read", + isActive: true, + startedDate: "2024-03-01", + completedDate: "2024-03-15", + }); + + // Get ordered sessions + const orderedSessions = await sessionRepository.findAllByBookIdOrdered(bookId); + + // Should have 3 sessions ordered chronologically + expect(orderedSessions).toHaveLength(3); + expect(orderedSessions[0].startedDate).toBe("2024-01-01"); + expect(orderedSessions[1].startedDate).toBe("2024-02-01"); + expect(orderedSessions[2].startedDate).toBe("2024-03-01"); + + // Get sessions with display numbers + const sessionsWithDisplay = await sessionService.getSessionsWithDisplayNumbers(bookId); + + // Display numbers should be 1, 2, 3 (based on array position) + expect(sessionsWithDisplay[0].displayNumber).toBe(1); + expect(sessionsWithDisplay[1].displayNumber).toBe(2); + expect(sessionsWithDisplay[2].displayNumber).toBe(3); + }); + + test("should renumber display numbers after deleting a session", async () => { + // Create 3 sessions + const session1 = await sessionRepository.create({ + bookId, + sessionNumber: 1, + status: "read", + isActive: false, + startedDate: "2024-01-01", + completedDate: "2024-01-15", + }); + + const session2 = await sessionRepository.create({ + bookId, + sessionNumber: 2, + status: "read", + isActive: false, + startedDate: "2024-02-01", + completedDate: "2024-02-15", + }); + + const session3 = await sessionRepository.create({ + bookId, + sessionNumber: 3, + status: "read", + isActive: true, + startedDate: "2024-03-01", + completedDate: "2024-03-15", + }); + + // Verify display numbers before deletion + let sessionsWithDisplay = await sessionService.getSessionsWithDisplayNumbers(bookId); + expect(sessionsWithDisplay.filter(s => s.status === "read")).toHaveLength(3); + + // Delete the second session + await sessionService.deleteSession(bookId, session2.id); + + // Get updated sessions + sessionsWithDisplay = await sessionService.getSessionsWithDisplayNumbers(bookId); + const completedSessions = sessionsWithDisplay.filter(s => s.status === "read"); + + // Should now have 2 "read" sessions (1st and 3rd) + expect(completedSessions).toHaveLength(2); + + // Display numbers should be renumbered to 1 and 2 (no gaps!) + expect(completedSessions[0].displayNumber).toBe(1); + expect(completedSessions[0].startedDate).toBe("2024-01-01"); + + expect(completedSessions[1].displayNumber).toBe(2); + expect(completedSessions[1].startedDate).toBe("2024-03-01"); + }); + + test("should handle sessions with null startedDate using createdAt fallback", async () => { + // Create session1 with startedDate + const session1 = await sessionRepository.create({ + bookId, + sessionNumber: 1, + status: "read", + isActive: false, + startedDate: "2024-02-01", + completedDate: "2024-02-15", + }); + + // Create session2 without startedDate + const session2 = await sessionRepository.create({ + bookId, + sessionNumber: 2, + status: "read", + isActive: false, + startedDate: null, + completedDate: "2024-01-15", + }); + + // Get ordered sessions - should use COALESCE(startedDate, createdAt) + const orderedSessions = await sessionRepository.findAllByBookIdOrdered(bookId); + expect(orderedSessions).toHaveLength(2); + + // Verify displayNumbers are calculated AND ordering is correct + // Session2 (null startedDate) was created second, so createdAt is later + // Therefore session1 should be first chronologically + const sessionsWithDisplay = await sessionService.getSessionsWithDisplayNumbers(bookId); + expect(sessionsWithDisplay).toHaveLength(2); + expect(sessionsWithDisplay[0].sessionNumber).toBe(1); // session1 comes first + expect(sessionsWithDisplay[0].displayNumber).toBe(1); + expect(sessionsWithDisplay[1].sessionNumber).toBe(2); // session2 comes second + expect(sessionsWithDisplay[1].displayNumber).toBe(2); + }); + + test("should only assign displayNumber to sessions that match display filter", async () => { + // Create an archived "to-read" session without startedDate (will use createdAt ~ today) + const toReadSession = await sessionRepository.create({ + bookId, + sessionNumber: 1, + status: "to-read", + isActive: false, // Archived immediately + }); + + // Create an archived "reading" session with startedDate in 2024 + const readingSession = await sessionRepository.create({ + bookId, + sessionNumber: 2, + status: "reading", + isActive: false, // Archived immediately + startedDate: "2024-02-01", + }); + + // Create a completed "read" session + const readSession1 = await sessionRepository.create({ + bookId, + sessionNumber: 3, + status: "read", + isActive: false, + startedDate: "2024-03-01", + completedDate: "2024-03-15", + }); + + // Create a "dnf" session + const dnfSession = await sessionRepository.create({ + bookId, + sessionNumber: 4, + status: "dnf", + isActive: false, + startedDate: "2024-04-01", + completedDate: "2024-04-10", + }); + + // Create another completed "read" session that's active + const readSession2 = await sessionRepository.create({ + bookId, + sessionNumber: 5, + status: "read", + isActive: true, + startedDate: "2024-05-01", + completedDate: "2024-05-15", + }); + + // Get sessions with display numbers + const sessionsWithDisplay = await sessionService.getSessionsWithDisplayNumbers(bookId); + + // Should have 5 total sessions + expect(sessionsWithDisplay).toHaveLength(5); + + // Find each session by sessionNumber + const toRead = sessionsWithDisplay.find(s => s.sessionNumber === 1); + const reading = sessionsWithDisplay.find(s => s.sessionNumber === 2); + const read1 = sessionsWithDisplay.find(s => s.sessionNumber === 3); + const dnf = sessionsWithDisplay.find(s => s.sessionNumber === 4); + const read2 = sessionsWithDisplay.find(s => s.sessionNumber === 5); + + // Expected chronological order based on startedDate (or createdAt fallback): + // 1. reading: 2024-02-01 + // 2. read1: 2024-03-01 + // 3. dnf: 2024-04-01 + // 4. read2: 2024-05-01 + // 5. toRead: ~2026-04-14 (createdAt converted to YYYY-MM-DD, created today) + + // All sessions match the display filter (!isActive || status=='read' || status=='dnf') + expect(reading?.displayNumber).toBe(1); + expect(read1?.displayNumber).toBe(2); + expect(dnf?.displayNumber).toBe(3); + expect(read2?.displayNumber).toBe(4); + expect(toRead?.displayNumber).toBe(5); + }); +}); diff --git a/__tests__/integration/services/streaks-coverage.test.ts b/__tests__/integration/services/streaks-coverage.test.ts index 7294bf41..91861622 100644 --- a/__tests__/integration/services/streaks-coverage.test.ts +++ b/__tests__/integration/services/streaks-coverage.test.ts @@ -2,8 +2,6 @@ import { describe, test, expect, beforeAll, afterAll, beforeEach } from 'vitest' import { streakService } from "@/lib/services/streak.service"; import { bookRepository, sessionRepository, progressRepository, streakRepository } from "@/lib/repositories"; import { setupTestDatabase, teardownTestDatabase, clearTestDatabase } from "@/__tests__/helpers/db-setup"; -import { startOfDay } from "date-fns"; -import { toZonedTime, fromZonedTime } from "date-fns-tz"; import { toProgressDate, toSessionDate } from "../../test-utils"; import { toDateString } from "@/utils/dateHelpers.server"; diff --git a/app/api/books/[id]/sessions/route.ts b/app/api/books/[id]/sessions/route.ts index ae43f5cc..c87c04ad 100644 --- a/app/api/books/[id]/sessions/route.ts +++ b/app/api/books/[id]/sessions/route.ts @@ -1,6 +1,7 @@ import { getLogger } from "@/lib/logger"; import { NextRequest, NextResponse } from "next/server"; import { bookRepository, sessionRepository } from "@/lib/repositories"; +import { sessionService } from "@/lib/services/session.service"; export const dynamic = 'force-dynamic'; @@ -22,7 +23,21 @@ export async function GET(request: NextRequest, props: { params: Promise<{ id: s // OPTIMIZED: Get all reading sessions with progress summaries in a single query const sessionsWithProgress = await sessionRepository.findAllByBookIdWithProgress(bookId); - return NextResponse.json(sessionsWithProgress, { + // Get display numbers using service layer (single source of truth) + const sessionsWithDisplayNumbers = await sessionService.getSessionsWithDisplayNumbers(bookId); + + // Create a map of sessionId -> displayNumber + const displayNumberMap = new Map( + sessionsWithDisplayNumbers.map(s => [s.id, s.displayNumber]) + ); + + // Add displayNumber to sessions (preserving original sort order from findAllByBookIdWithProgress) + const result = sessionsWithProgress.map(session => ({ + ...session, + displayNumber: displayNumberMap.get(session.id), + })); + + return NextResponse.json(result, { headers: { 'Cache-Control': 'no-cache, no-store, must-revalidate', 'Pragma': 'no-cache', diff --git a/components/CurrentlyReading/ReadingHistoryTab.tsx b/components/CurrentlyReading/ReadingHistoryTab.tsx index 485d4b48..56f702c8 100644 --- a/components/CurrentlyReading/ReadingHistoryTab.tsx +++ b/components/CurrentlyReading/ReadingHistoryTab.tsx @@ -15,6 +15,7 @@ import MarkdownRenderer from "@/components/Markdown/MarkdownRenderer"; interface ReadingSession { id: number; sessionNumber: number; + displayNumber?: number; // Calculated display number (1st read, 2nd read, etc.) status: string; startedDate?: string; completedDate?: string; @@ -47,6 +48,7 @@ export default function ReadingHistoryTab({ bookId, bookTitle = "this book" }: R const [viewProgressModal, setViewProgressModal] = useState<{ sessionId: number; sessionNumber: number; + displayNumber?: number; } | null>(null); // Fetch sessions using TanStack Query - automatic caching and background refetching @@ -165,11 +167,11 @@ export default function ReadingHistoryTab({ bookId, bookTitle = "this book" }: R key={session.id} className="p-5 bg-[var(--background)] border border-[var(--border-color)] rounded-lg" > -
@@ -275,6 +278,7 @@ export default function ReadingHistoryTab({ bookId, bookTitle = "this book" }: R onConfirm={handleSaveSession} bookTitle={bookTitle} sessionNumber={editingSession?.sessionNumber ?? 0} + displayNumber={editingSession?.displayNumber} sessionId={editingSession?.id ?? 0} bookId={bookId} currentStartedDate={editingSession?.startedDate ?? null} @@ -287,6 +291,7 @@ export default function ReadingHistoryTab({ bookId, bookTitle = "this book" }: R onClose={handleCloseDeleteModal} onConfirm={handleConfirmDelete} sessionNumber={deletingSession?.sessionNumber ?? 0} + displayNumber={deletingSession?.displayNumber} progressCount={deletingSession?.progressSummary.totalEntries ?? 0} bookTitle={bookTitle} isActive={deletingSession?.isActive ?? false} @@ -300,6 +305,7 @@ export default function ReadingHistoryTab({ bookId, bookTitle = "this book" }: R bookId={bookId} bookTitle={bookTitle} sessionNumber={viewProgressModal.sessionNumber} + displayNumber={viewProgressModal.displayNumber} /> )}