Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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 @@ -61,6 +61,26 @@ 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?

}

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 +94,6 @@ class TableUI extends PureComponent {
onDelete,
onExport,
selectedSavedObjects,
onQueryChange,
onTableChange,
goInApp,
getEditUrl,
Expand Down Expand Up @@ -182,11 +201,25 @@ class TableUI extends PureComponent {
},
];

let queryParseError;
if (!this.state.isSearchTextValid) {
const parseErrorMsg = intl.formatMessage({
id: 'kbn.management.objects.objectsTable.table.searchBarUnableToParseQuery',
Copy link
Contributor

Choose a reason for hiding this comment

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

Based on the i18n naming guidelines, I think this should be kbn.management.objects.objectsTable.searchBar.unableToParseQueryErrorMessage.

defaultMessage: 'Unable to parse query',
});
queryParseError = (
<div className="euiFormErrorText euiFormRow__text">
Copy link
Contributor

Choose a reason for hiding this comment

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

If we use EuiFormErrorText here instead then we become uncoupled from the CSS classes used (an implementation detail) and gain an accessibility improvement because the error will announce itself when it appears.

        <EuiFormErrorText>
          {`${parseErrorMsg}. ${this.state.parseErrorMessage}`}
        </EuiFormErrorText>

Formatting looks unchanged:

image

{`${parseErrorMsg}. ${this.state.parseErrorMessage}`}
</div>
);
}

return (
<Fragment>
<EuiSearchBar
box={{ ['data-test-subj']: 'savedObjectSearchBar' }}
Copy link
Contributor

Choose a reason for hiding this comment

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

I tested this and it looks like we can remove the brackets to make this slightly simpler:

box={{ 'data-test-subj': 'savedObjectSearchBar' }}

filters={filters}
onChange={onQueryChange}
onChange={this.onChange}
toolsRight={[
<EuiButton
key="deleteSO"
Expand All @@ -213,6 +246,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,60 @@
// 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",
"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,7 +17,7 @@
* 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';

Expand Down Expand Up @@ -46,8 +46,28 @@ 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.

}

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,
Expand All @@ -71,14 +91,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.searchBarUnableToParseQuery',
defaultMessage: 'Unable to parse query',
});
queryParseError = (
<div className="euiFormErrorText euiFormRow__text">
Copy link
Contributor

Choose a reason for hiding this comment

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

And here.

{`${parseErrorMsg}. ${this.state.parseErrorMessage}`}
</div>
);
}

return (
<Fragment>
<EuiSearchBar
box={box}
filters={filters}
onChange={this.onChange}
query={query}
/>
{queryParseError}
</Fragment>
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,27 @@ describe('Search', () => {
component.find('input').simulate('keyup', { target: { value: 'new filter' } });
expect(onQueryChange).toHaveBeenCalledTimes(1);
});

it('should handle query parse error', async () => {
const onQueryChangeMock = jest.fn();
const component = mountWithIntl(
<Search.WrappedComponent
query={query}
categories={categories}
onQueryChange={onQueryChangeMock}
/>
);

// Send invalid query
component.find('input').simulate('keyup', { target: { value: '?' } });
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to use a data-test-subj selector the way you did in the other test?

expect(onQueryChangeMock).toHaveBeenCalledTimes(0);
expect(component.state().isSearchTextValid).toBe(false);

onQueryChangeMock.mockReset();

// Send valid query to ensure component can recover from invalid query
component.find('input').simulate('keyup', { target: { value: 'dateFormat' } });
expect(onQueryChangeMock).toHaveBeenCalledTimes(1);
expect(component.state().isSearchTextValid).toBe(true);
});
});