-
-
Notifications
You must be signed in to change notification settings - Fork 6
Fix: Session numbering after deletion (Issue #413) #416
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
4f516d9
fix: correct session numbering after deletion (issue #413)
masonfox 5ca27f3
fix: only assign displayNumber to sessions shown in Reading History
masonfox f51acf4
fix: resolve timezone-based flaky test in streaks-coverage
masonfox 1d0cbea
fix: implement Copilot feedback (iteration 1)
masonfox e3e255b
fix: reset modal state when closing to prevent stuck 'Deleting...' bu…
masonfox File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
299 changes: 299 additions & 0 deletions
299
__tests__/integration/issues/issue-413-session-number-after-deletion.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,299 @@ | ||
| /** | ||
| * 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 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 | ||
| const sessionsWithDisplay = await sessionService.getSessionsWithDisplayNumbers(bookId); | ||
| expect(sessionsWithDisplay).toHaveLength(2); | ||
| expect(sessionsWithDisplay[0].displayNumber).toBe(1); | ||
|
masonfox marked this conversation as resolved.
|
||
| expect(sessionsWithDisplay[1].displayNumber).toBe(2); | ||
| }); | ||
|
|
||
| test("should only assign displayNumber to sessions that match display filter", async () => { | ||
| // Create an active "to-read" session (should NOT get displayNumber - active and not read/dnf) | ||
| const toReadSession = await sessionRepository.create({ | ||
| bookId, | ||
| sessionNumber: 1, | ||
| status: "to-read", | ||
| isActive: true, | ||
| }); | ||
|
|
||
| // Archive the to-read session (WILL get displayNumber=1 - archived sessions shown) | ||
| await sessionRepository.update(toReadSession.id, { isActive: false }); | ||
|
|
||
| // Create an active "reading" session (should NOT get displayNumber - active and not read/dnf) | ||
| const readingSession = await sessionRepository.create({ | ||
| bookId, | ||
| sessionNumber: 2, | ||
| status: "reading", | ||
| isActive: true, | ||
| startedDate: "2024-02-01", | ||
| }); | ||
|
|
||
| // Archive the reading session (WILL get displayNumber=2 - archived sessions shown) | ||
| await sessionRepository.update(readingSession.id, { isActive: false }); | ||
|
|
||
| // Create a completed "read" session (SHOULD get displayNumber=3) | ||
| const readSession1 = await sessionRepository.create({ | ||
| bookId, | ||
| sessionNumber: 3, | ||
| status: "read", | ||
| isActive: false, | ||
| startedDate: "2024-03-01", | ||
| completedDate: "2024-03-15", | ||
| }); | ||
|
|
||
| // Create a "dnf" session (SHOULD get displayNumber=4) | ||
| 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 (SHOULD get displayNumber=5 - status=read) | ||
| 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); | ||
|
|
||
| // Archived sessions should have displayNumbers (filter: !isActive = true) | ||
| expect(toRead?.displayNumber).toBe(1); | ||
| expect(reading?.displayNumber).toBe(2); | ||
|
|
||
| // Completed/DNF sessions should have displayNumbers | ||
| expect(read1?.displayNumber).toBe(3); | ||
| expect(dnf?.displayNumber).toBe(4); | ||
| expect(read2?.displayNumber).toBe(5); | ||
| }); | ||
| }); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.