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

[Cases] UI validation for assignees, tags and categories filters #162411

Merged
merged 7 commits into from
Jul 26, 2023
Merged
Show file tree
Hide file tree
Changes from 5 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 @@ -8,12 +8,14 @@
import React from 'react';
import userEvent from '@testing-library/user-event';
import { screen, fireEvent, waitFor, within } from '@testing-library/react';
import { waitForEuiPopoverOpen } from '@elastic/eui/lib/test/rtl';

import type { AppMockRenderer } from '../../common/mock';
import { createAppMockRenderer } from '../../common/mock';
import type { AssigneesFilterPopoverProps } from './assignees_filter';
import { AssigneesFilterPopover } from './assignees_filter';
import { userProfiles } from '../../containers/user_profiles/api.mock';
import { waitForEuiPopoverOpen } from '@elastic/eui/lib/test/rtl';
import { MAX_ASSIGNEES_FILTER_LENGTH } from '../../../common/constants';

jest.mock('../../containers/user_profiles/api');

Expand Down Expand Up @@ -309,4 +311,28 @@ describe('AssigneesFilterPopover', () => {
fireEvent.change(screen.getByPlaceholderText('Search users'), { target: { value: 'dingo' } });
expect(screen.queryByText('No assignees')).not.toBeInTheDocument();
});

