[v11.0.x] datatrails: recently loaded trails modify parent variables when making new steps (#87333)

datatrails: recently loaded trails modify parent variables when making new steps (#87284)

* fix: loaded trails modified parent var on new step

This ensures that recently loaded trails won't have variable changes
which create new steps modify the previous step.

(cherry picked from commit 046eedaa4c)

Co-authored-by: Darren Janeczek <38694490+darrenjaneczek@users.noreply.github.com>

public/app/features/trails/DataTrail.test.tsx

@@ -1,8 +1,8 @@
 import { locationService, setDataSourceSrv } from '@grafana/runtime';
 import { AdHocFiltersVariable, sceneGraph } from '@grafana/scenes';
+import { DataSourceType } from 'app/features/alerting/unified/utils/datasource';
 
 import { MockDataSourceSrv, mockDataSource } from '../alerting/unified/mocks';
-import { DataSourceType } from '../alerting/unified/utils/datasource';
 import { activateFullSceneTree } from '../dashboard-scene/utils/test-utils';
 
 import { DataTrail } from './DataTrail';
@@ -26,6 +26,22 @@ describe('DataTrail', () => {
     let trail: DataTrail;
     const preTrailUrl = '/';
 
+    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}`);
+    }
+
     beforeEach(() => {
       trail = new DataTrail({});
       locationService.push(preTrailUrl);
@@ -199,22 +215,6 @@ describe('DataTrail', () => {
         });
       });
 
-      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);
       });
@@ -410,5 +410,71 @@ describe('DataTrail', () => {
         expect(locationService.getSearch().has('metric')).toBe(false);
       });
     });
+
+    it('Filter should be empty', () => {
+      expect(getStepFilterVar(0).state.filters.length).toBe(0);
+    });
+
+    describe('And filter is added zone=a', () => {
+      beforeEach(() => {
+        getFilterVar().setState({ filters: [{ key: 'zone', operator: '=', value: 'a' }] });
+      });
+
+      it('Filter of trail should be zone=a', () => {
+        expect(getFilterVar().state.filters[0].key).toBe('zone');
+        expect(getFilterVar().state.filters[0].value).toBe('a');
+      });
+
+      it('Filter of step 1 should be zone=a', () => {
+        expect(getStepFilterVar(1).state.filters[0].key).toBe('zone');
+        expect(getStepFilterVar(1).state.filters[0].value).toBe('a');
+      });
+
+      it('Filter of step 0 should empty', () => {
+        expect(getStepFilterVar(0).state.filters.length).toBe(0);
+      });
+
+      describe('When returning to step 0', () => {
+        beforeEach(() => {
+          trail.state.history.goBackToStep(0);
+        });
+
+        it('Filter of trail should be empty', () => {
+          expect(getFilterVar().state.filters.length).toBe(0);
+        });
+      });
+    });
+
+    it('Time range `from` should be now-6h', () => {
+      expect(trail.state.$timeRange?.state.from).toBe('now-6h');
+    });
+
+    describe('And time range is changed to now-15m to now', () => {
+      beforeEach(() => {
+        trail.state.$timeRange?.setState({ from: 'now-15m' });
+      });
+
+      it('Time range `from` should be now-15m', () => {
+        expect(trail.state.$timeRange?.state.from).toBe('now-15m');
+      });
+
+      it('Time range `from` of step 1 should be now-15m', () => {
+        expect(trail.state.history.state.steps[1].trailState.$timeRange?.state.from).toBe('now-15m');
+      });
+
+      it('Time range `from` of step 0 should be now-6h', () => {
+        expect(trail.state.history.state.steps[0].trailState.$timeRange?.state.from).toBe('now-6h');
+      });
+
+      describe('When returning to step 0', () => {
+        beforeEach(() => {
+          trail.state.history.goBackToStep(0);
+        });
+
+        it('Time range `from` should be now-6h', () => {
+          expect(trail.state.$timeRange?.state.from).toBe('now-6h');
+        });
+      });
+    });
   });
 });

public/app/features/trails/DataTrail.tsx

@@ -93,6 +93,10 @@ export class DataTrail extends SceneObjectBase<DataTrailState> {
       );
     }
 
+    // Disconnects the current step history state from the current state, to prevent changes affecting history state
+    const currentState = this.state.history.state.steps[this.state.history.state.currentStep].trailState;
+    this.restoreFromHistoryStep(currentState);
+
     this.enableUrlSync();
 
     // Save the current trail as a recent if the browser closes or reloads

public/app/features/trails/TrailStore/TrailStore.test.ts

@@ -1,4 +1,10 @@
-import { BOOKMARKED_TRAILS_KEY, RECENT_TRAILS_KEY } from '../shared';
+import { locationService, setDataSourceSrv } from '@grafana/runtime';
+import { AdHocFiltersVariable, getUrlSyncManager, sceneGraph } from '@grafana/scenes';
+import { DataSourceType } from 'app/features/alerting/unified/utils/datasource';
+
+import { MockDataSourceSrv, mockDataSource } from '../../alerting/unified/mocks';
+import { DataTrail } from '../DataTrail';
+import { BOOKMARKED_TRAILS_KEY, RECENT_TRAILS_KEY, VAR_FILTERS } from '../shared';
 
 import { SerializedTrail, getTrailStore } from './TrailStore';
 
@@ -21,6 +27,17 @@ describe('TrailStore', () => {
     global.localStorage = localStorageMock as unknown as Storage;
 
     jest.useFakeTimers();
+
+    // Having the mock service set up is required for activating the loaded trails
+    setDataSourceSrv(
+      new MockDataSourceSrv({
+        prom: mockDataSource({
+          name: 'Prometheus',
+          type: DataSourceType.Prometheus,
+          uid: 'ds',
+        }),
+      })
+    );
   });
 
   describe('Empty store', () => {
@@ -35,7 +52,7 @@ describe('TrailStore', () => {
     });
   });
 
-  describe('Initialize store with one recent trail', () => {
+  describe('Initialize store with one recent trail with final current step', () => {
     const history: SerializedTrail['history'] = [
       {
         urlValues: {
@@ -134,7 +151,7 @@ describe('TrailStore', () => {
       ['metric', 'different_metric'],
       ['from', 'now-1y'],
       ['to', 'now-30m'],
-      ['var-ds', '1234'],
+      ['var-ds', 'ds'],
       ['var-groupby', 'job'],
       ['var-filters', 'cluster|=|dev-eu-west-2'],
     ])(`new recent trails with a different '%p' value should insert new entry`, (key, differentValue) => {
@@ -253,6 +270,126 @@ describe('TrailStore', () => {
       expect(trail.state.history.state.currentStep).toBe(1);
     });
 
+    function getFilterVar(trail: DataTrail) {
+      const variable = sceneGraph.lookupVariable(VAR_FILTERS, trail);
+      if (variable instanceof AdHocFiltersVariable) {
+        return variable;
+      }
+      throw new Error('getFilterVar failed');
+    }
+
+    function getStepFilterVar(trail: DataTrail, 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('Recent trail filter should be empty at current step 1', () => {
+      const store = getTrailStore();
+      const trail = store.recent[0].resolve();
+
+      expect(getStepFilterVar(trail, 1).state.filters.length).toBe(0);
+      expect(trail.state.history.state.currentStep).toBe(1);
+      expect(trail.state.history.state.steps.length).toBe(3);
+    });
+
+    describe('And filter is added zone=a', () => {
+      let trail: DataTrail;
+      beforeEach(() => {
+        localStorage.clear();
+        localStorage.setItem(RECENT_TRAILS_KEY, JSON.stringify([{ history, currentStep: 1 }]));
+        getTrailStore().load();
+        const store = getTrailStore();
+        trail = store.recent[0].resolve();
+        const urlState = getUrlSyncManager().getUrlState(trail);
+        locationService.partial(urlState);
+        trail.activate();
+        trail.state.history.activate();
+        getFilterVar(trail).setState({ filters: [{ key: 'zone', operator: '=', value: 'a' }] });
+      });
+
+      it('This should create step 3', () => {
+        expect(trail.state.history.state.steps.length).toBe(4);
+        expect(trail.state.history.state.currentStep).toBe(3);
+      });
+
+      it('Filter of trail should be zone=a', () => {
+        expect(getFilterVar(trail).state.filters[0].key).toBe('zone');
+        expect(getFilterVar(trail).state.filters[0].value).toBe('a');
+      });
+
+      it('Filter of step 3 should be zone=a', () => {
+        expect(getStepFilterVar(trail, 3).state.filters[0].key).toBe('zone');
+        expect(getStepFilterVar(trail, 3).state.filters[0].value).toBe('a');
+      });
+
+      it('Filter of step 1 should be empty', () => {
+        expect(getStepFilterVar(trail, 1).state.filters.length).toBe(0);
+      });
+
+      describe('When returning to step 1', () => {
+        beforeEach(() => {
+          trail.state.history.goBackToStep(1);
+        });
+
+        it('Filter of trail should be empty', () => {
+          expect(getFilterVar(trail).state.filters.length).toBe(0);
+        });
+      });
+    });
+
+    it('Time range `from` should be now-1h', () => {
+      const store = getTrailStore();
+      const trail = store.recent[0].resolve();
+
+      expect(trail.state.$timeRange?.state.from).toBe('now-1h');
+    });
+
+    describe('And time range is changed to now-15m to now', () => {
+      let trail: DataTrail;
+      beforeEach(() => {
+        localStorage.clear();
+        localStorage.setItem(RECENT_TRAILS_KEY, JSON.stringify([{ history, currentStep: 1 }]));
+        getTrailStore().load();
+        const store = getTrailStore();
+        trail = store.recent[0].resolve();
+        const urlState = getUrlSyncManager().getUrlState(trail);
+        locationService.partial(urlState);
+        trail.activate();
+        trail.state.history.activate();
+        trail.state.$timeRange?.setState({ from: 'now-15m' });
+      });
+
+      it('This should create step 3', () => {
+        expect(trail.state.history.state.steps.length).toBe(4);
+        expect(trail.state.history.state.currentStep).toBe(3);
+      });
+
+      it('Time range `from` should be now-15m', () => {
+        expect(trail.state.$timeRange?.state.from).toBe('now-15m');
+      });
+
+      it('Time range `from` of step 2 should be now-15m', () => {
+        expect(trail.state.history.state.steps[3].trailState.$timeRange?.state.from).toBe('now-15m');
+      });
+
+      it('Time range `from` of step 1 should be now-1h', () => {
+        expect(trail.state.history.state.steps[1].trailState.$timeRange?.state.from).toBe('now-1h');
+      });
+
+      describe('When returning to step 1', () => {
+        beforeEach(() => {
+          trail.state.history.goBackToStep(1);
+        });
+
+        it('Time range `from` should be now-1h', () => {
+          expect(trail.state.$timeRange?.state.from).toBe('now-1h');
+        });
+      });
+    });
+
     it('should have no bookmarked trails', () => {
       const store = getTrailStore();
       expect(store.bookmarks.length).toBe(0);
@@ -309,7 +446,7 @@ describe('TrailStore', () => {
       ['metric', 'different_metric'],
       ['from', 'now-1y'],
       ['to', 'now-30m'],
-      ['var-ds', '1234'],
+      ['var-ds', 'different'],
       ['var-groupby', 'job'],
       ['var-filters', 'cluster|=|dev-eu-west-2'],
     ])(`new recent trails with a different '%p' value should insert new entry`, (key, differentValue) => {