Keep reading history when its book is hard-deleted - #44
Merged
Merged
Conversation
reading_sessions and read_completions cascaded on books.id, so purging deleted books or removing a library erased every statistic derived from that reading, with nothing left to rebuild it from. book_id is now nullable on both tables with ON DELETE SET NULL: the row survives and loses only its attribution. The user cascade is unchanged. PostgreSQL alters the constraint in place. SQLite rebuilds each table in one transaction, checking the copied row count before the original is dropped and recreating every index. Nothing references either table, so the rebuild never has to switch foreign key enforcement off. Rolling back deletes the orphaned rows first, since they cannot satisfy NOT NULL.
Reading history now survives its book being hard-deleted, with a null book_id. Totals, the per-period chart, the per-device split and coverage never joined books, so they keep counting that time unchanged. The series and format breakdowns did join books, so they now LEFT JOIN and gather those sessions into one row flagged removedFromLibrary, with a null series or format. The orphan row sorts after named rows on both engines, which disagree about where a null goes. Books read and books finished count distinct books, and deleted books cannot be told apart, so those two figures drop a deleted book's contribution. Time, pages and sittings are per-row sums and stay exact. DELETE /api/v1/reading-stats/orphaned removes the caller's detached sessions and completions in one transaction and reports the counts. Nothing else is touched, and no other user's history is reachable.
…te it The series and format panels render the removedFromLibrary row as an italic "Removed from library" line with no link, and the series row carries a Delete action. The confirmation states what will actually go. The dashboard row covers only the dates on screen while a purge is not windowed, so the dialog reads its figures from a new GET /api/v1/reading-stats/orphaned, which totals every detached session and completion for the caller. The user guide explains what survives a permanent delete, why books read and books finished drop the deleted book, and how to clear it.
Deploying codex with
|
| Latest commit: |
0ed41c7
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c1fc95f3.codex-asm.pages.dev |
| Branch Preview URL: | https://feat-reading-history-survive.codex-asm.pages.dev |
API contract changesCompared against Report |
The purge control lived on the "Removed from library" series row, which only exists when that reading falls inside the dates on screen, ranks in the top eight, and is non-zero for the chosen metric. Under "books finished" it never is, and orphaned completions with no sittings never produce a row at all. The entry point is now a page-level notice driven by the unwindowed totals, shown whenever any removed history exists. The confirmation re-reads those totals on open and stays disabled until they arrive, so it cannot confirm against figures from before it opened or alongside a failed refresh. "Nothing left" requires no sittings and no completions, and sittings are counted as the dashboard counts them. Also drop doc comments promising a reading-progress import that does not exist yet, give the trigger an accessible name, and in the migration: run a rollback's orphan delete inside the SQLite rebuild's transaction, name the table when pre-existing rows would fail the copy, and add a book_id index so each deleted book stops scanning both tables in full for the foreign key action.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Reading statistics are derived entirely from
reading_sessions, andread_completionsis the onlyrecord that a book was ever finished. Both cascaded on
books.id, so purging deleted books ordeleting a library silently erased hours a reader really spent reading, with nothing left to rebuild
them from. Soft deletes were always safe; this is about the two paths that reach a real
DELETE.Schema:
book_idbecomes nullable,ON DELETE SET NULLOn both history tables. The row survives the delete and loses only its attribution. The
user_idcascade is unchanged and tested: deleting a user still removes their history.
PostgreSQL alters the column and swaps the constraint in place. SQLite cannot, so each table is
rebuilt: create, copy, check the row count before dropping the original, drop, rename, recreate every
index,
foreign_key_check. It runs in one transaction the migration opens itself (sea-orm-migrationonly wraps PostgreSQL), and it never touches
PRAGMA foreign_keys: that step of SQLite'sprocedure exists to stop dropping a parent from cascading into its children, and nothing references
either table. That is also what avoids the pragma trouble
m20260508_000081ran into.The migration tests compare index SQL,
pragma_table_infoandpragma_foreign_key_listagainst adatabase migrated to just before this change, so a lost index, column or
DESCfails loudly.Rolling back is lossy by construction:
downhas to delete the orphaned rows to restoreNOT NULL,and the doc comment says so.
The rebuild also adds an index on
book_idto both tables (PostgreSQL gets the same). Neither hadone leading with it, so every deleted book made the foreign key action scan both tables in full,
and deleting a library does that once per book. If pre-existing rows already reference a missing
user or book, the SQLite path now fails naming the table instead of a bare constraint error.
Also run against a copy of a real dev database (13,419 books, 2,808 progress rows): every session and
completion kept, all five indexes present, no FK violations, and a hard delete leaves orphans.
Statistics keep counting it
Totals, the per-period chart, the per-device split and coverage never joined
books, so they areunchanged by a hard delete. The series and format breakdowns now
LEFT JOINand gather that timeinto one row:
removedFromLibrary: true, with a nullseriesId/seriesNameorformat. It isranked like any other row and placed after named rows on ties, explicitly, because SQLite and
PostgreSQL disagree about where a null sorts.
One figure does move, by design.
booksandbooksFinishedare distinct counts keyed on thebook, and two deleted books cannot be told apart, so a deleted book drops out of both. Time, pages
and sittings are per-row sums and stay exact. I chose undercounting over guessing; recovering it
would need per-session snapshot columns, which were considered and rejected as not worth the
backfill.
A reader can clear it, knowingly
GET /api/v1/reading-stats/orphaned: unwindowed totals of the caller's detached history.DELETE /api/v1/reading-stats/orphaned: removes the caller's detached sessions and completions inone transaction and returns the counts. Scoped to the caller; attributed history is never touched.
The dashboard row is windowed and the purge is not, so the confirmation reads its numbers from the
GETrather than from the row, which would understate what is deleted. Sittings are counted the waythe dashboard counts them.
UI and docs
"Removed from library" appears as an italic, unlinked row in the series and format panels. The way to
discard it is a page-level Reading of removed books notice driven by the unwindowed totals, not
an action on that row: the row only exists when the reading is inside the window, in the top eight,
and non-zero for the chosen metric, so orphaned completions with no sittings, or anything under
"books finished", would otherwise have no control at all. The confirmation re-reads the totals on
open and stays disabled until they arrive, so it never confirms against stale or failed figures. The user guide explains what survives a permanent delete, why books read/finished
drop the deleted book, and how to clear it.
Verification
make test-fast: 5405 passedmake test-fast-postgres-run: 5444 passedweb:npm run test:run3708 passed,npm run lint,npm run buildcleancargo clippy --workspace --all-targets -- -D warningsclean#45, stacked on this one, adds reading-progress export/import, which can put orphaned sessions
back on their books after a library reorganisation.