Skip to content

Commit

Permalink
[Uptime] Improve filter group (elastic#88185)
Browse files Browse the repository at this point in the history 
* Unskip "Observer location" test block.

* Commit temp "describe.only" to make flaky test runner go faster.

* Add optional chain for some potentially-null props.

* Make overview filters type partial.

* Repair broken types.

* Remove only call from test.

* Add unit tests and mark areas for improvement in \`FilterGroup\` component.

* Add aria-label translations and new labels.

* Refactor existing tests and add tests for new labels.

* Fix bug in event handler and update tests.

* Delete a comment.

* Delete a comment.

* Add some line breaks to help readability.

* Add additional tests, fix a bug.

Co-authored-by: Kibana Machine <[email protected]>
  • Loading branch information
@justinkambic @kibanamachine
justinkambic and kibanamachine authored Jan 21, 2021
1 parent 0b798f7 commit f0be0ad
Show file tree
Hide file tree
Showing 10 changed files with 330 additions and 292 deletions.
264 changes: 150 additions & 114 deletions .../public/components/overview/alerts/monitor_expressions/filters_expression_select.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,18 @@

import React from 'react';
import { shallowWithIntl } from '@kbn/test/jest';
import { fireEvent, waitFor } from '@testing-library/react';
import { FiltersExpressionsSelect } from './filters_expression_select';
import { render } from '../../../../lib/helper/rtl_helpers';
import { filterAriaLabels as aria } from './translations';
import { filterLabels } from '../../filter_group/translations';

describe('FiltersExpressionSelect', () => {
const LOCATION_FIELD_NAME = 'observer.geo.name';
const PORT_FIELD_NAME = 'url.port';
const SCHEME_FIELD_NAME = 'monitor.type';
const TAG_FIELD_NAME = 'tags';

describe('filters expression select component', () => {
it('is empty when no filters available', () => {
const component = shallowWithIntl(
<FiltersExpressionsSelect
Expand All @@ -35,11 +44,24 @@ describe('filters expression select component', () => {
`);
});

