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] Table Solution Filter Not Rendering Any Checked Option When All Selected #172460

Merged
merged 8 commits into from
Dec 13, 2023
Merged

[Cases] Table Solution Filter Not Rendering Any Checked Option When All Selected #172460

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
396e695
first commit
jcger Dec 4, 2023
8f27552
Merge branch 'main' of github.com:jcger/kibana into issue-172454
jcger Dec 5, 2023
39925ed
add api test
jcger Dec 5, 2023
6b8d832
test + remove not needed stuff
jcger Dec 5, 2023
d803ad3
Merge branch 'main' of github.com:jcger/kibana into issue-172454
jcger Dec 7, 2023
6ed1faa
Merge branch 'main' into issue-172454
cnasikas Dec 12, 2023
6442558
Move owner logic inside the useGetCases hook
cnasikas Dec 12, 2023
cf70ea6
Fix tests
cnasikas Dec 12, 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
102 changes: 0 additions & 102 deletions x-pack/plugins/cases/public/components/all_cases/all_cases_list.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,6 @@ describe('AllCasesListGeneric', () => {
filterOptions: {
...DEFAULT_FILTER_OPTIONS,
searchFields: ['title', 'description'],
owner: ['securitySolution'],
category: ['twix'],
},
queryParams: DEFAULT_QUERY_PARAMS,
Expand Down Expand Up @@ -645,82 +644,6 @@ describe('AllCasesListGeneric', () => {
});

describe('Solutions', () => {
it('should set the owner to all available solutions when deselecting all solutions', async () => {
Copy link
Member

Choose a reason for hiding this comment

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

The logic and the test moved to the useGetCases hook.

const { getByTestId } = render(
<TestProviders owner={[]}>
<AllCasesList />
</TestProviders>
);

expect(useGetCasesMock).toHaveBeenCalledWith({
filterOptions: {
search: '',
searchFields: ['title', 'description'],
severity: [],
reporters: [],
status: [],
tags: [],
assignees: [],
owner: ['securitySolution', 'observability'],
category: [],
customFields: {},
},
queryParams: DEFAULT_QUERY_PARAMS,
});

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

await waitForEuiPopoverOpen();

userEvent.click(
getByTestId(`options-filter-popover-item-${SECURITY_SOLUTION_OWNER}`),
undefined,
{
skipPointerEventsCheck: true,
}
);

expect(useGetCasesMock).toBeCalledWith({
filterOptions: {
search: '',
searchFields: ['title', 'description'],
severity: [],
reporters: [],
status: [],
tags: [],
assignees: [],
owner: ['securitySolution'],
category: [],
customFields: {},
},
queryParams: DEFAULT_QUERY_PARAMS,
});

userEvent.click(
getByTestId(`options-filter-popover-item-${SECURITY_SOLUTION_OWNER}`),
undefined,
{
skipPointerEventsCheck: true,
}
);

expect(useGetCasesMock).toHaveBeenLastCalledWith({
filterOptions: {
search: '',
searchFields: ['title', 'description'],
severity: [],
reporters: [],
status: [],
tags: [],
assignees: [],
owner: ['securitySolution', 'observability'],
category: [],
customFields: {},
},
queryParams: DEFAULT_QUERY_PARAMS,
});
});

it('should hide the solutions filter if the owner is provided', async () => {
const { queryByTestId } = render(
<TestProviders owner={[SECURITY_SOLUTION_OWNER]}>
Expand All @@ -730,30 +653,6 @@ describe('AllCasesListGeneric', () => {

expect(queryByTestId('options-filter-popover-button-owner')).toBeFalsy();
});

it('should call useGetCases with the correct owner on initial render', async () => {
render(
<TestProviders owner={[SECURITY_SOLUTION_OWNER]}>
<AllCasesList />
</TestProviders>
);

expect(useGetCasesMock).toHaveBeenCalledWith({
filterOptions: {
search: '',
searchFields: ['title', 'description'],
severity: [],
reporters: [],
status: [],
tags: [],
assignees: [],
owner: ['securitySolution'],
category: [],
customFields: {},
},
queryParams: DEFAULT_QUERY_PARAMS,
});
});
});

describe('Actions', () => {
Expand Down Expand Up @@ -1102,7 +1001,6 @@ describe('AllCasesListGeneric', () => {
expect(useGetCasesMock).toHaveBeenLastCalledWith({
filterOptions: {
...DEFAULT_FILTER_OPTIONS,
owner: [SECURITY_SOLUTION_OWNER],
assignees: [],
},
queryParams: DEFAULT_QUERY_PARAMS,
Expand Down
3 changes: 1 addition & 2 deletions x-pack/plugins/cases/public/components/all_cases/all_cases_list.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,10 @@ export const AllCasesList = React.memo<AllCasesListProps>(
const isLoading = useIsLoadingCases();

const hasOwner = !!owner.length;

const firstAvailableStatus = head(difference(caseStatuses, hiddenStatuses));
const initialFilterOptions = {
...(!isEmpty(hiddenStatuses) && firstAvailableStatus && { status: [firstAvailableStatus] }),
owner: hasOwner ? owner : availableSolutions,
Copy link
Member

Choose a reason for hiding this comment

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

Logic moved inside the useGetCases hook.

};

const { queryParams, setQueryParams, filterOptions, setFilterOptions } = useAllCasesState(
Expand Down Expand Up @@ -210,7 +210,6 @@ export const AllCasesList = React.memo<AllCasesListProps>(
availableSolutions={hasOwner ? [] : availableSolutions}
hiddenStatuses={hiddenStatuses}
onCreateCasePressed={onCreateCasePressed}
initialFilterOptions={initialFilterOptions}
isSelectorView={isSelectorView}
isLoading={isLoadingCurrentUserProfile}
currentUserProfile={currentUserProfile}
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/cases/public/components/all_cases/solution_filter.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ describe('SolutionFilter ', () => {

expect(onChange).toHaveBeenCalledWith({
filterId: 'owner',
selectedOptionKeys: [solutions[0]],
selectedOptionKeys: [],
});
});
});
Expand Down Expand Up @@ -168,7 +168,7 @@ describe('SolutionFilter ', () => {

expect(onChange).toHaveBeenCalledWith({
filterId: 'owner',
selectedOptionKeys: [solutions[0], solutions[1]],
selectedOptionKeys: [],
});
});
});
Expand Down
35 changes: 2 additions & 33 deletions x-pack/plugins/cases/public/components/all_cases/solution_filter.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,37 +43,6 @@ export const SolutionFilterComponent = ({
const options = mapToMultiSelectOption(hasOwner ? owner : availableSolutions);
const solutions = availableSolutions.map((solution) => mapToReadableSolutionName(solution));

/**
* If the user selects and deselects all solutions then the owner is set to an empty array.
* This results in fetching all cases the user has access to including
* the ones with read access. We want to show only the cases the user has full access to.
* For that reason we fallback to availableSolutions if the owner is empty.
*
* If the consumer of cases has passed an owner we fallback to the provided owner
*/
const _onChange = ({
filterId,
selectedOptionKeys: newOptions,
}: {
filterId: string;
selectedOptionKeys: string[];
}) => {
if (hasOwner) {
onChange({
filterId,
selectedOptionKeys: newOptions.length === 0 ? owner : newOptions,
});
} else {
onChange({
filterId,
selectedOptionKeys: newOptions.length === 0 ? availableSolutions : newOptions,
});
}
};

const selectedOptionsInFilter =
selectedOptionKeys.length === availableSolutions.length ? [] : selectedOptionKeys;

const renderOption = (option: EuiSelectableOption) => {
const solution = solutions.find((solutionData) => solutionData.id === option.label) as Solution;
return (
Expand All @@ -90,10 +59,10 @@ export const SolutionFilterComponent = ({
<MultiSelectFilter
buttonLabel={i18n.SOLUTION}
id={'owner'}
onChange={_onChange}
onChange={onChange}
options={options}
renderOption={renderOption}
selectedOptionKeys={selectedOptionsInFilter}
selectedOptionKeys={selectedOptionKeys}
/>
);
};
Expand Down
17 changes: 6 additions & 11 deletions ...lugins/cases/public/components/all_cases/table_filter_config/use_system_filter_config.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

import React from 'react';

import type { FilterOptions } from '../../../../common/ui';
import type { CaseStatuses } from '../../../../common/types/domain';
import { MAX_TAGS_FILTER_LENGTH, MAX_CATEGORY_FILTER_LENGTH } from '../../../../common/constants';
import { MultiSelectFilter, mapToMultiSelectOption } from '../multi_select_filter';
Expand All @@ -28,7 +27,6 @@ interface UseFilterConfigProps {
countOpenCases: number | null;
currentUserProfile: CurrentUserProfile;
hiddenStatuses?: CaseStatuses[];
initialFilterOptions: Partial<FilterOptions>;
isLoading: boolean;
isSelectorView?: boolean;
onFilterOptionsChange: FilterChangeHandler;
Expand All @@ -44,7 +42,6 @@ export const getSystemFilterConfig = ({
countOpenCases,
currentUserProfile,
hiddenStatuses,
initialFilterOptions,
isLoading,
isSelectorView,
onFilterOptionsChange,
Expand All @@ -69,7 +66,7 @@ export const getSystemFilterConfig = ({
isAvailable: true,
getEmptyOptions: () => {
return {
severity: initialFilterOptions.severity || [],
severity: [],
};
},
render: ({ filterOptions }: FilterConfigRenderParams) => (
Expand All @@ -86,7 +83,7 @@ export const getSystemFilterConfig = ({
isAvailable: true,
getEmptyOptions: () => {
return {
status: initialFilterOptions.status || [],
status: [],
};
},
render: ({ filterOptions }: FilterConfigRenderParams) => (
Expand All @@ -107,7 +104,7 @@ export const getSystemFilterConfig = ({
isAvailable: caseAssignmentAuthorized && !isSelectorView,
getEmptyOptions: () => {
return {
assignees: initialFilterOptions.assignees || [],
assignees: [],
};
},
render: ({ filterOptions }: FilterConfigRenderParams) => {
Expand All @@ -128,7 +125,7 @@ export const getSystemFilterConfig = ({
isAvailable: true,
getEmptyOptions: () => {
return {
tags: initialFilterOptions.tags || [],
tags: [],
};
},
render: ({ filterOptions }: FilterConfigRenderParams) => (
Expand All @@ -150,7 +147,7 @@ export const getSystemFilterConfig = ({
isAvailable: true,
getEmptyOptions: () => {
return {
category: initialFilterOptions.category || [],
category: [],
};
},
render: ({ filterOptions }: FilterConfigRenderParams) => (
Expand All @@ -172,7 +169,7 @@ export const getSystemFilterConfig = ({
isAvailable: availableSolutions.length > 1,
getEmptyOptions: () => {
return {
owner: initialFilterOptions.owner || [],
owner: [],
};
},
render: ({ filterOptions }: FilterConfigRenderParams) => (
Expand All @@ -195,7 +192,6 @@ export const useSystemFilterConfig = ({
countOpenCases,
currentUserProfile,
hiddenStatuses,
initialFilterOptions,
isLoading,
isSelectorView,
onFilterOptionsChange,
Expand All @@ -210,7 +206,6 @@ export const useSystemFilterConfig = ({
countOpenCases,
currentUserProfile,
hiddenStatuses,
initialFilterOptions,
isLoading,
isSelectorView,
onFilterOptionsChange,
Expand Down
Loading
Loading