Query History: Split data and view models (#44922)

* Remove unused properties

* Fix unit tests

* Fix unit tests

* Split data models

* Simplify updating items in rich history

* Update tests

* Fix starring an item and add a unit test

* Move the converter to a separate file and add unit tests

* Convert a private function to an inline function

* Add more docs and clean up the code

* Update public/app/core/history/localStorageConverter.ts

Co-authored-by: Giordano Ricci <me@giordanoricci.com>

* Update public/app/core/utils/richHistory.test.ts

Co-authored-by: Giordano Ricci <me@giordanoricci.com>

* Use template literals over explicit casting

* Split updateRichHistory to three separate functions

Co-authored-by: Giordano Ricci <me@giordanoricci.com>

.betterer.results

@@ -263,7 +263,7 @@ exports[`no enzyme tests`] = {
     "public/app/features/explore/RichHistory/RichHistory.test.tsx:409631018": [
       [1, 17, 13, "RegExp match", "2409514259"]
     ],
-    "public/app/features/explore/RichHistory/RichHistoryCard.test.tsx:357697997": [
+    "public/app/features/explore/RichHistory/RichHistoryCard.test.tsx:689438177": [
       [1, 19, 13, "RegExp match", "2409514259"]
     ],
     "public/app/features/explore/RichHistory/RichHistoryContainer.test.tsx:396471778": [

public/app/core/history/RichHistoryLocalStorage.test.ts

@@ -4,23 +4,47 @@ import { RichHistoryQuery } from '../../types';
 import { DataQuery } from '@grafana/data';
 import { afterEach, beforeEach } from '../../../test/lib/common';
 import { RichHistoryStorageWarning } from './RichHistoryStorage';
+import { backendSrv } from '../services/backend_srv';
 
 const key = 'grafana.explore.richHistory';
 
-const mockItem: RichHistoryQuery = {
-  ts: 2,
+jest.mock('@grafana/runtime', () => ({
+  ...(jest.requireActual('@grafana/runtime') as unknown as object),
+  getBackendSrv: () => backendSrv,
+  getDataSourceSrv: () => {
+    return {
+      getList: () => {
+        return [
+          { uid: 'dev-test-uid', name: 'dev-test' },
+          { uid: 'dev-test-2-uid', name: 'dev-test-2' },
+        ];
+      },
+    };
+  },
+}));
+
+interface MockQuery extends DataQuery {
+  query: string;
+}
+
+const mockItem: RichHistoryQuery<MockQuery> = {
+  id: '2',
+  createdAt: 2,
   starred: true,
+  datasourceUid: 'dev-test-uid',
   datasourceName: 'dev-test',
   comment: 'test',
-  queries: [{ refId: 'ref', query: 'query-test' } as DataQuery],
+  queries: [{ refId: 'ref', query: 'query-test' }],
 };
 
-const mockItem2: RichHistoryQuery = {
-  ts: 3,
+const mockItem2: RichHistoryQuery<MockQuery> = {
+  id: '3',
+  createdAt: 3,
   starred: true,
+  datasourceUid: 'dev-test-2-uid',
   datasourceName: 'dev-test-2',
   comment: 'test-2',
-  queries: [{ refId: 'ref-2', query: 'query-2' } as DataQuery],
+  queries: [{ refId: 'ref-2', query: 'query-2' }],
 };
 
 describe('RichHistoryLocalStorage', () => {
@@ -45,7 +69,7 @@ describe('RichHistoryLocalStorage', () => {
     it('should save query history to localStorage', async () => {
       await storage.addToRichHistory(mockItem);
       expect(store.exists(key)).toBeTruthy();
-      expect(store.getObject(key)).toMatchObject([mockItem]);
+      expect(await storage.getRichHistory()).toMatchObject([mockItem]);
     });
 
     it('should not save duplicated query to localStorage', async () => {
@@ -54,24 +78,25 @@ describe('RichHistoryLocalStorage', () => {
       await expect(async () => {
         await storage.addToRichHistory(mockItem2);
       }).rejects.toThrow('Entry already exists');
-      expect(store.getObject(key)).toMatchObject([mockItem2, mockItem]);
+      expect(await storage.getRichHistory()).toMatchObject([mockItem2, mockItem]);
     });
 
     it('should update starred in localStorage', async () => {
       await storage.addToRichHistory(mockItem);
-      await storage.updateStarred(mockItem.ts, false);
-      expect(store.getObject(key)[0].starred).toEqual(false);
+      await storage.updateStarred(mockItem.id, false);
+      expect((await storage.getRichHistory())[0].starred).toEqual(false);
     });
 
     it('should update comment in localStorage', async () => {
       await storage.addToRichHistory(mockItem);
-      await storage.updateComment(mockItem.ts, 'new comment');
-      expect(store.getObject(key)[0].comment).toEqual('new comment');
+      await storage.updateComment(mockItem.id, 'new comment');
+      expect((await storage.getRichHistory())[0].comment).toEqual('new comment');
     });
 
     it('should delete query in localStorage', async () => {
       await storage.addToRichHistory(mockItem);
-      await storage.deleteRichHistory(mockItem.ts);
+      await storage.deleteRichHistory(mockItem.id);
+      expect(await storage.getRichHistory()).toEqual([]);
       expect(store.getObject(key)).toEqual([]);
     });
   });
@@ -92,9 +117,9 @@ describe('RichHistoryLocalStorage', () => {
 
       expect(richHistory).toMatchObject([
         mockItem,
-        { starred: true, ts: 0, queries: [] },
-        { starred: true, ts: now, queries: [] },
-        { starred: false, ts: now, queries: [] },
+        { starred: true, createdAt: 0, queries: [] },
+        { starred: true, createdAt: now, queries: [] },
+        { starred: false, createdAt: now, queries: [] },
       ]);
     });
 
@@ -119,7 +144,7 @@ describe('RichHistoryLocalStorage', () => {
       expect(history.filter((h) => !h.starred)).toHaveLength(notStarredItemsInHistory);
 
       store.setObject(key, history);
-      const warning = await storage.addToRichHistory(mockItem);
+      const { warning } = await storage.addToRichHistory(mockItem);
       expect(warning).toMatchObject({
         type: RichHistoryStorageWarning.LimitExceeded,
       });
@@ -143,13 +168,22 @@ describe('RichHistoryLocalStorage', () => {
 
     describe('should load from localStorage data in old formats', () => {
       it('should load when queries are strings', async () => {
-        const oldHistoryItem = {
-          ...mockItem,
-          queries: ['test query 1', 'test query 2', 'test query 3'],
-        };
-        store.setObject(key, [oldHistoryItem]);
+        store.setObject(key, [
+          {
+            ts: 2,
+            starred: true,
+            datasourceName: 'dev-test',
+            comment: 'test',
+            queries: ['test query 1', 'test query 2', 'test query 3'],
+          },
+        ]);
         const expectedHistoryItem = {
-          ...mockItem,
+          id: '2',
+          createdAt: 2,
+          starred: true,
+          datasourceUid: 'dev-test-uid',
+          datasourceName: 'dev-test',
+          comment: 'test',
           queries: [
             {
               expr: 'test query 1',
@@ -171,13 +205,22 @@ describe('RichHistoryLocalStorage', () => {
       });
 
       it('should load when queries are json-encoded strings', async () => {
-        const oldHistoryItem = {
-          ...mockItem,
-          queries: ['{"refId":"A","key":"key1","metrics":[]}', '{"refId":"B","key":"key2","metrics":[]}'],
-        };
-        store.setObject(key, [oldHistoryItem]);
+        store.setObject(key, [
+          {
+            ts: 2,
+            starred: true,
+            datasourceName: 'dev-test',
+            comment: 'test',
+            queries: ['{"refId":"A","key":"key1","metrics":[]}', '{"refId":"B","key":"key2","metrics":[]}'],
+          },
+        ]);
         const expectedHistoryItem = {
-          ...mockItem,
+          id: '2',
+          createdAt: 2,
+          starred: true,
+          datasourceUid: 'dev-test-uid',
+          datasourceName: 'dev-test',
+          comment: 'test',
           queries: [
             {
               refId: 'A',

public/app/core/history/RichHistoryLocalStorage.ts

@@ -2,12 +2,22 @@ import RichHistoryStorage, { RichHistoryServiceError, RichHistoryStorageWarning
 import { RichHistoryQuery } from '../../types';
 import store from '../store';
 import { DataQuery } from '@grafana/data';
-import { isEqual, omit } from 'lodash';
+import { find, isEqual, omit } from 'lodash';
 import { createRetentionPeriodBoundary, RICH_HISTORY_SETTING_KEYS } from './richHistoryLocalStorageUtils';
+import { fromDTO, toDTO } from './localStorageConverter';
 
 export const RICH_HISTORY_KEY = 'grafana.explore.richHistory';
 export const MAX_HISTORY_ITEMS = 10000;
 
+export type RichHistoryLocalStorageDTO = {
+  // works as an unique identifier
+  ts: number;
+  datasourceName: string;
+  starred: boolean;
+  comment: string;
+  queries: DataQuery[];
+};
+
 /**
  * Local storage implementation for Rich History. It keeps all entries in browser's local storage.
  */
@@ -16,21 +26,27 @@ export default class RichHistoryLocalStorage implements RichHistoryStorage {
    * Return all history entries, perform migration and clean up entries not matching retention policy.
    */
   async getRichHistory() {
-    const richHistory: RichHistoryQuery[] = store.getObject(RICH_HISTORY_KEY, []);
-    const transformedRichHistory = migrateRichHistory(richHistory);
-    return transformedRichHistory;
+    return getRichHistoryDTOs().map(fromDTO);
   }
 
-  async addToRichHistory(richHistoryQuery: RichHistoryQuery) {
-    const richHistory = cleanUp(await this.getRichHistory());
+  async addToRichHistory(newRichHistoryQuery: Omit<RichHistoryQuery, 'id' | 'createdAt'>) {
+    const ts = Date.now();
+    const richHistoryQuery = {
+      id: ts.toString(),
+      createdAt: ts,
+      ...newRichHistoryQuery,
+    };
+
+    const newRichHistoryQueryDTO = toDTO(richHistoryQuery);
+    const currentRichHistoryDTOs = cleanUp(getRichHistoryDTOs());
 
     /* Compare queries of a new query and last saved queries. If they are the same, (except selected properties,
      * which can be different) don't save it in rich history.
      */
-    const newQueriesToCompare = richHistoryQuery.queries.map((q) => omit(q, ['key', 'refId']));
+    const newQueriesToCompare = newRichHistoryQueryDTO.queries.map((q) => omit(q, ['key', 'refId']));
     const lastQueriesToCompare =
-      richHistory.length > 0 &&
-      richHistory[0].queries.map((q) => {
+      currentRichHistoryDTOs.length > 0 &&
+      currentRichHistoryDTOs[0].queries.map((q) => {
         return omit(q, ['key', 'refId']);
       });
 
@@ -40,9 +56,9 @@ export default class RichHistoryLocalStorage implements RichHistoryStorage {
       throw error;
     }
 
-    const { queriesToKeep, limitExceeded } = checkLimits(richHistory);
+    const { queriesToKeep, limitExceeded } = checkLimits(currentRichHistoryDTOs);
 
-    const updatedHistory: RichHistoryQuery[] = [richHistoryQuery, ...queriesToKeep];
+    const updatedHistory: RichHistoryLocalStorageDTO[] = [newRichHistoryQueryDTO, ...queriesToKeep];
 
     try {
       store.setObject(RICH_HISTORY_KEY, updatedHistory);
@@ -56,52 +72,59 @@ export default class RichHistoryLocalStorage implements RichHistoryStorage {
 
     if (limitExceeded) {
       return {
-        type: RichHistoryStorageWarning.LimitExceeded,
-        message: `Query history reached the limit of ${MAX_HISTORY_ITEMS}. Old, not-starred items have been removed.`,
+        warning: {
+          type: RichHistoryStorageWarning.LimitExceeded,
+          message: `Query history reached the limit of ${MAX_HISTORY_ITEMS}. Old, not-starred items have been removed.`,
+        },
+        richHistoryQuery,
       };
     }
 
-    return undefined;
+    return { richHistoryQuery };
   }
 
   async deleteAll() {
     store.delete(RICH_HISTORY_KEY);
   }
 
-  async deleteRichHistory(id: number) {
-    const richHistory: RichHistoryQuery[] = store.getObject(RICH_HISTORY_KEY, []);
-    const updatedHistory = richHistory.filter((query) => query.ts !== id);
+  async deleteRichHistory(id: string) {
+    const ts = parseInt(id, 10);
+    const richHistory: RichHistoryLocalStorageDTO[] = store.getObject(RICH_HISTORY_KEY, []);
+    const updatedHistory = richHistory.filter((query) => query.ts !== ts);
     store.setObject(RICH_HISTORY_KEY, updatedHistory);
   }
 
-  async updateStarred(id: number, starred: boolean) {
-    const richHistory: RichHistoryQuery[] = store.getObject(RICH_HISTORY_KEY, []);
-    const updatedHistory = richHistory.map((query) => {
-      if (query.ts === id) {
-        query.starred = starred;
-      }
-      return query;
-    });
+  async updateStarred(id: string, starred: boolean) {
+    return updateRichHistory(id, (richHistoryDTO) => (richHistoryDTO.starred = starred));
+  }
 
-    store.setObject(RICH_HISTORY_KEY, updatedHistory);
+  async updateComment(id: string, comment: string) {
+    return updateRichHistory(id, (richHistoryDTO) => (richHistoryDTO.comment = comment));
   }
+}
 
-  async updateComment(id: number, comment: string) {
-    const richHistory: RichHistoryQuery[] = store.getObject(RICH_HISTORY_KEY, []);
-    const updatedHistory = richHistory.map((query) => {
-      if (query.ts === id) {
-        query.comment = comment;
-      }
-      return query;
-    });
-    store.setObject(RICH_HISTORY_KEY, updatedHistory);
+function updateRichHistory(
+  id: string,
+  updateCallback: (richHistoryDTO: RichHistoryLocalStorageDTO) => void
+): RichHistoryQuery {
+  const ts = parseInt(id, 10);
+  const richHistoryDTOs: RichHistoryLocalStorageDTO[] = store.getObject(RICH_HISTORY_KEY, []);
+  const richHistoryDTO = find(richHistoryDTOs, { ts });
+
+  if (!richHistoryDTO) {
+    throw new Error('Rich history item not found.');
   }
+
+  updateCallback(richHistoryDTO);
+
+  store.setObject(RICH_HISTORY_KEY, richHistoryDTOs);
+  return fromDTO(richHistoryDTO);
 }
 
 /**
  * Removes entries that do not match retention policy criteria.
  */
-function cleanUp(richHistory: RichHistoryQuery[]): RichHistoryQuery[] {
+function cleanUp(richHistory: RichHistoryLocalStorageDTO[]): RichHistoryLocalStorageDTO[] {
   const retentionPeriod: number = store.getObject(RICH_HISTORY_SETTING_KEYS.retentionPeriod, 7);
   const retentionPeriodLastTs = createRetentionPeriodBoundary(retentionPeriod, false);
 
@@ -115,7 +138,10 @@ function cleanUp(richHistory: RichHistoryQuery[]): RichHistoryQuery[] {
  * Ensures the entry can be added. Throws an error if current limit has been hit.
  * Returns queries that should be saved back giving space for one extra query.
  */
-function checkLimits(queriesToKeep: RichHistoryQuery[]): { queriesToKeep: RichHistoryQuery[]; limitExceeded: boolean } {
+function checkLimits(queriesToKeep: RichHistoryLocalStorageDTO[]): {
+  queriesToKeep: RichHistoryLocalStorageDTO[];
+  limitExceeded: boolean;
+} {
   // remove oldest non-starred items to give space for the recent query
   let limitExceeded = false;
   let current = queriesToKeep.length - 1;
@@ -130,7 +156,12 @@ function checkLimits(queriesToKeep: RichHistoryQuery[]): { queriesToKeep: RichHi
   return { queriesToKeep, limitExceeded };
 }
 
-function migrateRichHistory(richHistory: RichHistoryQuery[]) {
+function getRichHistoryDTOs(): RichHistoryLocalStorageDTO[] {
+  const richHistory: RichHistoryLocalStorageDTO[] = store.getObject(RICH_HISTORY_KEY, []);
+  return migrateRichHistory(richHistory);
+}
+
+function migrateRichHistory(richHistory: RichHistoryLocalStorageDTO[]): RichHistoryLocalStorageDTO[] {
   const transformedRichHistory = richHistory.map((query) => {
     const transformedQueries: DataQuery[] = query.queries.map((q, index) => createDataQuery(query, q, index));
     return { ...query, queries: transformedQueries };
@@ -139,7 +170,7 @@ function migrateRichHistory(richHistory: RichHistoryQuery[]) {
   return transformedRichHistory;
 }
 
-function createDataQuery(query: RichHistoryQuery, individualQuery: DataQuery | string, index: number) {
+function createDataQuery(query: RichHistoryLocalStorageDTO, individualQuery: DataQuery | string, index: number) {
   const letters = 'ABCDEFGHIJKLMNOPQRSTUVXYZ';
   if (typeof individualQuery === 'object') {
     // the current format

public/app/core/history/RichHistoryStorage.ts

@@ -32,9 +32,16 @@ export type RichHistoryStorageWarningDetails = {
  */
 export default interface RichHistoryStorage {
   getRichHistory(): Promise<RichHistoryQuery[]>;
-  addToRichHistory(richHistoryQuery: RichHistoryQuery): Promise<RichHistoryStorageWarningDetails | undefined>;
+
+  /**
+   * Creates new RichHistoryQuery, returns object with unique id and created date
+   */
+  addToRichHistory(
+    newRichHistoryQuery: Omit<RichHistoryQuery, 'id' | 'createdAt'>
+  ): Promise<{ warning?: RichHistoryStorageWarningDetails; richHistoryQuery: RichHistoryQuery }>;
+
   deleteAll(): Promise<void>;
-  deleteRichHistory(id: number): Promise<void>;
-  updateStarred(id: number, starred: boolean): Promise<void>;
-  updateComment(id: number, comment: string | undefined): Promise<void>;
+  deleteRichHistory(id: string): Promise<void>;
+  updateStarred(id: string, starred: boolean): Promise<RichHistoryQuery>;
+  updateComment(id: string, comment: string | undefined): Promise<RichHistoryQuery>;
 }

public/app/core/history/localStorageConverter.test.ts

@@ -0,0 +1,60 @@
+import { backendSrv } from '../services/backend_srv';
+import { fromDTO, toDTO } from './localStorageConverter';
+import { RichHistoryQuery } from '../../types';
+import { RichHistoryLocalStorageDTO } from './RichHistoryLocalStorage';
+
+jest.mock('@grafana/runtime', () => ({
+  ...(jest.requireActual('@grafana/runtime') as unknown as object),
+  getBackendSrv: () => backendSrv,
+  getDataSourceSrv: () => {
+    return {
+      getList: () => {
+        return [{ uid: 'uid', name: 'dev-test' }];
+      },
+    };
+  },
+}));
+
+const validRichHistory: RichHistoryQuery = {
+  comment: 'comment',
+  createdAt: 1,
+  datasourceName: 'dev-test',
+  datasourceUid: 'uid',
+  id: '1',
+  queries: [{ refId: 'A' }],
+  starred: true,
+};
+
+const validDTO: RichHistoryLocalStorageDTO = {
+  comment: 'comment',
+  datasourceName: 'dev-test',
+  queries: [{ refId: 'A' }],
+  starred: true,
+  ts: 1,
+};
+
+describe('LocalStorage converted', () => {
+  it('converts RichHistoryQuery to local storage DTO', () => {
+    expect(toDTO(validRichHistory)).toMatchObject(validDTO);
+  });
+
+  it('throws an error when data source for RichHistory does not exist to avoid saving invalid items', () => {
+    const invalidRichHistory = { ...validRichHistory, datasourceUid: 'invalid' };
+    expect(() => {
+      toDTO(invalidRichHistory);
+    }).toThrow();
+  });
+
+  it('converts DTO to RichHistoryQuery', () => {
+    expect(fromDTO(validDTO)).toMatchObject(validRichHistory);
+  });
+
+  it('uses empty uid when datasource does not exist for a DTO to fail gracefully for queries from removed datasources', () => {
+    const invalidDto = { ...validDTO, datasourceName: 'removed' };
+    expect(fromDTO(invalidDto)).toMatchObject({
+      ...validRichHistory,
+      datasourceName: 'removed',
+      datasourceUid: '',
+    });
+  });
+});

public/app/core/history/localStorageConverter.ts

@@ -0,0 +1,41 @@
+import { find } from 'lodash';
+import { DataSourceInstanceSettings } from '@grafana/data';
+import { getDataSourceSrv } from '@grafana/runtime';
+import { RichHistoryLocalStorageDTO } from './RichHistoryLocalStorage';
+import { RichHistoryQuery } from '../../types';
+
+export const fromDTO = (dto: RichHistoryLocalStorageDTO): RichHistoryQuery => {
+  const datasource = find(
+    getDataSourceSrv().getList(),
+    (settings: DataSourceInstanceSettings) => settings.name === dto.datasourceName
+  );
+
+  return {
+    id: dto.ts.toString(),
+    createdAt: dto.ts,
+    datasourceName: dto.datasourceName,
+    datasourceUid: datasource?.uid || '', // will be show on the list as coming from a removed data source
+    starred: dto.starred,
+    comment: dto.comment,
+    queries: dto.queries,
+  };
+};
+
+export const toDTO = (richHistoryQuery: RichHistoryQuery): RichHistoryLocalStorageDTO => {
+  const datasource = find(
+    getDataSourceSrv().getList(),
+    (settings: DataSourceInstanceSettings) => settings.uid === richHistoryQuery.datasourceUid
+  );
+
+  if (!datasource) {
+    throw new Error('Datasource not found.');
+  }
+
+  return {
+    ts: richHistoryQuery.createdAt,
+    datasourceName: richHistoryQuery.datasourceName,
+    starred: richHistoryQuery.starred,
+    comment: richHistoryQuery.comment,
+    queries: richHistoryQuery.queries,
+  };
+};

public/app/core/history/richHistoryLocalStorageUtils.ts

@@ -23,7 +23,7 @@ export const createRetentionPeriodBoundary = (days: number, isLastTs: boolean) =
 export function filterQueriesByTime(queries: RichHistoryQuery[], timeFilter: [number, number]) {
   const filter1 = createRetentionPeriodBoundary(timeFilter[0], true); // probably the vars should have a different name
   const filter2 = createRetentionPeriodBoundary(timeFilter[1], false);
-  return queries.filter((q) => q.ts < filter1 && q.ts > filter2);
+  return queries.filter((q) => q.createdAt < filter1 && q.createdAt > filter2);
 }
 
 export function filterQueriesByDataSource(queries: RichHistoryQuery[], listOfDatasourceFilters: string[]) {
@@ -53,10 +53,12 @@ export const sortQueries = (array: RichHistoryQuery[], sortOrder: SortOrder) =>
   let sortFunc;
 
   if (sortOrder === SortOrder.Ascending) {
-    sortFunc = (a: RichHistoryQuery, b: RichHistoryQuery) => (a.ts < b.ts ? -1 : a.ts > b.ts ? 1 : 0);
+    sortFunc = (a: RichHistoryQuery, b: RichHistoryQuery) =>
+      a.createdAt < b.createdAt ? -1 : a.createdAt > b.createdAt ? 1 : 0;
   }
   if (sortOrder === SortOrder.Descending) {
-    sortFunc = (a: RichHistoryQuery, b: RichHistoryQuery) => (a.ts < b.ts ? 1 : a.ts > b.ts ? -1 : 0);
+    sortFunc = (a: RichHistoryQuery, b: RichHistoryQuery) =>
+      a.createdAt < b.createdAt ? 1 : a.createdAt > b.createdAt ? -1 : 0;
   }
 
   if (sortOrder === SortOrder.DatasourceZA) {

public/app/core/utils/richHistory.test.ts

@@ -25,6 +25,8 @@ jest.mock('../history/richHistoryStorageProvider', () => {
 const mock: any = {
   storedHistory: [
     {
+      id: '1',
+      createdAt: 1,
       comment: '',
       datasourceName: 'datasource history name',
       queries: [
@@ -32,11 +34,10 @@ const mock: any = {
         { expr: 'query2', refId: '2' },
       ],
       starred: true,
-      ts: 1,
     },
   ],
   testComment: '',
-  testDatasourceId: 'datasourceId',
+  testDatasourceUid: 'datasourceUid',
   testDatasourceName: 'datasourceName',
   testQueries: [
     { expr: 'query3', refId: 'B' },
@@ -53,12 +54,24 @@ describe('richHistory', () => {
     jest.useFakeTimers('modern');
     jest.setSystemTime(new Date(1970, 0, 1));
 
-    richHistoryStorageMock.addToRichHistory = jest.fn().mockResolvedValue(undefined);
+    richHistoryStorageMock.addToRichHistory = jest.fn((r) => {
+      return Promise.resolve({ richHistoryQuery: { ...r, id: 'GENERATED ID', createdAt: Date.now() } });
+    });
     richHistoryStorageMock.deleteAll = jest.fn().mockResolvedValue({});
     richHistoryStorageMock.deleteRichHistory = jest.fn().mockResolvedValue({});
     richHistoryStorageMock.getRichHistory = jest.fn().mockResolvedValue({});
-    richHistoryStorageMock.updateComment = jest.fn().mockResolvedValue({});
-    richHistoryStorageMock.updateStarred = jest.fn().mockResolvedValue({});
+    richHistoryStorageMock.updateComment = jest.fn((id, comment) => {
+      return {
+        ...mock,
+        comment,
+      };
+    });
+    richHistoryStorageMock.updateStarred = jest.fn((id, starred) => {
+      return {
+        ...mock,
+        starred,
+      };
+    });
   });
 
   afterEach(() => {
@@ -73,10 +86,12 @@ describe('richHistory', () => {
     const expectedResult = [
       {
         comment: mock.testComment,
+        datasourceUid: mock.testDatasourceUid,
         datasourceName: mock.testDatasourceName,
         queries: mock.testQueries,
         starred: mock.testStarred,
-        ts: 2,
+        createdAt: 2,
+        id: 'GENERATED ID',
       },
       mock.storedHistory[0],
     ];
@@ -85,6 +100,7 @@ describe('richHistory', () => {
       Date.now = jest.fn(() => 2);
       const { richHistory: newHistory } = await addToRichHistory(
         mock.storedHistory,
+        mock.testDatasourceUid,
         mock.testDatasourceName,
         mock.testQueries,
         mock.testStarred,
@@ -100,6 +116,7 @@ describe('richHistory', () => {
 
       const { richHistory } = await addToRichHistory(
         mock.storedHistory,
+        mock.testDatasourceUid,
         mock.testDatasourceName,
         mock.testQueries,
         mock.testStarred,
@@ -109,11 +126,11 @@ describe('richHistory', () => {
       );
       expect(richHistory).toMatchObject(expectedResult);
       expect(richHistoryStorageMock.addToRichHistory).toBeCalledWith({
+        datasourceUid: mock.testDatasourceUid,
         datasourceName: mock.testDatasourceName,
         starred: mock.testStarred,
         comment: mock.testComment,
         queries: mock.testQueries,
-        ts: 2,
       });
     });
 
@@ -126,6 +143,7 @@ describe('richHistory', () => {
 
       const { richHistory: newHistory } = await addToRichHistory(
         mock.storedHistory,
+        mock.storedHistory[0].datasourceUid,
         mock.storedHistory[0].datasourceName,
         [{ expr: 'query1', maxLines: null, refId: 'A' } as DataQuery, { expr: 'query2', refId: 'B' } as DataQuery],
         mock.testStarred,
@@ -139,13 +157,19 @@ describe('richHistory', () => {
     it('it should append new items even when the limit is exceeded', async () => {
       Date.now = jest.fn(() => 2);
 
-      richHistoryStorageMock.addToRichHistory = jest.fn().mockReturnValue({
-        type: RichHistoryStorageWarning.LimitExceeded,
-        message: 'Limit exceeded',
+      richHistoryStorageMock.addToRichHistory = jest.fn((query) => {
+        return Promise.resolve({
+          richHistoryQuery: { ...query, id: 'GENERATED ID', createdAt: Date.now() },
+          warning: {
+            type: RichHistoryStorageWarning.LimitExceeded,
+            message: 'Limit exceeded',
+          },
+        });
       });
 
       const { richHistory, limitExceeded } = await addToRichHistory(
         mock.storedHistory,
+        mock.testDatasourceUid,
         mock.testDatasourceName,
         mock.testQueries,
         mock.testStarred,
@@ -160,21 +184,21 @@ describe('richHistory', () => {
 
   describe('updateStarredInRichHistory', () => {
     it('should update starred in query in history', async () => {
-      const updatedStarred = await updateStarredInRichHistory(mock.storedHistory, 1);
-      expect(updatedStarred[0].starred).toEqual(false);
+      const updatedStarred = await updateStarredInRichHistory(mock.storedHistory, '1', !mock.starred);
+      expect(updatedStarred[0].starred).toEqual(!mock.starred);
     });
   });
 
   describe('updateCommentInRichHistory', () => {
     it('should update comment in query in history', async () => {
-      const updatedComment = await updateCommentInRichHistory(mock.storedHistory, 1, 'new comment');
+      const updatedComment = await updateCommentInRichHistory(mock.storedHistory, '1', 'new comment');
       expect(updatedComment[0].comment).toEqual('new comment');
     });
   });
 
   describe('deleteQueryInRichHistory', () => {
     it('should delete query in query in history', async () => {
-      const deletedHistory = await deleteQueryInRichHistory(mock.storedHistory, 1);
+      const deletedHistory = await deleteQueryInRichHistory(mock.storedHistory, '1');
       expect(deletedHistory).toEqual([]);
     });
   });
@@ -230,7 +254,7 @@ describe('richHistory', () => {
   describe('createQueryHeading', () => {
     it('should correctly create heading for queries when sort order is ascending ', () => {
       // Have to offset the timezone of a 1 microsecond epoch, and then reverse the changes
-      mock.storedHistory[0].ts = 1 + -1 * dateTime().utcOffset() * 60 * 1000;
+      mock.storedHistory[0].createdAt = 1 + -1 * dateTime().utcOffset() * 60 * 1000;
       const heading = createQueryHeading(mock.storedHistory[0], SortOrder.Ascending);
       expect(heading).toEqual('January 1');
     });

public/app/core/utils/richHistory.ts

@@ -34,6 +34,7 @@ export { SortOrder };
 
 export async function addToRichHistory(
   richHistory: RichHistoryQuery[],
+  datasourceUid: string,
   datasourceName: string | null,
   queries: DataQuery[],
   starred: boolean,
@@ -41,25 +42,25 @@ export async function addToRichHistory(
   showQuotaExceededError: boolean,
   showLimitExceededWarning: boolean
 ): Promise<{ richHistory: RichHistoryQuery[]; richHistoryStorageFull?: boolean; limitExceeded?: boolean }> {
-  const ts = Date.now();
   /* Save only queries, that are not falsy (e.g. empty object, null, ...) */
   const newQueriesToSave: DataQuery[] = queries && queries.filter((query) => notEmptyQuery(query));
 
   if (newQueriesToSave.length > 0) {
-    const newRichHistory: RichHistoryQuery = {
-      queries: newQueriesToSave,
-      ts,
-      datasourceName: datasourceName ?? '',
-      starred,
-      comment: comment ?? '',
-    };
-
     let richHistoryStorageFull = false;
     let limitExceeded = false;
     let warning: RichHistoryStorageWarningDetails | undefined;
+    let newRichHistory: RichHistoryQuery;
 
     try {
-      warning = await getRichHistoryStorage().addToRichHistory(newRichHistory);
+      const result = await getRichHistoryStorage().addToRichHistory({
+        datasourceUid: datasourceUid,
+        datasourceName: datasourceName ?? '',
+        queries: newQueriesToSave,
+        starred,
+        comment: comment ?? '',
+      });
+      warning = result.warning;
+      newRichHistory = result.richHistoryQuery;
     } catch (error) {
       if (error.name === RichHistoryServiceError.StorageFull) {
         richHistoryStorageFull = true;
@@ -93,26 +94,10 @@ export async function deleteAllFromRichHistory(): Promise<void> {
   return getRichHistoryStorage().deleteAll();
 }
 
-export async function updateStarredInRichHistory(richHistory: RichHistoryQuery[], ts: number) {
-  let updatedQuery: RichHistoryQuery | undefined;
-
-  const updatedHistory = richHistory.map((query) => {
-    /* Timestamps are currently unique - we can use them to identify specific queries */
-    if (query.ts === ts) {
-      const isStarred = query.starred;
-      updatedQuery = Object.assign({}, query, { starred: !isStarred });
-      return updatedQuery;
-    }
-    return query;
-  });
-
-  if (!updatedQuery) {
-    return richHistory;
-  }
-
+export async function updateStarredInRichHistory(richHistory: RichHistoryQuery[], id: string, starred: boolean) {
   try {
-    await getRichHistoryStorage().updateStarred(ts, updatedQuery.starred);
-    return updatedHistory;
+    const updatedQuery = await getRichHistoryStorage().updateStarred(id, starred);
+    return richHistory.map((query) => (query.id === id ? updatedQuery : query));
   } catch (error) {
     dispatch(notifyApp(createErrorNotification('Saving rich history failed', error.message)));
     return richHistory;
@@ -121,25 +106,12 @@ export async function updateStarredInRichHistory(richHistory: RichHistoryQuery[]
 
 export async function updateCommentInRichHistory(
   richHistory: RichHistoryQuery[],
-  ts: number,
+  id: string,
   newComment: string | undefined
 ) {
-  let updatedQuery: RichHistoryQuery | undefined;
-  const updatedHistory = richHistory.map((query) => {
-    if (query.ts === ts) {
-      updatedQuery = Object.assign({}, query, { comment: newComment });
-      return updatedQuery;
-    }
-    return query;
-  });
-
-  if (!updatedQuery) {
-    return richHistory;
-  }
-
   try {
-    await getRichHistoryStorage().updateComment(ts, newComment);
-    return updatedHistory;
+    const updatedQuery = await getRichHistoryStorage().updateComment(id, newComment);
+    return richHistory.map((query) => (query.id === id ? updatedQuery : query));
   } catch (error) {
     dispatch(notifyApp(createErrorNotification('Saving rich history failed', error.message)));
     return richHistory;
@@ -148,11 +120,11 @@ export async function updateCommentInRichHistory(
 
 export async function deleteQueryInRichHistory(
   richHistory: RichHistoryQuery[],
-  ts: number
+  id: string
 ): Promise<RichHistoryQuery[]> {
-  const updatedHistory = richHistory.filter((query) => query.ts !== ts);
+  const updatedHistory = richHistory.filter((query) => query.id !== id);
   try {
-    await getRichHistoryStorage().deleteRichHistory(ts);
+    await getRichHistoryStorage().deleteRichHistory(id);
     return updatedHistory;
   } catch (error) {
     dispatch(notifyApp(createErrorNotification('Saving rich history failed', error.message)));
@@ -234,7 +206,7 @@ export function createQueryHeading(query: RichHistoryQuery, sortOrder: SortOrder
   if (sortOrder === SortOrder.DatasourceAZ || sortOrder === SortOrder.DatasourceZA) {
     heading = query.datasourceName;
   } else {
-    heading = createDateStringFromTs(query.ts);
+    heading = createDateStringFromTs(query.createdAt);
   }
   return heading;
 }

public/app/features/explore/RichHistory/RichHistoryCard.test.tsx

@@ -1,26 +1,36 @@
 import React from 'react';
 import { shallow } from 'enzyme';
 import { RichHistoryCard, Props } from './RichHistoryCard';
-import { ExploreId } from '../../../types/explore';
+import { ExploreId, RichHistoryQuery } from '../../../types/explore';
 import { DataSourceApi, DataQuery } from '@grafana/data';
 
-const setup = (propOverrides?: Partial<Props>) => {
-  const props: Props = {
+const starRichHistoryMock = jest.fn();
+
+interface MockQuery extends DataQuery {
+  query: string;
+}
+
+const setup = (propOverrides?: Partial<Props<MockQuery>>) => {
+  const props: Props<MockQuery> = {
     query: {
-      ts: 1,
+      id: '1',
+      createdAt: 1,
+      datasourceUid: 'Test datasource uid',
       datasourceName: 'Test datasource',
       starred: false,
       comment: '',
       queries: [
-        { expr: 'query1', refId: 'A' } as DataQuery,
-        { expr: 'query2', refId: 'B' } as DataQuery,
-        { expr: 'query3', refId: 'C' } as DataQuery,
+        { query: 'query1', refId: 'A' },
+        { query: 'query2', refId: 'B' },
+        { query: 'query3', refId: 'C' },
       ],
     },
     dsImg: '/app/img',
     isRemoved: false,
     changeDatasource: jest.fn(),
-    updateRichHistory: jest.fn(),
+    starHistoryItem: starRichHistoryMock,
+    commentHistoryItem: jest.fn(),
+    deleteHistoryItem: jest.fn(),
     setQueries: jest.fn(),
     exploreId: ExploreId.left,
     datasourceInstance: { name: 'Datasource' } as DataSourceApi,
@@ -32,8 +42,10 @@ const setup = (propOverrides?: Partial<Props>) => {
   return wrapper;
 };
 
-const starredQueryWithComment = {
-  ts: 1,
+const starredQueryWithComment: RichHistoryQuery<MockQuery> = {
+  id: '1',
+  createdAt: 1,
+  datasourceUid: 'Test datasource uid',
   datasourceName: 'Test datasource',
   starred: true,
   comment: 'test comment',
@@ -48,9 +60,9 @@ describe('RichHistoryCard', () => {
   it('should render all queries', () => {
     const wrapper = setup();
     expect(wrapper.find({ 'aria-label': 'Query text' })).toHaveLength(3);
-    expect(wrapper.find({ 'aria-label': 'Query text' }).at(0).text()).toEqual('{"expr":"query1"}');
-    expect(wrapper.find({ 'aria-label': 'Query text' }).at(1).text()).toEqual('{"expr":"query2"}');
-    expect(wrapper.find({ 'aria-label': 'Query text' }).at(2).text()).toEqual('{"expr":"query3"}');
+    expect(wrapper.find({ 'aria-label': 'Query text' }).at(0).text()).toEqual('{"query":"query1"}');
+    expect(wrapper.find({ 'aria-label': 'Query text' }).at(1).text()).toEqual('{"query":"query2"}');
+    expect(wrapper.find({ 'aria-label': 'Query text' }).at(2).text()).toEqual('{"query":"query3"}');
   });
   it('should render data source icon', () => {
     const wrapper = setup();
@@ -120,11 +132,17 @@ describe('RichHistoryCard', () => {
   describe('starring', () => {
     it('should have title "Star query", if not starred', () => {
       const wrapper = setup();
-      expect(wrapper.find({ title: 'Star query' })).toHaveLength(1);
+      const starButton = wrapper.find({ title: 'Star query' });
+      expect(starButton).toHaveLength(1);
+      starButton.simulate('click');
+      expect(starRichHistoryMock).toBeCalledWith(starredQueryWithComment.id, true);
     });
     it('should have title "Unstar query", if not starred', () => {
       const wrapper = setup({ query: starredQueryWithComment });
-      expect(wrapper.find({ title: 'Unstar query' })).toHaveLength(1);
+      const starButton = wrapper.find({ title: 'Unstar query' });
+      expect(starButton).toHaveLength(1);
+      starButton.simulate('click');
+      expect(starRichHistoryMock).toBeCalledWith(starredQueryWithComment.id, false);
     });
   });
 });

public/app/features/explore/RichHistory/RichHistoryCard.tsx

@@ -3,7 +3,7 @@ import { connect, ConnectedProps } from 'react-redux';
 import { css, cx } from '@emotion/css';
 import { stylesFactory, useTheme, TextArea, Button, IconButton } from '@grafana/ui';
 import { getDataSourceSrv } from '@grafana/runtime';
-import { GrafanaTheme, DataSourceApi } from '@grafana/data';
+import { GrafanaTheme, DataSourceApi, DataQuery } from '@grafana/data';
 import { RichHistoryQuery, ExploreId } from 'app/types/explore';
 import { createUrlFromRichHistory, createQueryText } from 'app/core/utils/richHistory';
 import { createAndCopyShortLink } from 'app/core/utils/shortLinks';
@@ -14,7 +14,7 @@ import { notifyApp } from 'app/core/actions';
 import { createSuccessNotification } from 'app/core/copy/appNotification';
 import { StoreState } from 'app/types';
 
-import { updateRichHistory } from '../state/history';
+import { starHistoryItem, commentHistoryItem, deleteHistoryItem } from '../state/history';
 import { changeDatasource } from '../state/datasource';
 import { setQueries } from '../state/query';
 import { ShowConfirmModalEvent } from '../../../types/events';
@@ -30,19 +30,21 @@ function mapStateToProps(state: StoreState, { exploreId }: { exploreId: ExploreI
 
 const mapDispatchToProps = {
   changeDatasource,
-  updateRichHistory,
+  deleteHistoryItem,
+  commentHistoryItem,
+  starHistoryItem,
   setQueries,
 };
 
 const connector = connect(mapStateToProps, mapDispatchToProps);
 
-interface OwnProps {
-  query: RichHistoryQuery;
+interface OwnProps<T extends DataQuery = DataQuery> {
+  query: RichHistoryQuery<T>;
   dsImg: string;
   isRemoved: boolean;
 }
 
-export type Props = ConnectedProps<typeof connector> & OwnProps;
+export type Props<T extends DataQuery = DataQuery> = ConnectedProps<typeof connector> & OwnProps<T>;
 
 const getStyles = stylesFactory((theme: GrafanaTheme, isRemoved: boolean) => {
   /* Hard-coded value so all buttons and icons on right side of card are aligned */
@@ -143,8 +145,18 @@ const getStyles = stylesFactory((theme: GrafanaTheme, isRemoved: boolean) => {
 });
 
 export function RichHistoryCard(props: Props) {
-  const { query, dsImg, isRemoved, updateRichHistory, changeDatasource, exploreId, datasourceInstance, setQueries } =
-    props;
+  const {
+    query,
+    dsImg,
+    isRemoved,
+    commentHistoryItem,
+    starHistoryItem,
+    deleteHistoryItem,
+    changeDatasource,
+    exploreId,
+    datasourceInstance,
+    setQueries,
+  } = props;
   const [activeUpdateComment, setActiveUpdateComment] = useState(false);
   const [comment, setComment] = useState<string | undefined>(query.comment);
   const [queryDsInstance, setQueryDsInstance] = useState<DataSourceApi | undefined>(undefined);
@@ -192,25 +204,25 @@ export function RichHistoryCard(props: Props) {
           yesText: 'Delete',
           icon: 'trash-alt',
           onConfirm: () => {
-            updateRichHistory(query.ts, 'delete');
+            deleteHistoryItem(query.id);
             dispatch(notifyApp(createSuccessNotification('Query deleted')));
           },
         })
       );
     } else {
-      updateRichHistory(query.ts, 'delete');
+      deleteHistoryItem(query.id);
       dispatch(notifyApp(createSuccessNotification('Query deleted')));
     }
   };
 
   const onStarrQuery = () => {
-    updateRichHistory(query.ts, 'starred');
+    starHistoryItem(query.id, !query.starred);
   };
 
   const toggleActiveUpdateComment = () => setActiveUpdateComment(!activeUpdateComment);
 
   const onUpdateComment = () => {
-    updateRichHistory(query.ts, 'comment', comment);
+    commentHistoryItem(query.id, comment);
     setActiveUpdateComment(false);
   };
 

public/app/features/explore/RichHistory/RichHistoryQueriesTab.tsx

@@ -240,7 +240,7 @@ export function RichHistoryQueriesTab(props: Props) {
                 return (
                   <RichHistoryCard
                     query={q}
-                    key={q.ts}
+                    key={q.id}
                     exploreId={exploreId}
                     dsImg={listOfDatasources[idx].imgUrl}
                     isRemoved={listOfDatasources[idx].isRemoved}

public/app/features/explore/RichHistory/RichHistoryStarredTab.tsx

@@ -151,7 +151,7 @@ export function RichHistoryStarredTab(props: Props) {
           return (
             <RichHistoryCard
               query={q}
-              key={q.ts}
+              key={q.id}
               exploreId={exploreId}
               dsImg={listOfDatasources[idx].imgUrl}
               isRemoved={listOfDatasources[idx].isRemoved}

public/app/features/explore/state/history.ts

@@ -23,19 +23,23 @@ export const historyUpdatedAction = createAction<HistoryUpdatedPayload>('explore
 // Action creators
 //
 
-export const updateRichHistory = (ts: number, property: string, updatedProperty?: string): ThunkResult<void> => {
+export const starHistoryItem = (id: string, starred: boolean): ThunkResult<void> => {
   return async (dispatch, getState) => {
-    // Side-effect: Saving rich history in localstorage
-    let nextRichHistory;
-    if (property === 'starred') {
-      nextRichHistory = await updateStarredInRichHistory(getState().explore.richHistory, ts);
-    }
-    if (property === 'comment') {
-      nextRichHistory = await updateCommentInRichHistory(getState().explore.richHistory, ts, updatedProperty);
-    }
-    if (property === 'delete') {
-      nextRichHistory = await deleteQueryInRichHistory(getState().explore.richHistory, ts);
-    }
+    const nextRichHistory = await updateStarredInRichHistory(getState().explore.richHistory, id, starred);
+    dispatch(richHistoryUpdatedAction({ richHistory: nextRichHistory }));
+  };
+};
+
+export const commentHistoryItem = (id: string, comment?: string): ThunkResult<void> => {
+  return async (dispatch, getState) => {
+    const nextRichHistory = await updateCommentInRichHistory(getState().explore.richHistory, id, comment);
+    dispatch(richHistoryUpdatedAction({ richHistory: nextRichHistory }));
+  };
+};
+
+export const deleteHistoryItem = (id: string): ThunkResult<void> => {
+  return async (dispatch, getState) => {
+    const nextRichHistory = await deleteQueryInRichHistory(getState().explore.richHistory, id);
     dispatch(richHistoryUpdatedAction({ richHistory: nextRichHistory }));
   };
 };

public/app/features/explore/state/query.ts

@@ -326,6 +326,7 @@ async function handleHistory(
     limitExceeded,
   } = await addToRichHistory(
     state.richHistory || [],
+    datasource.uid,
     datasource.name,
     queries,
     false,

public/app/types/explore.ts

@@ -195,12 +195,14 @@ export interface QueryTransaction {
   scanning?: boolean;
 }
 
-export type RichHistoryQuery = {
-  ts: number;
+export type RichHistoryQuery<T extends DataQuery = DataQuery> = {
+  id: string;
+  createdAt: number;
+  datasourceUid: string;
   datasourceName: string;
   starred: boolean;
   comment: string;
-  queries: DataQuery[];
+  queries: T[];
 };
 
 export interface ExplorePanelData extends PanelData {