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

Jump to bottom

[Cases] Convert remaining hooks to React Query #157254

Merged
merged 21 commits into from
May 12, 2023
Merged

[Cases] Convert remaining hooks to React Query #157254

Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
615a0fd
Convert useCreateAttachments
cnasikas May 10, 2023
5b96c98
Convert usePostPushToService
cnasikas May 10, 2023
42ec0ab
Convert useUpdateCase
cnasikas May 10, 2023
b41526b
Convert useUpdateComment
cnasikas May 10, 2023
7f5f203
Fix tests
cnasikas May 10, 2023
81e4cee
Fixes
cnasikas May 10, 2023
55375b3
Add query client to the cases context
cnasikas May 10, 2023
e10b9b7
Fix types
cnasikas May 10, 2023
362a6a1
Merge branch 'main' into react_query_hooks
kibanamachine May 10, 2023
03e46c3
Merge branch 'main' into react_query_hooks
cnasikas May 11, 2023
27b08a4
Fix cypress tests
cnasikas May 11, 2023
b2f1105
Merge branch 'react_query_hooks' of github.com:cnasikas/kibana into r…
cnasikas May 11, 2023
e5dc03a
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine May 11, 2023
e431f0d
Merge branch 'main' into react_query_hooks
cnasikas May 11, 2023
b5efd8b
Merge branch 'main' into react_query_hooks
cnasikas May 12, 2023
4921e9f
Refresh when pushing
cnasikas May 12, 2023
41e16dc
Delete connector after the end of the test
cnasikas May 12, 2023
c046e0a
Use function from library
cnasikas May 12, 2023
6655997
Remove unused variable
cnasikas May 12, 2023
49069b6
Bring toaster variable back
cnasikas May 12, 2023
fff79e1
Skip flaky test
cnasikas May 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
2 changes: 2 additions & 0 deletions x-pack/plugins/cases/common/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ export {
export {
CommentType,
CaseStatuses,
CaseSeverity,
ConnectorTypes,
getCasesFromAlertsUrl,
throwErrors,
ExternalReferenceStorageType,
Expand Down
62 changes: 30 additions & 32 deletions x-pack/plugins/cases/public/common/mock/test_providers.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,22 +99,22 @@ const TestProvidersComponent: React.FC<TestProviderProps> = ({
<I18nProvider>
<KibanaContextProvider services={services}>
<ThemeProvider theme={() => ({ eui: euiDarkVars, darkMode: true })}>
<QueryClientProvider client={queryClient}>
<MemoryRouter>
<CasesProvider
value={{
externalReferenceAttachmentTypeRegistry,
persistableStateAttachmentTypeRegistry,
features,
owner,
permissions,
getFilesClient,
}}
>
<MemoryRouter>
<CasesProvider
value={{
externalReferenceAttachmentTypeRegistry,
persistableStateAttachmentTypeRegistry,
features,
owner,
permissions,
getFilesClient,
}}
>
<QueryClientProvider client={queryClient}>
<FilesContext client={createMockFilesClient()}>{children}</FilesContext>
</CasesProvider>
</MemoryRouter>
</QueryClientProvider>
</QueryClientProvider>
</CasesProvider>
</MemoryRouter>
</ThemeProvider>
</KibanaContextProvider>
</I18nProvider>
Expand Down Expand Up @@ -181,23 +181,21 @@ export const createAppMockRenderer = ({
<I18nProvider>
<KibanaContextProvider services={services}>
<ThemeProvider theme={() => ({ eui: euiDarkVars, darkMode: true })}>
<QueryClientProvider client={queryClient}>
<MemoryRouter>
<CasesProvider
value={{
externalReferenceAttachmentTypeRegistry,
persistableStateAttachmentTypeRegistry,
features,
owner,
permissions,
releasePhase,
getFilesClient,
}}
>
{children}
</CasesProvider>
</MemoryRouter>
</QueryClientProvider>
<MemoryRouter>
<CasesProvider
value={{
externalReferenceAttachmentTypeRegistry,
persistableStateAttachmentTypeRegistry,
features,
owner,
permissions,
releasePhase,
getFilesClient,
}}
>
<QueryClientProvider client={queryClient}>{children}</QueryClientProvider>
</CasesProvider>
</MemoryRouter>
</ThemeProvider>
</KibanaContextProvider>
</I18nProvider>
Expand Down
44 changes: 31 additions & 13 deletions x-pack/plugins/cases/public/components/add_comment/index.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ const addCommentProps: AddCommentProps = {
const defaultResponse = {
isLoading: false,
isError: false,
createAttachments,
mutate: createAttachments,
};

const sampleData: CaseAttachmentWithoutOwner = {
Expand Down Expand Up @@ -79,12 +79,21 @@ describe('AddComment ', () => {
wrapper.find(`button[data-test-subj="submit-comment"]`).first().simulate('click');
await waitFor(() => {
expect(onCommentSaving).toBeCalled();
expect(createAttachments).toBeCalledWith({
caseId: addCommentProps.caseId,
caseOwner: SECURITY_SOLUTION_OWNER,
data: [sampleData],
updateCase: onCommentPosted,
});
expect(createAttachments).toBeCalledWith(
{
caseId: addCommentProps.caseId,
caseOwner: SECURITY_SOLUTION_OWNER,
attachments: [sampleData],
},
{ onSuccess: expect.anything() }
);
});

act(() => {
createAttachments.mock.calls[0][1].onSuccess();
});

await waitFor(() => {
expect(wrapper.find(`[data-test-subj="add-comment"] textarea`).text()).toBe('');
});
});
Expand Down Expand Up @@ -256,12 +265,21 @@ describe('draft comment ', () => {

await waitFor(() => {
expect(onCommentSaving).toBeCalled();
expect(createAttachments).toBeCalledWith({
caseId: addCommentProps.caseId,
caseOwner: SECURITY_SOLUTION_OWNER,
data: [sampleData],
updateCase: onCommentPosted,
});
expect(createAttachments).toBeCalledWith(
{
caseId: addCommentProps.caseId,
caseOwner: SECURITY_SOLUTION_OWNER,
attachments: [sampleData],
},
{ onSuccess: expect.anything() }
);
});

act(() => {
createAttachments.mock.calls[0][1].onSuccess();
});

await waitFor(() => {
expect(result.getByLabelText('caseComment').textContent).toBe('');
expect(sessionStorage.getItem(draftKey)).toBe('');
});
Expand Down
22 changes: 15 additions & 7 deletions x-pack/plugins/cases/public/components/add_comment/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export const AddComment = React.memo(
const editorRef = useRef<EuiMarkdownEditorRef>(null);
const [focusOnContext, setFocusOnContext] = useState(false);
const { permissions, owner, appId } = useCasesContext();
const { isLoading, createAttachments } = useCreateAttachments();
const { isLoading, mutate: createAttachments } = useCreateAttachments();
Copy link
Contributor

Choose a reason for hiding this comment

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

This might have been how it was before these changes, but it seems like other hooks like useUpdateComment leverages an onError. Should the calls of createAttachments do the same?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it depends on the scenario. Most of the calls to createAttachments call mutateAsync (which is of the form await createAttachments(...)) and are surrounded by a try/catch. In the scenario of adding a comment I think showing the error toaster (is being automatically by the hook) is enough. Do you have any scenarios in your mind where the onError is needed?

const draftStorageKey = getMarkdownEditorStorageKey(appId, caseId, id);

const { form } = useForm<AddCommentFormSchema>({
Expand Down Expand Up @@ -113,15 +113,23 @@ export const AddComment = React.memo(
if (onCommentSaving != null) {
onCommentSaving();
}
createAttachments({
caseId,
caseOwner: owner[0],
data: [{ ...data, type: CommentType.user }],
updateCase: onCommentPosted,
});

createAttachments(
{
caseId,
caseOwner: owner[0],
attachments: [{ ...data, type: CommentType.user }],
},
{
onSuccess: (theCase) => {
onCommentPosted(theCase);
},
}
);

reset({ defaultValue: {} });
}

removeItemFromSessionStorage(draftStorageKey);
}, [
submit,
Expand Down
9 changes: 1 addition & 8 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 @@ -34,7 +34,6 @@ import { triggersActionsUiMock } from '@kbn/triggers-actions-ui-plugin/public/mo
import { registerConnectorsToMockActionRegistry } from '../../common/mock/register_connectors';
import { createStartServicesMock } from '../../common/lib/kibana/kibana_react.mock';
import { waitForComponentToUpdate } from '../../common/test_utils';
import { useCreateAttachments } from '../../containers/use_create_attachments';
Copy link
Member Author

Choose a reason for hiding this comment

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

not used anymore by the component.

import { useGetSupportedActionConnectors } from '../../containers/configure/use_get_supported_action_connectors';
import { useGetTags } from '../../containers/use_get_tags';
import { useUpdateCase } from '../../containers/use_update_case';
Expand All @@ -45,7 +44,6 @@ import { useBulkGetUserProfiles } from '../../containers/user_profiles/use_bulk_
import { useLicense } from '../../common/use_license';
import * as api from '../../containers/api';

jest.mock('../../containers/use_create_attachments');
jest.mock('../../containers/use_get_cases');
jest.mock('../../containers/use_get_action_license');
jest.mock('../../containers/use_get_tags');
Expand All @@ -66,7 +64,6 @@ const useGetCurrentUserProfileMock = useGetCurrentUserProfile as jest.Mock;
const useBulkGetUserProfilesMock = useBulkGetUserProfiles as jest.Mock;
const useKibanaMock = useKibana as jest.MockedFunction<typeof useKibana>;
const useGetConnectorsMock = useGetSupportedActionConnectors as jest.Mock;
const useCreateAttachmentsMock = useCreateAttachments as jest.Mock;
const useUpdateCaseMock = useUpdateCase as jest.Mock;
const useLicenseMock = useLicense as jest.Mock;

Expand All @@ -88,10 +85,6 @@ describe.skip('AllCasesListGeneric', () => {
const updateCaseProperty = jest.fn();

const emptyTag = getEmptyTagValue().props.children;
useCreateAttachmentsMock.mockReturnValue({
status: { isLoading: false },
createAttachments: jest.fn(),
});

const defaultGetCases = {
...useGetCasesMockState,
Expand Down Expand Up @@ -125,7 +118,7 @@ describe.skip('AllCasesListGeneric', () => {
useGetCurrentUserProfileMock.mockReturnValue({ data: userProfiles[0], isLoading: false });
useBulkGetUserProfilesMock.mockReturnValue({ data: userProfilesMap });
useGetConnectorsMock.mockImplementation(() => ({ data: connectorsMock, isLoading: false }));
useUpdateCaseMock.mockReturnValue({ updateCaseProperty });
useUpdateCaseMock.mockReturnValue({ mutate: updateCaseProperty });
useLicenseMock.mockReturnValue({ isAtLeastPlatinum: () => false });
mockKibana();
moment.tz.setDefault('UTC');
Expand Down
6 changes: 2 additions & 4 deletions x-pack/plugins/cases/public/components/all_cases/selector_modal/all_cases_selector_modal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,10 @@ import {
EuiModalHeaderTitle,
} from '@elastic/eui';
import styled from 'styled-components';
import { QueryClientProvider } from '@tanstack/react-query';
import { ReactQueryDevtools } from '@tanstack/react-query-devtools';
import type { CaseUI, CaseStatusWithAllStatus } from '../../../../common/ui/types';
import * as i18n from '../../../common/translations';
import { AllCasesList } from '../all_cases_list';
import { casesQueryClient } from '../../cases_context/query_client';

export interface AllCasesSelectorModalProps {
hiddenStatuses?: CaseStatusWithAllStatus[];
Expand Down Expand Up @@ -56,7 +54,7 @@ export const AllCasesSelectorModal = React.memo<AllCasesSelectorModalProps>(
);

return isModalOpen ? (
<QueryClientProvider client={casesQueryClient}>
<>
<ReactQueryDevtools initialIsOpen={false} />
<Modal onClose={closeModal} data-test-subj="all-cases-modal">
<EuiModalHeader>
Expand All @@ -79,7 +77,7 @@ export const AllCasesSelectorModal = React.memo<AllCasesSelectorModalProps>(
</EuiButtonEmpty>
</EuiModalFooter>
</Modal>
</QueryClientProvider>
</>
) : null;
}
);
Expand Down
32 changes: 16 additions & 16 deletions .../public/components/all_cases/selector_modal/use_cases_add_to_existing_case_modal.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ const persistableStateAttachmentTypeRegistry = new PersistableStateAttachmentTyp

describe('use cases add to existing case modal hook', () => {
useCreateAttachmentsMock.mockReturnValue({
createAttachments: jest.fn(),
mutateAsync: jest.fn(),
});

const dispatch = jest.fn();
Expand Down Expand Up @@ -144,7 +144,7 @@ describe('use cases add to existing case modal hook', () => {

it('should call getAttachments with the case info', async () => {
AllCasesSelectorModalMock.mockImplementation(({ onRowClick }) => {
onRowClick({ id: 'test' } as CaseUI);
onRowClick({ id: 'test', owner: 'cases' } as CaseUI);
return null;
});

Expand All @@ -153,13 +153,13 @@ describe('use cases add to existing case modal hook', () => {

await waitFor(() => {
expect(getAttachments).toHaveBeenCalledTimes(1);
expect(getAttachments).toHaveBeenCalledWith({ theCase: { id: 'test' } });
expect(getAttachments).toHaveBeenCalledWith({ theCase: { id: 'test', owner: 'cases' } });
});
});

it('should show a toaster info when no attachments are defined and noAttachmentsToaster is defined', async () => {
AllCasesSelectorModalMock.mockImplementation(({ onRowClick }) => {
onRowClick({ id: 'test' } as CaseUI);
onRowClick({ id: 'test', owner: 'cases' } as CaseUI);
return null;
});

Expand All @@ -182,7 +182,7 @@ describe('use cases add to existing case modal hook', () => {

it('should show a toaster info when no attachments are defined and noAttachmentsToaster is not defined', async () => {
AllCasesSelectorModalMock.mockImplementation(({ onRowClick }) => {
onRowClick({ id: 'test' } as CaseUI);
onRowClick({ id: 'test', owner: 'cases' } as CaseUI);
return null;
});

Expand All @@ -204,7 +204,7 @@ describe('use cases add to existing case modal hook', () => {
it('should call createAttachments when a case is selected and show a toast message', async () => {
const mockBulkCreateAttachments = jest.fn();
useCreateAttachmentsMock.mockReturnValueOnce({
createAttachments: mockBulkCreateAttachments,
mutateAsync: mockBulkCreateAttachments,
});

const mockedToastSuccess = jest.fn();
Expand All @@ -213,7 +213,7 @@ describe('use cases add to existing case modal hook', () => {
});

AllCasesSelectorModalMock.mockImplementation(({ onRowClick }) => {
onRowClick({ id: 'test' } as CaseUI);
onRowClick({ id: 'test', owner: 'cases' } as CaseUI);
return null;
});

Expand All @@ -224,8 +224,8 @@ describe('use cases add to existing case modal hook', () => {
expect(mockBulkCreateAttachments).toHaveBeenCalledTimes(1);
expect(mockBulkCreateAttachments).toHaveBeenCalledWith({
caseId: 'test',
data: [alertComment],
throwOnError: true,
caseOwner: 'cases',
attachments: [alertComment],
});
});
expect(mockedToastSuccess).toHaveBeenCalled();
Expand All @@ -235,7 +235,7 @@ describe('use cases add to existing case modal hook', () => {
const mockBulkCreateAttachments = jest.fn();

useCreateAttachmentsMock.mockReturnValueOnce({
createAttachments: mockBulkCreateAttachments,
mutateAsync: mockBulkCreateAttachments,
});

const mockedToastSuccess = jest.fn();
Expand All @@ -244,7 +244,7 @@ describe('use cases add to existing case modal hook', () => {
});

AllCasesSelectorModalMock.mockImplementation(({ onRowClick }) => {
onRowClick({ id: 'test' } as CaseUI);
onRowClick({ id: 'test', owner: 'cases' } as CaseUI);
return null;
});

Expand All @@ -259,7 +259,7 @@ describe('use cases add to existing case modal hook', () => {
it('should not call createAttachments nor show toast success when a case is not selected', async () => {
const mockBulkCreateAttachments = jest.fn();
useCreateAttachmentsMock.mockReturnValueOnce({
createAttachments: mockBulkCreateAttachments,
mutateAsync: mockBulkCreateAttachments,
});

const mockedToastSuccess = jest.fn();
Expand All @@ -285,7 +285,7 @@ describe('use cases add to existing case modal hook', () => {
it('should not show toast success when a case is selected with attachments and fails to update attachments', async () => {
const mockBulkCreateAttachments = jest.fn().mockRejectedValue(new Error('Impossible'));
useCreateAttachmentsMock.mockReturnValueOnce({
createAttachments: mockBulkCreateAttachments,
mutateAsync: mockBulkCreateAttachments,
});

const mockedToast = jest.fn();
Expand All @@ -295,7 +295,7 @@ describe('use cases add to existing case modal hook', () => {

// simulate a case selected
AllCasesSelectorModalMock.mockImplementation(({ onRowClick }) => {
onRowClick({ id: 'test' } as CaseUI);
onRowClick({ id: 'test', owner: 'cases' } as CaseUI);
return null;
});

Expand All @@ -305,8 +305,8 @@ describe('use cases add to existing case modal hook', () => {
await waitFor(() => {
expect(mockBulkCreateAttachments).toHaveBeenCalledWith({
caseId: 'test',
data: [alertComment],
throwOnError: true,
caseOwner: 'cases',
attachments: [alertComment],
});
});

Expand Down
Loading