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

[Uptime] Improve filter group #88185

Merged
merged 19 commits into from
Jan 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
0e55ef9
Unskip "Observer location" test block.
justinkambic Jan 6, 2021
e1c5a39
Commit temp "describe.only" to make flaky test runner go faster.
justinkambic Jan 6, 2021
223eb59
Add optional chain for some potentially-null props.
justinkambic Jan 11, 2021
19f9d07
Make overview filters type partial.
justinkambic Jan 12, 2021
f73911a
Repair broken types.
justinkambic Jan 12, 2021
7e4e098
Remove only call from test.
justinkambic Jan 13, 2021
d03b0a4
Add unit tests and mark areas for improvement in \`FilterGroup\` comp…
justinkambic Jan 13, 2021
ca13fe8
Add aria-label translations and new labels.
justinkambic Jan 13, 2021
ed2c964
Refactor existing tests and add tests for new labels.
justinkambic Jan 13, 2021
0d27256
Fix bug in event handler and update tests.
justinkambic Jan 14, 2021
117b17c
Merge branch 'master' into uptime_improve-filter-group
justinkambic Jan 15, 2021
fbdcbb3
Delete a comment.
justinkambic Jan 15, 2021
19ea207
Delete a comment.
justinkambic Jan 15, 2021
b7345fb
Add some line breaks to help readability.
justinkambic Jan 15, 2021
9b624e4
Merge branch 'master' into uptime_improve-filter-group
kibanamachine Jan 19, 2021
ba1969c
Merge branch 'master' into uptime_improve-filter-group
kibanamachine Jan 20, 2021
04706e9
Merge branch 'master' into uptime_improve-filter-group
justinkambic Jan 20, 2021
60984e9
Merge branch 'uptime_improve-filter-group' of github.com:justinkambic…
justinkambic Jan 20, 2021
8fb99f5
Add additional tests, fix a bug.
justinkambic Jan 20, 2021
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 @@ -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}.`))
);
});
}
);
});
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