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

handle EuiSearchBar query parse failures #25235

Merged
merged 5 commits into from
Nov 13, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@
exports[`Table should render normally 1`] = `
<React.Fragment>
<EuiSearchBar
box={
Object {
"data-test-subj": "savedObjectSearchBar",
}
}
filters={
Array [
Object {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@
*/

import React from 'react';
import { shallowWithIntl } from 'test_utils/enzyme_helpers';
import { shallowWithIntl, mountWithIntl } from 'test_utils/enzyme_helpers';
import { findTestSubject } from '@elastic/eui/lib/test';
import { keyCodes } from '@elastic/eui/lib/services';

jest.mock('ui/errors', () => ({
SavedObjectNotFound: class SavedObjectNotFound extends Error {
Expand All @@ -39,37 +41,62 @@ jest.mock('ui/chrome', () => ({

import { Table } from '../table';

const defaultProps = {
selectedSavedObjects: [1],
selectionConfig: {
onSelectionChange: () => {},
},
filterOptions: [{ value: 2 }],
onDelete: () => {},
onExport: () => {},
getEditUrl: () => {},
goInApp: () => {},
pageIndex: 1,
pageSize: 2,
items: [3],
itemId: 'id',
totalItemCount: 3,
onQueryChange: () => {},
onTableChange: () => {},
isSearching: false,
onShowRelationships: () => {},
};

describe('Table', () => {
it('should render normally', () => {
const props = {
selectedSavedObjects: [1],
selectionConfig: {
onSelectionChange: () => {},
},
filterOptions: [{ value: 2 }],
onDelete: () => {},
onExport: () => {},
getEditUrl: () => {},
goInApp: () => {},
const component = shallowWithIntl(
<Table.WrappedComponent
{...defaultProps}
/>
);

pageIndex: 1,
pageSize: 2,
items: [3],
itemId: 'id',
totalItemCount: 3,
onQueryChange: () => {},
onTableChange: () => {},
isSearching: false,
expect(component).toMatchSnapshot();
});

onShowRelationships: () => {},
it('should handle query parse error', () => {
const onQueryChangeMock = jest.fn();
const customizedProps = {
...defaultProps,
onQueryChange: onQueryChangeMock
};

const component = shallowWithIntl(
const component = mountWithIntl(
<Table.WrappedComponent
{...props}
{...customizedProps}
/>
);
const searchBar = findTestSubject(component, 'savedObjectSearchBar');

expect(component).toMatchSnapshot();
// Send invalid query
searchBar.simulate('keyup', { keyCode: keyCodes.ENTER, target: { value: '?' } });
expect(onQueryChangeMock).toHaveBeenCalledTimes(0);
expect(component.state().isSearchTextValid).toBe(false);

onQueryChangeMock.mockReset();

// Send valid query to ensure component can recover from invalid query
searchBar.simulate('keyup', { keyCode: keyCodes.ENTER, target: { value: 'I am valid' } });
expect(onQueryChangeMock).toHaveBeenCalledTimes(1);
expect(component.state().isSearchTextValid).toBe(true);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ import {
EuiIcon,
EuiLink,
EuiSpacer,
EuiToolTip
EuiToolTip,
EuiFormErrorText
} from '@elastic/eui';
import { getSavedObjectLabel, getSavedObjectIcon } from '../../../../lib';
import { FormattedMessage, injectI18n } from '@kbn/i18n/react';
Expand Down Expand Up @@ -61,6 +62,27 @@ class TableUI extends PureComponent {
onShowRelationships: PropTypes.func.isRequired,
};

state = {
isSearchTextValid: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe set parseErrorMessage: null here, to give people one place to see all of the state properties in use?

parseErrorMessage: null,
}

onChange = ({ query, error }) => {
if (error) {
this.setState({
isSearchTextValid: false,
parseErrorMessage: error.message,
});
return;
}

this.setState({
isSearchTextValid: true,
parseErrorMessage: null,
});
this.props.onQueryChange({ query });
}

render() {
const {
pageIndex,
Expand All @@ -74,7 +96,6 @@ class TableUI extends PureComponent {
onDelete,
onExport,
selectedSavedObjects,
onQueryChange,
onTableChange,
goInApp,
getEditUrl,
Expand Down Expand Up @@ -182,11 +203,25 @@ class TableUI extends PureComponent {
},
];

let queryParseError;
if (!this.state.isSearchTextValid) {
const parseErrorMsg = intl.formatMessage({
id: 'kbn.management.objects.objectsTable.searchBar.unableToParseQueryErrorMessage',
defaultMessage: 'Unable to parse query',
});
queryParseError = (
<EuiFormErrorText>
{`${parseErrorMsg}. ${this.state.parseErrorMessage}`}
</EuiFormErrorText>
);
}

return (
<Fragment>
<EuiSearchBar
box={{ 'data-test-subj': 'savedObjectSearchBar' }}
filters={filters}
onChange={onQueryChange}
onChange={this.onChange}
toolsRight={[
<EuiButton
key="deleteSO"
Expand All @@ -213,6 +248,7 @@ class TableUI extends PureComponent {
</EuiButton>,
]}
/>
{queryParseError}
<EuiSpacer size="s" />
<div data-test-subj="savedObjectsTable">
<EuiBasicTable
Expand Down
Original file line number Diff line number Diff line change
@@ -1,58 +1,61 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Search should render normally 1`] = `
<EuiSearchBar
box={
Object {
"aria-label": "Search advanced settings",
"incremental": true,
}
}
filters={
Array [
<React.Fragment>
<EuiSearchBar
box={
Object {
"field": "category",
"multiSelect": "or",
"name": "Category",
"options": Array [
Object {
"name": "General",
"value": "general",
},
Object {
"name": "Dashboard",
"value": "dashboard",
},
Object {
"name": "HiddenCategory",
"value": "hiddenCategory",
},
Object {
"name": "X-pack",
"value": "x-pack",
"aria-label": "Search advanced settings",
"data-test-subj": "settingsSearchBar",
"incremental": true,
}
}
filters={
Array [
Object {
"field": "category",
"multiSelect": "or",
"name": "Category",
"options": Array [
Object {
"name": "General",
"value": "general",
},
Object {
"name": "Dashboard",
"value": "dashboard",
},
Object {
"name": "HiddenCategory",
"value": "hiddenCategory",
},
Object {
"name": "X-pack",
"value": "x-pack",
},
],
"type": "field_value_selection",
},
]
}
onChange={[Function]}
query={
Query {
"ast": _AST {
"_clauses": Array [],
"_indexedClauses": Object {
"field": Object {},
"is": Object {},
"term": Array [],
},
],
"type": "field_value_selection",
},
]
}
onChange={[Function]}
query={
Query {
"ast": _AST {
"_clauses": Array [],
"_indexedClauses": Object {
"field": Object {},
"is": Object {},
"term": Array [],
},
},
"syntax": Object {
"parse": [Function],
"print": [Function],
},
"text": "",
"syntax": Object {
"parse": [Function],
"print": [Function],
},
"text": "",
}
}
}
/>
/>
</React.Fragment>
`;
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,13 @@
* under the License.
*/

