Query History: Migrate local storage to remote storage (#48572)

* Load Rich History when the container is opened

* Store rich history for each pane separately

* Do not update currently opened query history when an item is added

It's impossible to figure out if the item should be added or not, because filters are applied in the backend. We don't want to replicate that filtering logic in frontend. One way to make it work could be by refreshing both panes.

* Test starring and deleting query history items when both panes are open

* Remove e2e dependency on ExploreId

* Fix unit test

* Assert exact queries

* Simplify test

* Fix e2e tests

* Fix toolbar a11y

* Reload the history after an item is added

* Fix unit test

* Remove references to Explore from generic PageToolbar component

* Update test name

* Fix test assertion

* Add issue item to TODO

* Improve test assertion

* Simplify test setup

* Move query history settings to persistence layer

* Fix test import

* Fix unit test

* Fix unit test

* Test local storage settings API

* Code formatting

* Fix linting errors

* Add an integration test

* Add missing aria role

* Fix a11y issues

* Fix a11y issues

* Use divs instead of ul/li

Otherwis,e pa11y-ci reports the error below claiming there are no children with role=tab:

Certain ARIA roles must contain particular children
   (https://dequeuniversity.com/rules/axe/4.3/aria-required-children?application=axeAPI)

   (#reactRoot > div > main > div:nth-child(3) > div > div:nth-child(1) > div >
   div:nth-child(1) > div > div > nav > div:nth-child(2) > ul)

   <ul class="css-af3vye" role="tablist"><li class="css-1ciwanz"><a href...</ul>

* Clean up settings tab

* Remove redundant aria label

* Remove redundant container

* Clean up test assertions

* Move filtering to persistence layer

* Move filtering to persistence layer

* Simplify applying filters

* Split applying filters and reloading the history

* Debounce updating filters

* Update tests

* Fix waiting for debounced results

* Clear results when switching tabs

* Improve test coverage

* Update docs

* Revert extra handling for uid (will be added when we introduce remote storage)

* Create basic plan

* Rename query history toggle

* Add list of supported features and add ds name to RichHistoryQuery object

* Clean up

Removed planned items will be addressed in upcoming prs (filtering and pagination)

* Handle data source filters

* Simplify DTO conversion

* Clean up

* Fix betterer conflicts

* Fix imports

* Fix imports

* Post-merge fixes

* Use config instead of a feature flag

* Use config instead of a feature flag

* Update converter tests

* Add tests for RichHistoryRemoteStorage

* Simplify test setup

* Simplify assertion

* Add e2e test for query history

* Remove duplicated entry

* Fix unit tests

* Improve readability

* Remove unnecessary casting

* Mock backend in integration tests

* Remove unnecessary casting

* Fix integration test

* Update betterer results

* Fix unit tests

* Simplify testing with DataSourceSrv

* Fix sorting and add to/from filtering

* Add migration for local storage query history

* Test query history migration

* Simplify testing DataSourceSettings

* Skip redundant migrations

* Revert error logging test

* Fix tests

* Update betterer results

* Change notification message after migration

* Ensure previous request is canceled when getting search results

* Add loading message when results are being loaded

* Show info message only if local storage is enabled

* Fix unit test

* Post-merge fixes

* Fix intergration tests

* Fix incorrect filtering

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

@@ -34,8 +34,8 @@ describe('RichHistoryRemoteStorage', () => {
     storage = new RichHistoryRemoteStorage();
   });
 
-  it('returns list of query history items', async () => {
-    const expectedViewModel: RichHistoryQuery<any> = {
+  const setup = (): { richHistoryQuery: RichHistoryQuery; dto: RichHistoryRemoteStorageDTO } => {
+    const richHistoryQuery: RichHistoryQuery<any> = {
       id: '123',
       createdAt: 200 * 1000,
       datasourceUid: 'ds1',
@@ -44,16 +44,25 @@ describe('RichHistoryRemoteStorage', () => {
       comment: 'comment',
       queries: [{ foo: 'bar ' }],
     };
-    const returnedDTOs: RichHistoryRemoteStorageDTO[] = [
-      {
-        uid: expectedViewModel.id,
-        createdAt: expectedViewModel.createdAt / 1000,
-        datasourceUid: expectedViewModel.datasourceUid,
-        starred: expectedViewModel.starred,
-        comment: expectedViewModel.comment,
-        queries: expectedViewModel.queries,
-      },
-    ];
+
+    const dto = {
+      uid: richHistoryQuery.id,
+      createdAt: richHistoryQuery.createdAt / 1000,
+      datasourceUid: richHistoryQuery.datasourceUid,
+      starred: richHistoryQuery.starred,
+      comment: richHistoryQuery.comment,
+      queries: richHistoryQuery.queries,
+    };
+
+    return {
+      richHistoryQuery,
+      dto,
+    };
+  };
+
+  it('returns list of query history items', async () => {
+    const { richHistoryQuery, dto } = setup();
+    const returnedDTOs: RichHistoryRemoteStorageDTO[] = [dto];
     fetchMock.mockReturnValue(
       of({
         data: {
@@ -79,6 +88,18 @@ describe('RichHistoryRemoteStorage', () => {
       url: `/api/query-history?datasourceUid=ds1&datasourceUid=ds2&searchString=${search}&sort=time-desc&to=now-${from}d&from=now-${to}d&limit=${expectedLimit}&page=${expectedPage}&onlyStarred=${starred}`,
       requestId: 'query-history-get-all',
     });
-    expect(items).toMatchObject([expectedViewModel]);
+    expect(items).toMatchObject([richHistoryQuery]);
+  });
+
+  it('migrates provided rich history items', async () => {
+    const { richHistoryQuery, dto } = setup();
+    fetchMock.mockReturnValue(of({}));
+    await storage.migrate([richHistoryQuery]);
+    expect(fetchMock).toBeCalledWith({
+      url: '/api/query-history/migrate',
+      method: 'POST',
+      data: { queries: [dto] },
+      showSuccessAlert: false,
+    });
   });
 });

public/app/core/history/RichHistoryRemoteStorage.ts

@@ -7,7 +7,7 @@ import { DataQuery } from '../../../../packages/grafana-data';
 import { RichHistorySearchFilters, RichHistorySettings, SortOrder } from '../utils/richHistoryTypes';
 
 import RichHistoryStorage, { RichHistoryStorageWarningDetails } from './RichHistoryStorage';
-import { fromDTO } from './remoteStorageConverter';
+import { fromDTO, toDTO } from './remoteStorageConverter';
 
 export type RichHistoryRemoteStorageDTO = {
   uid: string;
@@ -18,6 +18,18 @@ export type RichHistoryRemoteStorageDTO = {
   queries: DataQuery[];
 };
 
+type RichHistoryRemoteStorageMigrationDTO = {
+  datasourceUid: string;
+  queries: DataQuery[];
+  createdAt: number;
+  starred: boolean;
+  comment: string;
+};
+
+type RichHistoryRemoteStorageMigrationPayloadDTO = {
+  queries: RichHistoryRemoteStorageMigrationDTO[];
+};
+
 type RichHistoryRemoteStorageResultsPayloadDTO = {
   result: {
     queryHistory: RichHistoryRemoteStorageDTO[];
@@ -78,6 +90,20 @@ export default class RichHistoryRemoteStorage implements RichHistoryStorage {
   async updateStarred(id: string, starred: boolean): Promise<RichHistoryQuery> {
     throw new Error('not supported yet');
   }
+
+  /**
+   * @internal Used only for migration purposes. Will be removed in future.
+   */
+  async migrate(richHistory: RichHistoryQuery[]) {
+    await lastValueFrom(
+      getBackendSrv().fetch({
+        url: '/api/query-history/migrate',
+        method: 'POST',
+        data: { queries: richHistory.map(toDTO) } as RichHistoryRemoteStorageMigrationPayloadDTO,
+        showSuccessAlert: false,
+      })
+    );
+  }
 }
 
 function buildQueryParams(filters: RichHistorySearchFilters): string {

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

@@ -3,7 +3,7 @@ import { RichHistoryQuery } from '../../types';
 import { backendSrv } from '../services/backend_srv';
 
 import { RichHistoryRemoteStorageDTO } from './RichHistoryRemoteStorage';
-import { fromDTO } from './remoteStorageConverter';
+import { fromDTO, toDTO } from './remoteStorageConverter';
 
 const dsMock = new DatasourceSrv();
 dsMock.init(
@@ -43,4 +43,7 @@ describe('RemoteStorage converter', () => {
   it('converts DTO to RichHistoryQuery', () => {
     expect(fromDTO(validDTO)).toMatchObject(validRichHistory);
   });
+  it('convert RichHistoryQuery to DTO', () => {
+    expect(toDTO(validRichHistory)).toMatchObject(validDTO);
+  });
 });

public/app/core/history/remoteStorageConverter.ts

@@ -17,3 +17,14 @@ export const fromDTO = (dto: RichHistoryRemoteStorageDTO): RichHistoryQuery => {
     queries: dto.queries,
   };
 };
+
+export const toDTO = (richHistory: RichHistoryQuery): RichHistoryRemoteStorageDTO => {
+  return {
+    uid: richHistory.id,
+    createdAt: Math.floor(richHistory.createdAt / 1000),
+    datasourceUid: richHistory.datasourceUid,
+    starred: richHistory.starred,
+    comment: richHistory.comment,
+    queries: richHistory.queries,
+  };
+};

public/app/core/history/richHistoryLocalStorageUtils.ts

@@ -96,4 +96,5 @@ export const RICH_HISTORY_SETTING_KEYS = {
   starredTabAsFirstTab: 'grafana.explore.richHistory.starredTabAsFirstTab',
   activeDatasourceOnly: 'grafana.explore.richHistory.activeDatasourceOnly',
   datasourceFilters: 'grafana.explore.richHistory.datasourceFilters',
+  migrated: 'grafana.explore.richHistory.migrated',
 };

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

@@ -13,6 +13,7 @@ import {
   createQueryHeading,
   deleteAllFromRichHistory,
   deleteQueryInRichHistory,
+  migrateQueryHistoryFromLocalStorage,
   SortOrder,
 } from './richHistory';
 
@@ -24,6 +25,20 @@ jest.mock('../history/richHistoryStorageProvider', () => {
   };
 });
 
+const richHistoryLocalStorageMock = { getRichHistory: jest.fn() };
+jest.mock('../history/RichHistoryLocalStorage', () => {
+  return function () {
+    return richHistoryLocalStorageMock;
+  };
+});
+
+const richHistoryRemoteStorageMock = { migrate: jest.fn() };
+jest.mock('../history/RichHistoryRemoteStorage', () => {
+  return function () {
+    return richHistoryRemoteStorageMock;
+  };
+});
+
 interface MockQuery extends DataQuery {
   expr: string;
   maxLines?: number | null;
@@ -163,6 +178,25 @@ describe('richHistory', () => {
     });
   });
 
+  describe('migration', () => {
+    beforeEach(() => {
+      richHistoryRemoteStorageMock.migrate.mockReset();
+    });
+
+    it('migrates history', async () => {
+      const history = [{ id: 'test' }, { id: 'test2' }];
+
+      richHistoryLocalStorageMock.getRichHistory.mockReturnValue(history);
+      await migrateQueryHistoryFromLocalStorage();
+      expect(richHistoryRemoteStorageMock.migrate).toBeCalledWith(history);
+    });
+    it('does not migrate if there are no entries', async () => {
+      richHistoryLocalStorageMock.getRichHistory.mockReturnValue([]);
+      await migrateQueryHistoryFromLocalStorage();
+      expect(richHistoryRemoteStorageMock.migrate).not.toBeCalled();
+    });
+  });
+
   describe('mapNumbertoTimeInSlider', () => {
     it('should correctly map number to value', () => {
       const value = mapNumbertoTimeInSlider(25);

public/app/core/utils/richHistory.ts

@@ -4,10 +4,16 @@ import { DataQuery, DataSourceApi, dateTimeFormat, ExploreUrlState, urlUtil } fr
 import { serializeStateToUrlParam } from '@grafana/data/src/utils/url';
 import { getDataSourceSrv } from '@grafana/runtime';
 import { notifyApp } from 'app/core/actions';
-import { createErrorNotification, createWarningNotification } from 'app/core/copy/appNotification';
+import {
+  createErrorNotification,
+  createSuccessNotification,
+  createWarningNotification,
+} from 'app/core/copy/appNotification';
 import { dispatch } from 'app/store/store';
 import { RichHistoryQuery } from 'app/types/explore';
 
+import RichHistoryLocalStorage from '../history/RichHistoryLocalStorage';
+import RichHistoryRemoteStorage from '../history/RichHistoryRemoteStorage';
 import {
   RichHistoryServiceError,
   RichHistoryStorageWarning,
@@ -118,6 +124,37 @@ export async function deleteQueryInRichHistory(id: string) {
   }
 }
 
+export enum LocalStorageMigrationStatus {
+  Successful = 'successful',
+  Failed = 'failed',
+  NotNeeded = 'not-needed',
+}
+
+export async function migrateQueryHistoryFromLocalStorage(): Promise<LocalStorageMigrationStatus> {
+  const richHistoryLocalStorage = new RichHistoryLocalStorage();
+  const richHistoryRemoteStorage = new RichHistoryRemoteStorage();
+
+  try {
+    const richHistory: RichHistoryQuery[] = await richHistoryLocalStorage.getRichHistory({
+      datasourceFilters: [],
+      from: 0,
+      search: '',
+      sortOrder: SortOrder.Descending,
+      starred: false,
+      to: 14,
+    });
+    if (richHistory.length === 0) {
+      return LocalStorageMigrationStatus.NotNeeded;
+    }
+    await richHistoryRemoteStorage.migrate(richHistory);
+    dispatch(notifyApp(createSuccessNotification('Query history successfully migrated from local storage')));
+    return LocalStorageMigrationStatus.Successful;
+  } catch (error) {
+    dispatch(notifyApp(createWarningNotification(`Query history migration failed. ${error.message}`)));
+    return LocalStorageMigrationStatus.Failed;
+  }
+}
+
 export const createUrlFromRichHistory = (query: RichHistoryQuery) => {
   const exploreState: ExploreUrlState = {
     /* Default range, as we are not saving timerange in rich history */

public/app/features/explore/QueryRows.test.tsx

@@ -55,6 +55,7 @@ function setup(queries: DataQuery[]) {
     right: undefined,
     richHistoryStorageFull: false,
     richHistoryLimitExceededWarningShown: false,
+    richHistoryMigrationFailed: false,
   };
   const store = configureStore({ explore: initialState, user: { orgId: 1 } as UserState });
 

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

@@ -138,7 +138,7 @@ export function RichHistoryQueriesTab(props: Props) {
 
   useEffect(() => {
     const datasourceFilters =
-      richHistorySettings.activeDatasourceOnly && richHistorySettings.lastUsedDatasourceFilters
+      !richHistorySettings.activeDatasourceOnly && richHistorySettings.lastUsedDatasourceFilters
         ? richHistorySettings.lastUsedDatasourceFilters
         : [activeDatasourceInstance];
     const filters: RichHistorySearchFilters = {

public/app/features/explore/spec/helper/interactions.ts

@@ -31,9 +31,7 @@ export const openQueryHistory = async (exploreId: ExploreId = ExploreId.left) =>
   const selector = withinExplore(exploreId);
   const button = selector.getByRole('button', { name: 'Rich history button' });
   await userEvent.click(button);
-  expect(
-    await selector.findByText('The history is local to your browser and is not shared with others.')
-  ).toBeInTheDocument();
+  expect(await selector.findByPlaceholderText('Search queries')).toBeInTheDocument();
 };
 
 export const closeQueryHistory = async (exploreId: ExploreId = ExploreId.left) => {

public/app/features/explore/spec/helper/setup.tsx

@@ -11,6 +11,7 @@ import { GrafanaRoute } from 'app/core/navigation/GrafanaRoute';
 import { Echo } from 'app/core/services/echo/Echo';
 import { configureStore } from 'app/store/configureStore';
 
+import { RICH_HISTORY_KEY, RichHistoryLocalStorageDTO } from '../../../../core/history/RichHistoryLocalStorage';
 import { LokiDatasource } from '../../../../plugins/datasource/loki/datasource';
 import { LokiQuery } from '../../../../plugins/datasource/loki/types';
 import { ExploreId } from '../../../../types';
@@ -156,3 +157,18 @@ export const withinExplore = (exploreId: ExploreId) => {
   const container = screen.getAllByTestId('data-testid Explore');
   return within(container[exploreId === ExploreId.left ? 0 : 1]);
 };
+
+export const setupLocalStorageRichHistory = (dsName: string) => {
+  window.localStorage.setItem(
+    RICH_HISTORY_KEY,
+    JSON.stringify([
+      {
+        ts: Date.now(),
+        datasourceName: dsName,
+        starred: true,
+        comment: '',
+        queries: [{ refId: 'A' }],
+      } as RichHistoryLocalStorageDTO,
+    ])
+  );
+};

public/app/features/explore/spec/queryHistory.test.tsx

@@ -1,6 +1,8 @@
 import React from 'react';
+import { of } from 'rxjs';
 
 import { serializeStateToUrlParam } from '@grafana/data';
+import { config } from '@grafana/runtime';
 
 import { silenceConsoleOutput } from '../../../../test/core/utils/silenceConsoleOutput';
 import { ExploreId } from '../../../types';
@@ -24,12 +26,14 @@ import {
   switchToQueryHistoryTab,
 } from './helper/interactions';
 import { makeLogsQueryResponse } from './helper/query';
-import { setupExplore, tearDown, waitForExplore } from './helper/setup';
+import { setupExplore, setupLocalStorageRichHistory, tearDown, waitForExplore } from './helper/setup';
 
-const fetch = jest.fn();
+const fetchMock = jest.fn();
+const postMock = jest.fn();
+const getMock = jest.fn();
 jest.mock('@grafana/runtime', () => ({
   ...jest.requireActual('@grafana/runtime'),
-  getBackendSrv: () => ({ fetch }),
+  getBackendSrv: () => ({ fetch: fetchMock, post: postMock, get: getMock }),
 }));
 
 jest.mock('react-virtualized-auto-sizer', () => {
@@ -48,6 +52,10 @@ describe('Explore: Query History', () => {
   silenceConsoleOutput();
 
   afterEach(() => {
+    config.queryHistoryEnabled = false;
+    fetchMock.mockClear();
+    postMock.mockClear();
+    getMock.mockClear();
     tearDown();
   });
 
@@ -153,4 +161,45 @@ describe('Explore: Query History', () => {
     assertQueryHistoryTabIsSelected('Starred');
     assertDataSourceFilterVisibility(false);
   });
+
+  describe('local storage migration', () => {
+    it('does not migrate if query history is not enabled', async () => {
+      config.queryHistoryEnabled = false;
+      const { datasources } = setupExplore();
+      setupLocalStorageRichHistory('loki');
+      (datasources.loki.query as jest.Mock).mockReturnValueOnce(makeLogsQueryResponse());
+      getMock.mockReturnValue({ result: { queryHistory: [] } });
+      await waitForExplore();
+
+      await openQueryHistory();
+      expect(postMock).not.toBeCalledWith('/api/query-history/migrate', { queries: [] });
+    });
+
+    it('migrates query history from local storage', async () => {
+      config.queryHistoryEnabled = true;
+      const { datasources } = setupExplore();
+      setupLocalStorageRichHistory('loki');
+      (datasources.loki.query as jest.Mock).mockReturnValueOnce(makeLogsQueryResponse());
+      fetchMock.mockReturnValue(of({ data: { result: { queryHistory: [] } } }));
+      await waitForExplore();
+
+      await openQueryHistory();
+      expect(fetchMock).toBeCalledWith(
+        expect.objectContaining({
+          url: expect.stringMatching('/api/query-history/migrate'),
+          data: { queries: [expect.objectContaining({ datasourceUid: 'loki' })] },
+        })
+      );
+      fetchMock.mockReset();
+      fetchMock.mockReturnValue(of({ data: { result: { queryHistory: [] } } }));
+
+      await closeQueryHistory();
+      await openQueryHistory();
+      expect(fetchMock).not.toBeCalledWith(
+        expect.objectContaining({
+          url: expect.stringMatching('/api/query-history/migrate'),
+        })
+      );
+    });
+  });
 });

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

@@ -1,12 +1,17 @@
 import { AnyAction, createAction } from '@reduxjs/toolkit';
 
 import { DataQuery, HistoryItem } from '@grafana/data';
+import { config } from '@grafana/runtime';
+import { RICH_HISTORY_SETTING_KEYS } from 'app/core/history/richHistoryLocalStorageUtils';
+import store from 'app/core/store';
 import {
   addToRichHistory,
   deleteAllFromRichHistory,
   deleteQueryInRichHistory,
   getRichHistory,
   getRichHistorySettings,
+  LocalStorageMigrationStatus,
+  migrateQueryHistoryFromLocalStorage,
   updateCommentInRichHistory,
   updateRichHistorySettings,
   updateStarredInRichHistory,
@@ -18,6 +23,7 @@ import { RichHistorySearchFilters, RichHistorySettings } from '../../../core/uti
 
 import {
   richHistoryLimitExceededAction,
+  richHistoryMigrationFailedAction,
   richHistorySearchFiltersUpdatedAction,
   richHistorySettingsUpdatedAction,
   richHistoryStorageFullAction,
@@ -140,6 +146,20 @@ export const clearRichHistoryResults = (exploreId: ExploreId): ThunkResult<void>
  */
 export const initRichHistory = (): ThunkResult<void> => {
   return async (dispatch, getState) => {
+    const queriesMigrated = store.getBool(RICH_HISTORY_SETTING_KEYS.migrated, false);
+    const migrationFailedDuringThisSession = getState().explore.richHistoryMigrationFailed;
+
+    // Query history migration should always be successful, but in case of unexpected errors we ensure
+    // the migration attempt happens only once per session, and the user is informed about the failure
+    // in a way that can help with potential investigation.
+    if (config.queryHistoryEnabled && !queriesMigrated && !migrationFailedDuringThisSession) {
+      const migrationStatus = await migrateQueryHistoryFromLocalStorage();
+      if (migrationStatus === LocalStorageMigrationStatus.Failed) {
+        dispatch(richHistoryMigrationFailedAction());
+      } else {
+        store.set(RICH_HISTORY_SETTING_KEYS.migrated, true);
+      }
+    }
     let settings = getState().explore.richHistorySettings;
     if (!settings) {
       settings = await getRichHistorySettings();

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

@@ -27,6 +27,7 @@ export const richHistoryUpdatedAction =
   createAction<{ richHistory: RichHistoryQuery[]; exploreId: ExploreId }>('explore/richHistoryUpdated');
 export const richHistoryStorageFullAction = createAction('explore/richHistoryStorageFullAction');
 export const richHistoryLimitExceededAction = createAction('explore/richHistoryLimitExceededAction');
+export const richHistoryMigrationFailedAction = createAction('explore/richHistoryMigrationFailedAction');
 
 export const richHistorySettingsUpdatedAction = createAction<RichHistorySettings>('explore/richHistorySettingsUpdated');
 export const richHistorySearchFiltersUpdatedAction = createAction<{
@@ -170,6 +171,7 @@ export const initialExploreState: ExploreState = {
   right: undefined,
   richHistoryStorageFull: false,
   richHistoryLimitExceededWarningShown: false,
+  richHistoryMigrationFailed: false,
 };
 
 /**
@@ -231,6 +233,13 @@ export const exploreReducer = (state = initialExploreState, action: AnyAction):
     };
   }
 
+  if (richHistoryMigrationFailedAction.match(action)) {
+    return {
+      ...state,
+      richHistoryMigrationFailed: true,
+    };
+  }
+
   if (resetExploreAction.match(action)) {
     const payload: ResetExplorePayload = action.payload;
     const leftState = state[ExploreId.left];

public/app/types/explore.ts

@@ -60,6 +60,11 @@ export interface ExploreState {
    * True if a warning message of hitting the exceeded number of items has been shown already.
    */
   richHistoryLimitExceededWarningShown: boolean;
+
+  /**
+   * True if a warning message about failed rich history has been shown already in this session.
+   */
+  richHistoryMigrationFailed: boolean;
 }
 
 export const EXPLORE_GRAPH_STYLES = ['lines', 'bars', 'points', 'stacked_lines', 'stacked_bars'] as const;