fix: DNF status transitions now create new sessions by masonfox · Pull Request #391 · masonfox/tome

masonfox · 2026-02-26T20:45:15Z

Summary

Fixes a bug where transitioning a book from DNF (Did Not Finish) status back to "Read Next" or "Want To Read" was overwriting the existing DNF session instead of creating a new session with an incremented session number, causing DNF history to be lost.

Changes:

DNF is now treated as a fully terminal state (like "read")
Transitioning FROM DNF to any other status archives the DNF session and creates a new session
DNF → read transitions are blocked with validation error (user must go DNF → reading → read with progress)
DNF session completedDate is preserved when archiving
New sessions from DNF → reading auto-set startedDate

Technical Details

Modified: lib/services/session.service.ts

Updated backward movement detection to include DNF transitions
Added validation to block direct DNF → read transitions
Modified archival logic to always archive DNF sessions
Preserved DNF completedDate when archiving
Auto-set startedDate for new sessions when status is "reading"

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

Fixed error handling to return 400 for DNF validation errors (was returning 500)

Tests Added:

5 new unit tests in __tests__/integration/services/sessions/session/core.test.ts
5 new E2E API tests in __tests__/e2e/api/books/dnf-transitions.test.ts

Valid Transitions FROM DNF

From	To	Behavior	Result
`dnf`	`to-read`	Archive DNF, create new session #2	2 sessions
`dnf`	`read-next`	Archive DNF, create new session #2	2 sessions
`dnf`	`reading`	Archive DNF, create new session #2	2 sessions
`dnf`	`read`	BLOCKED - validation error	Throws error

Testing

✅ All 3904 tests passing (including new DNF transition tests)
✅ Manually verified on production database (book ID 743302)
✅ CI passing with all E2E tests working correctly

Files Changed

lib/services/session.service.ts (+58, -22)
app/api/books/[id]/status/route.ts (+1, -1)
__tests__/integration/services/sessions/session/core.test.ts (+131)
__tests__/e2e/api/books/dnf-transitions.test.ts (+190, new file)

Total: +380 insertions, -23 deletions

