Skip to content

ref(dashboards): Drop DashboardLastVisited table#114930

Merged
wedamija merged 2 commits intomasterfrom
flag-cleanup/drop-dashboard-last-visited-table
May 7, 2026
Merged

ref(dashboards): Drop DashboardLastVisited table#114930
wedamija merged 2 commits intomasterfrom
flag-cleanup/drop-dashboard-last-visited-table

Conversation

@wedamija
Copy link
Copy Markdown
Member

@wedamija wedamija commented May 5, 2026

Drop the sentry_dashboardlastvisited table from PostgreSQL via
SafeDeleteModel(DELETE).

Do not merge until #(prev PR) has been deployed and verified — the
prior PR removes Django's knowledge of the model and drops FK
constraints, but leaves the table in place for rolling deploy safety.

PR Stack:

  1. Remove model + MOVE_TO_PENDING migration
  2. this PR — DELETE migration to drop the table

@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label May 5, 2026
@wedamija wedamija changed the title Flag cleanup/drop dashboard last visited table ref(dashboards): Drop DashboardLastVisited table May 5, 2026
@wedamija wedamija force-pushed the flag-cleanup/drop-dashboard-last-visited-table branch from e8e4c65 to 30d75ac Compare May 6, 2026 19:08
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

This PR has a migration; here is the generated SQL for src/sentry/migrations/1083_remove_dashboardlastvisited.py src/sentry/migrations/1084_delete_dashboardlastvisited.py

for 1083_remove_dashboardlastvisited in sentry

--
-- Alter field dashboard on dashboardlastvisited
--
SET CONSTRAINTS "sentry_dashboardlast_dashboard_id_31d3a51b_fk_sentry_da" IMMEDIATE; ALTER TABLE "sentry_dashboardlastvisited" DROP CONSTRAINT "sentry_dashboardlast_dashboard_id_31d3a51b_fk_sentry_da";
--
-- Alter field member on dashboardlastvisited
--
SET CONSTRAINTS "sentry_dashboardlast_member_id_28801a0a_fk_sentry_or" IMMEDIATE; ALTER TABLE "sentry_dashboardlastvisited" DROP CONSTRAINT "sentry_dashboardlast_member_id_28801a0a_fk_sentry_or";
--
-- Moved model DashboardLastVisited to pending deletion state
--
-- (no-op)

for 1084_delete_dashboardlastvisited in sentry

--
-- Delete model DashboardLastVisited
--
DROP TABLE "sentry_dashboardlastvisited" CASCADE;

wedamija and others added 2 commits May 6, 2026 12:35
Drop the sentry_dashboardlastvisited table from PostgreSQL via
SafeDeleteModel(DELETE).

Do not merge until #(prev PR) has been deployed and verified — the
prior PR removes Django's knowledge of the model and drops FK
constraints, but leaves the table in place for rolling deploy safety.

PR Stack:
1. Remove model + MOVE_TO_PENDING migration
2. **this PR** — DELETE migration to drop the table

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
SafeDeleteModel(DELETE) needs the table → silo mapping to know which
DB to drop the table from. Once the model class is removed (prior PR),
the router can no longer resolve this on its own, so we record it here
alongside dashboardtombstone and the rest.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@wedamija wedamija force-pushed the flag-cleanup/drop-dashboard-last-visited-table branch from 30d75ac to 4ff58bf Compare May 6, 2026 19:35
@wedamija wedamija marked this pull request as ready for review May 6, 2026 19:35
@wedamija wedamija requested a review from a team as a code owner May 6, 2026 19:35
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

This PR has a migration; here is the generated SQL for src/sentry/migrations/1084_delete_dashboardlastvisited.py

for 1084_delete_dashboardlastvisited in sentry

--
-- Delete model DashboardLastVisited
--
DROP TABLE "sentry_dashboardlastvisited" CASCADE;

@wedamija wedamija merged commit d004017 into master May 7, 2026
83 checks passed
@wedamija wedamija deleted the flag-cleanup/drop-dashboard-last-visited-table branch May 7, 2026 07:45
wedamija added a commit that referenced this pull request May 7, 2026
…_PENDING (#115087)

The `historical_silo_assignments` check in `SafeDeleteModel` only fired
for `DeletionAction.DELETE`, with an early return for `MOVE_TO_PENDING`.
That missed the dangerous case where the model class is deleted in the
same PR as the `MOVE_TO_PENDING` migration: once the class is gone, the
silo router can no longer resolve the table via the live app registry,
and without a historical entry `allow_migrate` returns False so the
migration silently no-ops in prod.

This is what bit us in #114929 (`dashboardlastvisited`). This caused an
error in production where we couldn't delete related rows from
`OrganizationMember`, and this was only fixed when we dropped the table
fully in #114930.

This pr makes sure that the test fails if we move a model to pending and
don't add the historical routing.

Also update the skill to guide llms to always add this historical entry.

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants