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] refactor useGetCases to use react-query #133505

Merged
merged 22 commits into from
Jun 8, 2022
Merged

[Cases] refactor useGetCases to use react-query #133505

Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
5691e40
refactor usetags to use react-query
Jun 3, 2022
181a3b5
rename hook to useGetTags
Jun 3, 2022
9280332
Remove caseId parameter from useUpdateField
Jun 3, 2022
9cd7cb6
remove dispatchUpdateCaseProperty from getCases hook
Jun 3, 2022
b63fc1f
Merge remote-tracking branch 'upstream/main' into refactor/create-for…
Jun 3, 2022
08a17a0
Merge remote-tracking branch 'origin/refactor/create-form-hooks-p2' i…
Jun 3, 2022
9c83ab4
WIP: fixing tests
Jun 3, 2022
8ed85c8
Merge remote-tracking branch 'upstream/main' into refactor/create-for…
Jun 7, 2022
c02acac
Update broken tests
Jun 7, 2022
4b5dab6
Fix tests
Jun 7, 2022
74cbacc
Merge remote-tracking branch 'upstream/main' into refactor/create-for…
Jun 7, 2022
82bca36
Fix possible problem with empty prefixed key
Jun 7, 2022
c8ee55a
invalidate tags cache view when refreshing case view pag
Jun 7, 2022
b63e63b
Merge remote-tracking branch 'upstream/main' into refactor/create-for…
Jun 7, 2022
9e6c917
Fix jest tests timing out
Jun 7, 2022
1c91720
fix broken test
Jun 7, 2022
2453d45
remove unused variable
Jun 7, 2022
20df8b9
Merge remote-tracking branch 'origin/refactor/create-form-hooks-p2' i…
Jun 7, 2022
c35f5e2
Merge remote-tracking branch 'upstream/main' into refactor/create-for…
Jun 8, 2022
15e167a
rename hook to previous name
Jun 8, 2022
64abe42
Use previous params
Jun 8, 2022
24ee910
do not check for data length to show loading bar
Jun 8, 2022
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
263 changes: 101 additions & 162 deletions x-pack/plugins/cases/public/components/all_cases/all_cases_list.test.tsx
View file
Open in desktop

Large diffs are not rendered by default.

56 changes: 31 additions & 25 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 @@ -14,10 +14,11 @@ import {
Case,
CaseStatusWithAllStatus,
FilterOptions,
QueryParams,
SortFieldCase,
StatusAll,
} from '../../../common/ui/types';
import { CaseStatuses, caseStatuses } from '../../../common/api';
import { useGetCases } from '../../containers/use_get_cases';

import { useAvailableCasesOwners } from '../app/use_available_owners';
import { useCasesColumns } from './columns';
Expand All @@ -28,6 +29,12 @@ import { CasesTable } from './table';
import { useCasesContext } from '../cases_context/use_cases_context';
import { CasesMetrics } from './cases_metrics';
import { useGetConnectors } from '../../containers/configure/use_connectors';
import {
DEFAULT_FILTER_OPTIONS,
DEFAULT_QUERY_PARAMS,
initialData,
useGetCases,
} from '../../containers/use_get_cases';

