Skip to content

Show referenced records before deleting an admin account - #225

Merged
calebyhan merged 3 commits into
mainfrom
fix/224-account-referenced-records
Sep 8, 2026
Merged

calebyhan merged 3 commits into
mainfrom
fix/224-account-referenced-records

Conversation

@MasonMines2006

Copy link
Copy Markdown
Collaborator

Summary

  • Closes Accounts tab should show referenced records to manage #224. Deleting an admin account gave no visibility into what it had authored or last touched before the foreign key link was silently nulled out.
  • Adds GET /api/admin/accounts/{id}/references, which groups linked records (news, static pages, app config, budget entries, finance hearing settings, calendar events) by type with counts, sample items, and manage-page links.
  • Replaces the old window.confirm() delete flow with an AccountReferencesDialog that shows those linked records before an admin confirms deletion, and clearly surfaces a retryable error state if the safety check itself fails (rather than silently treating a failed check as "safe to delete").
  • Fixes FinanceHearingConfig.updated_by, the one FK to admin.id that was still NOT NULL with no ondelete rule — it was missed by Admin/Staff roles rework #223's pass over the rest of these and would have thrown an IntegrityError on delete.
  • No manual DB migration needed — init_db.py's existing self-healing migration (sync_missing_columns / sync_foreign_key_actions) picks up the nullable + SET NULL change automatically.

Test plan

  • Backend: added TestAccountReferences (11 cases covering 404, empty/each reference type, item-limit truncation, staff/unauthenticated access) and a delete test for the FinanceHearingConfig fix; full backend suite passes (634/639 — the 5 unrelated failures are a pre-existing macOS-only /dev/shm pytest config issue, not related to this change).
  • Frontend: tsc --noEmit clean, production build passes.
  • Manual verification in a live browser (Playwright): confirmed dialog shows linked records per type, confirmed empty-state "safe to delete" message, confirmed error state + disabled delete button + working retry when the references check fails (simulated via request interception).
  • Reviewed by code-reviewer and security-reviewer agents; one HIGH finding (failed reference check being indistinguishable from a genuine empty result) was fixed and re-verified live.

Admins deleting an account had no visibility into what it authored or
last touched (news, static pages, app config, budget entries, finance
hearing settings, calendar events) before the FK link was silently
nulled out. Adds GET /api/admin/accounts/{id}/references, grouped by
record type with counts and manage-page links, and a confirmation
dialog that shows them before delete instead of a plain confirm().

Also fixes FinanceHearingConfig.updated_by, the one FK to admin.id
still NOT NULL with no ondelete rule (missed by #223's pass over the
rest of these).
@github-actions

github-actions Bot commented Sep 5, 2026 •

Copy link
Copy Markdown

Test Results

642 tests  +15   642 ✅ +15   59s ⏱️ +5s
  1 suites ± 0     0 💤 ± 0 
  1 files   ± 0     0 ❌ ± 0 

Results for commit b7eb56c. ± Comparison against base commit dd856b4.

This pull request removes 1 and adds 16 tests. Note that renamed tests count towards both.
tests.models.test_other_models.TestFinanceHearingConfigModel ‑ test_updated_by_not_nullable
tests.models.test_other_models.TestFinanceHearingConfigModel ‑ test_updated_by_nullable
tests.routes.test_admin_accounts.TestAccountReferences ‑ test_count_exceeds_returned_items
tests.routes.test_admin_accounts.TestAccountReferences ‑ test_empty_when_no_linked_records
tests.routes.test_admin_accounts.TestAccountReferences ‑ test_includes_app_config_updated
tests.routes.test_admin_accounts.TestAccountReferences ‑ test_includes_budget_data_updated
tests.routes.test_admin_accounts.TestAccountReferences ‑ test_includes_calendar_events_created
tests.routes.test_admin_accounts.TestAccountReferences ‑ test_includes_finance_hearing_config_updated
tests.routes.test_admin_accounts.TestAccountReferences ‑ test_includes_news_authored
tests.routes.test_admin_accounts.TestAccountReferences ‑ test_includes_section_membership
tests.routes.test_admin_accounts.TestAccountReferences ‑ test_includes_static_pages_edited
…

♻️ This comment has been updated with latest results.

@calebyhan
calebyhan force-pushed the fix/224-account-referenced-records branch from 19556f1 to b7eb56c Compare September 8, 2026 19:40

@calebyhan calebyhan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@calebyhan
calebyhan merged commit abbd6f0 into main Sep 8, 2026
3 checks passed
@calebyhan
calebyhan deleted the fix/224-account-referenced-records branch September 8, 2026 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Accounts tab should show referenced records to manage

2 participants