it('contains provided new filter values', () => {
const component = shallowWithIntl(
it.each([
[[LOCATION_FIELD_NAME], [aria.LOCATION], [aria.TAG, aria.PORT, aria.SCHEME]],
[
[LOCATION_FIELD_NAME, TAG_FIELD_NAME],
[aria.LOCATION, aria.TAG],
[aria.PORT, aria.SCHEME],
],
[
[PORT_FIELD_NAME, SCHEME_FIELD_NAME],
[aria.PORT, aria.SCHEME],
[aria.LOCATION, aria.TAG],
],
[[TAG_FIELD_NAME], [aria.TAG], [aria.LOCATION, aria.PORT, aria.SCHEME]],
])('contains provided new filter values', (newFilters, expectedLabels, absentLabels) => {
const { getByLabelText, queryByLabelText } = render(
<FiltersExpressionsSelect
alertParams={{}}
newFilters={['observer.geo.name']}
newFilters={newFilters}
onRemoveFilter={jest.fn()}
filters={{
tags: [],
Expand All @@ -52,125 +74,139 @@ describe('filters expression select component', () => {
shouldUpdateUrl={false}
/>
);
expect(component).toMatchInlineSnapshot(`
<Fragment>
<EuiFlexGroup
key="filter_location"
>
<EuiFlexItem>
<FilterPopover
btnContent={
<EuiExpression
aria-label="ariaLabel"
color="secondary"
data-test-subj="uptimeCreateStatusAlert.filter_location"
description="From"
onClick={[Function]}
value="any location"
/>
}
disabled={true}
fieldName="observer.geo.name"
forceOpen={false}
id="filter_location"
items={Array []}
loading={false}
onFilterFieldChange={[Function]}
selectedItems={Array []}
setForceOpen={[Function]}
title="Scheme"
/>
</EuiFlexItem>
<EuiFlexItem
grow={false}
>
<EuiButtonIcon
aria-label="Remove filter"
color="danger"
iconType="trash"
onClick={[Function]}
/>
</EuiFlexItem>
<EuiSpacer
size="xs"
/>
</EuiFlexGroup>
<EuiSpacer
size="xs"
/>
</Fragment>
`);
expectedLabels.forEach((label) => expect(getByLabelText(label)));
absentLabels.forEach((label) => expect(queryByLabelText(label)).toBeNull());
});

it('contains provided selected filter values', () => {
const component = shallowWithIntl(
it.each([
['Remove filter Location', LOCATION_FIELD_NAME],
['Remove filter Scheme', SCHEME_FIELD_NAME],
['Remove filter Port', PORT_FIELD_NAME],
['Remove filter Tag', TAG_FIELD_NAME],
])('fires remove filter handler', async (removeButtonLabel, expectedFieldName) => {
const onRemoveFilterMock = jest.fn();
const setAlertParamsMock = jest.fn();
const { getByLabelText } = render(
<FiltersExpressionsSelect
alertParams={{}}
newFilters={['tags']}
onRemoveFilter={jest.fn()}
newFilters={[LOCATION_FIELD_NAME, SCHEME_FIELD_NAME, PORT_FIELD_NAME, TAG_FIELD_NAME]}
onRemoveFilter={onRemoveFilterMock}
filters={{
tags: ['foo', 'bar'],
ports: [],
schemes: [],
locations: [],
tags: ['prod'],
ports: [5601],
schemes: ['http'],
locations: ['nyc'],
}}
setAlertParams={jest.fn()}
setAlertParams={setAlertParamsMock}
setUpdatedFieldValues={jest.fn()}
shouldUpdateUrl={false}
/>
);
expect(component).toMatchInlineSnapshot(`
<Fragment>
<EuiFlexGroup
key="filter_tags"
>
<EuiFlexItem>
<FilterPopover
btnContent={
<EuiExpression
aria-label="ariaLabel"
color="secondary"
data-test-subj="uptimeCreateStatusAlert.filter_tags"
description="Using"
onClick={[Function]}
value="any tag"
/>
}
disabled={false}
fieldName="tags"
forceOpen={false}
id="filter_tags"
items={
Array [
"foo",
"bar",
]
}
loading={false}
onFilterFieldChange={[Function]}
selectedItems={Array []}
setForceOpen={[Function]}
title="Tags"
/>
</EuiFlexItem>
<EuiFlexItem
grow={false}
>
<EuiButtonIcon
aria-label="Remove filter"
color="danger"
iconType="trash"
onClick={[Function]}
/>
</EuiFlexItem>
<EuiSpacer
size="xs"
/>
</EuiFlexGroup>
<EuiSpacer
size="xs"
/>
</Fragment>
`);

const removeButton = getByLabelText(removeButtonLabel);
fireEvent.click(removeButton);
expect(onRemoveFilterMock).toHaveBeenCalledTimes(1);
expect(onRemoveFilterMock).toHaveBeenCalledWith(expectedFieldName);
expect(setAlertParamsMock).toHaveBeenCalledTimes(1);
expect(setAlertParamsMock).toHaveBeenCalledWith('filters', {
[SCHEME_FIELD_NAME]: [],
[LOCATION_FIELD_NAME]: [],
[TAG_FIELD_NAME]: [],
[PORT_FIELD_NAME]: [],
});
});

const TEST_TAGS = ['foo', 'bar'];
const TEST_PORTS = [5601, 9200];
const TEST_SCHEMES = ['http', 'tcp'];
const TEST_LOCATIONS = ['nyc', 'fairbanks'];

it.each([
[
{
tags: TEST_TAGS,
ports: [5601, 9200],
schemes: ['http', 'tcp'],
locations: ['nyc', 'fairbanks'],
},
[TAG_FIELD_NAME],
aria.TAG,
filterLabels.TAG,
TEST_TAGS,
],
[
{
tags: [],
ports: TEST_PORTS,
schemes: [],
locations: [],
},
[PORT_FIELD_NAME],
aria.PORT,
filterLabels.PORT,
TEST_PORTS,
],
[
{
tags: [],
ports: [],
schemes: TEST_SCHEMES,
locations: [],
},
[SCHEME_FIELD_NAME],
aria.SCHEME,
filterLabels.SCHEME,
TEST_SCHEMES,
],
[
{
tags: [],
ports: [],
schemes: [],
locations: TEST_LOCATIONS,
},
[LOCATION_FIELD_NAME],
aria.LOCATION,
filterLabels.LOCATION,
TEST_LOCATIONS,
],
])(
'applies accessible label to filter expressions, and contains selected filters',
/**
* @param filters the set of filters the test should supply as props
* @param newFilters the set of filter item types the component should render
* @param expectedFilterButtonAriaLabel the aria label for the popover button for the targeted filter
* @param filterLabel the name of the filter label expected in each item's aria-label
* @param expectedFilterItems the set of filter options the component should render
*/
async (
filters,
newFilters,
expectedFilterButtonAriaLabel,
filterLabel,
expectedFilterItems
) => {
const { getByLabelText } = render(
<FiltersExpressionsSelect
alertParams={{}}
newFilters={newFilters}
onRemoveFilter={jest.fn()}
filters={filters}
setAlertParams={jest.fn()}
setUpdatedFieldValues={jest.fn()}
shouldUpdateUrl={false}
/>
);

const filterButton = getByLabelText(expectedFilterButtonAriaLabel);

fireEvent.click(filterButton);

await waitFor(() => {
expectedFilterItems.forEach((filterItem: string | number) =>
expect(getByLabelText(`Filter by ${filterLabel} ${filterItem}.`))
);
});
}
);
});
14 changes: 9 additions & 5 deletions ...ptime/public/components/overview/alerts/monitor_expressions/filters_expression_select.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import React, { useState } from 'react';
import { EuiButtonIcon, EuiExpression, EuiFlexGroup, EuiFlexItem, EuiSpacer } from '@elastic/eui';
import { FilterPopover } from '../../filter_group/filter_popover';
import { filterLabels } from '../../filter_group/translations';
import { alertFilterLabels } from './translations';
import { alertFilterLabels, filterAriaLabels } from './translations';
import { FilterExpressionsSelectProps } from './filters_expression_select_container';
import { OverviewFiltersState } from '../../../../state/reducers/overview_filters';

Expand Down Expand Up @@ -58,6 +58,7 @@ export const FiltersExpressionsSelect: React.FC<Props> = ({

const monitorFilters = [
{
'aria-label': filterAriaLabels.PORT,
onFilterFieldChange,
loading: false,
fieldName: 'url.port',
Expand All @@ -71,18 +72,20 @@ export const FiltersExpressionsSelect: React.FC<Props> = ({
value: selectedPorts.length === 0 ? alertFilterLabels.ANY_PORT : selectedPorts?.join(','),
},
{
'aria-label': filterAriaLabels.TAG,
onFilterFieldChange,
loading: false,
fieldName: 'tags',
id: 'filter_tags',
disabled: tags?.length === 0,
items: tags ?? [],
selectedItems: selectedTags,
title: filterLabels.TAGS,
title: filterLabels.TAG,
description: selectedTags.length === 0 ? alertFilterLabels.WITH : alertFilterLabels.WITH_TAG,
value: selectedTags.length === 0 ? alertFilterLabels.ANY_TAG : selectedTags?.join(','),
},
{
'aria-label': filterAriaLabels.SCHEME,
onFilterFieldChange,
loading: false,
fieldName: 'monitor.type',
Expand All @@ -95,14 +98,15 @@ export const FiltersExpressionsSelect: React.FC<Props> = ({
value: selectedSchemes.length === 0 ? alertFilterLabels.ANY_TYPE : selectedSchemes?.join(','),
},
{
'aria-label': filterAriaLabels.LOCATION,
onFilterFieldChange,
loading: false,
fieldName: 'observer.geo.name',
id: 'filter_location',
disabled: locations?.length === 0,
items: locations ?? [],
selectedItems: selectedLocations,
title: filterLabels.SCHEME,
title: filterLabels.LOCATION,
description:
selectedLocations.length === 0 ? alertFilterLabels.FROM : alertFilterLabels.FROM_LOCATION,
value:
Expand Down Expand Up @@ -132,7 +136,7 @@ export const FiltersExpressionsSelect: React.FC<Props> = ({
{...item}
btnContent={
<EuiExpression
aria-label={'ariaLabel'}
aria-label={item['aria-label']}
color={'secondary'}
data-test-subj={'uptimeCreateStatusAlert.' + item.id}
description={description}
Expand All @@ -148,7 +152,7 @@ export const FiltersExpressionsSelect: React.FC<Props> = ({
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiButtonIcon
aria-label="Remove filter"
aria-label={alertFilterLabels.REMOVE_FILTER_LABEL(item.title)}
iconType="trash"
color="danger"
onClick={() => {
Expand Down
Loading

0 comments on commit f0be0ad

Please sign in to comment.