-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[Upgrade Assistant] Fix filter deprecations search filter #57541
Changes from 5 commits
850459f
64e5478
3f8f45e
f6aced9
b4cdaf0
518d010
21abaa0
084f40d
855dae4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 |
---|---|---|
|
@@ -3,23 +3,25 @@ | |
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
import React, { FunctionComponent } from 'react'; | ||
|
||
import { EuiButton, EuiFieldSearch, EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; | ||
import React, { FunctionComponent, useState } from 'react'; | ||
import { i18n } from '@kbn/i18n'; | ||
import { EuiButton, EuiFieldSearch, EuiFlexGroup, EuiFlexItem, EuiCallOut } from '@elastic/eui'; | ||
import { FormattedMessage, injectI18n } from '@kbn/i18n/react'; | ||
|
||
import { DeprecationInfo } from 'src/legacy/core_plugins/elasticsearch'; | ||
import { GroupByOption, LevelFilterOption, LoadingState } from '../../types'; | ||
import { FilterBar } from './filter_bar'; | ||
import { GroupByBar } from './group_by_bar'; | ||
|
||
import { validateRegExpString } from '../../../utils'; | ||
|
||
interface CheckupControlsProps extends ReactIntl.InjectedIntlProps { | ||
allDeprecations?: DeprecationInfo[]; | ||
loadingState: LoadingState; | ||
loadData: () => void; | ||
currentFilter: LevelFilterOption; | ||
onFilterChange: (filter: LevelFilterOption) => void; | ||
search: string; | ||
onSearchChange: (filter: string) => void; | ||
availableGroupByOptions: GroupByOption[]; | ||
currentGroupBy: GroupByOption; | ||
|
@@ -32,44 +34,79 @@ export const CheckupControlsUI: FunctionComponent<CheckupControlsProps> = ({ | |
loadData, | ||
currentFilter, | ||
onFilterChange, | ||
search, | ||
onSearchChange, | ||
availableGroupByOptions, | ||
currentGroupBy, | ||
onGroupByChange, | ||
intl, | ||
}) => ( | ||
<EuiFlexGroup alignItems="center" wrap={true} responsive={false}> | ||
<EuiFlexItem grow={true}> | ||
<EuiFieldSearch | ||
aria-label="Filter" | ||
placeholder={intl.formatMessage({ | ||
id: 'xpack.upgradeAssistant.checkupTab.controls.searchBarPlaceholder', | ||
defaultMessage: 'Filter', | ||
})} | ||
value={search} | ||
onChange={e => onSearchChange(e.target.value)} | ||
/> | ||
</EuiFlexItem> | ||
}) => { | ||
const [searchTermError, setSearchTermError] = useState<null | string>(null); | ||
const filterInvalid = Boolean(searchTermError); | ||
return ( | ||
<EuiFlexGroup direction="column" responsive={false}> | ||
<EuiFlexItem grow={true}> | ||
<EuiFlexGroup alignItems="center" wrap={true} responsive={false}> | ||
<EuiFlexItem grow={true}> | ||
<EuiFieldSearch | ||
isInvalid={filterInvalid} | ||
aria-label="Filter" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i18n |
||
placeholder={intl.formatMessage({ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you switch to |
||
id: 'xpack.upgradeAssistant.checkupTab.controls.searchBarPlaceholder', | ||
defaultMessage: 'Filter', | ||
})} | ||
onChange={e => { | ||
const string = e.target.value; | ||
const errorMessage = validateRegExpString(string); | ||
if (errorMessage) { | ||
// Emit an empty search term to listeners if search term is invalid. | ||
onSearchChange(''); | ||
setSearchTermError(errorMessage); | ||
} else { | ||
onSearchChange(e.target.value); | ||
if (searchTermError) { | ||
setSearchTermError(null); | ||
} | ||
} | ||
}} | ||
/> | ||
</EuiFlexItem> | ||
|
||
{/* These two components provide their own EuiFlexItem wrappers */} | ||
<FilterBar {...{ allDeprecations, currentFilter, onFilterChange }} /> | ||
<GroupByBar {...{ availableGroupByOptions, currentGroupBy, onGroupByChange }} /> | ||
{/* These two components provide their own EuiFlexItem wrappers */} | ||
<FilterBar {...{ allDeprecations, currentFilter, onFilterChange }} /> | ||
<GroupByBar {...{ availableGroupByOptions, currentGroupBy, onGroupByChange }} /> | ||
|
||
<EuiFlexItem grow={false}> | ||
<EuiButton | ||
fill | ||
onClick={loadData} | ||
iconType="refresh" | ||
isLoading={loadingState === LoadingState.Loading} | ||
> | ||
<FormattedMessage | ||
id="xpack.upgradeAssistant.checkupTab.controls.refreshButtonLabel" | ||
defaultMessage="Refresh" | ||
/> | ||
</EuiButton> | ||
</EuiFlexItem> | ||
</EuiFlexGroup> | ||
); | ||
<EuiFlexItem grow={false}> | ||
<EuiButton | ||
fill | ||
onClick={loadData} | ||
iconType="refresh" | ||
isLoading={loadingState === LoadingState.Loading} | ||
> | ||
<FormattedMessage | ||
id="xpack.upgradeAssistant.checkupTab.controls.refreshButtonLabel" | ||
defaultMessage="Refresh" | ||
/> | ||
</EuiButton> | ||
</EuiFlexItem> | ||
</EuiFlexGroup> | ||
</EuiFlexItem> | ||
{filterInvalid && ( | ||
<EuiFlexItem grow={false}> | ||
<EuiCallOut | ||
color="danger" | ||
title={i18n.translate( | ||
'xpack.upgradeAssistant.checkupTab.controls.filterErrorMessageLabel', | ||
{ | ||
defaultMessage: 'Filter Invalid: {searchTermError}', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: this should probably be sentence case per https://elastic.github.io/eui/#/guidelines/writing. |
||
values: { searchTermError }, | ||
} | ||
)} | ||
iconType="faceSad" | ||
/> | ||
</EuiFlexItem> | ||
)} | ||
</EuiFlexGroup> | ||
); | ||
}; | ||
|
||
export const CheckupControls = injectI18n(CheckupControlsUI); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
import { validateRegExpString } from './utils'; | ||
|
||
describe('validRegExpString', () => { | ||
it('correctly returns false for invalid strings', () => { | ||
expect(validateRegExpString('?asd')).toContain(`Invalid regular expression`); | ||
expect(validateRegExpString('*asd')).toContain(`Invalid regular expression`); | ||
expect(validateRegExpString('(')).toContain(`Invalid regular expression`); | ||
}); | ||
|
||
it('correctly returns true for valid strings', () => { | ||
expect(validateRegExpString('asd')).toBe(''); | ||
expect(validateRegExpString('.*asd')).toBe(''); | ||
expect(validateRegExpString('')).toBe(''); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
import { pipe } from 'fp-ts/lib/pipeable'; | ||
import { tryCatch, fold } from 'fp-ts/lib/Either'; | ||
|
||
export const validateRegExpString = (s: string) => | ||
pipe( | ||
tryCatch( | ||
() => new RegExp(s), | ||
e => (e as Error).message | ||
), | ||
fold( | ||
(errorMessage: string) => errorMessage, | ||
() => '' | ||
) | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe
true
is the default