const ProgressLoader = styled(EuiProgress)`
${({ $isShow }: { $isShow: boolean }) =>
Expand Down Expand Up @@ -65,19 +72,21 @@ export const AllCasesList = React.memo<AllCasesListProps>(
...(!isEmpty(hiddenStatuses) && firstAvailableStatus && { status: firstAvailableStatus }),
owner: hasOwner ? owner : availableSolutions,
};
const [filterOptions, setFilterOptions] = useState<FilterOptions>({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the second logic change in the PR. the previous hook was working both as a fetch data hook and state management. I removed the state management part from it and move it here.

...DEFAULT_FILTER_OPTIONS,
...initialFilterOptions,
});
const [queryParams, setQueryParams] = useState<QueryParams>(DEFAULT_QUERY_PARAMS);
const [selectedCases, setSelectedCases] = useState<Case[]>([]);

const {
data,
dispatchUpdateCaseProperty,
data = initialData,
isFetching: isLoadingCases,
refetch: refetchCases,
} = useGetCases({
filterOptions,
loading,
queryParams,
selectedCases,
refetchCases,
setFilters,
setQueryParams,
setSelectedCases,
} = useGetCases({ initialFilterOptions });
});

const { data: connectors = [] } = useGetConnectors();

Expand Down Expand Up @@ -147,30 +156,28 @@ export const AllCasesList = React.memo<AllCasesListProps>(
const onFilterChangedCallback = useCallback(
(newFilterOptions: Partial<FilterOptions>) => {
if (newFilterOptions.status && newFilterOptions.status === CaseStatuses.closed) {
setQueryParams({ sortField: SortFieldCase.closedAt });
setQueryParams({ ...DEFAULT_QUERY_PARAMS, sortField: SortFieldCase.closedAt });
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we use the previous state?

setQueryParams((prev) => ({ ...prev, sortField: SortFieldCase.closedAt }))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct! thanks :D

} else if (newFilterOptions.status && newFilterOptions.status === CaseStatuses.open) {
setQueryParams({ sortField: SortFieldCase.createdAt });
setQueryParams({ ...DEFAULT_QUERY_PARAMS, sortField: SortFieldCase.createdAt });
} else if (
newFilterOptions.status &&
newFilterOptions.status === CaseStatuses['in-progress']
) {
setQueryParams({ sortField: SortFieldCase.createdAt });
setQueryParams({ ...DEFAULT_QUERY_PARAMS, sortField: SortFieldCase.createdAt });
}

deselectCases();
setFilters(newFilterOptions);
setFilterOptions({ ...DEFAULT_FILTER_OPTIONS, ...newFilterOptions });
refreshCases(false);
},
[deselectCases, setFilters, refreshCases, setQueryParams]
[deselectCases, setFilterOptions, refreshCases, setQueryParams]
);

const showActions = userCanCrud && !isSelectorView;

const columns = useCasesColumns({
dispatchUpdateCaseProperty,
filterStatus: filterOptions.status,
filterStatus: filterOptions.status ?? StatusAll,
handleIsLoading,
isLoadingCases: loading,
refreshCases,
isSelectorView,
userCanCrud,
Expand All @@ -181,9 +188,9 @@ export const AllCasesList = React.memo<AllCasesListProps>(

const pagination = useMemo(
() => ({
pageIndex: queryParams.page - 1,
pageSize: queryParams.perPage,
totalItemCount: data.total,
pageIndex: (queryParams?.page ?? DEFAULT_QUERY_PARAMS.page) - 1,
pageSize: queryParams?.perPage ?? DEFAULT_QUERY_PARAMS.perPage,
totalItemCount: data.total ?? 0,
pageSizeOptions: [5, 10, 15, 20, 25],
}),
[data, queryParams]
Expand All @@ -196,7 +203,6 @@ export const AllCasesList = React.memo<AllCasesListProps>(
}),
[selectedCases, setSelectedCases]
);
const isCasesLoading = useMemo(() => loading.indexOf('cases') > -1, [loading]);
const isDataEmpty = useMemo(() => data.total === 0, [data]);

const tableRowProps = useCallback(
Expand All @@ -212,7 +218,7 @@ export const AllCasesList = React.memo<AllCasesListProps>(
size="xs"
color="accent"
className="essentialAnimation"
$isShow={(isCasesLoading || isLoading) && !isDataEmpty}
$isShow={isLoading || isLoadingCases}
/>
{!isSelectorView ? <CasesMetrics refresh={refresh} /> : null}
<CasesTableFilters
Expand Down Expand Up @@ -240,8 +246,8 @@ export const AllCasesList = React.memo<AllCasesListProps>(
filterOptions={filterOptions}
goToCreateCase={onRowClick}
handleIsLoading={handleIsLoading}
isCasesLoading={isCasesLoading}
isCommentUpdating={isCasesLoading}
isCasesLoading={isLoadingCases && data.cases.length === 0}
Copy link
Member

Choose a reason for hiding this comment

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

What will happen if the are no cases in the response?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This actually a mistake. I removed the length check.

isCommentUpdating={isLoadingCases}
isDataEmpty={isDataEmpty}
isSelectorView={isSelectorView}
onChange={tableOnChangeCallback}
Expand Down
31 changes: 15 additions & 16 deletions x-pack/plugins/cases/public/components/all_cases/columns.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import {
import { RIGHT_ALIGNMENT } from '@elastic/eui/lib/services';
import styled from 'styled-components';

import { Case, DeleteCase } from '../../../common/ui/types';
import { Case, DeleteCase, UpdateByKey } from '../../../common/ui/types';
import { CaseStatuses, ActionConnector, CaseSeverity } from '../../../common/api';
import { OWNER_INFO } from '../../../common/constants';
import { getEmptyTagValue } from '../empty_value';
Expand All @@ -33,7 +33,6 @@ import { CaseDetailsLink } from '../links';
import * as i18n from './translations';
import { ALERTS } from '../../common/translations';
import { getActions } from './actions';
import { UpdateCase } from '../../containers/use_get_cases';
import { useDeleteCases } from '../../containers/use_delete_cases';
import { ConfirmDeleteCaseModal } from '../confirm_delete_case';
import { useApplicationCapabilities, useKibana } from '../../common/lib/kibana';
Expand All @@ -43,6 +42,7 @@ import { getConnectorIcon } from '../utils';
import type { CasesOwners } from '../../client/helpers/can_use_cases';
import { useCasesFeatures } from '../cases_context/use_cases_features';
import { severities } from '../severity/config';
import { useUpdateCase } from '../../containers/use_update_case';

export type CasesColumns =
| EuiTableActionsColumnType<Case>
Expand All @@ -57,10 +57,8 @@ const renderStringField = (field: string, dataTestSubj: string) =>
field != null ? <span data-test-subj={dataTestSubj}>{field}</span> : getEmptyTagValue();

export interface GetCasesColumn {
dispatchUpdateCaseProperty: (u: UpdateCase) => void;
filterStatus: string;
handleIsLoading: (a: boolean) => void;
isLoadingCases: string[];
refreshCases?: (a?: boolean) => void;
isSelectorView: boolean;
userCanCrud: boolean;
Expand All @@ -70,10 +68,8 @@ export interface GetCasesColumn {
showSolutionColumn?: boolean;
}
export const useCasesColumns = ({
dispatchUpdateCaseProperty,
filterStatus,
handleIsLoading,
isLoadingCases,
refreshCases,
isSelectorView,
userCanCrud,
Expand All @@ -98,6 +94,8 @@ export const useCasesColumns = ({
title: '',
});

const { updateCaseProperty, isLoading: isLoadingUpdateCase } = useUpdateCase();

const toggleDeleteModal = useCallback(
(deleteCase: Case) => {
handleToggleModal();
Expand All @@ -107,15 +105,17 @@ export const useCasesColumns = ({
);

const handleDispatchUpdate = useCallback(
(args: Omit<UpdateCase, 'refetchCasesStatus'>) => {
dispatchUpdateCaseProperty({
...args,
refetchCasesStatus: () => {
({ updateKey, updateValue, caseData }: UpdateByKey) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is one of the main logic-wise changes in this PR. The previous useGetCases hook was providing a "sub-hook" to update a case and I had to remove that.

Instead I reused the existing updateCaseproperty hook.

updateCaseProperty({
updateKey,
updateValue,
caseData,
onSuccess: () => {
if (refreshCases != null) refreshCases();
},
});
},
[dispatchUpdateCaseProperty, refreshCases]
[refreshCases, updateCaseProperty]
);

const actions = useMemo(
Expand All @@ -136,8 +136,8 @@ export const useCasesColumns = ({
);

useEffect(() => {
handleIsLoading(isDeleting || isLoadingCases.indexOf('caseUpdate') > -1);
}, [handleIsLoading, isDeleting, isLoadingCases]);
handleIsLoading(isDeleting || isLoadingUpdateCase);
}, [handleIsLoading, isDeleting, isLoadingUpdateCase]);

useEffect(() => {
if (isDeleted) {
Expand Down Expand Up @@ -319,13 +319,12 @@ export const useCasesColumns = ({
return (
<StatusContextMenu
currentStatus={theCase.status}
disabled={!userCanCrud || isLoadingCases.length > 0}
disabled={!userCanCrud || isLoadingUpdateCase}
onStatusChanged={(status) =>
handleDispatchUpdate({
updateKey: 'status',
updateValue: status,
caseId: theCase.id,
version: theCase.version,
caseData: theCase,
})
}
/>
Expand Down
6 changes: 1 addition & 5 deletions x-pack/plugins/cases/public/components/all_cases/index.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,11 @@ import { AllCases } from '.';
import { TestProviders } from '../../common/mock';
import { useGetReporters } from '../../containers/use_get_reporters';
import { useGetActionLicense } from '../../containers/use_get_action_license';
import { CaseStatuses } from '../../../common/api';
import { casesStatus, connectorsMock, useGetCasesMockState } from '../../containers/mock';
import { useGetCases } from '../../containers/use_get_cases';
import { useGetCasesStatus } from '../../containers/use_get_cases_status';
import { useGetConnectors } from '../../containers/configure/use_connectors';
import { useGetTags } from '../../containers/use_get_tags';
import { useGetCases } from '../../containers/use_get_cases';

jest.mock('../../containers/use_get_reporters');
jest.mock('../../containers/use_get_tags');
Expand All @@ -38,7 +37,6 @@ const useGetCasesStatusMock = useGetCasesStatus as jest.Mock;
const useGetActionLicenseMock = useGetActionLicense as jest.Mock;

describe('AllCases', () => {
const dispatchUpdateCaseProperty = jest.fn();
const refetchCases = jest.fn();
const setFilters = jest.fn();
const setQueryParams = jest.fn();
Expand All @@ -47,7 +45,6 @@ describe('AllCases', () => {

const defaultGetCases = {
...useGetCasesMockState,
dispatchUpdateCaseProperty,
refetchCases,
setFilters,
setQueryParams,
Expand Down Expand Up @@ -89,7 +86,6 @@ describe('AllCases', () => {
it('should render the stats', async () => {
useGetCasesMock.mockReturnValue({
...defaultGetCases,
filterOptions: { ...defaultGetCases.filterOptions, status: CaseStatuses.closed },
});

const wrapper = mount(
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/cases/public/components/case_view/use_on_update_field.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { getTypedPayload } from '../../containers/utils';
import { OnUpdateFields } from './types';

export const useOnUpdateField = ({ caseData, caseId }: { caseData: Case; caseId: string }) => {
const { isLoading, updateKey: loadingKey, updateCaseProperty } = useUpdateCase({ caseId });
const { isLoading, updateKey: loadingKey, updateCaseProperty } = useUpdateCase();

const onUpdateField = useCallback(
({ key, value, onSuccess, onError }: OnUpdateFields) => {
Expand Down
Loading