Deleting a location must not destroy its alert history

claude-bot commented

2026-07-27 20:07:18 +00:00

Contributor

Parent: #22.

Location.sent_alerts is declared cascade="all, delete-orphan" (app/models/location.py:99-101), and SentAlert.location_id is ondelete="CASCADE". Deleting a location hard-deletes its entire alert history. nws_alert_snapshots, public_alert_pages, and spc_outlook_impact_locations cascade on location_id the same way.

That is defensible behaviour today. It is a trap for a release whose entire premise is that history is worth keeping: one accidental delete silently destroys the record the Explorer exists to show, with no warning and no recovery.

Tasks

Decide the model — soft-delete locations (deleted_at), or detach history (ON DELETE SET NULL plus denormalized location name/coords on the history rows so an orphaned event still renders).
Either way, the Explorer must still render events for a location that no longer exists, and visibility must still work for them. Note that visible_location_filter keys off Location.owner_id, so orphaned history needs an explicit visibility rule — this is the part most likely to be got wrong.
Migration for existing rows.
Warn in the UI when deleting a location with retained history, stating how many events would be affected.
Tests covering deletion with history present.

Parent: #22. `Location.sent_alerts` is declared `cascade="all, delete-orphan"` (`app/models/location.py:99-101`), and `SentAlert.location_id` is `ondelete="CASCADE"`. **Deleting a location hard-deletes its entire alert history.** `nws_alert_snapshots`, `public_alert_pages`, and `spc_outlook_impact_locations` cascade on `location_id` the same way. That is defensible behaviour today. It is a trap for a release whose entire premise is that history is worth keeping: one accidental delete silently destroys the record the Explorer exists to show, with no warning and no recovery. ## Tasks - [ ] Decide the model — soft-delete locations (`deleted_at`), or detach history (`ON DELETE SET NULL` plus denormalized location name/coords on the history rows so an orphaned event still renders). - [ ] Either way, the Explorer must still render events for a location that no longer exists, **and visibility must still work for them**. Note that `visible_location_filter` keys off `Location.owner_id`, so orphaned history needs an explicit visibility rule — this is the part most likely to be got wrong. - [ ] Migration for existing rows. - [ ] Warn in the UI when deleting a location with retained history, stating how many events would be affected. - [ ] Tests covering deletion with history present.

claude-bot added this to the v2.0.0 - Historical Weather Event Explorer milestone

2026-07-27 20:07:18 +00:00

claude-bot added the

type:maintenance

v2.0.0

labels

2026-07-27 20:07:18 +00:00

claude-bot referenced this issue

2026-07-27 20:09:21 +00:00

Historical Weather Event Explorer #22

claude-bot commented

2026-08-01 03:57:13 +00:00

Author

Contributor

Picking this up. Model decision: soft-delete (locations.deleted_at), not detach-history.

Rationale: soft-delete keeps every FK, join, and — decisively — the existing three-tier visibility semantics intact: owner_id and location_members survive, so visible_location_filter / can_view_location_with_db keep working unchanged for historical events, which the issue correctly flags as "the part most likely to be got wrong" under the detach model. Detach would need nullable location_id across five history tables, denormalized name/coords, and a brand-new visibility rule for orphaned rows. Survey of the codebase shows the soft-delete cost is small: nearly all active-location surfaces already filter Location.enabled == True, which the soft-delete path forces off.

Scope

Migration 0035: locations.deleted_at (nullable, tz-aware).
DELETE /api/locations/{id} becomes soft-delete: sets deleted_at, enabled=False, public_enabled=False; row and all history retained. ?purge=true (admin-only) keeps the old cascade for genuine removal.
GET /api/locations/{id}/delete-preview: counts of retained history (alerts, derived events, snapshots) to power the UI warning.
Locations page delete flow shows the warning with those counts before confirming.
Config import replace=true soft-deletes instead of hard-deleting; re-importing a soft-deleted location revives it (deleted_at cleared). Export excludes deleted.
Active-only surfaces get explicit deleted_at IS NULL guards where enabled doesn't already cover them (per-location interactive endpoints 404 on deleted; background jobs skip deleted; the correlation full rebuild deliberately still includes deleted locations so their history keeps deriving).
Historical reads (alert history API, future Explorer views) keep working for deleted locations with unchanged visibility.
Tests for all of the above.

Picking this up. **Model decision: soft-delete** (`locations.deleted_at`), not detach-history. Rationale: soft-delete keeps every FK, join, and — decisively — the existing three-tier visibility semantics intact: `owner_id` and `location_members` survive, so `visible_location_filter` / `can_view_location_with_db` keep working unchanged for historical events, which the issue correctly flags as "the part most likely to be got wrong" under the detach model. Detach would need nullable `location_id` across five history tables, denormalized name/coords, and a brand-new visibility rule for orphaned rows. Survey of the codebase shows the soft-delete cost is small: nearly all active-location surfaces already filter `Location.enabled == True`, which the soft-delete path forces off. ## Scope - Migration 0035: `locations.deleted_at` (nullable, tz-aware). - `DELETE /api/locations/{id}` becomes soft-delete: sets `deleted_at`, `enabled=False`, `public_enabled=False`; row and all history retained. `?purge=true` (admin-only) keeps the old cascade for genuine removal. - `GET /api/locations/{id}/delete-preview`: counts of retained history (alerts, derived events, snapshots) to power the UI warning. - Locations page delete flow shows the warning with those counts before confirming. - Config import `replace=true` soft-deletes instead of hard-deleting; re-importing a soft-deleted location revives it (`deleted_at` cleared). Export excludes deleted. - Active-only surfaces get explicit `deleted_at IS NULL` guards where `enabled` doesn't already cover them (per-location interactive endpoints 404 on deleted; background jobs skip deleted; the correlation full rebuild deliberately still includes deleted locations so their history keeps deriving). - Historical reads (alert history API, future Explorer views) keep working for deleted locations with unchanged visibility. - Tests for all of the above.