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.

Sign up for GitHub

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

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
95d5765
add validation fir max filter length for assignees and tags
js-jankisalvi Jul 20, 2023
5d0cf94
Merge remote-tracking branch 'upstream/main' into ui-validation-for-f…
js-jankisalvi Jul 24, 2023
4e9ea30
add tests, category validation
js-jankisalvi Jul 24, 2023
7994b6f
Added border to warning
js-jankisalvi Jul 24, 2023
5c1d653
Merge branch 'main' into ui-validation-for-filters
js-jankisalvi Jul 24, 2023
ae951cd
PR feedback
js-jankisalvi Jul 25, 2023
277eb31
Merge branch 'main' into ui-validation-for-filters
js-jankisalvi Jul 25, 2023
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
31 changes: 30 additions & 1 deletion x-pack/plugins/cases/public/components/all_cases/assignees_filter.test.tsx
View file
Open in desktop
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,31 @@ 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 •
edited
Loading

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} assignees`
)
).toBeInTheDocument();
js-jankisalvi marked this conversation as resolved.
Show resolved Hide resolved

expect(screen.getByTitle('No assignees')).toHaveAttribute('aria-selected', 'false');
expect(screen.getByTitle('No assignees')).toHaveAttribute('aria-disabled', 'true');
});
});
8 changes: 8 additions & 0 deletions x-pack/plugins/cases/public/components/all_cases/assignees_filter.tsx
View file
Open in desktop
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
93 changes: 93 additions & 0 deletions x-pack/plugins/cases/public/components/all_cases/table_filters.test.tsx
View file
Open in desktop
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,97 @@ 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 not show warning message when one of the tags deselected after reaching the limit', 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();

userEvent.click(screen.getAllByTestId(`options-filter-popover-item-${newTags[0]}`)[0]);

expect(screen.queryByTestId('maximum-length-warning')).not.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
5 changes: 5 additions & 0 deletions x-pack/plugins/cases/public/components/all_cases/table_filters.tsx
View file
Open in desktop
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,17 @@ const CasesTableFiltersComponent = ({
selectedOptions={selectedTags}
options={tags}
optionsEmptyLabel={i18n.NO_TAGS_AVAILABLE}
limit={MAX_TAGS_FILTER_LENGTH}
limitReachedMessage={i18n.MAX_SELECTED_FILTER(MAX_TAGS_FILTER_LENGTH, 'tags')}
/>
<FilterPopover
buttonLabel={i18n.CATEGORIES}
onSelectedOptionsChanged={handleSelectedCategories}
selectedOptions={selectedCategories}
options={categories}
optionsEmptyLabel={i18n.NO_CATEGORIES_AVAILABLE}
limit={MAX_CATEGORY_FILTER_LENGTH}
limitReachedMessage={i18n.MAX_SELECTED_FILTER(MAX_CATEGORY_FILTER_LENGTH, 'categories')}
/>
{availableSolutions.length > 1 && (
<SolutionFilter
Expand Down
6 changes: 6 additions & 0 deletions x-pack/plugins/cases/public/components/all_cases/translations.ts
View file
Open in desktop
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 = (count: number, field: string) =>
i18n.translate('xpack.cases.userProfile.maxSelectedAssigneesFilter', {
defaultMessage: "You've selected the maximum number of {count} {field}",
values: { count, field },
});

export const SHOW_LESS = i18n.translate('xpack.cases.allCasesView.showLessAvatars', {
defaultMessage: 'show less',
});
Expand Down
91 changes: 90 additions & 1 deletion x-pack/plugins/cases/public/components/filter_popover/index.test.tsx
View file
Open in desktop
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,93 @@ 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}
limit={maxLength}
limitReachedMessage={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[3]}`)).toHaveProperty('disabled');
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}
limit={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[3]}`)).toHaveProperty('disabled');
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
limitReachedMessage={maxLengthLabel}
/>
);

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]]);
});
});
});
21 changes: 21 additions & 0 deletions x-pack/plugins/cases/public/components/filter_popover/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@

import React, { useCallback, useState } from 'react';
import {
EuiCallOut,
EuiFilterButton,
EuiFilterSelectItem,
EuiFlexGroup,
EuiFlexItem,
EuiHorizontalRule,
EuiPanel,
EuiPopover,
EuiText,
Expand All @@ -22,6 +24,8 @@ interface FilterPopoverProps {
onSelectedOptionsChanged: (value: string[]) => void;
options: string[];
optionsEmptyLabel?: string;
limit?: number;
limitReachedMessage?: string;
selectedOptions: string[];
}

Expand Down Expand Up @@ -56,6 +60,8 @@ export const FilterPopoverComponent = ({
options,
optionsEmptyLabel,
selectedOptions,
limit,
limitReachedMessage,
}: FilterPopoverProps) => {
const [isPopoverOpen, setIsPopoverOpen] = useState(false);

Expand Down Expand Up @@ -87,10 +93,25 @@ export const FilterPopoverComponent = ({
panelPaddingSize="none"
repositionOnScroll
>
{limit && limitReachedMessage && selectedOptions.length >= limit ? (
<>
<EuiHorizontalRule margin="none" />
<EuiCallOut
title={limitReachedMessage}
color="warning"
size="s"
data-test-subj="maximum-length-warning"
/>
<EuiHorizontalRule margin="none" />
</>
) : null}
<ScrollableDiv>
{options.map((option, index) => (
<EuiFilterSelectItem
checked={selectedOptions.includes(option) ? 'on' : undefined}
disabled={Boolean(
limit && selectedOptions.length >= limit && !selectedOptions.includes(option)
)}
data-test-subj={`options-filter-popover-item-${option}`}
key={`${index}-${option}`}
onClick={toggleSelectedGroupCb.bind(null, option)}
Expand Down