Query History: Load history when QueryHistoryContainer is opened (#46457)

* 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
This commit is contained in:
Piotr Jamróz
2022-04-06 13:49:25 +02:00
committed by GitHub
parent 8ef5212b0e
commit 88ec750728
21 changed files with 342 additions and 235 deletions

View File

@@ -236,9 +236,6 @@ exports[`no enzyme tests`] = {
"public/app/features/explore/RichHistory/RichHistoryCard.test.tsx:689438177": [
[1, 19, 13, "RegExp match", "2409514259"]
],
"public/app/features/explore/RichHistory/RichHistoryContainer.test.tsx:396471778": [
[1, 17, 13, "RegExp match", "2409514259"]
],
"public/app/features/explore/RichHistory/RichHistoryQueriesTab.test.tsx:3436519226": [
[1, 17, 13, "RegExp match", "2409514259"]
],

View File

@@ -21,11 +21,25 @@ export interface Props {
children?: ReactNode;
className?: string;
isFullscreen?: boolean;
'aria-label'?: string;
}
/** @alpha */
export const PageToolbar: FC<Props> = React.memo(
({ title, parent, pageIcon, onGoBack, children, titleHref, parentHref, leftItems, isFullscreen, className }) => {
({
title,
parent,
pageIcon,
onGoBack,
children,
titleHref,
parentHref,
leftItems,
isFullscreen,
className,
/** main nav-container aria-label **/
'aria-label': ariaLabel,
}) => {
const styles = useStyles2(getStyles);
/**
@@ -44,7 +58,7 @@ export const PageToolbar: FC<Props> = React.memo(
);
return (
<div className={mainStyle}>
<nav className={mainStyle} aria-label={ariaLabel}>
{pageIcon && !onGoBack && (
<div className={styles.pageIcon}>
<Icon name={pageIcon} size="lg" aria-hidden />
@@ -110,7 +124,7 @@ export const PageToolbar: FC<Props> = React.memo(
</div>
);
})}
</div>
</nav>
);
}
);

View File

@@ -12,7 +12,8 @@ import {
} from './richHistory';
import store from 'app/core/store';
import { dateTime, DataQuery } from '@grafana/data';
import RichHistoryStorage, { RichHistoryServiceError, RichHistoryStorageWarning } from '../history/RichHistoryStorage';
import RichHistoryStorage, { RichHistoryStorageWarning } from '../history/RichHistoryStorage';
import { RichHistoryQuery } from '../../types';
const richHistoryStorageMock: RichHistoryStorage = {} as RichHistoryStorage;
@@ -22,20 +23,27 @@ jest.mock('../history/richHistoryStorageProvider', () => {
};
});
interface MockQuery extends DataQuery {
expr: string;
maxLines?: number | null;
}
const storedHistory: Array<RichHistoryQuery<MockQuery>> = [
{
id: '1',
createdAt: 1,
comment: '',
datasourceUid: 'datasource uid',
datasourceName: 'datasource history name',
queries: [
{ expr: 'query1', maxLines: null, refId: '1' },
{ expr: 'query2', refId: '2' },
],
starred: true,
},
];
const mock: any = {
storedHistory: [
{
id: '1',
createdAt: 1,
comment: '',
datasourceName: 'datasource history name',
queries: [
{ expr: 'query1', maxLines: null, refId: '1' },
{ expr: 'query2', refId: '2' },
],
starred: true,
},
],
testComment: '',
testDatasourceUid: 'datasourceUid',
testDatasourceName: 'datasourceName',
@@ -83,23 +91,10 @@ describe('richHistory', () => {
deleteAllFromRichHistory();
expect(store.exists(key)).toBeFalsy();
});
const expectedResult = [
{
comment: mock.testComment,
datasourceUid: mock.testDatasourceUid,
datasourceName: mock.testDatasourceName,
queries: mock.testQueries,
starred: mock.testStarred,
createdAt: 2,
id: 'GENERATED ID',
},
mock.storedHistory[0],
];
it('should append query to query history', async () => {
Date.now = jest.fn(() => 2);
const { richHistory: newHistory } = await addToRichHistory(
mock.storedHistory,
const { limitExceeded, richHistoryStorageFull } = await addToRichHistory(
mock.testDatasourceUid,
mock.testDatasourceName,
mock.testQueries,
@@ -108,23 +103,8 @@ describe('richHistory', () => {
true,
true
);
expect(newHistory).toEqual(expectedResult);
});
it('should add query history to storage', async () => {
Date.now = jest.fn(() => 2);
const { richHistory } = await addToRichHistory(
mock.storedHistory,
mock.testDatasourceUid,
mock.testDatasourceName,
mock.testQueries,
mock.testStarred,
mock.testComment,
true,
true
);
expect(richHistory).toMatchObject(expectedResult);
expect(limitExceeded).toBeFalsy();
expect(richHistoryStorageFull).toBeFalsy();
expect(richHistoryStorageMock.addToRichHistory).toBeCalledWith({
datasourceUid: mock.testDatasourceUid,
datasourceName: mock.testDatasourceName,
@@ -134,27 +114,7 @@ describe('richHistory', () => {
});
});
it('should not append duplicated query to query history', async () => {
Date.now = jest.fn(() => 2);
const duplicatedEntryError = new Error();
duplicatedEntryError.name = RichHistoryServiceError.DuplicatedEntry;
richHistoryStorageMock.addToRichHistory = jest.fn().mockRejectedValue(duplicatedEntryError);
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,
mock.testComment,
true,
true
);
expect(newHistory).toEqual([mock.storedHistory[0]]);
});
it('it should append new items even when the limit is exceeded', async () => {
it('it should return a flag indicating that the limit has been exceed', async () => {
Date.now = jest.fn(() => 2);
richHistoryStorageMock.addToRichHistory = jest.fn((query) => {
@@ -167,8 +127,7 @@ describe('richHistory', () => {
});
});
const { richHistory, limitExceeded } = await addToRichHistory(
mock.storedHistory,
const { richHistoryStorageFull, limitExceeded } = await addToRichHistory(
mock.testDatasourceUid,
mock.testDatasourceName,
mock.testQueries,
@@ -177,29 +136,29 @@ describe('richHistory', () => {
true,
true
);
expect(richHistory).toEqual(expectedResult);
expect(richHistoryStorageFull).toBeFalsy();
expect(limitExceeded).toBeTruthy();
});
});
describe('updateStarredInRichHistory', () => {
it('should update starred in query in history', async () => {
const updatedStarred = await updateStarredInRichHistory(mock.storedHistory, '1', !mock.starred);
expect(updatedStarred[0].starred).toEqual(!mock.starred);
const updatedStarred = await updateStarredInRichHistory('1', !mock.starred);
expect(updatedStarred!.starred).toEqual(!mock.starred);
});
});
describe('updateCommentInRichHistory', () => {
it('should update comment in query in history', async () => {
const updatedComment = await updateCommentInRichHistory(mock.storedHistory, '1', 'new comment');
expect(updatedComment[0].comment).toEqual('new comment');
const updatedComment = await updateCommentInRichHistory('1', 'new comment');
expect(updatedComment!.comment).toEqual('new comment');
});
});
describe('deleteQueryInRichHistory', () => {
it('should delete query in query in history', async () => {
const deletedHistory = await deleteQueryInRichHistory(mock.storedHistory, '1');
expect(deletedHistory).toEqual([]);
const deletedHistoryId = await deleteQueryInRichHistory('1');
expect(deletedHistoryId).toEqual('1');
});
});
@@ -220,7 +179,7 @@ describe('richHistory', () => {
describe('filterQueries', () => {
it('should filter out queries based on data source filter', () => {
const filteredQueries = filterAndSortQueries(
mock.storedHistory,
storedHistory,
SortOrder.Ascending,
['not provided data source'],
''
@@ -228,25 +187,15 @@ describe('richHistory', () => {
expect(filteredQueries).toHaveLength(0);
});
it('should keep queries based on data source filter', () => {
const filteredQueries = filterAndSortQueries(
mock.storedHistory,
SortOrder.Ascending,
['datasource history name'],
''
);
const filteredQueries = filterAndSortQueries(storedHistory, SortOrder.Ascending, ['datasource history name'], '');
expect(filteredQueries).toHaveLength(1);
});
it('should filter out all queries based on search filter', () => {
const filteredQueries = filterAndSortQueries(
mock.storedHistory,
SortOrder.Ascending,
[],
'i do not exist in query'
);
const filteredQueries = filterAndSortQueries(storedHistory, SortOrder.Ascending, [], 'i do not exist in query');
expect(filteredQueries).toHaveLength(0);
});
it('should include queries based on search filter', () => {
const filteredQueries = filterAndSortQueries(mock.storedHistory, SortOrder.Ascending, [], 'query1');
const filteredQueries = filterAndSortQueries(storedHistory, SortOrder.Ascending, [], 'query1');
expect(filteredQueries).toHaveLength(1);
});
});
@@ -254,13 +203,13 @@ 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].createdAt = 1 + -1 * dateTime().utcOffset() * 60 * 1000;
const heading = createQueryHeading(mock.storedHistory[0], SortOrder.Ascending);
storedHistory[0].createdAt = 1 + -1 * dateTime().utcOffset() * 60 * 1000;
const heading = createQueryHeading(storedHistory[0], SortOrder.Ascending);
expect(heading).toEqual('January 1');
});
it('should correctly create heading for queries when sort order is datasourceAZ ', () => {
const heading = createQueryHeading(mock.storedHistory[0], SortOrder.DatasourceAZ);
expect(heading).toEqual(mock.storedHistory[0].datasourceName);
const heading = createQueryHeading(storedHistory[0], SortOrder.DatasourceAZ);
expect(heading).toEqual(storedHistory[0].datasourceName);
});
});
});

View File

@@ -33,7 +33,6 @@ export { SortOrder };
*/
export async function addToRichHistory(
richHistory: RichHistoryQuery[],
datasourceUid: string,
datasourceName: string | null,
queries: DataQuery[],
@@ -41,7 +40,7 @@ export async function addToRichHistory(
comment: string | null,
showQuotaExceededError: boolean,
showLimitExceededWarning: boolean
): Promise<{ richHistory: RichHistoryQuery[]; richHistoryStorageFull?: boolean; limitExceeded?: boolean }> {
): Promise<{ richHistoryStorageFull?: boolean; limitExceeded?: boolean }> {
/* Save only queries, that are not falsy (e.g. empty object, null, ...) */
const newQueriesToSave: DataQuery[] = queries && queries.filter((query) => notEmptyQuery(query));
@@ -49,7 +48,6 @@ export async function addToRichHistory(
let richHistoryStorageFull = false;
let limitExceeded = false;
let warning: RichHistoryStorageWarningDetails | undefined;
let newRichHistory: RichHistoryQuery;
try {
const result = await getRichHistoryStorage().addToRichHistory({
@@ -60,7 +58,6 @@ export async function addToRichHistory(
comment: comment ?? '',
});
warning = result.warning;
newRichHistory = result.richHistoryQuery;
} catch (error) {
if (error.name === RichHistoryServiceError.StorageFull) {
richHistoryStorageFull = true;
@@ -69,7 +66,7 @@ export async function addToRichHistory(
dispatch(notifyApp(createErrorNotification('Rich History update failed', error.message)));
}
// Saving failed. Do not add new entry.
return { richHistory, richHistoryStorageFull, limitExceeded };
return { richHistoryStorageFull, limitExceeded };
}
// Limit exceeded but new entry was added. Notify that old entries have been removed.
@@ -78,12 +75,11 @@ export async function addToRichHistory(
showLimitExceededWarning && dispatch(notifyApp(createWarningNotification(warning.message)));
}
// Saving successful - add new entry.
return { richHistory: [newRichHistory, ...richHistory], richHistoryStorageFull, limitExceeded };
return { richHistoryStorageFull, limitExceeded };
}
// Nothing to save
return { richHistory };
// Nothing to change
return {};
}
export async function getRichHistory(): Promise<RichHistoryQuery[]> {
@@ -94,41 +90,31 @@ export async function deleteAllFromRichHistory(): Promise<void> {
return getRichHistoryStorage().deleteAll();
}
export async function updateStarredInRichHistory(richHistory: RichHistoryQuery[], id: string, starred: boolean) {
export async function updateStarredInRichHistory(id: string, starred: boolean) {
try {
const updatedQuery = await getRichHistoryStorage().updateStarred(id, starred);
return richHistory.map((query) => (query.id === id ? updatedQuery : query));
return await getRichHistoryStorage().updateStarred(id, starred);
} catch (error) {
dispatch(notifyApp(createErrorNotification('Saving rich history failed', error.message)));
return richHistory;
return undefined;
}
}
export async function updateCommentInRichHistory(
richHistory: RichHistoryQuery[],
id: string,
newComment: string | undefined
) {
export async function updateCommentInRichHistory(id: string, newComment: string | undefined) {
try {
const updatedQuery = await getRichHistoryStorage().updateComment(id, newComment);
return richHistory.map((query) => (query.id === id ? updatedQuery : query));
return await getRichHistoryStorage().updateComment(id, newComment);
} catch (error) {
dispatch(notifyApp(createErrorNotification('Saving rich history failed', error.message)));
return richHistory;
return undefined;
}
}
export async function deleteQueryInRichHistory(
richHistory: RichHistoryQuery[],
id: string
): Promise<RichHistoryQuery[]> {
const updatedHistory = richHistory.filter((query) => query.id !== id);
export async function deleteQueryInRichHistory(id: string) {
try {
await getRichHistoryStorage().deleteRichHistory(id);
return updatedHistory;
return id;
} catch (error) {
dispatch(notifyApp(createErrorNotification('Saving rich history failed', error.message)));
return richHistory;
return undefined;
}
}

View File

@@ -81,6 +81,7 @@ class UnConnectedExploreToolbar extends PureComponent<Props> {
return (
<div ref={topOfExploreViewRef}>
<PageToolbar
aria-label="Explore toolbar"
title={exploreId === ExploreId.left ? 'Explore' : undefined}
pageIcon={exploreId === ExploreId.left ? 'compass' : undefined}
leftItems={[

View File

@@ -44,12 +44,12 @@ function setup(queries: DataQuery[]) {
const initialState: ExploreState = {
left: {
...leftState,
richHistory: [],
datasourceInstance: datasources['someDs-uid'],
queries,
},
syncedTimes: false,
right: undefined,
richHistory: [],
richHistoryStorageFull: false,
richHistoryLimitExceededWarningShown: false,
};

View File

@@ -1,6 +1,5 @@
import React from 'react';
import { mount } from 'enzyme';
import { Resizable } from 're-resizable';
import { render } from '@testing-library/react';
import { ExploreId } from '../../../types/explore';
import { RichHistoryContainer, Props } from './RichHistoryContainer';
@@ -16,26 +15,33 @@ const setup = (propOverrides?: Partial<Props>) => {
richHistory: [],
firstTab: Tabs.RichHistory,
deleteRichHistory: jest.fn(),
loadRichHistory: jest.fn(),
onClose: jest.fn(),
};
Object.assign(props, propOverrides);
const wrapper = mount(<RichHistoryContainer {...props} />);
return wrapper;
return render(<RichHistoryContainer {...props} />);
};
describe('RichHistoryContainer', () => {
it('should render reseizable component', () => {
const wrapper = setup();
expect(wrapper.find(Resizable)).toHaveLength(1);
});
it('should render component with correct width', () => {
const wrapper = setup();
expect(wrapper.getDOMNode().getAttribute('style')).toContain('width: 531.5px');
const { container } = setup();
expect(container.firstElementChild!.getAttribute('style')).toContain('width: 531.5px');
});
it('should render component with correct height', () => {
const wrapper = setup();
expect(wrapper.getDOMNode().getAttribute('style')).toContain('height: 400px');
const { container } = setup();
expect(container.firstElementChild!.getAttribute('style')).toContain('height: 400px');
});
it('should re-request rich history every time the component is mounted', () => {
const loadRichHistory = jest.fn();
const { unmount } = setup({ loadRichHistory });
expect(loadRichHistory).toBeCalledTimes(1);
unmount();
expect(loadRichHistory).toBeCalledTimes(1);
setup({ loadRichHistory });
expect(loadRichHistory).toBeCalledTimes(2);
});
});

View File

@@ -1,5 +1,5 @@
// Libraries
import React, { useState } from 'react';
import React, { useEffect, useState } from 'react';
import { connect, ConnectedProps } from 'react-redux';
// Services & Utils
@@ -14,7 +14,7 @@ import { ExploreId } from 'app/types/explore';
import { RichHistory, Tabs } from './RichHistory';
//Actions
import { deleteRichHistory } from '../state/history';
import { deleteRichHistory, loadRichHistory } from '../state/history';
import { ExploreDrawer } from '../ExploreDrawer';
function mapStateToProps(state: StoreState, { exploreId }: { exploreId: ExploreId }) {
@@ -25,7 +25,7 @@ function mapStateToProps(state: StoreState, { exploreId }: { exploreId: ExploreI
const firstTab = store.getBool(RICH_HISTORY_SETTING_KEYS.starredTabAsFirstTab, false)
? Tabs.Starred
: Tabs.RichHistory;
const { richHistory } = explore;
const { richHistory } = item;
return {
richHistory,
firstTab,
@@ -34,6 +34,7 @@ function mapStateToProps(state: StoreState, { exploreId }: { exploreId: ExploreI
}
const mapDispatchToProps = {
loadRichHistory,
deleteRichHistory,
};
@@ -49,7 +50,20 @@ export type Props = ConnectedProps<typeof connector> & OwnProps;
export function RichHistoryContainer(props: Props) {
const [height, setHeight] = useState(400);
const { richHistory, width, firstTab, activeDatasourceInstance, exploreId, deleteRichHistory, onClose } = props;
const {
richHistory,
width,
firstTab,
activeDatasourceInstance,
exploreId,
deleteRichHistory,
loadRichHistory,
onClose,
} = props;
useEffect(() => {
loadRichHistory(exploreId);
}, [loadRichHistory, exploreId]);
return (
<ExploreDrawer

View File

@@ -66,7 +66,7 @@ describe('Wrapper', () => {
range: { from: 'now-1h', to: 'now' },
}),
};
const { datasources, store } = setupExplore({ urlParams });
const { datasources } = setupExplore({ urlParams });
(datasources.loki.query as Mock).mockReturnValueOnce(makeLogsQueryResponse());
// Make sure we render the logs panel
@@ -84,11 +84,6 @@ describe('Wrapper', () => {
...urlParams,
});
expect(store.getState().explore.richHistory[0]).toMatchObject({
datasourceName: 'loki',
queries: [{ expr: '{ label="value"}', refId: 'A' }],
});
// We called the data source query method once
expect(datasources.loki.query).toBeCalledTimes(1);
expect((datasources.loki.query as Mock).mock.calls[0][0]).toMatchObject({

View File

@@ -3,7 +3,6 @@ import { connect, ConnectedProps } from 'react-redux';
import { ExploreId, ExploreQueryParams } from 'app/types/explore';
import { ErrorBoundaryAlert } from '@grafana/ui';
import { lastSavedUrl, resetExploreAction, richHistoryUpdatedAction } from './state/main';
import { getRichHistory } from '../../core/utils/richHistory';
import { ExplorePaneContainer } from './ExplorePaneContainer';
import { GrafanaRouteComponentProps } from 'app/core/navigation/types';
import { Branding } from '../../core/components/Branding/Branding';
@@ -52,10 +51,6 @@ class WrapperUnconnected extends PureComponent<Props> {
if (searchParams.from || searchParams.to) {
locationService.partial({ from: undefined, to: undefined }, true);
}
getRichHistory().then((richHistory) => {
this.props.richHistoryUpdatedAction({ richHistory });
});
}
componentDidUpdate(prevProps: Props) {

View File

@@ -1,7 +1,32 @@
import { screen } from '@testing-library/react';
import { waitFor } from '@testing-library/react';
import { ExploreId } from '../../../../types';
import { withinExplore } from './setup';
export const assertQueryHistoryExists = (query: string) => {
expect(screen.getByText('1 queries')).toBeInTheDocument();
const queryItem = screen.getByLabelText('Query text');
export const assertQueryHistoryExists = (query: string, exploreId: ExploreId = ExploreId.left) => {
const selector = withinExplore(exploreId);
expect(selector.getByText('1 queries')).toBeInTheDocument();
const queryItem = selector.getByLabelText('Query text');
expect(queryItem).toHaveTextContent(query);
};
export const assertQueryHistory = async (expectedQueryTexts: string[], exploreId: ExploreId = ExploreId.left) => {
const selector = withinExplore(exploreId);
await waitFor(() => {
expect(selector.getByText(`${expectedQueryTexts.length} queries`)).toBeInTheDocument();
const queryTexts = selector.getAllByLabelText('Query text');
expectedQueryTexts.forEach((expectedQueryText, queryIndex) => {
expect(queryTexts[queryIndex]).toHaveTextContent(expectedQueryText);
});
});
};
export const assertQueryHistoryIsStarred = async (expectedStars: boolean[], exploreId: ExploreId = ExploreId.left) => {
const selector = withinExplore(exploreId);
const starButtons = selector.getAllByRole('button', { name: /Star query|Unstar query/ });
await waitFor(() =>
expectedStars.forEach((starred, queryIndex) => {
expect(starButtons[queryIndex]).toHaveAccessibleName(starred ? 'Unstar query' : 'Star query');
})
);
};

View File

@@ -1,6 +1,8 @@
import { selectors } from '@grafana/e2e-selectors';
import userEvent from '@testing-library/user-event';
import { fireEvent, screen } from '@testing-library/react';
import { fireEvent, screen, within } from '@testing-library/react';
import { ExploreId } from '../../../../types';
import { withinExplore } from './setup';
export const changeDatasource = async (name: string) => {
const datasourcePicker = (await screen.findByLabelText(selectors.components.DataSourcePicker.container)).children[0];
@@ -9,20 +11,38 @@ export const changeDatasource = async (name: string) => {
fireEvent.click(option);
};
export const inputQuery = (query: string) => {
const input = screen.getByRole('textbox', { name: 'query' });
export const inputQuery = (query: string, exploreId: ExploreId = ExploreId.left) => {
const input = withinExplore(exploreId).getByRole('textbox', { name: 'query' });
userEvent.clear(input);
userEvent.type(input, query);
};
export const runQuery = () => {
const button = screen.getByRole('button', { name: /run query/i });
export const runQuery = (exploreId: ExploreId = ExploreId.left) => {
const explore = withinExplore(exploreId);
const toolbar = within(explore.getByLabelText('Explore toolbar'));
const button = toolbar.getByRole('button', { name: /run query/i });
userEvent.click(button);
};
export const openQueryHistory = async () => {
const button = screen.getByRole('button', { name: 'Rich history button' });
export const openQueryHistory = async (exploreId: ExploreId = ExploreId.left) => {
const selector = withinExplore(exploreId);
const button = selector.getByRole('button', { name: 'Rich history button' });
userEvent.click(button);
expect(
await screen.findByText('The history is local to your browser and is not shared with others.')
await selector.findByText('The history is local to your browser and is not shared with others.')
).toBeInTheDocument();
};
export const starQueryHistory = (queryIndex: number, exploreId: ExploreId = ExploreId.left) => {
invokeAction(queryIndex, 'Star query', exploreId);
};
export const deleteQueryHistory = (queryIndex: number, exploreId: ExploreId = ExploreId.left) => {
invokeAction(queryIndex, 'Delete query', exploreId);
};
const invokeAction = (queryIndex: number, actionAccessibleName: string, exploreId: ExploreId) => {
const selector = withinExplore(exploreId);
const buttons = selector.getAllByRole('button', { name: actionAccessibleName });
userEvent.click(buttons[queryIndex]);
};

View File

@@ -1,5 +1,6 @@
import React from 'react';
import { render, screen } from '@testing-library/react';
import { within } from '@testing-library/dom';
import { EnhancedStore } from '@reduxjs/toolkit';
import { Provider } from 'react-redux';
import { Route, Router } from 'react-router-dom';
@@ -16,6 +17,7 @@ import { initialUserState } from '../../../profile/state/reducers';
import { LokiDatasource } from '../../../../plugins/datasource/loki/datasource';
import { LokiQuery } from '../../../../plugins/datasource/loki/types';
import { ExploreId } from '../../../../types';
type DatasourceSetup = { settings: DataSourceInstanceSettings; api: DataSourceApi };
@@ -31,6 +33,7 @@ export function setupExplore(options?: SetupOptions): {
datasources: { [name: string]: DataSourceApi };
store: EnhancedStore;
unmount: () => void;
container: HTMLElement;
} {
// Clear this up otherwise it persists data source selection
// TODO: probably add test for that too
@@ -87,7 +90,7 @@ export function setupExplore(options?: SetupOptions): {
const route = { component: Wrapper };
const { unmount } = render(
const { unmount, container } = render(
<Provider store={store}>
<Router history={locationService.getHistory()}>
<Route path="/explore" exact render={(props) => <GrafanaRoute {...props} route={route as any} />} />
@@ -95,7 +98,7 @@ export function setupExplore(options?: SetupOptions): {
</Provider>
);
return { datasources: fromPairs(dsSettings.map((d) => [d.api.name, d.api])), store, unmount };
return { datasources: fromPairs(dsSettings.map((d) => [d.api.name, d.api])), store, unmount, container };
}
function makeDatasourceSetup({ name = 'loki', id = 1 }: { name?: string; id?: number } = {}): DatasourceSetup {
@@ -137,16 +140,21 @@ function makeDatasourceSetup({ name = 'loki', id = 1 }: { name?: string; id?: nu
name: name,
uid: name,
query: jest.fn(),
getRef: jest.fn(),
getRef: jest.fn().mockReturnValue(name),
meta,
} as any,
};
}
export const waitForExplore = async () => {
await screen.findAllByText(/Editor/i);
export const waitForExplore = async (exploreId: ExploreId = ExploreId.left) => {
return await withinExplore(exploreId).findByText(/Editor/i);
};
export const tearDown = () => {
window.localStorage.clear();
};
export const withinExplore = (exploreId: ExploreId) => {
const container = screen.getAllByTestId('data-testid Explore');
return within(container[exploreId === ExploreId.left ? 0 : 1]);
};

View File

@@ -1,8 +1,11 @@
import React from 'react';
import { serializeStateToUrlParam } from '@grafana/data';
import { setupExplore, tearDown, waitForExplore } from './helper/setup';
import { inputQuery, openQueryHistory, runQuery } from './helper/interactions';
import { assertQueryHistoryExists } from './helper/assert';
import { deleteQueryHistory, inputQuery, openQueryHistory, runQuery, starQueryHistory } from './helper/interactions';
import { assertQueryHistory, assertQueryHistoryExists, assertQueryHistoryIsStarred } from './helper/assert';
import { makeLogsQueryResponse } from './helper/query';
import { ExploreId } from '../../../types';
import { silenceConsoleOutput } from '../../../../test/core/utils/silenceConsoleOutput';
jest.mock('react-virtualized-auto-sizer', () => {
return {
@@ -17,6 +20,8 @@ describe('Explore: Query History', () => {
const USER_INPUT = 'my query';
const RAW_QUERY = `{"expr":"${USER_INPUT}"}`;
silenceConsoleOutput();
afterEach(() => {
tearDown();
});
@@ -33,7 +38,7 @@ describe('Explore: Query History', () => {
await openQueryHistory();
// the query that was run is in query history
assertQueryHistoryExists(RAW_QUERY);
await assertQueryHistoryExists(RAW_QUERY);
// when Explore is opened again
unmount();
@@ -42,6 +47,60 @@ describe('Explore: Query History', () => {
// previously added query is in query history
await openQueryHistory();
assertQueryHistoryExists(RAW_QUERY);
await assertQueryHistoryExists(RAW_QUERY);
});
it('adds recently added query if the query history panel is already open', async () => {
const urlParams = {
left: serializeStateToUrlParam({
datasource: 'loki',
queries: [{ refId: 'A', expr: 'query #1' }],
range: { from: 'now-1h', to: 'now' },
}),
};
const { datasources } = setupExplore({ urlParams });
(datasources.loki.query as jest.Mock).mockReturnValueOnce(makeLogsQueryResponse());
await waitForExplore();
await openQueryHistory();
inputQuery('query #2');
runQuery();
await assertQueryHistory(['{"expr":"query #2"}', '{"expr":"query #1"}']);
});
it('updates the state in both Explore panes', async () => {
const urlParams = {
left: serializeStateToUrlParam({
datasource: 'loki',
queries: [{ refId: 'A', expr: 'query #1' }],
range: { from: 'now-1h', to: 'now' },
}),
right: serializeStateToUrlParam({
datasource: 'loki',
queries: [{ refId: 'A', expr: 'query #2' }],
range: { from: 'now-1h', to: 'now' },
}),
};
const { datasources } = setupExplore({ urlParams });
(datasources.loki.query as jest.Mock).mockReturnValue(makeLogsQueryResponse());
await waitForExplore();
await waitForExplore(ExploreId.right);
// queries in history
await openQueryHistory(ExploreId.left);
await assertQueryHistory(['{"expr":"query #2"}', '{"expr":"query #1"}'], ExploreId.left);
await openQueryHistory(ExploreId.right);
await assertQueryHistory(['{"expr":"query #2"}', '{"expr":"query #1"}'], ExploreId.right);
// star one one query
starQueryHistory(1, ExploreId.left);
await assertQueryHistoryIsStarred([false, true], ExploreId.left);
await assertQueryHistoryIsStarred([false, true], ExploreId.right);
deleteQueryHistory(0, ExploreId.left);
await assertQueryHistory(['{"expr":"query #1"}'], ExploreId.left);
await assertQueryHistory(['{"expr":"query #1"}'], ExploreId.right);
});
});

View File

@@ -36,7 +36,6 @@ import {
import { ThunkResult } from 'app/types';
import { getFiscalYearStartMonth, getTimeZone } from 'app/features/profile/state/selectors';
import { getDataSourceSrv } from '@grafana/runtime';
import { getRichHistory } from '../../../core/utils/richHistory';
import { richHistoryUpdatedAction, stateSave } from './main';
import { keybindingSrv } from 'app/core/services/keybindingSrv';
@@ -181,9 +180,6 @@ export function initializeExplore(
// user to go back to previous url.
dispatch(runQueries(exploreId, { replaceUrl: true }));
}
const richHistory = await getRichHistory();
dispatch(richHistoryUpdatedAction({ richHistory }));
};
}
@@ -259,6 +255,13 @@ export const paneReducer = (state: ExploreItemState = makeExplorePaneState(), ac
state = timeReducer(state, action);
state = historyReducer(state, action);
if (richHistoryUpdatedAction.match(action)) {
return {
...state,
richHistory: action.payload.richHistory,
};
}
if (changeSizeAction.match(action)) {
const containerWidth = action.payload.width;
return { ...state, containerWidth };

View File

@@ -38,6 +38,7 @@ export const createDefaultInitialState = () => {
value: 0,
},
cache: [],
richHistory: [],
},
},
};

View File

@@ -1,12 +1,14 @@
import {
addToRichHistory,
deleteAllFromRichHistory,
deleteQueryInRichHistory,
getRichHistory,
updateCommentInRichHistory,
updateStarredInRichHistory,
} from 'app/core/utils/richHistory';
import { ExploreId, ExploreItemState, ThunkResult } from 'app/types';
import { richHistoryUpdatedAction } from './main';
import { HistoryItem } from '@grafana/data';
import { ExploreId, ExploreItemState, ExploreState, RichHistoryQuery, ThunkResult } from 'app/types';
import { richHistoryLimitExceededAction, richHistoryStorageFullAction, richHistoryUpdatedAction } from './main';
import { DataQuery, HistoryItem } from '@grafana/data';
import { AnyAction, createAction } from '@reduxjs/toolkit';
//
@@ -23,31 +25,89 @@ export const historyUpdatedAction = createAction<HistoryUpdatedPayload>('explore
// Action creators
//
type SyncHistoryUpdatesOptions = {
updatedQuery?: RichHistoryQuery;
deletedId?: string;
};
/**
* Updates current state in both Explore panes after changing or deleting a query history item
*/
const updateRichHistoryState = ({ updatedQuery, deletedId }: SyncHistoryUpdatesOptions): ThunkResult<void> => {
return async (dispatch, getState) => {
forEachExplorePane(getState().explore, (item, exploreId) => {
const newRichHistory = item.richHistory
// update
.map((query) => (query.id === updatedQuery?.id ? updatedQuery : query))
// or remove
.filter((query) => query.id !== deletedId);
dispatch(richHistoryUpdatedAction({ richHistory: newRichHistory, exploreId }));
});
};
};
const forEachExplorePane = (state: ExploreState, callback: (item: ExploreItemState, exploreId: ExploreId) => void) => {
callback(state.left, ExploreId.left);
state.right && callback(state.right, ExploreId.right);
};
export const addHistoryItem = (
datasourceUid: string,
datasourceName: string,
queries: DataQuery[]
): ThunkResult<void> => {
return async (dispatch, getState) => {
const { richHistoryStorageFull, limitExceeded } = await addToRichHistory(
datasourceUid,
datasourceName,
queries,
false,
'',
!getState().explore.richHistoryStorageFull,
!getState().explore.richHistoryLimitExceededWarningShown
);
if (richHistoryStorageFull) {
dispatch(richHistoryStorageFullAction());
}
if (limitExceeded) {
dispatch(richHistoryLimitExceededAction());
}
};
};
export const starHistoryItem = (id: string, starred: boolean): ThunkResult<void> => {
return async (dispatch, getState) => {
const nextRichHistory = await updateStarredInRichHistory(getState().explore.richHistory, id, starred);
dispatch(richHistoryUpdatedAction({ richHistory: nextRichHistory }));
const updatedQuery = await updateStarredInRichHistory(id, starred);
dispatch(updateRichHistoryState({ updatedQuery }));
};
};
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 }));
return async (dispatch) => {
const updatedQuery = await updateCommentInRichHistory(id, comment);
dispatch(updateRichHistoryState({ updatedQuery }));
};
};
export const deleteHistoryItem = (id: string): ThunkResult<void> => {
return async (dispatch, getState) => {
const nextRichHistory = await deleteQueryInRichHistory(getState().explore.richHistory, id);
dispatch(richHistoryUpdatedAction({ richHistory: nextRichHistory }));
return async (dispatch) => {
const deletedId = await deleteQueryInRichHistory(id);
dispatch(updateRichHistoryState({ deletedId }));
};
};
export const deleteRichHistory = (): ThunkResult<void> => {
return async (dispatch) => {
await deleteAllFromRichHistory();
dispatch(richHistoryUpdatedAction({ richHistory: [] }));
dispatch(richHistoryUpdatedAction({ richHistory: [], exploreId: ExploreId.left }));
dispatch(richHistoryUpdatedAction({ richHistory: [], exploreId: ExploreId.right }));
};
};
export const loadRichHistory = (exploreId: ExploreId): ThunkResult<void> => {
return async (dispatch) => {
const richHistory = await getRichHistory();
dispatch(richHistoryUpdatedAction({ richHistory, exploreId }));
};
};

View File

@@ -2,7 +2,7 @@ import { AnyAction } from 'redux';
import { DataSourceSrv, getDataSourceSrv, locationService } from '@grafana/runtime';
import { ExploreUrlState, serializeStateToUrlParam, SplitOpen, UrlQueryMap } from '@grafana/data';
import { GetExploreUrlArguments, stopQueryState } from 'app/core/utils/explore';
import { ExploreId, ExploreItemState, ExploreState } from 'app/types/explore';
import { ExploreId, ExploreItemState, ExploreState, RichHistoryQuery } from 'app/types/explore';
import { paneReducer } from './explorePane';
import { createAction } from '@reduxjs/toolkit';
import { getUrlStateFromPaneState, makeExplorePaneState } from './utils';
@@ -19,7 +19,8 @@ export interface SyncTimesPayload {
}
export const syncTimesAction = createAction<SyncTimesPayload>('explore/syncTimes');
export const richHistoryUpdatedAction = createAction<any>('explore/richHistoryUpdated');
export const richHistoryUpdatedAction =
createAction<{ richHistory: RichHistoryQuery[]; exploreId: ExploreId }>('explore/richHistoryUpdated');
export const richHistoryStorageFullAction = createAction('explore/richHistoryStorageFullAction');
export const richHistoryLimitExceededAction = createAction('explore/richHistoryLimitExceededAction');
@@ -157,7 +158,6 @@ export const initialExploreState: ExploreState = {
syncedTimes: false,
left: initialExploreItemState,
right: undefined,
richHistory: [],
richHistoryStorageFull: false,
richHistoryLimitExceededWarningShown: false,
};
@@ -207,13 +207,6 @@ export const exploreReducer = (state = initialExploreState, action: AnyAction):
return { ...state, syncedTimes: action.payload.syncedTimes };
}
if (richHistoryUpdatedAction.match(action)) {
return {
...state,
richHistory: action.payload.richHistory,
};
}
if (richHistoryStorageFullAction.match(action)) {
return {
...state,

View File

@@ -27,7 +27,6 @@ import {
stopQueryState,
updateHistory,
} from 'app/core/utils/explore';
import { addToRichHistory } from 'app/core/utils/richHistory';
import { ExploreItemState, ExplorePanelData, ThunkDispatch, ThunkResult } from 'app/types';
import { ExploreId, ExploreState, QueryOptions } from 'app/types/explore';
import { getTimeZone } from 'app/features/profile/state/selectors';
@@ -36,15 +35,10 @@ import { notifyApp } from '../../../core/actions';
import { runRequest } from '../../query/state/runRequest';
import { decorateData } from '../utils/decorators';
import { createErrorNotification } from '../../../core/copy/appNotification';
import {
richHistoryStorageFullAction,
richHistoryLimitExceededAction,
richHistoryUpdatedAction,
stateSave,
} from './main';
import { stateSave } from './main';
import { AnyAction, createAction, PayloadAction } from '@reduxjs/toolkit';
import { updateTime } from './time';
import { historyUpdatedAction } from './history';
import { addHistoryItem, historyUpdatedAction, loadRichHistory } from './history';
import { createCacheKey, getResultsFromCache } from './utils';
import deepEqual from 'fast-deep-equal';
@@ -320,29 +314,14 @@ async function handleHistory(
) {
const datasourceId = datasource.meta.id;
const nextHistory = updateHistory(history, datasourceId, queries);
const {
richHistory: nextRichHistory,
richHistoryStorageFull,
limitExceeded,
} = await addToRichHistory(
state.richHistory || [],
datasource.uid,
datasource.name,
queries,
false,
'',
!state.richHistoryStorageFull,
!state.richHistoryLimitExceededWarningShown
);
dispatch(historyUpdatedAction({ exploreId, history: nextHistory }));
dispatch(richHistoryUpdatedAction({ richHistory: nextRichHistory }));
if (richHistoryStorageFull) {
dispatch(richHistoryStorageFullAction());
}
if (limitExceeded) {
dispatch(richHistoryLimitExceededAction());
}
dispatch(addHistoryItem(datasource.uid, datasource.name, queries));
// Because filtering happens in the backend we cannot add a new entry without checking if it matches currently
// used filters. Instead, we refresh the query history list.
// TODO: run only if Query History list is opened (#47252)
dispatch(loadRichHistory(exploreId));
}
/**

View File

@@ -61,6 +61,7 @@ export const makeExplorePaneState = (): ExploreItemState => ({
logsResult: null,
eventBridge: null as unknown as EventBusExtended,
cache: [],
richHistory: [],
logsVolumeDataProvider: undefined,
logsVolumeData: undefined,
graphStyle: loadGraphStyle(),

View File

@@ -42,10 +42,6 @@ export interface ExploreState {
* Explore state of the right area in split view.
*/
right?: ExploreItemState;
/**
* History of all queries
*/
richHistory: RichHistoryQuery[];
/**
* True if local storage quota was exceeded when a rich history item was added. This is to prevent showing
@@ -153,6 +149,11 @@ export interface ExploreItemState {
showTrace?: boolean;
showNodeGraph?: boolean;
/**
* History of all queries
*/
richHistory: RichHistoryQuery[];
/**
* We are using caching to store query responses of queries run from logs navigation.
* In logs navigation, we do pagination and we don't want our users to unnecessarily run the same queries that they've run just moments before.