diff --git a/public/app/features/trails/DataTrail.test.tsx b/public/app/features/trails/DataTrail.test.tsx index 9b62ddeff41..a6ccc31f354 100644 --- a/public/app/features/trails/DataTrail.test.tsx +++ b/public/app/features/trails/DataTrail.test.tsx @@ -1,4 +1,5 @@ import { locationService, setDataSourceSrv } from '@grafana/runtime'; +import { AdHocFiltersVariable, sceneGraph } from '@grafana/scenes'; import { MockDataSourceSrv, mockDataSource } from '../alerting/unified/mocks'; import { DataSourceType } from '../alerting/unified/utils/datasource'; @@ -7,7 +8,7 @@ import { activateFullSceneTree } from '../dashboard-scene/utils/test-utils'; import { DataTrail } from './DataTrail'; import { MetricScene } from './MetricScene'; import { MetricSelectScene } from './MetricSelectScene'; -import { MetricSelectedEvent } from './shared'; +import { MetricSelectedEvent, VAR_FILTERS } from './shared'; describe('DataTrail', () => { beforeAll(() => { @@ -61,10 +62,6 @@ describe('DataTrail', () => { expect(trail.state.history.state.steps[1].type).toBe('metric'); }); - it('Should set history current step to 1', () => { - expect(trail.state.history.state.currentStep).toBe(1); - }); - it('Should set history currentStep to 1', () => { expect(trail.state.history.state.currentStep).toBe(1); }); @@ -73,6 +70,10 @@ describe('DataTrail', () => { expect(trail.state.history.state.steps[1].parentIndex).toBe(0); }); + it('Should have time range `from` be default "now-6h"', () => { + expect(trail.state.$timeRange?.state.from).toBe('now-6h'); + }); + describe('And browser back button is pressed', () => { locationService.getHistory().goBack(); @@ -81,6 +82,271 @@ describe('DataTrail', () => { expect(pathname).toEqual(preTrailUrl); }); }); + + describe('And when changing the time range `from` to "now-1h"', () => { + beforeEach(() => { + trail.state.$timeRange?.setState({ from: 'now-1h' }); + }); + + it('should sync state with url', () => { + expect(locationService.getSearchObject().from).toBe('now-1h'); + }); + + it('should add history step', () => { + expect(trail.state.history.state.steps[2].type).toBe('time'); + }); + + it('Should set history currentStep to 2', () => { + expect(trail.state.history.state.currentStep).toBe(2); + }); + + it('Should set history step 2 parentIndex to 1', () => { + expect(trail.state.history.state.steps[2].parentIndex).toBe(1); + }); + + it('Should have time range `from` be updated "now-1h"', () => { + expect(trail.state.$timeRange?.state.from).toBe('now-1h'); + }); + + it('Previous history step should have previous default `from` of "now-6h"', () => { + expect(trail.state.history.state.steps[1].trailState.$timeRange?.state.from).toBe('now-6h'); + }); + + it('Current history step should have new `from` of "now-1h"', () => { + expect(trail.state.history.state.steps[2].trailState.$timeRange?.state.from).toBe('now-1h'); + }); + + describe('And when traversing back to step 1', () => { + beforeEach(() => { + trail.state.history.goBackToStep(1); + }); + + it('Should set history currentStep to 1', () => { + expect(trail.state.history.state.currentStep).toBe(1); + }); + + it('should sync state with url', () => { + expect(locationService.getSearchObject().from).toBe('now-6h'); + }); + + it('Should have time range `from` be set back to "now-6h"', () => { + expect(trail.state.$timeRange?.state.from).toBe('now-6h'); + }); + + describe('And then when changing the time range `from` to "now-15m"', () => { + beforeEach(() => { + trail.state.$timeRange?.setState({ from: 'now-15m' }); + }); + + it('should sync state with url', () => { + expect(locationService.getSearchObject().from).toBe('now-15m'); + }); + + it('should add history step', () => { + expect(trail.state.history.state.steps[3].type).toBe('time'); + }); + + it('Should set history currentStep to 3', () => { + expect(trail.state.history.state.currentStep).toBe(3); + }); + + it('Should set history step 3 parentIndex to 1', () => { + expect(trail.state.history.state.steps[3].parentIndex).toBe(1); + }); + + it('Should have time range `from` be updated "now-15m"', () => { + expect(trail.state.$timeRange?.state.from).toBe('now-15m'); + }); + + it('History step 1 (parent) should have previous default `from` of "now-6h"', () => { + expect(trail.state.history.state.steps[1].trailState.$timeRange?.state.from).toBe('now-6h'); + }); + + it('History step 2 should still have `from` of "now-1h"', () => { + expect(trail.state.history.state.steps[2].trailState.$timeRange?.state.from).toBe('now-1h'); + }); + + describe('And then when returning again to step 1', () => { + beforeEach(() => { + trail.state.history.goBackToStep(1); + }); + + it('Should set history currentStep to 1', () => { + expect(trail.state.history.state.currentStep).toBe(1); + }); + + it('should sync state with url', () => { + expect(locationService.getSearchObject().from).toBe('now-6h'); + }); + + it('History step 1 (parent) should have previous default `from` of "now-6h"', () => { + expect(trail.state.history.state.steps[1].trailState.$timeRange?.state.from).toBe('now-6h'); + }); + + it('History step 2 should still have `from` of "now-1h"', () => { + expect(trail.state.history.state.steps[2].trailState.$timeRange?.state.from).toBe('now-1h'); + }); + + it('History step 3 should still have `from` of "now-15m"', () => { + expect(trail.state.history.state.steps[3].trailState.$timeRange?.state.from).toBe('now-15m'); + }); + + it('Should have time range `from` be set back to "now-6h"', () => { + expect(trail.state.$timeRange?.state.from).toBe('now-6h'); + }); + }); + }); + }); + }); + + function getFilterVar() { + const variable = sceneGraph.lookupVariable(VAR_FILTERS, trail); + if (variable instanceof AdHocFiltersVariable) { + return variable; + } + throw new Error('getFilterVar failed'); + } + + function getStepFilterVar(step: number) { + const variable = trail.state.history.state.steps[step].trailState.$variables?.getByName(VAR_FILTERS); + if (variable instanceof AdHocFiltersVariable) { + return variable; + } + throw new Error(`getStepFilterVar failed for step ${step}`); + } + + it('Should have default empty filter', () => { + expect(getFilterVar().state.filters.length).toBe(0); + }); + + describe('And when changing the filter to zone=a', () => { + beforeEach(() => { + getFilterVar().setState({ filters: [{ key: 'zone', operator: '=', value: 'a' }] }); + }); + + it('should sync state with url', () => { + expect(decodeURIComponent(locationService.getSearchObject()['var-filters']?.toString()!)).toBe('zone|=|a'); + }); + + it('should add history step', () => { + expect(trail.state.history.state.steps[2].type).toBe('filters'); + }); + + it('Should set history currentStep to 2', () => { + expect(trail.state.history.state.currentStep).toBe(2); + }); + + it('Should set history step 2 parentIndex to 1', () => { + expect(trail.state.history.state.steps[2].parentIndex).toBe(1); + }); + + it('Should have filter be updated to "zone=a"', () => { + expect(getFilterVar().state.filters[0].key).toBe('zone'); + expect(getFilterVar().state.filters[0].value).toBe('a'); + }); + + it('Previous history step should have empty filter', () => { + expect(getStepFilterVar(1).state.filters.length).toBe(0); + }); + + it('Current history step should have new filter zone=a', () => { + expect(getStepFilterVar(2).state.filters[0].key).toBe('zone'); + expect(getStepFilterVar(2).state.filters[0].value).toBe('a'); + }); + + describe('And when traversing back to step 1', () => { + beforeEach(() => { + trail.state.history.goBackToStep(1); + }); + + it('Should set history currentStep to 1', () => { + expect(trail.state.history.state.currentStep).toBe(1); + }); + + it('should sync state with url', () => { + expect(locationService.getSearchObject()['var-filters']).toBe(''); + }); + + it('Should have filters set back to empty', () => { + expect(getFilterVar().state.filters.length).toBe(0); + }); + + describe('And when changing the filter to zone=b', () => { + beforeEach(() => { + getFilterVar().setState({ filters: [{ key: 'zone', operator: '=', value: 'b' }] }); + }); + + it('should sync state with url', () => { + expect(decodeURIComponent(locationService.getSearchObject()['var-filters']?.toString()!)).toBe( + 'zone|=|b' + ); + }); + + it('should add history step', () => { + expect(trail.state.history.state.steps[3].type).toBe('filters'); + }); + + it('Should set history currentStep to 3', () => { + expect(trail.state.history.state.currentStep).toBe(3); + }); + + it('Should set history step 3 parentIndex to 1', () => { + expect(trail.state.history.state.steps[3].parentIndex).toBe(1); + }); + + it('Should have filter be updated to "zone=b"', () => { + expect(getFilterVar().state.filters[0].key).toBe('zone'); + expect(getFilterVar().state.filters[0].value).toBe('b'); + }); + + it('Parent history step 1 should still have empty filter', () => { + expect(getStepFilterVar(1).state.filters.length).toBe(0); + }); + + it('History step 2 should still have old filter zone=a', () => { + expect(getStepFilterVar(2).state.filters[0].key).toBe('zone'); + expect(getStepFilterVar(2).state.filters[0].value).toBe('a'); + }); + + it('Current history step 3 should have new filter zone=b', () => { + expect(getStepFilterVar(3).state.filters[0].key).toBe('zone'); + expect(getStepFilterVar(3).state.filters[0].value).toBe('b'); + }); + + describe('And then when returning again to step 1', () => { + beforeEach(() => { + trail.state.history.goBackToStep(1); + }); + + it('Should set history currentStep to 1', () => { + expect(trail.state.history.state.currentStep).toBe(1); + }); + + it('should sync state with url', () => { + expect(locationService.getSearchObject()['var-filters']).toBe(''); + }); + + it('Should have filters set back to empty', () => { + expect(getFilterVar().state.filters.length).toBe(0); + }); + + it('History step 1 should still have empty filter', () => { + expect(getStepFilterVar(1).state.filters.length).toBe(0); + }); + + it('History step 2 should still have old filter zone=a', () => { + expect(getStepFilterVar(2).state.filters[0].key).toBe('zone'); + expect(getStepFilterVar(2).state.filters[0].value).toBe('a'); + }); + + it('History step 3 should have new filter zone=b', () => { + expect(getStepFilterVar(3).state.filters[0].key).toBe('zone'); + expect(getStepFilterVar(3).state.filters[0].value).toBe('b'); + }); + }); + }); + }); + }); }); describe('When going back to history step 1', () => { diff --git a/public/app/features/trails/DataTrail.tsx b/public/app/features/trails/DataTrail.tsx index 73d75201d87..e2ac91a180e 100644 --- a/public/app/features/trails/DataTrail.tsx +++ b/public/app/features/trails/DataTrail.tsx @@ -18,6 +18,7 @@ import { SceneRefreshPicker, SceneTimePicker, SceneTimeRange, + sceneUtils, SceneVariable, SceneVariableSet, VariableDependencyConfig, @@ -26,7 +27,7 @@ import { import { useStyles2 } from '@grafana/ui'; import { DataTrailSettings } from './DataTrailSettings'; -import { DataTrailHistory, DataTrailHistoryStep } from './DataTrailsHistory'; +import { DataTrailHistory } from './DataTrailsHistory'; import { MetricScene } from './MetricScene'; import { MetricSelectScene } from './MetricSelectScene'; import { MetricsHeader } from './MetricsHeader'; @@ -81,59 +82,43 @@ export class DataTrail extends SceneObjectBase { // Some scene elements publish this this.subscribeToEvent(MetricSelectedEvent, this._handleMetricSelectedEvent.bind(this)); - // Pay attention to changes in history (i.e., changing the step) - this.state.history.subscribeToState((newState, oldState) => { - const oldNumberOfSteps = oldState.steps.length; - const newNumberOfSteps = newState.steps.length; - - const newStepWasAppended = newNumberOfSteps > oldNumberOfSteps; - - if (newStepWasAppended) { - // A new step is a significant change. Update the URL to match the new state. - this.syncTrailToUrl(); - // In order for the `useBookmarkState` to re-evaluate after a new step was made: - this.forceRender(); - // Do nothing else because the step state is already up to date -- it created a new step! - return; - } - - if (oldState.currentStep === newState.currentStep) { - // The same step was clicked on -- no need to change anything. - return; - } - - // History changed because a different node was selected - const step = newState.steps[newState.currentStep]; - - if (!step) { - return; - } - - this.goBackToStep(step); - }); - const filtersVariable = sceneGraph.lookupVariable(VAR_FILTERS, this); - const stateSubscription = - filtersVariable instanceof AdHocFiltersVariable && - filtersVariable?.subscribeToState((newState, prevState) => { - if (!this._addingFilterWithoutReportingInteraction) { - reportChangeInLabelFilters(newState.filters, prevState.filters); - } - }); + if (filtersVariable instanceof AdHocFiltersVariable) { + this._subs.add( + filtersVariable?.subscribeToState((newState, prevState) => { + if (!this._addingFilterWithoutReportingInteraction) { + reportChangeInLabelFilters(newState.filters, prevState.filters); + } + }) + ); + } + + this.enableUrlSync(); return () => { + this.disableUrlSync(); + if (!this.state.embedded) { getTrailStore().setRecentTrail(this); } - if (stateSubscription) { - stateSubscription?.unsubscribe(); - } }; } + private enableUrlSync() { + if (!this.state.embedded) { + getUrlSyncManager().initSync(this); + } + } + + private disableUrlSync() { + if (!this.state.embedded) { + getUrlSyncManager().cleanUp(this); + } + } + protected _variableDependency = new VariableDependencyConfig(this, { variableNames: [VAR_DATASOURCE], - onReferencedVariableValueChanged: async (variable: SceneVariable) => { + onReferencedVariableValueChanged: (variable: SceneVariable) => { const { name } = variable.state; if (name === VAR_DATASOURCE) { this.datasourceHelper.reset(); @@ -153,13 +138,13 @@ export class DataTrail extends SceneObjectBase { } this._addingFilterWithoutReportingInteraction = true; - variable.setState({ - filters: [...variable.state.filters, filter], - }); + + variable.setState({ filters: [...variable.state.filters, filter] }); + this._addingFilterWithoutReportingInteraction = false; } - private _addingFilterWithoutReportingInteraction = false; + private _addingFilterWithoutReportingInteraction = false; private datasourceHelper = new MetricDatasourceHelper(this); public getMetricMetadata(metric?: string) { @@ -170,25 +155,21 @@ export class DataTrail extends SceneObjectBase { return this.getMetricMetadata(this.state.metric); } - private goBackToStep(step: DataTrailHistoryStep) { - if (!step.trailState.metric) { - step.trailState.metric = undefined; - } + public restoreFromHistoryStep(state: DataTrailState) { + this.disableUrlSync(); - this.setState(step.trailState); - this.syncTrailToUrl(); - } - - private syncTrailToUrl() { - if (this.state.embedded) { - // Embedded trails should not be altering the URL - return; - } + this.setState( + sceneUtils.cloneSceneObjectState(state, { + history: this.state.history, + metric: !state.metric ? undefined : state.metric, + }) + ); const urlState = getUrlSyncManager().getUrlState(this); const fullUrl = urlUtil.renderUrl(locationService.getLocation().pathname, urlState); + locationService.replace(fullUrl); - locationService.replace(encodeURI(fullUrl)); + this.enableUrlSync(); } private _handleMetricSelectedEvent(evt: MetricSelectedEvent) { diff --git a/public/app/features/trails/DataTrailsHistory.tsx b/public/app/features/trails/DataTrailsHistory.tsx index b69be31ab89..5c08c590e4e 100644 --- a/public/app/features/trails/DataTrailsHistory.tsx +++ b/public/app/features/trails/DataTrailsHistory.tsx @@ -124,14 +124,17 @@ export class DataTrailHistory extends SceneObjectBase { return; } - this.stepTransitionInProgress = true; const step = this.state.steps[stepIndex]; const type = step.type === 'metric' && step.trailState.metric === undefined ? 'metric-clear' : step.type; + reportExploreMetrics('history_step_clicked', { type }); + this.stepTransitionInProgress = true; this.setState({ currentStep: stepIndex }); - // The URL will update + getTrailFor(this).restoreFromHistoryStep(step.trailState); + + // The URL will update this.stepTransitionInProgress = false; }