fix: datatrails: setRecentTrail: remove equivalent entries to new recent trail (#80932)

* fix: datatrails: setRecentTrail: remove equivalent entries * fix: use lodash to simplify code
2024-01-22 10:07:47 -05:00
parent 6d15e16d2e
commit fcc11c9290
2 changed files with 128 additions and 35 deletions
@@ -1,6 +1,6 @@
 import { BOOKMARKED_TRAILS_KEY, RECENT_TRAILS_KEY } from '../shared';

-import { getTrailStore } from './TrailStore';
+import { SerializedTrail, getTrailStore } from './TrailStore';

 describe('TrailStore', () => {
  beforeAll(() => {
@@ -29,40 +29,37 @@ describe('TrailStore', () => {
  });

  describe('Initialize store with one recent trail', () => {
-    beforeAll(() => {
+    const history: SerializedTrail['history'] = [
+      {
+        urlValues: {
+          from: 'now-1h',
+          to: 'now',
+          'var-ds': 'cb3a3391-700f-4cc6-81be-a122488e93e6',
+          'var-filters': [],
+          refresh: '',
+        },
+        type: 'start',
+        description: 'Test',
+        parentIndex: -1,
+      },
+      {
+        urlValues: {
+          metric: 'access_permissions_duration_count',
+          from: 'now-1h',
+          to: 'now',
+          'var-ds': 'cb3a3391-700f-4cc6-81be-a122488e93e6',
+          'var-filters': [],
+          refresh: '',
+        },
+        type: 'metric',
+        description: 'Test',
+        parentIndex: 0,
+      },
+    ];
+
+    beforeEach(() => {
      localStorage.clear();
-      localStorage.setItem(
-        RECENT_TRAILS_KEY,
-        JSON.stringify([
-          {
-            history: [
-              {
-                urlValues: {
-                  from: 'now-1h',
-                  to: 'now',
-                  'var-ds': 'cb3a3391-700f-4cc6-81be-a122488e93e6',
-                  'var-filters': [],
-                  refresh: '',
-                },
-                type: 'start',
-                description: 'Test',
-              },
-              {
-                urlValues: {
-                  metric: 'access_permissions_duration_count',
-                  from: 'now-1h',
-                  to: 'now',
-                  'var-ds': 'cb3a3391-700f-4cc6-81be-a122488e93e6',
-                  'var-filters': [],
-                  refresh: '',
-                },
-                type: 'metric',
-                description: 'Test',
-              },
-            ],
-          },
-        ])
-      );
+      localStorage.setItem(RECENT_TRAILS_KEY, JSON.stringify([{ history }]));
      getTrailStore().load();
    });

@@ -79,6 +76,88 @@ describe('TrailStore', () => {
      const store = getTrailStore();
      expect(store.bookmarks.length).toBe(0);
    });
+
+    describe('Add a new recent trail with equivalent current step state', () => {
+      const store = getTrailStore();
+
+      const duplicateTrailSerialized: SerializedTrail = {
+        history: [
+          history[0],
+          history[1],
+          {
+            ...history[1],
+            urlValues: {
+              ...history[1].urlValues,
+              metric: 'different_metric_in_the_middle',
+            },
+          },
+          {
+            ...history[1],
+          },
+        ],
+        currentStep: 3,
+      };
+
+      beforeEach(() => {
+        // We expect the initialized trail to be there
+        expect(store.recent.length).toBe(1);
+        expect(store.recent[0].resolve().state.history.state.steps.length).toBe(2);
+
+        // @ts-ignore #2341 -- deliberately access private method to construct trail object for testing purposes
+        const duplicateTrail = store._deserializeTrail(duplicateTrailSerialized);
+        store.setRecentTrail(duplicateTrail);
+      });
+
+      it('should still be only one recent trail', () => {
+        expect(store.recent.length).toBe(1);
+      });
+
+      it('it should only contain the new trail', () => {
+        const newRecentTrail = store.recent[0].resolve();
+        expect(newRecentTrail.state.history.state.steps.length).toBe(duplicateTrailSerialized.history.length);
+
+        // @ts-ignore #2341 -- deliberately access private method to construct trail object for testing purposes
+        const newRecent = store._serializeTrail(newRecentTrail);
+        expect(newRecent.currentStep).toBe(duplicateTrailSerialized.currentStep);
+        expect(newRecent.history.length).toBe(duplicateTrailSerialized.history.length);
+      });
+    });
+
+    it.each([
+      ['metric', 'different_metric'],
+      ['from', 'now-1y'],
+      ['to', 'now-30m'],
+      ['var-ds', '1234'],
+      ['var-groupby', 'job'],
+      ['var-filters', 'test'],
+    ])(`new recent trails with a different '%p' value should insert new entry`, (key, differentValue) => {
+      const store = getTrailStore();
+      // We expect the initialized trail to be there
+      expect(store.recent.length).toBe(1);
+
+      const differentTrailSerialized: SerializedTrail = {
+        history: [
+          history[0],
+          history[1],
+          {
+            ...history[1],
+            urlValues: {
+              ...history[1].urlValues,
+              [key]: differentValue,
+            },
+            parentIndex: 1,
+          },
+        ],
+        currentStep: 2,
+      };
+
+      // @ts-ignore #2341 -- deliberately access private method to construct trail object for testing purposes
+      const differentTrail = store._deserializeTrail(differentTrailSerialized);
+      store.setRecentTrail(differentTrail);
+
+      // There should now be two trails
+      expect(store.recent.length).toBe(2);
+    });
  });
  describe('Initialize store with one bookmark trail', () => {
    beforeAll(() => {
@@ -1,4 +1,4 @@
-import { debounce } from 'lodash';
+import { debounce, isEqual } from 'lodash';

 import { SceneObject, SceneObjectRef, SceneObjectUrlValues, getUrlSyncManager, sceneUtils } from '@grafana/scenes';

@@ -102,6 +102,16 @@ export class TrailStore {

  setRecentTrail(trail: DataTrail) {
    this._recent = this._recent.filter((t) => t !== trail.getRef());
+
+    // Check if any existing "recent" entries have equivalent 'current' urlValue to the new trail
+    const newTrailUrlValues = getCurrentUrlValues(this._serializeTrail(trail)) || {};
+    this._recent = this._recent.filter((t) => {
+      // Use the current step urlValues to filter out equivalent states
+      const urlValues = getCurrentUrlValues(this._serializeTrail(t.resolve()));
+      // Only keep trails with sufficiently unique urlValues on their current step
+      return !isEqual(newTrailUrlValues, urlValues);
+    });
+
    this._recent.unshift(trail.getRef());
    this._save();
  }
@@ -132,3 +142,7 @@ export function getTrailStore(): TrailStore {

  return store;
 }
+
+function getCurrentUrlValues({ history, currentStep }: SerializedTrail) {
+  return history[currentStep]?.urlValues || history.at(-1)?.urlValues;
+}