mirror of https://github.com/grafana/grafana.git
Revert "Dashboards: Refactor URL state preservation functionality when reloading on params change" (#105931)
Revert "Dashboards: Refactor URL state preservation functionality when reload…"
This reverts commit 10628e8741.
This commit is contained in:
Victor Marin
GitHub
parent
b02a375338
commit
968bb08703
|
|
@ -15,7 +15,6 @@ import { DashboardRoutes } from 'app/types';
|
|||
|
||||
import { DashboardPrompt } from '../saving/DashboardPrompt';
|
||||
import { DashboardPreviewBanner } from '../saving/provisioned/DashboardPreviewBanner';
|
||||
import { preserveDashboardSceneStateInLocalStorage } from '../utils/dashboardSessionState';
|
||||
|
||||
import { getDashboardScenePageStateManager } from './DashboardScenePageStateManager';
|
||||
|
||||
|
|
@ -48,7 +47,6 @@ export function DashboardScenePage({ route, queryParams, location }: Props) {
|
|||
}
|
||||
|
||||
return () => {
|
||||
preserveDashboardSceneStateInLocalStorage(locationService.getSearch(), uid);
|
||||
stateManager.clearState();
|
||||
};
|
||||
|
||||
|
|
|
|||
|
|
@ -719,57 +719,6 @@ describe('DashboardScenePageStateManager v2', () => {
|
|||
fetchDashboardSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('should not use cache if cache version and current dashboard state version differ', async () => {
|
||||
const getDashSpy = jest.fn();
|
||||
setupDashboardAPI(
|
||||
{
|
||||
access: {},
|
||||
apiVersion: 'v2alpha1',
|
||||
kind: 'DashboardWithAccessInfo',
|
||||
metadata: {
|
||||
name: 'fake-dash',
|
||||
creationTimestamp: '',
|
||||
resourceVersion: '1',
|
||||
generation: 1,
|
||||
},
|
||||
spec: { ...defaultDashboardV2Spec() },
|
||||
},
|
||||
getDashSpy
|
||||
);
|
||||
|
||||
const loader = new DashboardScenePageStateManagerV2({});
|
||||
await loader.loadDashboard({ uid: 'fake-dash', route: DashboardRoutes.Normal });
|
||||
|
||||
expect(getDashSpy).toHaveBeenCalledTimes(1);
|
||||
|
||||
const mockDashboard: DashboardWithAccessInfo<DashboardV2Spec> = {
|
||||
access: {},
|
||||
apiVersion: 'v2alpha1',
|
||||
kind: 'DashboardWithAccessInfo',
|
||||
metadata: {
|
||||
name: 'fake-dash',
|
||||
creationTimestamp: '',
|
||||
resourceVersion: '1',
|
||||
generation: 2,
|
||||
},
|
||||
spec: { ...defaultDashboardV2Spec() },
|
||||
};
|
||||
|
||||
const fetchDashboardSpy = jest.spyOn(loader, 'fetchDashboard').mockResolvedValue(mockDashboard);
|
||||
|
||||
// mimic navigating from db1 to db2 and then back to db1, which maintains the cache. but on
|
||||
// db1 load the initial version will be 1. Since the cache is set we also need to verify against the
|
||||
// current dashboard state whether we should reload or not
|
||||
loader.setSceneCache('fake-dash', loader.state.dashboard!.clone({ version: 2 }));
|
||||
const options = { version: 2, scopes: [], timeRange: { from: 'now-1h', to: 'now' }, variables: {} };
|
||||
await loader.reloadDashboard(options);
|
||||
|
||||
expect(fetchDashboardSpy).toHaveBeenCalledTimes(1);
|
||||
expect(loader.state.dashboard?.state.version).toBe(2);
|
||||
|
||||
fetchDashboardSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('should handle errors during reload', async () => {
|
||||
const getDashSpy = jest.fn();
|
||||
setupDashboardAPI(
|
||||
|
|
@ -994,44 +943,6 @@ describe('UnifiedDashboardScenePageStateManager', () => {
|
|||
|
||||
expect(manager['activeManager']).toBeInstanceOf(DashboardScenePageStateManagerV2);
|
||||
});
|
||||
|
||||
it('should not use cache if cache version and current dashboard state version differ in v1', async () => {
|
||||
const loadDashboardMock = setupLoadDashboardMock({
|
||||
dashboard: { uid: 'fake-dash', editable: true, version: 0 },
|
||||
meta: {},
|
||||
});
|
||||
|
||||
const manager = new UnifiedDashboardScenePageStateManager({});
|
||||
await manager.loadDashboard({ uid: 'fake-dash', route: DashboardRoutes.Normal });
|
||||
|
||||
expect(loadDashboardMock).toHaveBeenCalledWith('db', '', 'fake-dash', undefined);
|
||||
expect(manager['activeManager']).toBeInstanceOf(DashboardScenePageStateManager);
|
||||
|
||||
loadDashboardMock.mockClear();
|
||||
|
||||
const mockDashboard: DashboardDTO = {
|
||||
dashboard: {
|
||||
uid: 'fake-dash',
|
||||
version: 2,
|
||||
title: 'fake-dash',
|
||||
} as DashboardDataDTO,
|
||||
meta: {},
|
||||
};
|
||||
|
||||
const fetchDashboardSpy = jest.spyOn(manager['activeManager'], 'fetchDashboard').mockResolvedValue(mockDashboard);
|
||||
|
||||
// mimic navigating from db1 to db2 and then back to db1, which maintains the cache. but on
|
||||
// db1 load the initial version will be 1. Since the cache is set we also need to verify against the
|
||||
// current dashboard state whether we should reload or not
|
||||
manager.setSceneCache('fake-dash', manager.state.dashboard!.clone({ version: 2 }));
|
||||
const options = { version: 2, scopes: [], timeRange: { from: 'now-1h', to: 'now' }, variables: {} };
|
||||
await manager.reloadDashboard(options);
|
||||
|
||||
expect(fetchDashboardSpy).toHaveBeenCalledTimes(1);
|
||||
expect(manager.state.dashboard?.state.version).toBe(2);
|
||||
|
||||
fetchDashboardSpy.mockRestore();
|
||||
});
|
||||
});
|
||||
|
||||
describe('Home dashboard', () => {
|
||||
|
|
|
|||
|
|
@ -522,20 +522,7 @@ export class DashboardScenePageStateManager extends DashboardScenePageStateManag
|
|||
const rsp = await this.fetchDashboard(options);
|
||||
const fromCache = this.getSceneFromCache(options.uid);
|
||||
|
||||
// check if cached db version is same as both
|
||||
// response and current db state. There are scenarios where they can differ
|
||||
// e.g: when reloadOnParamsChange ff is on the first loaded dashboard could be version 0
|
||||
// then on this reload call the rsp increments the version. When the cache is not set
|
||||
// it creates a new scene based on the new rsp. But if we navigate to another dashboard
|
||||
// and then back to the initial one, the cache is still set, but the dashboard will be loaded
|
||||
// again with version 0. Because the cache is set with the incremented version and the rsp on
|
||||
// reload will match the cached version we return and do nothing, but the set scene is still
|
||||
// the one for the version 0 dashboard, thus we verify dashboard state version as well
|
||||
if (
|
||||
fromCache &&
|
||||
fromCache.state.version === rsp?.dashboard.version &&
|
||||
fromCache.state.version === this.state.dashboard?.state.version
|
||||
) {
|
||||
if (fromCache && fromCache.state.version === rsp?.dashboard.version) {
|
||||
this.setState({ isLoading: false });
|
||||
return;
|
||||
}
|
||||
|
|
@ -553,11 +540,6 @@ export class DashboardScenePageStateManager extends DashboardScenePageStateManag
|
|||
|
||||
const scene = transformSaveModelToScene(rsp);
|
||||
|
||||
// we need to call and restore dashboard state on every reload that pulls a new dashboard version
|
||||
if (config.featureToggles.preserveDashboardStateWhenNavigating && Boolean(options.uid)) {
|
||||
restoreDashboardStateFromLocalStorage(scene);
|
||||
}
|
||||
|
||||
this.setSceneCache(options.uid, scene);
|
||||
|
||||
this.setState({ dashboard: scene, isLoading: false, options });
|
||||
|
|
@ -714,11 +696,7 @@ export class DashboardScenePageStateManagerV2 extends DashboardScenePageStateMan
|
|||
const rsp = await this.fetchDashboard(options);
|
||||
const fromCache = this.getSceneFromCache(options.uid);
|
||||
|
||||
if (
|
||||
fromCache &&
|
||||
fromCache.state.version === rsp?.metadata.generation &&
|
||||
fromCache.state.version === this.state.dashboard?.state.version
|
||||
) {
|
||||
if (fromCache && fromCache.state.version === rsp?.metadata.generation) {
|
||||
this.setState({ isLoading: false });
|
||||
return;
|
||||
}
|
||||
|
|
@ -736,11 +714,6 @@ export class DashboardScenePageStateManagerV2 extends DashboardScenePageStateMan
|
|||
|
||||
const scene = transformSaveModelSchemaV2ToScene(rsp);
|
||||
|
||||
// we need to call and restore dashboard state on every reload that pulls a new dashboard version
|
||||
if (config.featureToggles.preserveDashboardStateWhenNavigating && Boolean(options.uid)) {
|
||||
restoreDashboardStateFromLocalStorage(scene);
|
||||
}
|
||||
|
||||
this.setSceneCache(options.uid, scene);
|
||||
|
||||
this.setState({ dashboard: scene, isLoading: false, options });
|
||||
|
|
|
|||
|
|
@ -60,6 +60,7 @@ import { registerDashboardMacro } from '../scene/DashboardMacro';
|
|||
import { DashboardReloadBehavior } from '../scene/DashboardReloadBehavior';
|
||||
import { DashboardScene } from '../scene/DashboardScene';
|
||||
import { DashboardLayoutManager } from '../scene/types/DashboardLayoutManager';
|
||||
import { preserveDashboardSceneStateInLocalStorage } from '../utils/dashboardSessionState';
|
||||
import { getIntervalsFromQueryString } from '../utils/utils';
|
||||
|
||||
import { SnapshotVariable } from './custom-variables/SnapshotVariable';
|
||||
|
|
@ -195,6 +196,7 @@ export function transformSaveModelSchemaV2ToScene(dto: DashboardWithAccessInfo<D
|
|||
registerDashboardMacro,
|
||||
registerPanelInteractionsReporter,
|
||||
new behaviors.LiveNowTimer({ enabled: dashboard.liveNow }),
|
||||
preserveDashboardSceneStateInLocalStorage,
|
||||
addPanelsOnLoadBehavior,
|
||||
new DashboardReloadBehavior({
|
||||
reloadOnParamsChange: config.featureToggles.reloadDashboardsOnParamsChange && false,
|
||||
|
|
|
|||
|
|
@ -46,6 +46,7 @@ import { RowRepeaterBehavior } from '../scene/layout-default/RowRepeaterBehavior
|
|||
import { RowActions } from '../scene/layout-default/row-actions/RowActions';
|
||||
import { setDashboardPanelContext } from '../scene/setDashboardPanelContext';
|
||||
import { createPanelDataProvider } from '../utils/createPanelDataProvider';
|
||||
import { preserveDashboardSceneStateInLocalStorage } from '../utils/dashboardSessionState';
|
||||
import { DashboardInteractions } from '../utils/interactions';
|
||||
import { getVizPanelKeyForPanelId } from '../utils/utils';
|
||||
import { createVariablesForDashboard, createVariablesForSnapshot } from '../utils/variables';
|
||||
|
|
@ -234,6 +235,7 @@ export function createDashboardSceneFromDashboardModel(oldModel: DashboardModel,
|
|||
registerDashboardMacro,
|
||||
registerPanelInteractionsReporter,
|
||||
new behaviors.LiveNowTimer({ enabled: oldModel.liveNow }),
|
||||
preserveDashboardSceneStateInLocalStorage,
|
||||
addPanelsOnLoadBehavior,
|
||||
new DashboardReloadBehavior({
|
||||
reloadOnParamsChange: config.featureToggles.reloadDashboardsOnParamsChange && oldModel.meta.reloadOnParamsChange,
|
||||
|
|
|
|||
|
|
@ -4,11 +4,7 @@ import { DashboardDataDTO } from 'app/types';
|
|||
|
||||
import { transformSaveModelToScene } from '../serialization/transformSaveModelToScene';
|
||||
|
||||
import {
|
||||
PRESERVED_SCENE_STATE_KEY,
|
||||
preserveDashboardSceneStateInLocalStorage,
|
||||
restoreDashboardStateFromLocalStorage,
|
||||
} from './dashboardSessionState';
|
||||
import { PRESERVED_SCENE_STATE_KEY, restoreDashboardStateFromLocalStorage } from './dashboardSessionState';
|
||||
|
||||
describe('dashboardSessionState', () => {
|
||||
beforeAll(() => {
|
||||
|
|
@ -22,21 +18,35 @@ describe('dashboardSessionState', () => {
|
|||
beforeEach(() => {});
|
||||
|
||||
describe('behavior', () => {
|
||||
it('should do nothing if no uid', () => {
|
||||
it('should do nothing for default home dashboard', () => {
|
||||
const scene = buildTestScene();
|
||||
scene.setState({ uid: undefined });
|
||||
|
||||
const deactivate = scene.activate();
|
||||
expect(window.sessionStorage.getItem(PRESERVED_SCENE_STATE_KEY)).toBeNull();
|
||||
|
||||
preserveDashboardSceneStateInLocalStorage({} as URLSearchParams, undefined);
|
||||
|
||||
deactivate();
|
||||
expect(window.sessionStorage.getItem(PRESERVED_SCENE_STATE_KEY)).toBeNull();
|
||||
});
|
||||
|
||||
it('should capture dashboard scene state and save it to session storage', () => {
|
||||
const params = new URLSearchParams('from=now-6h&to=now&timezone=browser&var-customVar=a');
|
||||
it('should do nothing if dashboard version is 0', () => {
|
||||
const scene = buildTestScene();
|
||||
scene.setState({ version: 0 });
|
||||
|
||||
const deactivate = scene.activate();
|
||||
expect(window.sessionStorage.getItem(PRESERVED_SCENE_STATE_KEY)).toBeNull();
|
||||
|
||||
preserveDashboardSceneStateInLocalStorage(params, 'uid');
|
||||
deactivate();
|
||||
expect(window.sessionStorage.getItem(PRESERVED_SCENE_STATE_KEY)).toBeNull();
|
||||
});
|
||||
|
||||
it('should capture dashboard scene state and save it to session storage on deactivation', () => {
|
||||
const scene = buildTestScene();
|
||||
|
||||
const deactivate = scene.activate();
|
||||
expect(window.sessionStorage.getItem(PRESERVED_SCENE_STATE_KEY)).toBeNull();
|
||||
|
||||
deactivate();
|
||||
expect(window.sessionStorage.getItem(PRESERVED_SCENE_STATE_KEY)).toBe(
|
||||
'?from=now-6h&to=now&timezone=browser&var-customVar=a'
|
||||
);
|
||||
|
|
@ -61,45 +71,6 @@ describe('dashboardSessionState', () => {
|
|||
expect(timeRange?.state.to).toEqual('now');
|
||||
});
|
||||
|
||||
it('should use preserved state filters if current location has empty filters state', () => {
|
||||
// we have a saved state with filters set
|
||||
window.sessionStorage.setItem(
|
||||
PRESERVED_SCENE_STATE_KEY,
|
||||
'?var-customVar=b&var-nonApplicableVar=b&from=now-5m&to=now&timezone=browser&var-filters=cluster%7C%3D%7Cdev&var-filters=test'
|
||||
);
|
||||
|
||||
// but the target location also has filters key with an empty state
|
||||
locationService.replace({
|
||||
search: 'var-customVar=b&from=now-5m&to=now&timezone=browser&var-filters=',
|
||||
});
|
||||
|
||||
// var-filters must also be set on the scene otherwise restore fn will drop the filter url key
|
||||
const scene = buildTestScene({
|
||||
templating: {
|
||||
list: [
|
||||
{
|
||||
multi: true,
|
||||
name: 'customVar',
|
||||
query: 'a,b,c',
|
||||
type: 'custom',
|
||||
},
|
||||
{
|
||||
name: 'filters',
|
||||
type: 'adhoc',
|
||||
},
|
||||
],
|
||||
},
|
||||
});
|
||||
|
||||
restoreDashboardStateFromLocalStorage(scene);
|
||||
|
||||
expect(locationService.getLocation().search).toBe(
|
||||
'?var-customVar=b&from=now-5m&to=now&timezone=browser&var-filters=cluster%7C%3D%7Cdev&var-filters=test'
|
||||
);
|
||||
|
||||
jest.clearAllMocks();
|
||||
});
|
||||
|
||||
it('should remove query params that are not applicable on a target dashboard', () => {
|
||||
window.sessionStorage.setItem(
|
||||
PRESERVED_SCENE_STATE_KEY,
|
||||
|
|
@ -124,10 +95,23 @@ describe('dashboardSessionState', () => {
|
|||
|
||||
expect(locationService.getLocation().search).toBe('?var-customVar=b&from=now-6h&to=now&timezone=browser');
|
||||
});
|
||||
|
||||
it('should not restore state if dashboard version is 0', () => {
|
||||
window.sessionStorage.setItem(
|
||||
PRESERVED_SCENE_STATE_KEY,
|
||||
'?var-customVarNotOnDB=b&from=now-5m&to=now&timezone=browser'
|
||||
);
|
||||
const scene = buildTestScene();
|
||||
scene.setState({ version: 0 });
|
||||
|
||||
restoreDashboardStateFromLocalStorage(scene);
|
||||
|
||||
expect(locationService.getLocation().search).toBe('?var-customVar=b&from=now-6h&to=now&timezone=browser');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
function buildTestScene(overrides?: Partial<DashboardDataDTO>) {
|
||||
function buildTestScene() {
|
||||
const testDashboard: DashboardDataDTO = {
|
||||
annotations: { list: [] },
|
||||
editable: true,
|
||||
|
|
@ -158,7 +142,6 @@ function buildTestScene(overrides?: Partial<DashboardDataDTO>) {
|
|||
uid: 'edhmd9stpd6o0a',
|
||||
version: 24,
|
||||
weekStart: '',
|
||||
...overrides,
|
||||
};
|
||||
|
||||
const scene = transformSaveModelToScene({ dashboard: testDashboard, meta: {} });
|
||||
|
|
|
|||
|
|
@ -1,36 +1,30 @@
|
|||
import { UrlQueryMap, urlUtil } from '@grafana/data';
|
||||
import { config, locationService } from '@grafana/runtime';
|
||||
import { UrlSyncManager } from '@grafana/scenes';
|
||||
|
||||
import { DashboardScene } from '../scene/DashboardScene';
|
||||
|
||||
export const PRESERVED_SCENE_STATE_KEY = `grafana.dashboard.preservedUrlFiltersState`;
|
||||
|
||||
// TODO - deal with all this complexity, more details here https://github.com/grafana/grafana/pull/104780
|
||||
export function restoreDashboardStateFromLocalStorage(dashboard: DashboardScene) {
|
||||
if (!dashboard.state.version) {
|
||||
return;
|
||||
}
|
||||
|
||||
const preservedUrlState = window.sessionStorage.getItem(PRESERVED_SCENE_STATE_KEY);
|
||||
|
||||
if (preservedUrlState) {
|
||||
const preservedQueryParams = new URLSearchParams(preservedUrlState);
|
||||
const currentQueryParams = locationService.getSearch();
|
||||
const cleanedQueryParams = new URLSearchParams();
|
||||
|
||||
// iterate over preserved query params and append them to current query params if they don't already exist
|
||||
preservedQueryParams.forEach((value, key) => {
|
||||
// if somehow there are keys set with no values, we append new key-value pairs,
|
||||
// but need to clean empty ones after this loop so we don't lose any values
|
||||
if (!currentQueryParams.has(key) || currentQueryParams.get(key) === '') {
|
||||
if (!currentQueryParams.has(key)) {
|
||||
currentQueryParams.append(key, value);
|
||||
}
|
||||
});
|
||||
|
||||
// remove empty values
|
||||
currentQueryParams.forEach((value, key) => {
|
||||
if (value !== '') {
|
||||
cleanedQueryParams.append(key, value);
|
||||
}
|
||||
});
|
||||
|
||||
for (const key of Array.from(cleanedQueryParams.keys())) {
|
||||
for (const key of Array.from(currentQueryParams.keys())) {
|
||||
// preserve non-variable query params, i.e. time range
|
||||
if (!key.startsWith('var-')) {
|
||||
continue;
|
||||
|
|
@ -38,11 +32,11 @@ export function restoreDashboardStateFromLocalStorage(dashboard: DashboardScene)
|
|||
|
||||
// remove params for variables that are not present on the target dashboard
|
||||
if (!dashboard.state.$variables?.getByName(key.replace('var-', ''))) {
|
||||
cleanedQueryParams.delete(key);
|
||||
currentQueryParams.delete(key);
|
||||
}
|
||||
}
|
||||
|
||||
const finalParams = cleanedQueryParams.toString();
|
||||
const finalParams = currentQueryParams.toString();
|
||||
if (finalParams) {
|
||||
locationService.replace({ search: finalParams });
|
||||
}
|
||||
|
|
@ -52,35 +46,32 @@ export function restoreDashboardStateFromLocalStorage(dashboard: DashboardScene)
|
|||
/**
|
||||
* Scenes behavior that will capture currently selected variables and time range and save them to local storage, so that they can be applied when the next dashboard is loaded.
|
||||
*/
|
||||
export function preserveDashboardSceneStateInLocalStorage(search: URLSearchParams, uid?: string) {
|
||||
export function preserveDashboardSceneStateInLocalStorage(scene: DashboardScene) {
|
||||
if (!config.featureToggles.preserveDashboardStateWhenNavigating) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Skipping saving state for default home dashboard
|
||||
if (!uid) {
|
||||
return;
|
||||
}
|
||||
return () => {
|
||||
// Skipping saving state for default home dashboard
|
||||
if (!scene.state.uid || !scene.state.version) {
|
||||
return;
|
||||
}
|
||||
|
||||
const queryParams: Record<string, string> = {};
|
||||
search.forEach((value, key) => {
|
||||
queryParams[key] = value;
|
||||
});
|
||||
const urlStates: UrlQueryMap = Object.fromEntries(
|
||||
Object.entries(new UrlSyncManager().getUrlState(scene)).filter(
|
||||
([key]) => key.startsWith('var-') || key === 'from' || key === 'to' || key === 'timezone'
|
||||
)
|
||||
);
|
||||
|
||||
const urlStates: UrlQueryMap = Object.fromEntries(
|
||||
Object.entries(queryParams).filter(
|
||||
([key]) => key.startsWith('var-') || key === 'from' || key === 'to' || key === 'timezone'
|
||||
)
|
||||
);
|
||||
const nonEmptyUrlStates = Object.fromEntries(
|
||||
Object.entries(urlStates).filter(([key, value]) => !(Array.isArray(value) && value.length === 0))
|
||||
);
|
||||
|
||||
const nonEmptyUrlStates = Object.fromEntries(
|
||||
Object.entries(urlStates).filter(([key, value]) => !(Array.isArray(value) && value.length === 0))
|
||||
);
|
||||
|
||||
// If there's anything to preserve, save it to local storage
|
||||
if (Object.keys(nonEmptyUrlStates).length > 0) {
|
||||
window.sessionStorage.setItem(PRESERVED_SCENE_STATE_KEY, urlUtil.renderUrl('', nonEmptyUrlStates));
|
||||
} else {
|
||||
window.sessionStorage.removeItem(PRESERVED_SCENE_STATE_KEY);
|
||||
}
|
||||
// If there's anything to preserve, save it to local storage
|
||||
if (Object.keys(nonEmptyUrlStates).length > 0) {
|
||||
window.sessionStorage.setItem(PRESERVED_SCENE_STATE_KEY, urlUtil.renderUrl('', nonEmptyUrlStates));
|
||||
} else {
|
||||
window.sessionStorage.removeItem(PRESERVED_SCENE_STATE_KEY);
|
||||
}
|
||||
};
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue