-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat(dashboards): link query overview to summary #103530
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(dashboards): link query overview to summary #103530
Conversation
| linkedDashboard => linkedDashboard.field === field | ||
| ); | ||
| if (dashboardLink) { | ||
| if (dashboardLink && dashboardLink.dashboardId !== '-1') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Fix Dashboard Linking for Static IDs
The getDashboardUrl function excludes links with dashboardId === '-1' but doesn't handle the corresponding staticDashboardId property for prebuilt dashboard links. This breaks the feature to link from queries overview to summary dashboard, as the condition will return undefined without processing the prebuilt dashboard ID. The code needs to handle the case where staticDashboardId should be used instead of dashboardId.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commented about this below
| const dashboardLink = widget.queries[0]?.linkedDashboards?.find( | ||
| linkedDashboard => linkedDashboard.field === field | ||
| ); | ||
| if (dashboardLink) { | ||
| if (dashboardLink && dashboardLink.dashboardId !== '-1') { | ||
| const newTemporaryFilters: GlobalFilter[] = | ||
| dashboardFilters[DashboardFilterKeys.GLOBAL_FILTER] ?? []; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: getDashboardUrl() does not handle staticDashboardId for prebuilt dashboards, causing drill-down links to fail when dashboardId is '-1'.
Severity: CRITICAL | Confidence: 0.95
🔍 Detailed Analysis
The getDashboardUrl() function in fieldRenderers.tsx fails to generate a URL for prebuilt dashboards. When a linkedDashboard entry has dashboardId: '-1' and a staticDashboardId, the condition dashboardLink.dashboardId !== '-1' evaluates to false. This prevents the URL generation logic for saved dashboards from executing, and no alternative handling for staticDashboardId is present. As a result, the function returns undefined, leading to a broken drill-down navigation feature for prebuilt dashboards.
💡 Suggested Fix
Implement logic within getDashboardUrl() to handle linkedDashboard entries where dashboardId is '-1' and staticDashboardId is present. This logic should use staticDashboardId to construct the appropriate URL for prebuilt dashboards, ensuring the drill-down feature functions correctly.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: static/app/utils/discover/fieldRenderers.tsx#L1424-L1430
Potential issue: The `getDashboardUrl()` function in `fieldRenderers.tsx` fails to
generate a URL for prebuilt dashboards. When a `linkedDashboard` entry has `dashboardId:
'-1'` and a `staticDashboardId`, the condition `dashboardLink.dashboardId !== '-1'`
evaluates to false. This prevents the URL generation logic for saved dashboards from
executing, and no alternative handling for `staticDashboardId` is present. As a result,
the function returns `undefined`, leading to a broken drill-down navigation feature for
prebuilt dashboards.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference_id: 2780090
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This wrong, the plan is to automatically fetch and populate the dashboardId, links should never be based on prebuilt id
…d-link-between-query-summary-and-query-detail
…n-query-summary-and-query-detail' of github.com:getsentry/sentry into dominikbuszowiecki/browse-102-add-dashboard-link-between-query-summary-and-query-detail
PrebuiltDashboardRendererwhich makes it easier to share logic for how prebuilt dashboards will renderNote: There is one more piece to this puzzle, we need to update the dashboards endpoint to allow us to fetch the actual dashboard id from the static id. See BROWSE-121