feat(campaigns): deleting a campaign no longer destroys everything in it (#405) #478
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/405-campaign-soft-delete"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #405.
The gap
delete_campaignwasdb.delete(campaign)followed immediately by a commit, and 17 tables carryON DELETE CASCADEback tocampaigns.id. Every session, transcript, summary, wiki entry, note, NPC, arc, plot thread and ledger row a group had built over months of play went in a single statement — behind one nativeconfirm(), available to any GM on the campaign rather than only its creator, with no undo, no grace, and no prompt to export first.Campaigns now go to a trash with a 30-day grace (7-day floor, shared with the wiki trash), a typed-name confirmation, an export offered at the moment of deletion, and a
purge_trashed_campaignsBeat task that runs the cascade once the window closes. Migrationf4a5b6c7d8ea, verified up → down → up.Why this is not #408's convention
A lore entry is only ever reached by selecting one, so filtering each of its 13 reads works and is checkable. A campaign is different in kind: its children are routinely selected without the query mentioning
Campaignat all — 35select(Session)sites alone, across 17 FK'd tables. "Filter every read" would mean auditing every session, note, ledger and lore query in the codebase, and would still silently fail to cover the next one anyone writes.So the gate is
_reject_if_trashedinapp/auth/dependencies.py, called by all four campaign- and session-scoped authorisation dependencies. Every such route is covered at once, and a route added tomorrow is covered by construction — it cannot authorise itself without passing through it.Five doors do not open with a session cookie, and filter explicitly:
The tasks go through
campaign_service.get_live_campaign— adb.getthat returnsNonefor a trashed campaign, so theif not campaign: continueguard all 11 call sites already had does the work. I checked each of the 11 for that guard before routing them through it; without one, the change would have turned a skip into anAttributeError.Three things checked rather than assumed
is_archivedis not a read filter anywhere in the backend — it appears in the model, the schemas, archive/restore, and one join-by-invite guard. Archiving suppresses nothing, so it gave soft delete nothing to build on.deleted_atis a separate column, and the two states stay distinct: collapsing them would make "archive for the season" and "destroy everything" the same database state.enforce_retentionskips trashed campaigns in all three of its passes, because a restored campaign must come back intact. Notably not by dropping them from the policy map — I wrote that first and it was wrong:get_effective_retention(db, None)falls back to the instance defaults, which can be shorter than the campaign's own override, so that version deleted their audio sooner rather than not at all.confirm()747 → 839). The substance held.Endpoints
DELETEnow takesconfirm_nameand trashes (still 204 — the caller need not care which kind of gone it is), plusGET /campaigns/trash/mineandPOST /campaigns/{id}/undelete./undelete, because/restorealready means un-archive — two states, two verbs. Neither trash route can sit under/{campaign_id}withrequire_gm, since that dependency 404s trashed campaigns by design; the trash has to live outside the gate it exists to see behind.Frontend
The native
confirm()is replaced by a typed-name modal matching the existing erase-member dialog (#118), with the export offered inside it — at the moment it is relevant, rather than somewhere it must be remembered. Deleted campaigns appear on the dashboard with a countdown and a Restore button, which is what makes the grace period something a GM can actually act on.Verification
28 backend mutations and 3 frontend mutations, all caught — reverting to a hard delete, removing the gate from each of the four dependencies, each of the five explicit filters, a restore that does not clear the flag or that also un-archives, a purge that ignores the grace or sweeps live rows, a retention floor that is only a default, and each of the three retention passes.
One test initially survived its mutation: the detail-route test claimed to cover the dependency gate, but that route is defended twice (the gate and
get_campaignfiltering), so removing the gate changed nothing and the test could not tell which one was working. Added a member-list test — it readscampaign_membersand never selects aCampaign, which is exactly the shape per-read filtering misses.Backend 1609 passed, frontend 470 passed;
ruff check/formatand eslint clean. NoBOT_CONTRACT_VERSIONbump: the bot response shape is unchanged, and a trashed campaign's guild returning 404 is a path an older bot already handles correctly — it denies with "cannot establish who the GM is", which is the right answer for a deleted campaign.Two pre-existing test-harness failures are unrelated and excluded from the local run (
test_version_sync.py,test_backup_client_version.py): both resolvePath(__file__).parents[3]to the repo root, which the local container does not mount. They pass in CI.🤖 Generated with Claude Code