Fix: Session numbering after deletion (Issue #413) by masonfox · Pull Request #416 · masonfox/tome

masonfox · 2026-04-14T01:26:08Z

Summary

Fixes #413 - Session numbers now display correctly after deleting sessions.

Problem: When a session was deleted and a new one was created, the session number would increment incorrectly (e.g., showing "Read # 2" when it should be "Read # 1").

Root Cause: The deleteSession() method hardcoded sessionNumber: 1 when creating a new "to-read" session after deletion, which conflicted with the getNextSessionNumber() logic that increments based on the highest existing session number.

Solution: Implemented array index-based display numbers that are calculated from chronologically ordered sessions.

Changes

Backend

Quick Fix: Updated deleteSession() to use getNextSessionNumber() instead of hardcoded 1
Repository: Added findAllByBookIdOrdered() method that sorts sessions by COALESCE(startedDate, createdAt) for chronological ordering
Service: Added getSessionsWithDisplayNumbers() method that calculates displayNumber based on array position (index + 1)
API: Updated /api/books/[id]/sessions endpoint to include displayNumber in responses

Frontend

Updated ReadingHistoryTab.tsx to display displayNumber (with fallback to sessionNumber)
Updated modal components to use displayNumber:
- SessionEditModal.tsx
- DeleteSessionModal.tsx
- SessionProgressModal.tsx

Testing

Added comprehensive integration tests in __tests__/integration/issues/issue-413-session-number-after-deletion.test.ts
All 4 new tests pass:
- ✅ Quick fix prevents session number conflicts
- ✅ Display numbers calculated chronologically
- ✅ Display numbers renumber after deletion (no gaps)
- ✅ Handles null startedDate with createdAt fallback
Full test suite: 4016 passing tests (up from 4012)

Behavior

Before:

Delete session # 1 → Creates "to-read" with sessionNumber: 1
Change to "Read" → Creates "read" with sessionNumber: 2 ❌ (should be # 1)

After:

Delete session # 1 → Creates "to-read" with sessionNumber: 1 (via getNextSessionNumber())
Change to "Read" → Creates "read" with sessionNumber: 2
Display number: Shows as "Read # 1" (calculated from array position) ✅

Key Features:

Display numbers are always sequential (1, 2, 3...) regardless of deletions
No gaps in numbering after deletion
Database sessionNumber preserved for backward compatibility
Display numbers represent "1st read, 2nd read, 3rd read" chronologically

Testing

npm test __tests__/integration/issues/issue-413-session-number-after-deletion.test.ts
# ✅ All 4 tests pass

npm test
# ✅ 4016 / 4017 tests pass (1 pre-existing failure in streaks)

Files Changed

lib/services/session.service.ts - Quick fix + display number service method
lib/repositories/session.repository.ts - Chronological ordering method
app/api/books/[id]/sessions/route.ts - Add displayNumber to API response
components/CurrentlyReading/ReadingHistoryTab.tsx - Display calculated numbers
components/Modals/*.tsx - Update modals to use displayNumber
__tests__/integration/issues/issue-413-session-number-after-deletion.test.ts - Integration tests

Manual Testing Needed

Delete a session and verify "Read #" numbers display correctly
Create multiple sessions, delete middle one, verify renumbering
Verify modals show correct "Read #X" titles

- Replace hardcoded sessionNumber=1 with getNextSessionNumber() in deleteSession() - Add findAllByBookIdOrdered() repository method for chronological sorting - Implement getSessionsWithDisplayNumbers() service method - Calculate displayNumber based on array index from chronologically ordered sessions - Update API endpoint to include displayNumber in responses - Update UI components (ReadingHistoryTab, modals) to display calculated numbers - Add comprehensive integration tests for session deletion and renumbering Fixes #413 - session numbers now display correctly after deletion Display numbers are sequential (1st read, 2nd read, 3rd read) without gaps Database sessionNumber preserved for backward compatibility

Filter sessions before calculating displayNumber to match the Reading History UI filter (!isActive || status=read || status=dnf). This ensures: - Active to-read/reading sessions don't get displayNumbers - Archived sessions (any status) get sequential displayNumbers - Completed read/dnf sessions get displayNumbers (even if active) - No gaps in numbering sequence Adds comprehensive integration test covering filter edge cases. Relates to #413

The 'should handle threshold change mid-day' test was failing between 8pm-midnight EST because it created progress using UTC dates while updateStreaks() queries using America/New_York timezone dates. When UTC and local dates differ (e.g., 11:50pm EST = 3:50am UTC next day), the test would create progress for April 14 (UTC) but updateStreaks would query for April 13 (EST), find no progress, and return early. Fix: Use getTodayInUserTimezone() helper to create progress dates in the same timezone that updateStreaks() uses (America/New_York). Relates to #413 (needed for PR merge)

codecov · 2026-04-14T03:55:31Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

@@             Coverage Diff             @@
##           develop     #416      +/-   ##
===========================================
+ Coverage    78.67%   78.85%   +0.18%     
===========================================
  Files          167      168       +1     
  Lines         7559     7620      +61     
  Branches      1850     1861      +11     
===========================================
+ Hits          5947     6009      +62     
  Misses        1127     1127              
+ Partials       485      484       -1

Flag	Coverage Δ
unittests	`78.85% <100.00%> (+0.18%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
app/api/books/[id]/sessions/route.ts	`88.23% <100.00%> (+3.61%)`	⬆️
lib/repositories/session.repository.ts	`79.27% <100.00%> (+0.38%)`	⬆️
lib/services/session.service.ts	`88.42% <100.00%> (+0.24%)`	⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot

Pull request overview

Fixes incorrect “Read #” display after deleting sessions by introducing a computed displayNumber derived from chronologically ordered sessions while keeping the persisted sessionNumber for compatibility.

Changes:

Update deleteSession() to compute the next sessionNumber via getNextSessionNumber() rather than hardcoding 1.
Add chronological session ordering + compute/return displayNumber from /api/books/[id]/sessions.
Update UI components/modals to render displayNumber (fallback to sessionNumber) and add integration tests for Issue #413.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
`lib/services/session.service.ts`	Adds `getSessionsWithDisplayNumbers()` and updates `deleteSession()` to use computed next session number.
`lib/repositories/session.repository.ts`	Adds `findAllByBookIdOrdered()` to order sessions chronologically for display numbering.
`app/api/books/[id]/sessions/route.ts`	Computes `displayNumber` and includes it in the sessions API response.
`components/CurrentlyReading/ReadingHistoryTab.tsx`	Displays `displayNumber` and passes it into modals/labels.
`components/Modals/SessionProgressModal.tsx`	Uses `displayNumber` (fallback to `sessionNumber`) in subtitle.
`components/Modals/SessionEditModal.tsx`	Uses `displayNumber` (fallback to `sessionNumber`) in title.
`components/Modals/DeleteSessionModal.tsx`	Uses `displayNumber` (fallback to `sessionNumber`) in confirmation header.
`__tests__/integration/services/streaks-coverage.test.ts`	Adjusts date handling for a streak test; adds a timezone helper.
`__tests__/integration/issues/issue-413-session-number-after-deletion.test.ts`	Adds integration tests covering Issue #413 scenarios.

- Fix CRITICAL COALESCE type mismatch in session ordering (TEXT vs INTEGER) - Remove code duplication in API route, use sessionService layer - Add ordering assertions to test null startedDate scenarios - Add regression test for getNextSessionNumber with existing sessions - Remove unused imports from streaks test - Preserve chronological ordering with createdAt fallback All 4019 tests passing. Co-authored-by: GitHub Copilot <copilot@github.com>

masonfox · 2026-04-14T04:20:15Z

@copilot please review this PR

Copilot · 2026-04-14T04:23:39Z

@copilot please review this PR

Here's a review of this PR:

✅ Overall Assessment: Ready to merge

The PR correctly fixes the session numbering regression (#413) and all previous reviewer feedback has been addressed. CI is green (4016 passing tests).

What's implemented

Root cause fix (session.service.ts):
deleteSession() now calls getNextSessionNumber() instead of hardcoding sessionNumber: 1. A dedicated regression test proves this works when other sessions still exist after deletion (verifies sessionNumber === 3, not 1).

Display number computation (session.service.ts → getSessionsWithDisplayNumbers()):
Single source of truth in the service layer. The API route delegates to this method rather than duplicating the logic. Display numbers are derived from chronological array position — sequential, no gaps.

Chronological ordering (session.repository.ts → findAllByBookIdOrdered()):
Uses COALESCE(startedDate, strftime('%Y-%m-%d', createdAt, 'unixepoch')) so both columns are TEXT (YYYY-MM-DD) when compared — correctly handles the previously flagged mixed INTEGER/TEXT type issue.

Frontend (ReadingHistoryTab.tsx, modals):
All components display displayNumber ?? sessionNumber as fallback, so this is backward-compatible.

Previous reviewer feedback — all addressed ✅

Issue	Status
Mixed TEXT/INTEGER in `COALESCE` ordering	✅ Fixed with `strftime` conversion
Duplicate display-number computation in API route	✅ Consolidated into service
Unused `startOfDay`, `toZonedTime`, etc. imports in streaks test	✅ Removed
Null startedDate test lacked ordering assertion	✅ Now asserts `sessionsWithDisplay[0].sessionNumber === 1`
First test didn't prove regression (hardcoded 1 would also pass)	✅ New test asserts `sessionNumber === 3` when other sessions exist

Minor observations (non-blocking)

The comment on line 213 of session.service.ts references a specific line number in ReadingHistoryTab.tsx:77-79. If that file ever changes line numbers, the comment becomes stale — consider referencing the filter logic semantically instead of by line.
displayNumber is typed as number | undefined throughout. A null coalescing fallback to sessionNumber is used consistently, which is correct.

No security or logic issues found. 🚀

…tton After successfully deleting a read session, the DeleteSessionModal's submitting state remained true when the modal closed. This caused the 'Delete Session' button to be stuck showing 'Deleting...' and be disabled when the modal was reopened for a subsequent deletion. Added useEffect hook that resets submitting state to false whenever the modal closes (isOpen changes to false). This ensures clean state on each modal open and handles all close scenarios (success, cancel, X button). - Import useEffect from React - Add cleanup effect that watches isOpen prop - Reset submitting to false when modal closes - All 4019 tests pass

masonfox added 3 commits April 13, 2026 21:25

masonfox requested a review from Copilot April 14, 2026 03:57

Copilot started reviewing on behalf of masonfox April 14, 2026 03:58 View session

Copilot AI reviewed Apr 14, 2026

View reviewed changes

Copilot started work on behalf of masonfox April 14, 2026 04:20 View session

Copilot finished work on behalf of masonfox April 14, 2026 04:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix: Session numbering after deletion (Issue #413)#416

Fix: Session numbering after deletion (Issue #413)#416
masonfox wants to merge 5 commits intodevelopfrom
fix/session-number-after-deletion

masonfox commented Apr 14, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Apr 14, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

masonfox commented Apr 14, 2026

Uh oh!

Copilot AI commented Apr 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

masonfox commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Backend

Frontend

Testing

Behavior

Testing

Files Changed

Manual Testing Needed

Uh oh!

codecov Bot commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

masonfox commented Apr 14, 2026

Uh oh!

Copilot AI commented Apr 14, 2026

✅ Overall Assessment: Ready to merge

What's implemented

Previous reviewer feedback — all addressed ✅

Minor observations (non-blocking)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

masonfox commented Apr 14, 2026 •

edited

Loading

codecov Bot commented Apr 14, 2026 •

edited

Loading