DashboardScenePage: Correct slug in self referencing data links (#100048)

* switch to useLayoutEffect so queries run after url correction

* add comment

* suparate useEffects to avoid update on slug change

* add url correction for missing slug

* account for slug change during dashboard rename

* simplify fix

* add e2e test for data link without slug

* remove old comment

* remove newly added path from useEffect dependencies
pull/103731/head^2
Sergej-Vlasov 1 month ago committed by GitHub
parent 79400018a4
commit 4648ba396b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 47
      e2e/dashboards-suite/dashboard-links-without-slug.spec.ts
  2. 256
      e2e/dashboards/DataLinkWithoutSlugTest.json
  3. 33
      public/app/features/dashboard-scene/pages/DashboardScenePage.tsx

@ -0,0 +1,47 @@
import testDashboard from '../dashboards/DataLinkWithoutSlugTest.json';
import { e2e } from '../utils';
describe('Dashboard with data links that have no slug', () => {
beforeEach(() => {
e2e.flows.login(Cypress.env('USERNAME'), Cypress.env('PASSWORD'));
});
it('Should not reload if linking to same dashboard', () => {
cy.intercept({
pathname: '/api/ds/query',
}).as('query');
e2e.flows.importDashboard(testDashboard, 1000, true);
cy.wait('@query');
e2e.components.Panels.Panel.title('Data links without slug').should('exist');
e2e.components.DataLinksContextMenu.singleLink().contains('9yy21uzzxypg').click();
cy.contains('Loading', { timeout: 500 })
.should(() => {}) // prevent test from failing if it does not find loading
.then(throwIfLoadingFound);
cy.url().should('include', urlShouldContain);
e2e.components.DataLinksContextMenu.singleLink().contains('dr199bpvpcru').click();
cy.contains('Loading', { timeout: 500 })
.should(() => {}) // prevent test from failing if it does not find loading
.then(throwIfLoadingFound);
cy.url().should('include', urlShouldContain);
e2e.components.DataLinksContextMenu.singleLink().contains('dre33fzyxcrz').click();
cy.contains('Loading', { timeout: 500 })
.should(() => {}) // prevent test from failing if it does not find loading
.then(throwIfLoadingFound);
cy.url().should('include', urlShouldContain);
});
});
const urlShouldContain = '/d/data-link-no-slug/data-link-without-slug-test';
const throwIfLoadingFound = (el: JQuery) => {
if (el.length) {
// This means dashboard refreshes when clicking self-referencing data link
// that has no slug in it
throw new Error('Should not contain Loading');
}
};

@ -0,0 +1,256 @@
{
"annotations": {
"list": [
{
"builtIn": 1,
"datasource": {
"type": "grafana",
"uid": "-- Grafana --"
},
"enable": true,
"hide": true,
"iconColor": "rgba(0, 211, 255, 1)",
"name": "Annotations & Alerts",
"type": "dashboard"
}
]
},
"editable": true,
"fiscalYearStartMonth": 0,
"graphTooltip": 0,
"id": 135,
"links": [],
"panels": [
{
"datasource": {
"type": "grafana-testdata-datasource",
"uid": "PD8C576611E62080A"
},
"fieldConfig": {
"defaults": {
"color": {
"mode": "thresholds"
},
"custom": {
"align": "auto",
"cellOptions": {
"type": "auto"
},
"inspect": false
},
"links": [
{
"title": "",
"url": "/d/${__dashboard.uid}?var-instance=${__data.fields.test1}&${__url_time_range}"
}
],
"mappings": [],
"thresholds": {
"mode": "absolute",
"steps": [
{
"color": "green"
},
{
"color": "red",
"value": 80
}
]
}
},
"overrides": []
},
"gridPos": {
"h": 8,
"w": 12,
"x": 0,
"y": 0
},
"id": 4,
"options": {
"cellHeight": "sm",
"footer": {
"countRows": false,
"fields": "",
"reducer": ["sum"],
"show": false
},
"showHeader": true
},
"pluginVersion": "11.6.0-pre",
"targets": [
{
"alias": "test1",
"datasource": {
"type": "grafana-testdata-datasource",
"uid": "PD8C576611E62080A"
},
"refId": "A",
"scenarioId": "csv_metric_values",
"stringInput": "9wvfgzurfzb, 9yy21uzzxypg, dr199bpvpcru, dre33fzyxcrz, gc6j7crvrcpf, u6g9zuxvxypv"
}
],
"title": "Data links without slug",
"type": "table"
},
{
"datasource": {
"type": "prometheus",
"uid": "gdev-prometheus"
},
"fieldConfig": {
"defaults": {
"color": {
"mode": "palette-classic"
},
"custom": {
"axisBorderShow": false,
"axisCenteredZero": false,
"axisColorMode": "text",
"axisLabel": "",
"axisPlacement": "auto",
"barAlignment": 0,
"barWidthFactor": 0.6,
"drawStyle": "line",
"fillOpacity": 0,
"gradientMode": "none",
"hideFrom": {
"legend": false,
"tooltip": false,
"viz": false
},
"insertNulls": false,
"lineInterpolation": "linear",
"lineWidth": 1,
"pointSize": 5,
"scaleDistribution": {
"type": "linear"
},
"showPoints": "auto",
"spanNulls": false,
"stacking": {
"group": "A",
"mode": "none"
},
"thresholdsStyle": {
"mode": "off"
}
},
"mappings": [],
"thresholds": {
"mode": "absolute",
"steps": [
{
"color": "green"
},
{
"color": "red",
"value": 80
}
]
}
},
"overrides": []
},
"gridPos": {
"h": 8,
"w": 12,
"x": 12,
"y": 0
},
"id": 3,
"options": {
"legend": {
"calcs": [],
"displayMode": "list",
"placement": "bottom",
"showLegend": true
},
"tooltip": {
"hideZeros": false,
"mode": "single",
"sort": "none"
}
},
"pluginVersion": "11.6.0-pre",
"targets": [
{
"datasource": {
"type": "prometheus",
"uid": "gdev-prometheus"
},
"disableTextWrap": false,
"editorMode": "builder",
"expr": "counters_logins{geohash=\"$instance\"}",
"fullMetaSearch": false,
"includeNullMetadata": true,
"instant": false,
"legendFormat": "__auto",
"range": true,
"refId": "A",
"useBackend": false
}
],
"title": "Panel Title",
"type": "timeseries"
}
],
"preload": false,
"refresh": "",
"schemaVersion": 41,
"tags": [],
"templating": {
"list": [
{
"current": {
"text": "9wvfgzurfzb",
"value": "9wvfgzurfzb"
},
"name": "instance",
"options": [
{
"selected": true,
"text": "9wvfgzurfzb",
"value": "9wvfgzurfzb"
},
{
"selected": false,
"text": "9yy21uzzxypg",
"value": "9yy21uzzxypg"
},
{
"selected": false,
"text": "dr199bpvpcru",
"value": "dr199bpvpcru"
},
{
"selected": false,
"text": "dre33fzyxcrz",
"value": "dre33fzyxcrz"
},
{
"selected": false,
"text": "gc6j7crvrcpf",
"value": "gc6j7crvrcpf"
},
{
"selected": false,
"text": "u6g9zuxvxypv",
"value": "u6g9zuxvxypv"
}
],
"query": "9wvfgzurfzb, 9yy21uzzxypg, dr199bpvpcru, dre33fzyxcrz, gc6j7crvrcpf, u6g9zuxvxypv",
"type": "custom"
}
]
},
"time": {
"from": "now-1h",
"to": "now"
},
"timepicker": {},
"timezone": "utc",
"title": "Data Link without slug test",
"uid": "data-link-no-slug",
"version": 3
}

@ -1,9 +1,9 @@
// Libraries
import { useEffect } from 'react';
import { useParams } from 'react-router-dom-v5-compat';
import { useEffect, useRef } from 'react';
import { Params, useParams } from 'react-router-dom-v5-compat';
import { usePrevious } from 'react-use';
import { PageLayoutType } from '@grafana/data';
import { locationService } from '@grafana/runtime';
import { UrlSyncContextProvider } from '@grafana/scenes';
import { Box } from '@grafana/ui';
import { Page } from 'app/core/components/Page/Page';
@ -31,6 +31,7 @@ export function DashboardScenePage({ route, queryParams, location }: Props) {
const { dashboard, isLoading, loadError } = stateManager.useState();
// After scene migration is complete and we get rid of old dashboard we should refactor dashboardWatcher so this route reload is not need
const routeReloadCounter = (location.state as any)?.routeReloadCounter;
const prevParams = useRef<Params<string>>(params);
useEffect(() => {
if (route.routeName === DashboardRoutes.Normal && type === 'snapshot') {
@ -48,7 +49,31 @@ export function DashboardScenePage({ route, queryParams, location }: Props) {
return () => {
stateManager.clearState();
};
}, [stateManager, uid, route.routeName, queryParams.folderUid, routeReloadCounter, slug, type, path]);
// removing slug and path (which has slug in it) from dependencies to prevent unmount when data links reference
// the same dashboard with no slug in url
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [stateManager, uid, route.routeName, queryParams.folderUid, routeReloadCounter, type]);
useEffect(() => {
// This use effect corrects URL without refresh when navigating to the same dashboard
// using data link that has no slug in url
if (route.routeName === DashboardRoutes.Normal) {
// correct URL only when there are no new slug
// if slug is defined and incorrect it will be corrected in stateManager
if (uid === prevParams.current.uid && prevParams.current.slug && !slug) {
const correctedUrl = `/d/${uid}/${prevParams.current.slug}`;
locationService.replace({
...locationService.getLocation(),
pathname: correctedUrl,
});
}
}
return () => {
prevParams.current = { uid, slug: !slug ? prevParams.current.slug : slug };
};
}, [route, slug, type, uid]);
if (!dashboard) {
let errorElement;

Loading…
Cancel
Save