Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[query assist] use badge to show agent errors #7998

Merged
merged 22 commits into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
6545d48
enable query assist for index patterns without MDS
joshuali925 Sep 4, 2024
4a5fe8e
add agent error parsing utils
joshuali925 Sep 4, 2024
6c07257
update ml-commons response schema processing
joshuali925 Sep 4, 2024
043f83b
use badge to show query assist error if possible
joshuali925 Sep 4, 2024
84d992a
avoid enter key triggering warning badge instead of submit button
joshuali925 Sep 4, 2024
f5140ea
address comments
joshuali925 Sep 5, 2024
898f072
update storage for upstream change
joshuali925 Sep 5, 2024
e3af9ac
add unit tests
joshuali925 Sep 5, 2024
40749a1
add i18n for badge ariaLabel
joshuali925 Sep 9, 2024
69e4545
change indices to indexes in unit tests
joshuali925 Sep 9, 2024
000bd30
Merge remote-tracking branch 'origin/main' into qa-2.17
joshuali925 Sep 9, 2024
d02c5a4
update sidebar
joshuali925 Sep 9, 2024
09354cf
add i18n for input placeholder
joshuali925 Sep 9, 2024
46ada24
move inline style to scss
joshuali925 Sep 9, 2024
8b8fb1d
change warning to error in query assist badge
joshuali925 Sep 9, 2024
f379ee1
calculate badge width at runtime
joshuali925 Sep 10, 2024
0de06f6
refactor: append badge and hide it when input is focused
joshuali925 Sep 10, 2024
48c42fc
avoid `display: none` when badge is focused
joshuali925 Sep 10, 2024
b83fdec
update unit tests
joshuali925 Sep 10, 2024
d948c9d
change unicode to eui icon
joshuali925 Sep 11, 2024
711fe18
truncate text in input when unfocused
joshuali925 Sep 11, 2024
e814ad2
change badge color to danger
joshuali925 Sep 11, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/_sidebar.md
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@
- [Opensearch dashboards.release notes 2.14.0](../release-notes/opensearch-dashboards.release-notes-2.14.0.md)
- [Opensearch dashboards.release notes 2.15.0](../release-notes/opensearch-dashboards.release-notes-2.15.0.md)
- [Opensearch dashboards.release notes 2.16.0](../release-notes/opensearch-dashboards.release-notes-2.16.0.md)
- [Opensearch dashboards.release notes 2.17.0](../release-notes/opensearch-dashboards.release-notes-2.17.0.md)
- [Opensearch dashboards.release notes 2.2.0](../release-notes/opensearch-dashboards.release-notes-2.2.0.md)
- [Opensearch dashboards.release notes 2.2.1](../release-notes/opensearch-dashboards.release-notes-2.2.1.md)
- [Opensearch dashboards.release notes 2.3.0](../release-notes/opensearch-dashboards.release-notes-2.3.0.md)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
* under the License.
*/

import { of } from 'rxjs';
import { QueryStringContract } from '.';
import { Query, Dataset } from '../../../common';
import { datasetServiceMock } from './dataset_service/dataset_service.mock';
Expand All @@ -44,10 +45,10 @@ const createSetupContractMock = (isEnhancementsEnabled: boolean = false) => {
};