import React, { PureComponent } from 'react';
import React, { Fragment, PureComponent } from 'react';
import PropTypes from 'prop-types';
import { injectI18n } from '@kbn/i18n/react';

import {
EuiSearchBar,
EuiFormErrorText,
} from '@elastic/eui';

import { getCategoryName } from '../../lib';
Expand All @@ -46,11 +47,33 @@ class SearchUI extends PureComponent {
});
}

state = {
isSearchTextValid: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Same suggestion here as in the other component.

parseErrorMessage: null,
}

onChange = ({ query, error }) => {
if (error) {
this.setState({
isSearchTextValid: false,
parseErrorMessage: error.message,
});
return;
}

this.setState({
isSearchTextValid: true,
parseErrorMessage: null,
});
this.props.onQueryChange({ query });
}

render() {
const { query, onQueryChange, intl } = this.props;
const { query, intl } = this.props;

const box = {
incremental: true,
'data-test-subj': 'settingsSearchBar',
'aria-label': intl.formatMessage({
id: 'kbn.management.settings.searchBarAriaLabel',
defaultMessage: 'Search advanced settings',
Expand All @@ -71,14 +94,29 @@ class SearchUI extends PureComponent {
}
];

return (
<EuiSearchBar
box={box}
filters={filters}
onChange={onQueryChange}
query={query}
/>
let queryParseError;
if (!this.state.isSearchTextValid) {
const parseErrorMsg = intl.formatMessage({
id: 'kbn.management.settings.searchBar.unableToParseQueryErrorMessage',
defaultMessage: 'Unable to parse query',
});
queryParseError = (
<EuiFormErrorText>
{`${parseErrorMsg}. ${this.state.parseErrorMessage}`}
</EuiFormErrorText>
);
}

return (
<Fragment>
<EuiSearchBar
box={box}
filters={filters}
onChange={this.onChange}
query={query}
/>
{queryParseError}
</Fragment>
);
}
}
Expand Down
Loading