datatrails: fix stability issues between conflict between browser history, URL sync, and trail history (#85134)

* fix: conflict between browser history and trail history

- ensure the back button or url changes don't generate trail steps
- ensure label breakdown TextLinks which appear on the summary tab work in embedded mode

public/app/features/trails/ActionTabs/MetricOverviewScene.tsx

@@ -12,8 +12,9 @@ import { Stack, Text, TextLink } from '@grafana/ui';
 
 import { PromMetricsMetadataItem } from '../../../plugins/datasource/prometheus/types';
 import { ALL_VARIABLE_VALUE } from '../../variables/constants';
+import { MetricScene } from '../MetricScene';
 import { StatusWrapper } from '../StatusWrapper';
-import { TRAILS_ROUTE, VAR_DATASOURCE_EXPR, VAR_GROUP_BY } from '../shared';
+import { VAR_DATASOURCE_EXPR, VAR_GROUP_BY } from '../shared';
 import { getMetricSceneFor, getTrailFor } from '../utils';
 
 import { getLabelOptions } from './utils';
@@ -91,26 +92,25 @@ export class MetricOverviewScene extends SceneObjectBase<MetricOverviewSceneStat
             <Stack direction="column" gap={0.5}>
               <Text weight={'medium'}>Labels</Text>
               {labelOptions.length === 0 && 'Unable to fetch labels.'}
-              {labelOptions.map((l) =>
-                getTrailFor(model).state.embedded ? (
-                  // Do not render as TextLink when in embedded mode, as any direct URL
-                  // manipulation will take the browser out out of the current page.
-                  <div key={l.label}>{l.label}</div>
-                ) : (
+              {labelOptions.map((l) => (
                 <TextLink
                   key={l.label}
-                    href={sceneGraph.interpolate(
-                      model,
-                      `${TRAILS_ROUTE}$\{__url.params:exclude:actionView,var-groupby}&actionView=breakdown&var-groupby=${encodeURIComponent(
-                        l.value!
-                      )}`
-                    )}
-                    title="View breakdown"
+                  href={`#View breakdown for ${l.label}`}
+                  title={`View breakdown for ${l.label}`}
+                  onClick={(event) => {
+                    event.preventDefault();
+                    event.stopPropagation();
+                    sceneGraph.getAncestor(model, MetricScene).setActionView('breakdown');
+                    const groupByVar = sceneGraph.lookupVariable(VAR_GROUP_BY, model);
+                    if (groupByVar instanceof QueryVariable) {
+                      groupByVar.setState({ value: l.value });
+                    }
+                    return false;
+                  }}
                 >
                   {l.label!}
                 </TextLink>
-                )
-              )}
+              ))}
             </Stack>
           </>
         </Stack>

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

@@ -1,5 +1,4 @@
 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';
@@ -22,13 +21,13 @@ describe('DataTrail', () => {
     );
   });
 