const queryStringManagerMock: jest.Mocked<QueryStringContract> = {
getQuery: jest.fn(),
getQuery: jest.fn().mockReturnValue(defaultQuery),
setQuery: jest.fn(),
getUpdates$: jest.fn(),
getDefaultQuery: jest.fn(),
getUpdates$: jest.fn().mockReturnValue(of(defaultQuery)),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice.

but i think one of the tests will fail now. so might have to just update the test to expect this now.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

weird i ran yarn test:jest before pushing, they passed locally

getDefaultQuery: jest.fn().mockReturnValue(defaultQuery),
formatQuery: jest.fn(),
clearQuery: jest.fn(),
addToQueryHistory: jest.fn(),
Expand Down
6 changes: 3 additions & 3 deletions src/plugins/query_enhancements/public/plugin.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
*/

import { CoreSetup, CoreStart, Plugin, PluginInitializerContext } from '../../../core/public';
import { IStorageWrapper, Storage } from '../../opensearch_dashboards_utils/public';
import { ConfigSchema } from '../common/config';
import { setData, setStorage } from './services';
import { createQueryAssistExtension } from './query_assist';
Expand All @@ -19,6 +18,7 @@ import { LanguageConfig, Query } from '../../data/public';
import { s3TypeConfig } from './datasets';
import { createEditor, DefaultInput, SingleLineInput } from '../../data/public';
import { QueryLanguageReference } from './query_editor/query_language_reference';
import { DataStorage } from '../../data/common';

export class QueryEnhancementsPlugin
implements
Expand All @@ -28,12 +28,12 @@ export class QueryEnhancementsPlugin
QueryEnhancementsPluginSetupDependencies,
QueryEnhancementsPluginStartDependencies
> {
private readonly storage: IStorageWrapper;
private readonly storage: DataStorage;
private readonly config: ConfigSchema;

constructor(initializerContext: PluginInitializerContext) {
this.config = initializerContext.config.get<ConfigSchema>();
this.storage = new Storage(window.localStorage);
this.storage = new DataStorage(window.localStorage, 'discover.queryAssist.');
}

public setup(
Expand Down
21 changes: 21 additions & 0 deletions src/plugins/query_enhancements/public/query_assist/_index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,27 @@
&.queryAssist__callout {
margin-top: $euiSizeXS;
}

.queryAssist__popoverWrapper {
display: flex;
align-items: center;
padding-right: $euiSizeS;
}

// Hide the error badge when the input is focused
.queryAssist__inputWrapper:has(.queryAssist__input:focus) .queryAssist__popoverWrapper {
display: none;
}
}

.queryAssist__popoverText {
padding: 0 $euiSizeM;
}

#queryAssistErrorTitle::before {
content: "⚠";
position: absolute;
left: $euiSizeS;
}

.osdQueryEditorExtensionComponent__query-assist {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@
*/

import { render } from '@testing-library/react';
import React, { ComponentProps } from 'react';
import React, { ComponentProps, PropsWithChildren } from 'react';
import { IntlProvider } from 'react-intl';
import { QueryAssistCallOut } from './call_outs';

type Props = ComponentProps<typeof QueryAssistCallOut>;

const IntlWrapper = ({ children }: { children: unknown }) => (
const IntlWrapper = ({ children }: PropsWithChildren<unknown>) => (
<IntlProvider locale="en">{children}</IntlProvider>
);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,218 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

import { fireEvent, render, screen, waitFor } from '@testing-library/react';
import React, { ComponentProps, PropsWithChildren } from 'react';
import { IntlProvider } from 'react-intl';
import { of } from 'rxjs';
import { QueryAssistBar } from '.';
import { notificationServiceMock, uiSettingsServiceMock } from '../../../../../core/public/mocks';
import { DataStorage } from '../../../../data/common';
import { QueryEditorExtensionDependencies, QueryStringContract } from '../../../../data/public';
import { dataPluginMock } from '../../../../data/public/mocks';
import { useOpenSearchDashboards } from '../../../../opensearch_dashboards_react/public';
import { setData, setStorage } from '../../services';
import { useGenerateQuery } from '../hooks';
import { AgentError, ProhibitedQueryError } from '../utils';
import { QueryAssistInput } from './query_assist_input';

jest.mock('../../../../opensearch_dashboards_react/public', () => ({
useOpenSearchDashboards: jest.fn(),
withOpenSearchDashboards: jest.fn((component: React.Component) => component),
}));

jest.mock('../hooks', () => ({
useGenerateQuery: jest.fn().mockReturnValue({ generateQuery: jest.fn(), loading: false }),
}));

jest.mock('./query_assist_input', () => ({
QueryAssistInput: ({ inputRef, error }: ComponentProps<typeof QueryAssistInput>) => (
<>
<input ref={inputRef} />
<div>{JSON.stringify(error)}</div>
</>
),
}));

const dataMock = dataPluginMock.createStartContract(true);
const queryStringMock = dataMock.query.queryString as jest.Mocked<QueryStringContract>;
const uiSettingsMock = uiSettingsServiceMock.createStartContract();
const notificationsMock = notificationServiceMock.createStartContract();

setData(dataMock);
setStorage(new DataStorage(window.localStorage, 'mock-prefix'));

const dependencies: QueryEditorExtensionDependencies = {
language: 'PPL',
onSelectLanguage: jest.fn(),
isCollapsed: false,
setIsCollapsed: jest.fn(),
};

type Props = ComponentProps<typeof QueryAssistBar>;

const IntlWrapper = ({ children }: PropsWithChildren<unknown>) => (
<IntlProvider locale="en">{children}</IntlProvider>
);

const renderQueryAssistBar = (overrideProps: Partial<Props> = {}) => {
const props: Props = Object.assign<Props, Partial<Props>>({ dependencies }, overrideProps);
const component = render(<QueryAssistBar {...props} />, {
wrapper: IntlWrapper,
});
return { component, props: props as jest.MockedObjectDeep<Props> };
};

describe('QueryAssistBar', () => {
beforeEach(() => {
(useOpenSearchDashboards as jest.Mock).mockReturnValue({
services: {
data: dataMock,
uiSettings: uiSettingsMock,
notifications: notificationsMock,
},
});
});
afterEach(() => {
jest.clearAllMocks();
});

it('renders null if collapsed', () => {
const { component } = renderQueryAssistBar({
dependencies: { ...dependencies, isCollapsed: true },
});
expect(component.container).toBeEmptyDOMElement();
});

it('matches snapshot', () => {
const { component } = renderQueryAssistBar();
expect(component.container).toMatchSnapshot();
});

it('displays callout when query input is empty on submit', async () => {
renderQueryAssistBar();

fireEvent.click(screen.getByRole('button'));

await waitFor(() => {
expect(screen.getByTestId('query-assist-empty-query-callout')).toBeInTheDocument();
});
});

it('displays callout when dataset is not selected on submit', async () => {
queryStringMock.getQuery.mockReturnValueOnce({ query: '', language: 'kuery' });
queryStringMock.getUpdates$.mockReturnValueOnce(of({ query: '', language: 'kuery' }));
renderQueryAssistBar();

fireEvent.change(screen.getByRole('textbox'), { target: { value: 'test query' } });
fireEvent.click(screen.getByRole('button'));

await waitFor(() => {
expect(screen.getByTestId('query-assist-empty-index-callout')).toBeInTheDocument();
});
});

it('displays callout for guardrail errors', async () => {
const generateQueryMock = jest.fn().mockResolvedValue({ error: new ProhibitedQueryError() });
(useGenerateQuery as jest.Mock).mockReturnValue({
generateQuery: generateQueryMock,
loading: false,
});

renderQueryAssistBar();

fireEvent.change(screen.getByRole('textbox'), { target: { value: 'test query' } });
fireEvent.click(screen.getByRole('button'));

await waitFor(() => {
expect(screen.getByTestId('query-assist-guard-callout')).toBeInTheDocument();
});
});

it('passes agent errors to input', async () => {
const generateQueryMock = jest.fn().mockResolvedValue({
error: new AgentError({
error: { type: 'mock-type', reason: 'mock-reason', details: 'mock-details' },
status: 303,
}),
});
(useGenerateQuery as jest.Mock).mockReturnValue({
generateQuery: generateQueryMock,
loading: false,
});

renderQueryAssistBar();

fireEvent.change(screen.getByRole('textbox'), { target: { value: 'test query' } });
fireEvent.click(screen.getByRole('button'));

await waitFor(() => {
expect(screen.getByText(/mock-reason/)).toBeInTheDocument();
});
});

it('displays toast for other unknown errors', async () => {
const mockError = new Error('mock-error');
const generateQueryMock = jest.fn().mockResolvedValue({
error: mockError,
});
(useGenerateQuery as jest.Mock).mockReturnValue({
generateQuery: generateQueryMock,
loading: false,
});

renderQueryAssistBar();

fireEvent.change(screen.getByRole('textbox'), { target: { value: 'test query' } });
fireEvent.click(screen.getByRole('button'));

await waitFor(() => {
expect(notificationsMock.toasts.addError).toHaveBeenCalledWith(mockError, {
title: 'Failed to generate results',
});
});
});

it('submits a valid query and updates services', async () => {
const generateQueryMock = jest
.fn()
.mockResolvedValue({ response: { query: 'generated query' } });
(useGenerateQuery as jest.Mock).mockReturnValue({
generateQuery: generateQueryMock,
loading: false,
});

renderQueryAssistBar();

fireEvent.change(screen.getByRole('textbox'), { target: { value: 'test query' } });
fireEvent.click(screen.getByRole('button'));

await waitFor(() => {
expect(generateQueryMock).toHaveBeenCalledWith({
question: 'test query',
index: 'Default Index Pattern',
language: 'PPL',
dataSourceId: 'mock-data-source-id',
});
});

expect(queryStringMock.setQuery).toHaveBeenCalledWith({
dataset: {
dataSource: {
id: 'mock-data-source-id',
title: 'Default Data Source',
type: 'OpenSearch',
},
id: 'default-index-pattern',
timeFieldName: '@timestamp',
title: 'Default Index Pattern',
type: 'INDEX_PATTERN',
},
language: 'PPL',
query: 'generated query',
});
expect(screen.getByTestId('query-assist-query-generated-callout')).toBeInTheDocument();
});
});
Loading
Loading