From 2a48f85a5dcb2ec9a0b291257c38e1adfb46d72a Mon Sep 17 00:00:00 2001 From: Nathan Reese Date: Tue, 6 Nov 2018 11:27:34 -0700 Subject: [PATCH 1/4] handle EuiSearchBar query parse failures --- .../__jest__/__snapshots__/table.test.js.snap | 5 + .../components/table/__jest__/table.test.js | 73 +++++++++---- .../objects_table/components/table/table.js | 34 +++++- .../search/__snapshots__/search.test.js.snap | 102 +++++++++--------- .../settings/components/search/search.js | 49 +++++++-- .../settings/components/search/search.test.js | 23 ++++ 6 files changed, 202 insertions(+), 84 deletions(-) diff --git a/src/core_plugins/kibana/public/management/sections/objects/components/objects_table/components/table/__jest__/__snapshots__/table.test.js.snap b/src/core_plugins/kibana/public/management/sections/objects/components/objects_table/components/table/__jest__/__snapshots__/table.test.js.snap index eecc496328f3d..d3ca955a95a54 100644 --- a/src/core_plugins/kibana/public/management/sections/objects/components/objects_table/components/table/__jest__/__snapshots__/table.test.js.snap +++ b/src/core_plugins/kibana/public/management/sections/objects/components/objects_table/components/table/__jest__/__snapshots__/table.test.js.snap @@ -3,6 +3,11 @@ exports[`Table should render normally 1`] = ` ({ SavedObjectNotFound: class SavedObjectNotFound extends Error { @@ -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( + + ); - 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( ); + 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); }); }); diff --git a/src/core_plugins/kibana/public/management/sections/objects/components/objects_table/components/table/table.js b/src/core_plugins/kibana/public/management/sections/objects/components/objects_table/components/table/table.js index 0effa28d9c40c..4774dea298410 100644 --- a/src/core_plugins/kibana/public/management/sections/objects/components/objects_table/components/table/table.js +++ b/src/core_plugins/kibana/public/management/sections/objects/components/objects_table/components/table/table.js @@ -61,6 +61,26 @@ class TableUI extends PureComponent { onShowRelationships: PropTypes.func.isRequired, }; + state = { + isSearchTextValid: true, + } + + 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, @@ -74,7 +94,6 @@ class TableUI extends PureComponent { onDelete, onExport, selectedSavedObjects, - onQueryChange, onTableChange, goInApp, getEditUrl, @@ -182,11 +201,21 @@ class TableUI extends PureComponent { }, ]; + let queryParseError; + if (!this.state.isSearchTextValid) { + queryParseError = ( +
+ {`Unable to parse query. ${this.state.parseErrorMessage}`} +
+ ); + } + return ( , ]} /> + {queryParseError}
+ + /> + `; diff --git a/src/core_plugins/kibana/public/management/sections/settings/components/search/search.js b/src/core_plugins/kibana/public/management/sections/settings/components/search/search.js index 919bcfe4da7a3..4d186ea5d8bc9 100644 --- a/src/core_plugins/kibana/public/management/sections/settings/components/search/search.js +++ b/src/core_plugins/kibana/public/management/sections/settings/components/search/search.js @@ -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'; @@ -46,8 +46,28 @@ class SearchUI extends PureComponent { }); } + state = { + isSearchTextValid: true, + } + + 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, @@ -71,14 +91,25 @@ class SearchUI extends PureComponent { } ]; - return ( - + let queryParseError; + if (!this.state.isSearchTextValid) { + queryParseError = ( +
+ {`Unable to parse query. ${this.state.parseErrorMessage}`} +
+ ); + } + return ( + + + {queryParseError} + ); } } diff --git a/src/core_plugins/kibana/public/management/sections/settings/components/search/search.test.js b/src/core_plugins/kibana/public/management/sections/settings/components/search/search.test.js index 2b76394ccc71e..2de310ed60363 100644 --- a/src/core_plugins/kibana/public/management/sections/settings/components/search/search.test.js +++ b/src/core_plugins/kibana/public/management/sections/settings/components/search/search.test.js @@ -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( + + ); + + // Send invalid query + component.find('input').simulate('keyup', { 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 + component.find('input').simulate('keyup', { target: { value: 'dateFormat' } }); + expect(onQueryChangeMock).toHaveBeenCalledTimes(1); + expect(component.state().isSearchTextValid).toBe(true); + }); }); From 534de7e78b40fd6fbb6ea2a4b4ad2b9d50a67c37 Mon Sep 17 00:00:00 2001 From: Nathan Reese Date: Tue, 6 Nov 2018 11:40:52 -0700 Subject: [PATCH 2/4] I18n parse failure messages --- .../components/objects_table/components/table/table.js | 6 +++++- .../sections/settings/components/search/search.js | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/core_plugins/kibana/public/management/sections/objects/components/objects_table/components/table/table.js b/src/core_plugins/kibana/public/management/sections/objects/components/objects_table/components/table/table.js index 4774dea298410..9976357cd3a46 100644 --- a/src/core_plugins/kibana/public/management/sections/objects/components/objects_table/components/table/table.js +++ b/src/core_plugins/kibana/public/management/sections/objects/components/objects_table/components/table/table.js @@ -203,9 +203,13 @@ class TableUI extends PureComponent { let queryParseError; if (!this.state.isSearchTextValid) { + const parseErrorMsg = intl.formatMessage({ + id: 'kbn.management.objects.objectsTable.table.searchBarUnableToParseQuery', + defaultMessage: 'Unable to parse query', + }); queryParseError = (
- {`Unable to parse query. ${this.state.parseErrorMessage}`} + {`${parseErrorMsg}. ${this.state.parseErrorMessage}`}
); } diff --git a/src/core_plugins/kibana/public/management/sections/settings/components/search/search.js b/src/core_plugins/kibana/public/management/sections/settings/components/search/search.js index 4d186ea5d8bc9..421a5cdbe1b92 100644 --- a/src/core_plugins/kibana/public/management/sections/settings/components/search/search.js +++ b/src/core_plugins/kibana/public/management/sections/settings/components/search/search.js @@ -93,9 +93,13 @@ class SearchUI extends PureComponent { let queryParseError; if (!this.state.isSearchTextValid) { + const parseErrorMsg = intl.formatMessage({ + id: 'kbn.management.settings.searchBarUnableToParseQuery', + defaultMessage: 'Unable to parse query', + }); queryParseError = (
- {`Unable to parse query. ${this.state.parseErrorMessage}`} + {`${parseErrorMsg}. ${this.state.parseErrorMessage}`}
); } From 386c5f6eefc0d80f1bf57e8b2809b7fbf11d4165 Mon Sep 17 00:00:00 2001 From: Nathan Reese Date: Mon, 12 Nov 2018 08:07:10 -0700 Subject: [PATCH 3/4] review updates --- .../objects_table/components/table/table.js | 12 +++++++----- .../search/__snapshots__/search.test.js.snap | 1 + .../sections/settings/components/search/search.js | 9 ++++++--- .../settings/components/search/search.test.js | 5 +++-- 4 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/core_plugins/kibana/public/management/sections/objects/components/objects_table/components/table/table.js b/src/core_plugins/kibana/public/management/sections/objects/components/objects_table/components/table/table.js index 9976357cd3a46..09ba15cc16b4b 100644 --- a/src/core_plugins/kibana/public/management/sections/objects/components/objects_table/components/table/table.js +++ b/src/core_plugins/kibana/public/management/sections/objects/components/objects_table/components/table/table.js @@ -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'; @@ -63,6 +64,7 @@ class TableUI extends PureComponent { state = { isSearchTextValid: true, + parseErrorMessage: null, } onChange = ({ query, error }) => { @@ -204,20 +206,20 @@ class TableUI extends PureComponent { let queryParseError; if (!this.state.isSearchTextValid) { const parseErrorMsg = intl.formatMessage({ - id: 'kbn.management.objects.objectsTable.table.searchBarUnableToParseQuery', + id: 'kbn.management.objects.objectsTable.searchBar.unableToParseQueryErrorMessage', defaultMessage: 'Unable to parse query', }); queryParseError = ( -
+ {`${parseErrorMsg}. ${this.state.parseErrorMessage}`} -
+ ); } return ( { @@ -71,6 +73,7 @@ class SearchUI extends PureComponent { const box = { incremental: true, + 'data-test-subj': 'settingsSearchBar', 'aria-label': intl.formatMessage({ id: 'kbn.management.settings.searchBarAriaLabel', defaultMessage: 'Search advanced settings', @@ -94,13 +97,13 @@ class SearchUI extends PureComponent { let queryParseError; if (!this.state.isSearchTextValid) { const parseErrorMsg = intl.formatMessage({ - id: 'kbn.management.settings.searchBarUnableToParseQuery', + id: 'kbn.management.settings.searchBar.unableToParseQueryErrorMessage', defaultMessage: 'Unable to parse query', }); queryParseError = ( -
+ {`${parseErrorMsg}. ${this.state.parseErrorMessage}`} -
+ ); } diff --git a/src/core_plugins/kibana/public/management/sections/settings/components/search/search.test.js b/src/core_plugins/kibana/public/management/sections/settings/components/search/search.test.js index 2de310ed60363..70b43fd6e416e 100644 --- a/src/core_plugins/kibana/public/management/sections/settings/components/search/search.test.js +++ b/src/core_plugins/kibana/public/management/sections/settings/components/search/search.test.js @@ -19,6 +19,7 @@ import React from 'react'; import { shallowWithIntl, mountWithIntl } from 'test_utils/enzyme_helpers'; +import { findTestSubject } from '@elastic/eui/lib/test'; import { Query } from '@elastic/eui'; @@ -52,7 +53,7 @@ describe('Search', () => { onQueryChange={onQueryChange} /> ); - component.find('input').simulate('keyup', { target: { value: 'new filter' } }); + findTestSubject(component, 'settingsSearchBar').simulate('keyup', { target: { value: 'new filter' } }); expect(onQueryChange).toHaveBeenCalledTimes(1); }); @@ -74,7 +75,7 @@ describe('Search', () => { onQueryChangeMock.mockReset(); // Send valid query to ensure component can recover from invalid query - component.find('input').simulate('keyup', { target: { value: 'dateFormat' } }); + findTestSubject(component, 'settingsSearchBar').simulate('keyup', { target: { value: 'dateFormat' } }); expect(onQueryChangeMock).toHaveBeenCalledTimes(1); expect(component.state().isSearchTextValid).toBe(true); }); From 8481ba011f356bebc22352d6f54e2884af3854cf Mon Sep 17 00:00:00 2001 From: Nathan Reese Date: Mon, 12 Nov 2018 08:10:59 -0700 Subject: [PATCH 4/4] more cleanup on settings search.test --- .../sections/settings/components/search/search.test.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/core_plugins/kibana/public/management/sections/settings/components/search/search.test.js b/src/core_plugins/kibana/public/management/sections/settings/components/search/search.test.js index 70b43fd6e416e..40532605d807a 100644 --- a/src/core_plugins/kibana/public/management/sections/settings/components/search/search.test.js +++ b/src/core_plugins/kibana/public/management/sections/settings/components/search/search.test.js @@ -67,15 +67,17 @@ describe('Search', () => { /> ); + const searchBar = findTestSubject(component, 'settingsSearchBar'); + // Send invalid query - component.find('input').simulate('keyup', { target: { value: '?' } }); + searchBar.simulate('keyup', { 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 - findTestSubject(component, 'settingsSearchBar').simulate('keyup', { target: { value: 'dateFormat' } }); + searchBar.simulate('keyup', { target: { value: 'dateFormat' } }); expect(onQueryChangeMock).toHaveBeenCalledTimes(1); expect(component.state().isSearchTextValid).toBe(true); });