Treat DNF as a terminal state similar to 'read'. When transitioning from DNF to any other status (read-next, to-read, reading), the system now: 1. Archives the existing DNF session (preserves history) 2. Creates a new session with incremented session number 3. Preserves the original DNF completedDate Changes: - Updated backward movement detection to include DNF transitions - DNF sessions always archived (no progress check needed) - Added validation to block direct DNF → read transitions - Auto-set startedDate when transitioning DNF → reading - Preserve DNF completedDate when archiving (don't overwrite) Tests: - Added 5 unit tests for DNF transitions in session/core.test.ts - Added 5 E2E API tests in dnf-transitions.test.ts - All 3900+ tests pass Fixes issue where DNF → read-next would overwrite the DNF session instead of creating a new session, losing the DNF history. Closes #[issue-number]

- Refactor DNF transition E2E tests to import and call route handlers directly - Fix error handling in status route to return 400 for DNF validation errors - All 5 E2E tests now passing (no more NetworkError) - Full test suite: 3904 tests passing Technical changes: - Import POST handler from route.ts instead of using fetch() - Use createMockRequest() helper to create NextRequest objects - Add vi.mock for next/cache revalidation - Update error handling to catch "Cannot mark" errors with 400 status - Adjust test expectations for flattened response structure when sessionArchived=true Co-authored-by: OpenCode <opencode@anomaly.co>

masonfox · 2026-02-26T22:55:03Z

✅ CI Test Failures Fixed

Fixed the 5 failing E2E tests in CI. The issue was that the tests were attempting to make real HTTP requests to localhost:3000 (which doesn't exist in CI), instead of following the project's E2E testing pattern.

Changes in commit `4f8bff5`:

1. Refactored E2E tests to use route handlers directly

Import POST handler from app/api/books/[id]/status/route.ts
Use createMockRequest() helper to create NextRequest objects
Added vi.mock('next/cache') to prevent side effects

2. Fixed error handling in status route

Updated error handling to return 400 status for "Cannot mark" errors
Previously returned 500 for DNF → read validation errors

3. Adjusted test expectations

When sessionArchived=true, the API spreads session properties at top level
Tests now expect data.id instead of data.session.id

Test Results

✅ All 5 DNF transition E2E tests passing
✅ Full test suite: 3904 tests passing

The fix follows the established pattern used in other E2E tests like reread.test.ts.

- Add calibreService mock to prevent file I/O during tests - Add streakService mock to prevent side effects - Align with other E2E test patterns (reread.test.ts, mark-as-read.test.ts, status.test.ts) - Add mock rationale comments explaining why each mock is needed - All 5 tests still passing Co-authored-by: OpenCode <opencode@anomaly.co>

masonfox · 2026-02-26T23:04:59Z

✅ Full E2E Pattern Alignment (commit `076a2ce`)

Added missing service mocks to fully align with project's E2E testing patterns:

Added Mocks

calibreService - Prevents file system I/O during tests
streakService - Prevents streak rebuilding side effects

Why This Matters

The session service calls both calibreService.updateRating() and streakService.rebuildStreak() during status updates. Without mocking these:

Tests would perform file I/O to Calibre database
Streak calculations would run unnecessarily
Tests would be slower and have side effects

Pattern Consistency

Now matches the same mock structure used in:

__tests__/e2e/api/books/reread.test.ts
__tests__/e2e/api/books/mark-as-read.test.ts
__tests__/e2e/api/status.test.ts
__tests__/e2e/api/books/complete-book.test.ts

All mocks include rationale comments explaining why they're needed. ✅

masonfox

Please review this PR for code quality, best practices, and potential issues.

…ix comments) - Add SessionValidationError class with typed error codes (PAGES_REQUIRED, DNF_TO_READ_BLOCKED) - Replace error string matching with instanceof checks in API route - Optimize progress lookup: skip DB query when completedDate already exists - Fix misleading comment: terminal sessions keep isActive=true (not false) All 3904 tests passing. Addresses GitHub Copilot review #3864087654

codecov · 2026-02-27T02:01:54Z

Codecov Report

❌ Patch coverage is 88.09524% with 5 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
lib/services/session.service.ts	86.84%	2 Missing and 3 partials ⚠️

@@             Coverage Diff             @@
##           develop     #391      +/-   ##
===========================================
- Coverage    78.55%   78.54%   -0.02%     
===========================================
  Files          167      167              
  Lines         7509     7533      +24     
  Branches      1829     1843      +14     
===========================================
+ Hits          5899     5917      +18     
- Misses        1129     1132       +3     
- Partials       481      484       +3

Flag	Coverage Δ
unittests	`78.54% <88.09%> (-0.02%)`	⬇️

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

Files with missing lines	Coverage Δ
app/api/books/[id]/status/route.ts	`92.10% <100.00%> (-0.21%)`	⬇️
lib/repositories/session.repository.ts	`78.89% <100.00%> (+0.19%)`	⬆️
lib/services/session.service.ts	`88.14% <86.84%> (-1.04%)`	⬇️

🚀 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.

masonfox

Re-requesting review after addressing feedback from Review #3864087654:

✅ Comment 1 (Misleading comment): Fixed comment to accurately reflect that terminal sessions keep isActive=true
✅ Comment 2 (Brittle error handling): Replaced string matching with typed SessionValidationError class and instanceof checks
✅ Comment 3 (Unnecessary query): Optimized to skip DB query when completedDate already exists

All 3904 tests still passing. Please review the updated implementation.

Resolves conflict by accepting deletion of review-loop.md from develop branch

- Set isActive=false for both 'read' and 'dnf' terminal statuses - Add readNextOrder assignment for DNF → read-next transitions (fixes Copilot Issue #1) - Fix misleading comments about terminal state behavior (fixes Copilot Issue #2) - Add 8 new tests covering terminal state transitions (fixes Copilot Issues #3 & #4) - Update 11 existing tests to expect isActive=false for terminal states - Add transaction support to SessionRepository.getNextReadNextOrder() Resolves Copilot PR review #3885064799 (all 4 issues addressed) Aligns with ADR-004, ADR-008, ADR-011 documentation Matches 97% of production data (62/64 read/dnf sessions have is_active=0) All 3999 tests passing ✅

- Extract complex boolean conditions into named variables (isReadingToPlanning, isDnfToAnyOther) - Consolidate completedDate calculation into helper function (getArchiveCompletedDate) - Reduce nesting with early returns and optional chaining - Improves maintainability without changing behavior All 3999 tests passing ✅ Co-authored-by: Claude <claude@anthropic.com>

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

masonfox and others added 2 commits February 26, 2026 15:44

masonfox commented Feb 26, 2026

View reviewed changes

masonfox requested a review from Copilot February 26, 2026 23:17

Copilot started reviewing on behalf of masonfox February 26, 2026 23:18 View session

This comment was marked as outdated.

Sign in to view

masonfox commented Feb 27, 2026

View reviewed changes

improve review loop

0d20e62

Copilot AI review requested due to automatic review settings March 3, 2026 21:03

Copilot started reviewing on behalf of masonfox March 3, 2026 21:04 View session

This comment was marked as outdated.

Sign in to view

masonfox and others added 4 commits March 4, 2026 18:14

Merge develop and remove review-loop file

c7ce6fa

Resolves conflict by accepting deletion of review-loop.md from develop branch

Merge branch 'develop' into fix/dnf-status-transitions

223dd98

masonfox requested a review from Copilot March 5, 2026 19:43

Copilot AI reviewed Mar 5, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: DNF status transitions now create new sessions#391

fix: DNF status transitions now create new sessions#391
masonfox wants to merge 9 commits intodevelopfrom
fix/dnf-status-transitions

masonfox commented Feb 26, 2026 •

edited

Loading

Uh oh!

masonfox commented Feb 26, 2026

Uh oh!

masonfox commented Feb 26, 2026

Uh oh!

masonfox left a comment

Uh oh!

This comment was marked as outdated.

Uh oh!

codecov bot commented Feb 27, 2026 •

edited

Loading

Uh oh!

masonfox left a comment

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

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

Uh oh!

Summary

Technical Details

Valid Transitions FROM DNF

Testing

Files Changed

Uh oh!

masonfox commented Feb 26, 2026

✅ CI Test Failures Fixed

Changes in commit 4f8bff5:

Test Results

Uh oh!

masonfox commented Feb 26, 2026

✅ Full E2E Pattern Alignment (commit 076a2ce)

Added Mocks

Why This Matters

Pattern Consistency

Uh oh!

masonfox left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

codecov bot commented Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

masonfox left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

masonfox commented Feb 26, 2026 •

edited

Loading

Changes in commit `4f8bff5`:

✅ Full E2E Pattern Alignment (commit `076a2ce`)

codecov bot commented Feb 27, 2026 •

edited

Loading