[Backend] Add a restore path and stop cascading version history on wiki entry deletion #408

Closed
opened 2026-08-25 20:44:48 +00:00 by claude-bot · 1 comment
Contributor

Severity: MEDIUM

Found in the August 2026 session lifecycle review (#319).

Deleting a wiki (lore) entry is a hard delete that takes its entire edit history down with it via a database cascade, and even short of deletion, there is no way to restore a version once you've clobbered an entry's current body — the version list is read-only, so recovering from a bad edit or a mistaken deletion is manual copy-paste at best and outright impossible at worst.

Evidence

  • webapp/backend/app/services/lore_service.py:851-853 (delete_lore_entry) performs a hard delete of the LoreEntry row.
  • webapp/backend/app/models/lore_entry_version.py:21-25lore_entry_versions.lore_entry_id is declared ForeignKey("lore_entries.id", ondelete="CASCADE"), so every version snapshot for an entry is destroyed the instant the entry itself is deleted.
  • webapp/backend/app/routers/campaigns.py:2296 (list_lore_versions) is the only version-related endpoint that exists — there is no corresponding restore/revert endpoint, so even while the entry is alive, recovering an earlier body is manual copy-paste out of the list view back into the editor.

Failure scenario
A GM deletes what they believe is a duplicate wiki entry, only to realise afterward it was the canonical one with months of accumulated lore and edit history. Because the delete cascades the version table too, there is nothing left to restore from — not even the version list survives to let them manually reconstruct it.

Proposed fix
Change wiki entry deletion to a soft delete (matching the campaign-deletion recommendation tracked separately — the same trash-window pattern), keeping the entry and its version history intact but hidden, purged only after a grace period. Independently of the delete question, add a restore/revert endpoint that takes a LoreEntryVersion and applies it back onto the live entry (itself snapshotting the pre-restore state first) — this is a small addition given the version table and its data already exist.

Acceptance criteria

  • Deleting a wiki entry no longer immediately destroys its version history; it is recoverable within a grace period.
  • A restore/revert endpoint exists that lets a GM roll a live entry back to a prior version.
  • Restoring a version itself creates a new version snapshot of the pre-restore state, so a restore is itself undoable.
  • Regression test: delete an entry within the grace period and confirm it (and its version history) can be recovered.
**Severity: MEDIUM** Found in the August 2026 session lifecycle review (#319). Deleting a wiki (lore) entry is a hard delete that takes its entire edit history down with it via a database cascade, and even short of deletion, there is no way to restore a version once you've clobbered an entry's current body — the version list is read-only, so recovering from a bad edit or a mistaken deletion is manual copy-paste at best and outright impossible at worst. **Evidence** - `webapp/backend/app/services/lore_service.py:851-853` (`delete_lore_entry`) performs a hard delete of the `LoreEntry` row. - `webapp/backend/app/models/lore_entry_version.py:21-25` — `lore_entry_versions.lore_entry_id` is declared `ForeignKey("lore_entries.id", ondelete="CASCADE")`, so every version snapshot for an entry is destroyed the instant the entry itself is deleted. - `webapp/backend/app/routers/campaigns.py:2296` (`list_lore_versions`) is the only version-related endpoint that exists — there is no corresponding restore/revert endpoint, so even while the entry is alive, recovering an earlier body is manual copy-paste out of the list view back into the editor. **Failure scenario** A GM deletes what they believe is a duplicate wiki entry, only to realise afterward it was the canonical one with months of accumulated lore and edit history. Because the delete cascades the version table too, there is nothing left to restore from — not even the version list survives to let them manually reconstruct it. **Proposed fix** Change wiki entry deletion to a soft delete (matching the campaign-deletion recommendation tracked separately — the same trash-window pattern), keeping the entry and its version history intact but hidden, purged only after a grace period. Independently of the delete question, add a restore/revert endpoint that takes a `LoreEntryVersion` and applies it back onto the live entry (itself snapshotting the pre-restore state first) — this is a small addition given the version table and its data already exist. **Acceptance criteria** - [ ] Deleting a wiki entry no longer immediately destroys its version history; it is recoverable within a grace period. - [ ] A restore/revert endpoint exists that lets a GM roll a live entry back to a prior version. - [ ] Restoring a version itself creates a new version snapshot of the pre-restore state, so a restore is itself undoable. - [ ] Regression test: delete an entry within the grace period and confirm it (and its version history) can be recovered.
Author
Contributor

Done, in two parts.

Restore-from-history (PR #476): a GM can roll an entry back to any earlier version, which covers the "edited badly" half of the issue.

Soft delete (PR #477): the "deleted it entirely" half, which was the worse failure — lore_entry_versions.lore_entry_id was ON DELETE CASCADE, so a hard delete took the entry's whole edit history with it in the same statement. There was nothing left to restore from. Entries now go to a trash (deleted_at, deleted_by_id), stay recoverable for a 30-day grace period, and are purged for real by the purge_trashed_lore_entries Beat task. Migration e3f4a5b6c7d9, verified up → down → up.

The actual work was the read surface: soft delete is only correct if every read excludes trashed rows, and a missed one shows up much later as "the deleted entry is still in search results". All 13 select(LoreEntry) sites filter explicitly rather than through a default loader criterion — greppable, and each one somewhere a reviewer can check. get_lore_entry(..., include_deleted=True) is the deliberate opt-in, with exactly two callers (restore and purge). There is a test per surface, not one test for the flag.

The grace period is a floor, not a preference: get_trash_retention_days clamps a stored value up to a 7-day minimum, the same treatment get_audio_trash_retention_days gets (#427), because months of edit history can't be regenerated at any price.

Two corrections to my own earlier notes, both caught by checking rather than assuming:

  • I'd described this as "80 places across 20 files". That counted every LoreEntry. mention including model declarations. The real read surface is 13 selects, 16 joins and 6 relationships across 5 files — which is what made explicit filtering right rather than a default loader.
  • Campaign.lore_entries is declared but never read anywhere. I'd written a comment justifying a filter on it before checking; it now records accurately that the relationship is unused and that anyone who starts reading it must filter trashed entries themselves.

21 new tests, 11 mutations all caught. One test initially survived its mutation — the trash-list test seeded only a trashed entry, so "show the trash" and "show everything" returned the same thing and dropping the filter changed nothing. It now seeds a live entry too and asserts it stays out. Full backend suite 1588 passed.

Endpoints: DELETE now trashes (still 204), plus GET /lore-trash and POST /lore/{id}/restore, both GM-only and audited.

Closing. #405 applies the same idea to campaigns — though on a different mechanism, since campaigns have 17 child tables reachable without going through Campaign and so need a gate rather than per-read filters.

Done, in two parts. **Restore-from-history** (PR #476): a GM can roll an entry back to any earlier version, which covers the "edited badly" half of the issue. **Soft delete** (PR #477): the "deleted it entirely" half, which was the worse failure — `lore_entry_versions.lore_entry_id` was `ON DELETE CASCADE`, so a hard delete took the entry's whole edit history with it in the same statement. There was nothing left to restore *from*. Entries now go to a trash (`deleted_at`, `deleted_by_id`), stay recoverable for a 30-day grace period, and are purged for real by the `purge_trashed_lore_entries` Beat task. Migration `e3f4a5b6c7d9`, verified up → down → up. The actual work was the read surface: soft delete is only correct if *every* read excludes trashed rows, and a missed one shows up much later as "the deleted entry is still in search results". All 13 `select(LoreEntry)` sites filter explicitly rather than through a default loader criterion — greppable, and each one somewhere a reviewer can check. `get_lore_entry(..., include_deleted=True)` is the deliberate opt-in, with exactly two callers (restore and purge). There is a test per surface, not one test for the flag. The grace period is a floor, not a preference: `get_trash_retention_days` clamps a stored value **up** to a 7-day minimum, the same treatment `get_audio_trash_retention_days` gets (#427), because months of edit history can't be regenerated at any price. Two corrections to my own earlier notes, both caught by checking rather than assuming: - I'd described this as "80 places across 20 files". That counted every `LoreEntry.` mention including model declarations. The real read surface is 13 selects, 16 joins and 6 relationships across 5 files — which is what made explicit filtering right rather than a default loader. - `Campaign.lore_entries` is declared but never read anywhere. I'd written a comment justifying a filter on it before checking; it now records accurately that the relationship is unused and that anyone who starts reading it must filter trashed entries themselves. 21 new tests, 11 mutations all caught. One test initially *survived* its mutation — the trash-list test seeded only a trashed entry, so "show the trash" and "show everything" returned the same thing and dropping the filter changed nothing. It now seeds a live entry too and asserts it stays out. Full backend suite 1588 passed. Endpoints: `DELETE` now trashes (still 204), plus `GET /lore-trash` and `POST /lore/{id}/restore`, both GM-only and audited. Closing. #405 applies the same idea to campaigns — though on a different mechanism, since campaigns have 17 child tables reachable without going through `Campaign` and so need a gate rather than per-read filters.
Sign in to join this conversation.
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
rbrooks/Quest-Board#408
No description provided.