Skip to content

ref(dashboards): Use favorited column for favorite status instead of row existence#110204

Merged
DominikB2014 merged 5 commits intomasterfrom
dominikbuszowiecki/dain-1287-use-starred-column-to-favourite-and-unfavourite
Mar 9, 2026
Merged

ref(dashboards): Use favorited column for favorite status instead of row existence#110204
DominikB2014 merged 5 commits intomasterfrom
dominikbuszowiecki/dain-1287-use-starred-column-to-favourite-and-unfavourite

Conversation

@DominikB2014
Copy link
Contributor

@DominikB2014 DominikB2014 commented Mar 9, 2026

Use the favorited boolean column on DashboardFavoriteUser to determine
favorite status, rather than checking for existence of a row in the table.

Previously, unfavoriting a dashboard deleted the DashboardFavoriteUser
row entirely. Now it sets favorited=False and clears the position, preserving
the row for potential re-favoriting. This avoids row churn and makes the
favorite state explicit.

Changes:

  • All manager queries (get_favorite_dashboards, get_favorite_dashboard,
    get_last_position, reorder_favorite_dashboards) now filter by favorited=True
  • insert_favorite_dashboard reactivates existing unfavorited entries instead
    of always creating new rows
  • Renamed delete_favorite_dashboardunfavorite_dashboard to reflect
    that the row is no longer deleted
  • Updated serializer, list endpoint filters, sort, and pin-by-favorites
    queries to scope by favorited=True
  • Fixed race condition in insert_favorite_dashboard using select_for_update()
    to prevent concurrent calls from causing IntegrityError
  • Fixed position assignment bug where re-favoriting the first dashboard after
    unfavoriting all others assigned position 1 instead of 0 due to a global
    unscoped count check

Refs DAIN-1287

…row existence

Check favorited=True instead of row existence to determine favorite
status. Unfavoriting now sets favorited=False and clears position
instead of deleting the row. Re-favoriting reactivates existing
unfavorited entries. Rename delete_favorite_dashboard to
unfavorite_dashboard to reflect the new behavior.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@linear-code
Copy link

linear-code bot commented Mar 9, 2026

@DominikB2014 DominikB2014 marked this pull request as ready for review March 9, 2026 14:26
@DominikB2014 DominikB2014 requested a review from a team as a code owner March 9, 2026 14:26
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Mar 9, 2026
Use select_for_update() to lock existing rows when favoriting a
dashboard, preventing concurrent calls from racing between the
check and insert and causing IntegrityError on the unique constraint.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…hboard

The global self.count() check included unfavorited rows and other
users' rows, causing position to be 1 instead of 0 when a user
favorites their first dashboard after having previously unfavorited
all others. Check for existing favorited rows scoped to the user
and organization instead.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…olumn

Update the deprecated favorited_by setter to set favorited=False
instead of deleting rows, consistent with the new unfavorite behavior.
Also handles re-favoriting existing unfavorited entries.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Copy link
Contributor

@edwardgou-sentry edwardgou-sentry left a comment

Choose a reason for hiding this comment

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

Makes sense to me. I'm assuming the update to prebuilt dashboard sync will be in another pr? There's also one cursor comment, but looks like an easy fix

@DominikB2014
Copy link
Contributor Author

Makes sense to me. I'm assuming the update to prebuilt dashboard sync will be in another pr? There's also one cursor comment, but looks like an easy fix

@edwardgou-sentry yup! Gonna follow up with a sync, will fix the cursor comment too

…vorite

The deprecated favorited_by setter was not clearing position when
unfavoriting, unlike unfavorite_dashboard which sets position=None.
The UniqueConstraint on (user_id, organization_id, position) applies
regardless of favorited status, so a stale non-null position on a
favorited=False row could cause IntegrityError when
insert_favorite_dashboard assigns the same position.

Also fix re-favoriting to assign a correct position instead of
leaving it null.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@DominikB2014 DominikB2014 merged commit ee3e0be into master Mar 9, 2026
56 checks passed
@DominikB2014 DominikB2014 deleted the dominikbuszowiecki/dain-1287-use-starred-column-to-favourite-and-unfavourite branch March 9, 2026 15:38
DominikB2014 added a commit that referenced this pull request Mar 9, 2026
…0209)

Adds a sync mechanism that automatically favorites certain prebuilt
dashboards for users on first load, mirroring the pattern used in
explore saved queries (`sync_prebuilt_queries_starred`).

An optional `pre_favorited` field is added to the `PrebuiltDashboard`
TypedDict. When set to `True`, `sync_prebuilt_dashboards_favorited()`
creates `DashboardFavoriteUser` records for users who haven't interacted
with those dashboards yet. This runs on the dashboards list GET endpoint
with a user-scoped lock (separate from the org-scoped lock used for
dashboard sync).

The enabled prebuilt dashboard logic (option + feature flag check) is
extracted into a shared `get_enabled_prebuilt_dashboards()` helper used
by both sync functions.

Pre-favorited dashboards: Backend Overview, Web Vitals, Mobile Vitals,
AI Agents Overview, MCP Overview.

Do not merge until #110204
merges

---------

Co-authored-by: Claude Opus 4.6 <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