Data-trails: prevent creating new metrics node when clicking on historical metric node (#78569)

* fix: prevent creating new metrics node when clicking on historical metric node

* Adding some initial tests

---------

Co-authored-by: Torkel Ödegaard <torkel@grafana.com>

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

@@ -0,0 +1,92 @@
+import { locationService, setDataSourceSrv } from '@grafana/runtime';
+import { getUrlSyncManager } from '@grafana/scenes';
+
+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';
+import { MetricScene } from './MetricScene';
+import { MetricSelectScene } from './MetricSelectScene';
+import { MetricSelectedEvent } from './shared';
+
+describe('DataTrail', () => {
+  beforeAll(() => {
+    setDataSourceSrv(
+      new MockDataSourceSrv({
+        prom: mockDataSource({
+          name: 'Prometheus',
+          type: DataSourceType.Prometheus,
+        }),
+      })
+    );
+  });
+
+  describe('Given starting trail with url sync and no url state', () => {
+    let trail: DataTrail;
+
+    beforeEach(() => {
+      trail = new DataTrail({});
+      locationService.push('/');
+      getUrlSyncManager().initSync(trail);
+      activateFullSceneTree(trail);
+    });
+
+    it('Should default to metric select scene', () => {
+      expect(trail.state.topScene).toBeInstanceOf(MetricSelectScene);
+    });
+
+    it('Should set stepIndex to 0', () => {
+      expect(trail.state.stepIndex).toBe(0);
+    });
+
+    describe('And metric is selected', () => {
+      beforeEach(() => {
+        trail.publishEvent(new MetricSelectedEvent('metric_bucket'));
+      });
+
+      it('should switch scene to MetricScene', () => {
+        expect(trail.state.metric).toBe('metric_bucket');
+        expect(trail.state.topScene).toBeInstanceOf(MetricScene);
+      });
+
+      it('should sync state with url', () => {
+        expect(locationService.getSearchObject().metric).toBe('metric_bucket');
+      });
+
+      it('should add history step', () => {
+        expect(trail.state.history.state.steps[1].type).toBe('metric');
+      });
+
+      it('Should set stepIndex to 1', () => {
+        expect(trail.state.stepIndex).toBe(1);
+      });
+    });
+
+    describe('When going back to history step', () => {
+      beforeEach(() => {
+        trail.publishEvent(new MetricSelectedEvent('first_metric'));
+        trail.publishEvent(new MetricSelectedEvent('second_metric'));
+        trail.goBackToStep(trail.state.history.state.steps[1]);
+      });
+
+      it('Should restore state and url', () => {
+        expect(trail.state.metric).toBe('first_metric');
+        expect(locationService.getSearchObject().metric).toBe('first_metric');
+      });
+
+      it('Should set stepIndex to 1', () => {
+        expect(trail.state.stepIndex).toBe(1);
+      });
+
+      it('Should not create another history step', () => {
+        expect(trail.state.history.state.steps.length).toBe(3);
+      });
+
+      it('But selecting a new metric should create another history step', () => {
+        trail.publishEvent(new MetricSelectedEvent('third_metric'));
+        expect(trail.state.history.state.steps.length).toBe(4);
+      });
+    });
+  });
+});

public/app/features/trails/DataTrail.tsx

@@ -42,6 +42,9 @@ export interface DataTrailState extends SceneObjectState {
 
   // Synced with url
   metric?: string;
+
+  // Indicates which step in the data trail this is
+  stepIndex: number;
 }
 
 export class DataTrail extends SceneObjectBase<DataTrailState> {
@@ -59,6 +62,7 @@ export class DataTrail extends SceneObjectBase<DataTrailState> {
       ],
       history: state.history ?? new DataTrailHistory({}),
       settings: state.settings ?? new DataTrailSettings({}),
+      stepIndex: state.stepIndex ?? 0,
       ...state,
     });
 

public/app/features/trails/DataTrailsHistory.tsx

@@ -50,7 +50,11 @@ export class DataTrailHistory extends SceneObjectBase<DataTrailsHistoryState> {
           this.state.steps[0].trailState = sceneUtils.cloneSceneObjectState(oldState, { history: this });
         }
 
-        if (newState.metric) {
+        // Check if new and old state are at the same step index
+        // Then we know this isn't a history transition
+        const isMovingThroughHistory = newState.stepIndex !== oldState.stepIndex;
+
+        if (newState.metric && !isMovingThroughHistory) {
           this.addTrailStep(trail, 'metric');
         }
       }
@@ -70,6 +74,10 @@ export class DataTrailHistory extends SceneObjectBase<DataTrailsHistoryState> {
   }
 
   public addTrailStep(trail: DataTrail, type: TrailStepType) {
+    const stepIndex = this.state.steps.length;
+    // Update the trail's current step state.  It is being given a step index.
+    trail.setState({ ...trail.state, stepIndex });
+
     this.setState({
       steps: [
         ...this.state.steps,