it('shows warning message when reaching maximum limit to filter', async () => {
const maxAssignees = Array(MAX_ASSIGNEES_FILTER_LENGTH).fill(userProfiles[0]);
const props = {
...defaultProps,
selectedAssignees: maxAssignees,
};
appMockRender.render(<AssigneesFilterPopover {...props} />);

await waitFor(async () => {
userEvent.click(screen.getByTestId('options-filter-popover-button-assignees'));
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: we use the old "@testing-library/user-event": "^13.5.0", which doesn't return a promise(I think it is here) so this could be moved outside.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to move userEvent.click(screen.getByTestId('options-filter-popover-button-assignees')); out of waitFor but test fails due to it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Weird, in the documentation for this version they don't use it. I don't know what to believe anymore 😆

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, we also don't put it in waitFor in other components tests. Maybe something wrong in rendering this component.

Copy link
Member

@cnasikas cnasikas Jul 27, 2023

Choose a reason for hiding this comment

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

FWIW, we can convert it to userEvent.click(await screen.findByTestId('options-filter-popover-button-assignees')); and remove the waitFor. All flavors of find* await for the element to appear.

expect(
screen.getByText(`${MAX_ASSIGNEES_FILTER_LENGTH} filters selected`)
).toBeInTheDocument();
});

await waitForEuiPopoverOpen();

expect(
screen.getByText(
`You've selected the maximum number of ${MAX_ASSIGNEES_FILTER_LENGTH} to filter assignees`
)
).toBeInTheDocument();
js-jankisalvi marked this conversation as resolved.
Show resolved Hide resolved
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { NoMatches } from '../user_profiles/no_matches';
import { bringCurrentUserToFrontAndSort, orderAssigneesIncludingNone } from '../user_profiles/sort';
import type { AssigneesFilteringSelection } from '../user_profiles/types';
import * as i18n from './translations';
import { MAX_ASSIGNEES_FILTER_LENGTH } from '../../../common/constants';

export const NO_ASSIGNEES_VALUE = null;

Expand Down Expand Up @@ -72,6 +73,11 @@ const AssigneesFilterPopoverComponent: React.FC<AssigneesFilterPopoverProps> = (
onDebounce,
});

const limitReachedMessage = useCallback(
(limit: number) => i18n.MAX_SELECTED_FILTER(limit, 'assignees'),
[]
);

const searchResultProfiles = useMemo(() => {
const sortedUsers = bringCurrentUserToFrontAndSort(currentUserProfile, userProfiles) ?? [];

Expand Down Expand Up @@ -117,6 +123,8 @@ const AssigneesFilterPopoverComponent: React.FC<AssigneesFilterPopoverProps> = (
clearButtonLabel: i18n.CLEAR_FILTERS,
emptyMessage: <EmptyMessage />,
noMatchesMessage: !isUserTyping && !isLoadingData ? <NoMatches /> : <EmptyMessage />,
limit: MAX_ASSIGNEES_FILTER_LENGTH,
limitReachedMessage,
singleSelection: false,
nullOptionLabel: i18n.NO_ASSIGNEES,
}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import {
OWNER_INFO,
SECURITY_SOLUTION_OWNER,
OBSERVABILITY_OWNER,
MAX_TAGS_FILTER_LENGTH,
MAX_CATEGORY_FILTER_LENGTH,
} from '../../../common/constants';
import type { AppMockRenderer } from '../../common/mock';
import { createAppMockRenderer } from '../../common/mock';
Expand Down Expand Up @@ -153,6 +155,72 @@ describe('CasesTableFilters ', () => {
expect(onFilterChanged).toHaveBeenCalledWith({ tags: ['pepsi'] });
});

it('should show warning message when maximum tags selected', async () => {
const newTags = Array(MAX_TAGS_FILTER_LENGTH).fill('coke');
(useGetTags as jest.Mock).mockReturnValue({ data: newTags, isLoading: false });

const ourProps = {
...props,
initial: {
...DEFAULT_FILTER_OPTIONS,
tags: newTags,
},
};

appMockRender.render(<CasesTableFilters {...ourProps} />);

userEvent.click(screen.getByTestId('options-filter-popover-button-Tags'));

await waitForEuiPopoverOpen();

expect(screen.getByTestId('maximum-length-warning')).toBeInTheDocument();
});

it('should show warning message when tags selection reaches maximum limit', async () => {
const newTags = Array(MAX_TAGS_FILTER_LENGTH - 1).fill('coke');
const tags = [...newTags, 'pepsi'];
(useGetTags as jest.Mock).mockReturnValue({ data: tags, isLoading: false });

const ourProps = {
...props,
initial: {
...DEFAULT_FILTER_OPTIONS,
tags: newTags,
},
};

appMockRender.render(<CasesTableFilters {...ourProps} />);

userEvent.click(screen.getByTestId('options-filter-popover-button-Tags'));

await waitForEuiPopoverOpen();

userEvent.click(screen.getByTestId(`options-filter-popover-item-${tags[tags.length - 1]}`));

expect(screen.getByTestId('maximum-length-warning')).toBeInTheDocument();
});

it('should show warning message when maximum categories selected', async () => {
const newCategories = Array(MAX_CATEGORY_FILTER_LENGTH).fill('snickers');
(useGetCategories as jest.Mock).mockReturnValue({ data: newCategories, isLoading: false });

const ourProps = {
...props,
initial: {
...DEFAULT_FILTER_OPTIONS,
category: newCategories,
},
};

appMockRender.render(<CasesTableFilters {...ourProps} />);

userEvent.click(screen.getByTestId('options-filter-popover-button-Categories'));

await waitForEuiPopoverOpen();

expect(screen.getByTestId('maximum-length-warning')).toBeInTheDocument();
});

it('should remove assignee from selected assignees when assignee no longer exists', async () => {
const overrideProps = {
...props,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import styled from 'styled-components';
import { EuiFlexGroup, EuiFlexItem, EuiFieldSearch, EuiFilterGroup, EuiButton } from '@elastic/eui';

import type { CaseStatusWithAllStatus, CaseSeverityWithAll } from '../../../common/ui/types';
import { MAX_TAGS_FILTER_LENGTH, MAX_CATEGORY_FILTER_LENGTH } from '../../../common/constants';
import { StatusAll } from '../../../common/ui/types';
import { CaseStatuses } from '../../../common/api';
import type { FilterOptions } from '../../containers/types';
Expand Down Expand Up @@ -227,13 +228,20 @@ const CasesTableFiltersComponent = ({
selectedOptions={selectedTags}
options={tags}
optionsEmptyLabel={i18n.NO_TAGS_AVAILABLE}
optionsMaxLEngth={MAX_TAGS_FILTER_LENGTH}
optionsMaxLengthLabel={i18n.MAX_SELECTED_FILTER(MAX_TAGS_FILTER_LENGTH, 'tags')}
/>
<FilterPopover
buttonLabel={i18n.CATEGORIES}
onSelectedOptionsChanged={handleSelectedCategories}
selectedOptions={selectedCategories}
options={categories}
optionsEmptyLabel={i18n.NO_CATEGORIES_AVAILABLE}
optionsMaxLEngth={MAX_CATEGORY_FILTER_LENGTH}
js-jankisalvi marked this conversation as resolved.
Show resolved Hide resolved
optionsMaxLengthLabel={i18n.MAX_SELECTED_FILTER(
MAX_CATEGORY_FILTER_LENGTH,
'categories'
)}
/>
{availableSolutions.length > 1 && (
<SolutionFilter
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,12 @@ export const NO_ASSIGNEES = i18n.translate(
}
);

export const MAX_SELECTED_FILTER = (limit: number, field: string) =>
js-jankisalvi marked this conversation as resolved.
Show resolved Hide resolved
i18n.translate('xpack.cases.userProfile.maxSelectedAssigneesFilter', {
defaultMessage: "You've selected the maximum number of {count} to filter {field}",
values: { count: limit, field },
});

export const SHOW_LESS = i18n.translate('xpack.cases.allCasesView.showLessAvatars', {
defaultMessage: 'show less',
});
Expand Down
js-jankisalvi marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@

import React from 'react';
import { waitForEuiPopoverOpen } from '@elastic/eui/lib/test/rtl';
import userEvent from '@testing-library/user-event';

import type { AppMockRenderer } from '../../common/mock';
import { createAppMockRenderer } from '../../common/mock';

import { FilterPopover } from '.';
import userEvent from '@testing-library/user-event';

describe('FilterPopover ', () => {
let appMockRender: AppMockRenderer;
Expand Down Expand Up @@ -110,4 +110,90 @@ describe('FilterPopover ', () => {

expect(onSelectedOptionsChanged).toHaveBeenCalledWith([]);
});

describe('maximum limit', () => {
const newTags = ['coke', 'pepsi', 'sprite', 'juice', 'water'];
const maxLength = 3;
const maxLengthLabel = `You have selected maximum number of ${maxLength} tags to filter`;

it('should show message when maximum options are selected', async () => {
const { getByTestId } = appMockRender.render(
<FilterPopover
buttonLabel={'Tags'}
onSelectedOptionsChanged={onSelectedOptionsChanged}
selectedOptions={[...newTags.slice(0, 3)]}
options={newTags}
optionsMaxLEngth={maxLength}
optionsMaxLengthLabel={maxLengthLabel}
/>
);

userEvent.click(getByTestId('options-filter-popover-button-Tags'));

await waitForEuiPopoverOpen();

expect(getByTestId('maximum-length-warning')).toHaveTextContent(maxLengthLabel);

expect(getByTestId(`options-filter-popover-item-${newTags[4]}`)).toHaveProperty('disabled');
js-jankisalvi marked this conversation as resolved.
Show resolved Hide resolved
});

it('should not show message when maximum length label is missing', async () => {
const { getByTestId, queryByTestId } = appMockRender.render(
<FilterPopover
buttonLabel={'Tags'}
onSelectedOptionsChanged={onSelectedOptionsChanged}
selectedOptions={[newTags[0], newTags[2]]}
options={newTags}
optionsMaxLEngth={maxLength}
/>
);

userEvent.click(getByTestId('options-filter-popover-button-Tags'));

await waitForEuiPopoverOpen();

expect(queryByTestId('maximum-length-warning')).not.toBeInTheDocument();
expect(getByTestId(`options-filter-popover-item-${newTags[4]}`)).toHaveProperty('disabled');
});

it('should not show message and disable options when maximum length property is missing', async () => {
const { getByTestId, queryByTestId } = appMockRender.render(
<FilterPopover
buttonLabel={'Tags'}
onSelectedOptionsChanged={onSelectedOptionsChanged}
selectedOptions={[newTags[0], newTags[2]]}
options={newTags}
js-jankisalvi marked this conversation as resolved.
Show resolved Hide resolved
/>
);

userEvent.click(getByTestId('options-filter-popover-button-Tags'));

await waitForEuiPopoverOpen();

expect(queryByTestId('maximum-length-warning')).not.toBeInTheDocument();
expect(getByTestId(`options-filter-popover-item-${newTags[4]}`)).toHaveProperty(
'disabled',
false
);
});

it('should allow to select more options when maximum length property is missing', async () => {
const { getByTestId } = appMockRender.render(
<FilterPopover
buttonLabel={'Tags'}
onSelectedOptionsChanged={onSelectedOptionsChanged}
selectedOptions={[newTags[0], newTags[2]]}
options={newTags}
/>
);

userEvent.click(getByTestId('options-filter-popover-button-Tags'));

await waitForEuiPopoverOpen();

userEvent.click(getByTestId(`options-filter-popover-item-${newTags[1]}`));

expect(onSelectedOptionsChanged).toHaveBeenCalledWith([newTags[0], newTags[2], newTags[1]]);
});
});
});
26 changes: 26 additions & 0 deletions x-pack/plugins/cases/public/components/filter_popover/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ interface FilterPopoverProps {
onSelectedOptionsChanged: (value: string[]) => void;
options: string[];
optionsEmptyLabel?: string;
optionsMaxLEngth?: number;
optionsMaxLengthLabel?: string;
selectedOptions: string[];
}

Expand All @@ -30,6 +32,12 @@ const ScrollableDiv = styled.div`
overflow: auto;
`;

const WarningPanel = styled(EuiPanel)`
${({ theme }) => `
border: 1px solid ${theme.eui.euiColorLightShade};
`}
`;

const toggleSelectedGroup = (group: string, selectedGroups: string[]): string[] => {
const selectedGroupIndex = selectedGroups.indexOf(group);
if (selectedGroupIndex >= 0) {
Expand All @@ -56,6 +64,8 @@ export const FilterPopoverComponent = ({
options,
optionsEmptyLabel,
selectedOptions,
optionsMaxLEngth,
optionsMaxLengthLabel,
}: FilterPopoverProps) => {
const [isPopoverOpen, setIsPopoverOpen] = useState(false);

Expand Down Expand Up @@ -87,10 +97,26 @@ export const FilterPopoverComponent = ({
panelPaddingSize="none"
repositionOnScroll
>
{optionsMaxLEngth && optionsMaxLengthLabel && selectedOptions.length >= optionsMaxLEngth && (
js-jankisalvi marked this conversation as resolved.
Show resolved Hide resolved
<EuiFlexGroup gutterSize="xs">
<EuiFlexItem grow={true}>
<WarningPanel color="warning" paddingSize="s">
<EuiText size="s" color="warning" data-test-subj="maximum-length-warning">
{optionsMaxLengthLabel}
</EuiText>
</WarningPanel>
</EuiFlexItem>
</EuiFlexGroup>
js-jankisalvi marked this conversation as resolved.
Show resolved Hide resolved
)}
<ScrollableDiv>
{options.map((option, index) => (
<EuiFilterSelectItem
checked={selectedOptions.includes(option) ? 'on' : undefined}
disabled={Boolean(
optionsMaxLEngth &&
selectedOptions.length >= optionsMaxLEngth &&
!selectedOptions.includes(option)
)}
data-test-subj={`options-filter-popover-item-${option}`}
key={`${index}-${option}`}
onClick={toggleSelectedGroupCb.bind(null, option)}
Expand Down