-  describe('Given starting trail with url sync and no url state', () => {
+  describe('Given starting non-embedded trail with url sync and no url state', () => {
     let trail: DataTrail;
+    const preTrailUrl = '/';
 
     beforeEach(() => {
       trail = new DataTrail({});
-      locationService.push('/');
-      getUrlSyncManager().initSync(trail);
+      locationService.push(preTrailUrl);
       activateFullSceneTree(trail);
     });
 
@@ -73,6 +72,15 @@ describe('DataTrail', () => {
       it('Should set history step 1 parentIndex to 0', () => {
         expect(trail.state.history.state.steps[1].parentIndex).toBe(0);
       });
+
+      describe('And browser back button is pressed', () => {
+        locationService.getHistory().goBack();
+
+        it('Should return to original URL', () => {
+          const { pathname } = locationService.getLocation();
+          expect(pathname).toEqual(preTrailUrl);
+        });
+      });
     });
 
     describe('When going back to history step 1', () => {
@@ -111,6 +119,15 @@ describe('DataTrail', () => {
         it('Should set history step 3 parent index to 1', () => {
           expect(trail.state.history.state.steps[3].parentIndex).toBe(1);
         });
+
+        describe('And browser back button is pressed', () => {
+          locationService.getHistory().goBack();
+
+          it('Should return to original URL', () => {
+            const { pathname } = locationService.getLocation();
+            expect(pathname).toEqual(preTrailUrl);
+          });
+        });
       });
     });
   });

public/app/features/trails/DataTrail.tsx

@@ -33,7 +33,6 @@ import { MetricsHeader } from './MetricsHeader';
 import { getTrailStore } from './TrailStore/TrailStore';
 import { MetricDatasourceHelper } from './helpers/MetricDatasourceHelper';
 import { MetricSelectedEvent, trailDS, VAR_DATASOURCE, VAR_FILTERS } from './shared';
-import { getUrlForTrail } from './utils';
 
 export interface DataTrailState extends SceneObjectState {
   topScene?: SceneObject;
@@ -89,9 +88,11 @@ export class DataTrail extends SceneObjectBase<DataTrailState> {
       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 because the state is already up to date -- it created a new step!
+        // Do nothing else because the step state is already up to date -- it created a new step!
         return;
       }
 
@@ -103,12 +104,15 @@ export class DataTrail extends SceneObjectBase<DataTrailState> {
       // History changed because a different node was selected
       const step = newState.steps[newState.currentStep];
 
+      if (!step) {
+        return;
+      }
+
       this.goBackToStep(step);
     });
 
     return () => {
       if (!this.state.embedded) {
-        getUrlSyncManager().cleanUp(this);
         getTrailStore().setRecentTrail(this);
       }
     };
@@ -135,29 +139,25 @@ export class DataTrail extends SceneObjectBase<DataTrailState> {
   }
 
   private goBackToStep(step: DataTrailHistoryStep) {
-    if (!this.state.embedded) {
-      getUrlSyncManager().cleanUp(this);
-    }
-
     if (!step.trailState.metric) {
       step.trailState.metric = undefined;
     }
 
     this.setState(step.trailState);
-
-    if (!this.state.embedded) {
-      locationService.replace(getUrlForTrail(this));
-
-      getUrlSyncManager().initSync(this);
+    this.syncTrailToUrl();
   }
+
+  private syncTrailToUrl() {
+    if (this.state.embedded) {
+      // Embedded trails should not be altering the URL
+      return;
+    }
+    const urlState = getUrlSyncManager().getUrlState(this);
+    locationService.partial(urlState, true);
   }
 
   private _handleMetricSelectedEvent(evt: MetricSelectedEvent) {
-    if (this.state.embedded) {
     this.setState(this.getSceneUpdatesForNewMetricValue(evt.payload));
-    } else {
-      locationService.partial({ metric: evt.payload, actionView: null });
-    }
 
     // Add metric to adhoc filters baseFilter
     const filterVar = sceneGraph.lookupVariable(VAR_FILTERS, this);

public/app/features/trails/DataTrailsApp.tsx

@@ -4,7 +4,7 @@ import { Route, Switch } from 'react-router-dom';
 
 import { GrafanaTheme2, PageLayoutType } from '@grafana/data';
 import { locationService } from '@grafana/runtime';
-import { getUrlSyncManager, SceneComponentProps, SceneObjectBase, SceneObjectState } from '@grafana/scenes';
+import { SceneComponentProps, SceneObjectBase, SceneObjectState, getUrlSyncManager } from '@grafana/scenes';
 import { useStyles2 } from '@grafana/ui';
 import { Page } from 'app/core/components/Page/Page';
 
@@ -75,7 +75,14 @@ function DataTrailView({ trail }: { trail: DataTrail }) {
 
   useEffect(() => {
     if (!isInitialized) {
+      // Set the initial state based on the URL.
       getUrlSyncManager().initSync(trail);
+      // Any further changes to the state should occur directly to the state, not through the URL.
+      // We want to stop automatically syncing the URL state (and vice versa) to the trail after this point.
+      // Moving forward in the lifecycle of the trail, we will make explicit calls to trail.syncTrailToUrl()
+      // so we can ensure the URL is kept up to date at key points.
+      getUrlSyncManager().cleanUp(trail);
+
       getTrailStore().setRecentTrail(trail);
       setIsInitialized(true);
     }

public/app/features/trails/DataTrailsHistory.tsx

@@ -30,7 +30,6 @@ export interface DataTrailHistoryStep {
 }
 
 export type TrailStepType = 'filters' | 'time' | 'metric' | 'start';
-
 export class DataTrailHistory extends SceneObjectBase<DataTrailsHistoryState> {
   public constructor(state: Partial<DataTrailsHistoryState>) {
     super({ steps: state.steps ?? [], currentStep: state.currentStep ?? 0 });
@@ -120,9 +119,13 @@ export class DataTrailHistory extends SceneObjectBase<DataTrailsHistoryState> {
   }
 
   public goBackToStep(stepIndex: number) {
-    this.stepTransitionInProgress = true;
+    if (stepIndex === this.state.currentStep) {
+      return;
+    }
 
+    this.stepTransitionInProgress = true;
     this.setState({ currentStep: stepIndex });
+    // The URL will update
 
     this.stepTransitionInProgress = false;
   }
@@ -142,10 +145,15 @@ export class DataTrailHistory extends SceneObjectBase<DataTrailsHistoryState> {
 
     const { ancestry, alternatePredecessorStyle } = useMemo(() => {
       const ancestry = new Set<number>();
+
       let cursor = currentStep;
       while (cursor >= 0) {
+        const step = steps[cursor];
+        if (!step) {
+          break;
+        }
         ancestry.add(cursor);
-        cursor = steps[cursor].parentIndex;
+        cursor = step.parentIndex;
       }
 
       const alternatePredecessorStyle = new Map<number, string>();