Skip to content

fix(dashboards): Resolve prebuilt dashboard placeholder IDs before duplication#110802

Merged
gggritso merged 4 commits intomasterfrom
worktree-georgegritsouk/dain-1300-resolve-prebuilt-linkeddashboards-placeholder-ids-before
Mar 17, 2026
Merged

fix(dashboards): Resolve prebuilt dashboard placeholder IDs before duplication#110802
gggritso merged 4 commits intomasterfrom
worktree-georgegritsouk/dain-1300-resolve-prebuilt-linkeddashboards-placeholder-ids-before

Conversation

@gggritso
Copy link
Copy Markdown
Member

@gggritso gggritso commented Mar 16, 2026

Prebuilt dashboards reference other prebuilt dashboards via linkedDashboards with placeholder dashboardId: '-1'. During duplication from list/grid views, these placeholders were sent directly to the backend, causing failures.

Extracts shared pure utilities (getLinkedDashboardPrebuiltIds, replacePlaceholderLinkedDashboardIds) from the existing usePopulateLinkedDashboards hook and adds an async resolver (resolveLinkedDashboardIds) that fetches real IDs via TanStack queryClient.fetchQuery before cloning. Unresolved linked dashboards are dropped to avoid sending invalid -1 IDs.

The resolver shares a makeDashboardsQueryKey helper with the existing hook to keep query key construction consistent.

This creates a bit of duplication with the code that resolves prebuilt IDs for displaying the pre-built dashboard, but that whole layer will need a re-look anyway. I filed a separate ticket for that: https://linear.app/getsentry/issue/DAIN-1341/consolidate-dashboards-data-fetching-code

DAIN-1300

…ore duplication

Prebuilt dashboards reference other prebuilt dashboards via
linkedDashboards with placeholder dashboardId '-1'. During duplication
from list/grid views, these placeholders were sent directly to the
backend, causing failures.

Extract shared pure utilities from usePopulateLinkedDashboards and add
an async resolver (resolveLinkedDashboardIds) that fetches real IDs via
TanStack queryClient.fetchQuery before cloning. Unresolved linked
dashboards are dropped to avoid sending invalid '-1' IDs.

DAIN-1300
@gggritso gggritso requested a review from a team as a code owner March 16, 2026 21:06
@linear-code
Copy link
Copy Markdown

linear-code bot commented Mar 16, 2026

@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Mar 16, 2026
Copy link
Copy Markdown
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.

Fix All in Cursor

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Loading state starts after resolver request
    • Moved setIsLoading(true) to the start of the duplication try-block so loading is active during linked-dashboard ID resolution and the button stays disabled throughout the async flow.

Create PR

Or push these changes by commenting:

@cursor push 6f48b888bd
Preview (6f48b888bd)
diff --git a/static/app/views/dashboards/hooks/useDuplicateDashboard.tsx b/static/app/views/dashboards/hooks/useDuplicateDashboard.tsx
--- a/static/app/views/dashboards/hooks/useDuplicateDashboard.tsx
+++ b/static/app/views/dashboards/hooks/useDuplicateDashboard.tsx
@@ -74,6 +74,7 @@
         );
       }
       try {
+        setIsLoading(true);
         const dashboardDetail = await resolveLinkedDashboardIds({
           queryClient,
           orgSlug: organization.slug,
@@ -83,7 +84,6 @@
         delete newDashboard.prebuiltId;
         newDashboard.title = `${newDashboard.title} copy`;
         newDashboard.widgets.map(widget => (widget.id = undefined));
-        setIsLoading(true);
         const copiedDashboard = await createDashboard(
           api,
           organization.slug,

This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.

@gggritso gggritso changed the title fix(dashboards): Resolve prebuilt linkedDashboard placeholder IDs before duplication fix(dashboards): Resolve prebuilt dashboard placeholder IDs before duplication Mar 17, 2026
Copy link
Copy Markdown
Contributor

@DominikB2014 DominikB2014 left a comment

Choose a reason for hiding this comment

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

lgtm! Although there's an optimization which might be nice to add imo

Comment on lines +47 to +51
const [resolvedDashboards] = await queryClient.fetchQuery({
queryKey,
queryFn: fetchDataQuery<DashboardDetails[]>,
staleTime: Infinity,
});
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.

Instead fetching the needed prebuiltIds, what if we fetch ALL prebuilt ids at once. This lets us maintain a constant query key, and we don't need to make a new request for every prebuilt dashboard.

I think with the current approach, the query key will be different per prebuilt dashboard because they all use different prebuilt ids in the linked dashboards

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That's true but on the flipside if we fetch all dashboards, that's slower! I don't think it's very common to duplicate many dashboards in a row 🤔

Copy link
Copy Markdown
Contributor

@DominikB2014 DominikB2014 Mar 17, 2026

Choose a reason for hiding this comment

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

I think going between prebuilt dashboards would benefit from the caching too! I guess in an ideal world we would have an endpoint that only returns the ids, we don't need all the other data associated with a dashboard, but probs not that important to optimize.

I'll leave it up to you tho, just throwing the idea out there

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

True, but also depends on how many pre-built dashboards we end up having, if we end up with a lot it'll continue to make sense to only fetch the right ones 🤔 anyway, fair suggestion! I'll leave it out for now, but I'll come back to this when we re-do the fetching logic

@gggritso gggritso merged commit 7c8666b into master Mar 17, 2026
65 checks passed
@gggritso gggritso deleted the worktree-georgegritsouk/dain-1300-resolve-prebuilt-linkeddashboards-placeholder-ids-before branch March 17, 2026 14:58
@github-actions github-actions bot locked and limited conversation to collaborators Apr 2, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants