[Cases] Refactor: AllCasesList component in selection mode should only select (#128915)

* Remove deprecated and unused attribute

* Do not accept attachments as part of allcaseslist component props

* Attach attachment in the hook

* Add tests for useCasesAddToExistingCaseModal postComment hook

* remove comment

* Add tests for postcomment hook

* Update tests for useCasesAddToExistingCaseModal hook

* Add tests for toast messages in the postComment hook

* Additional toast error validation

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
This commit is contained in:
Esteban Beltran 2022-04-01 17:57:02 +01:00 committed by GitHub
parent 02dca37fc6
commit 0c2ec496d0
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 249 additions and 192 deletions

View file

@ -18,19 +18,15 @@ const AllCasesSelectorModalLazy: React.FC<AllCasesSelectorModalProps> = lazy(
export const getAllCasesSelectorModalLazy = ({
owner,
userCanCrud,
alertData,
hiddenStatuses,
onRowClick,
updateCase,
onClose,
}: GetAllCasesSelectorModalProps) => (
<CasesProvider value={{ owner, userCanCrud }}>
<Suspense fallback={<EuiLoadingSpinner />}>
<AllCasesSelectorModalLazy
alertData={alertData}
hiddenStatuses={hiddenStatuses}
onRowClick={onRowClick}
updateCase={updateCase}
onClose={onClose}
/>
</Suspense>
@ -42,20 +38,14 @@ export const getAllCasesSelectorModalLazy = ({
* cases provider. to be further refactored https://github.com/elastic/kibana/issues/123183
*/
export const getAllCasesSelectorModalNoProviderLazy = ({
alertData,
attachments,
hiddenStatuses,
onRowClick,
updateCase,
onClose,
}: AllCasesSelectorModalProps) => (
<Suspense fallback={<EuiLoadingSpinner />}>
<AllCasesSelectorModalLazy
alertData={alertData}
attachments={attachments}
hiddenStatuses={hiddenStatuses}
onRowClick={onRowClick}
updateCase={updateCase}
onClose={onClose}
/>
</Suspense>

View file

@ -17,7 +17,7 @@ import { TestProviders } from '../../common/mock';
import { casesStatus, useGetCasesMockState, mockCase, connectorsMock } from '../../containers/mock';
import { StatusAll } from '../../../common/ui/types';
import { CaseStatuses, CommentType } from '../../../common/api';
import { CaseStatuses } from '../../../common/api';
import { SECURITY_SOLUTION_OWNER } from '../../../common/constants';
import { getEmptyTagValue } from '../empty_value';
import { useDeleteCases } from '../../containers/use_delete_cases';
@ -515,46 +515,6 @@ describe('AllCasesListGeneric', () => {
});
});
it('should call postComment when a case is selected in isSelectorView=true and has attachments', async () => {
const postCommentMockedValue = { status: { isLoading: false }, postComment: jest.fn() };
usePostCommentMock.mockReturnValueOnce(postCommentMockedValue);
const wrapper = mount(
<TestProviders>
<AllCasesList
isSelectorView={true}
attachments={[
{
type: CommentType.alert,
alertId: 'alert-id-201',
owner: 'test',
index: 'index-id-1',
rule: {
id: 'rule-id-1',
name: 'Awesome myrule',
},
},
]}
/>
</TestProviders>
);
wrapper.find('[data-test-subj="cases-table-row-select-1"]').first().simulate('click');
await waitFor(() => {
expect(postCommentMockedValue.postComment).toHaveBeenCalledWith({
caseId: '1',
data: {
alertId: 'alert-id-201',
index: 'index-id-1',
owner: 'test',
rule: {
id: 'rule-id-1',
name: 'Awesome myrule',
},
type: 'alert',
},
});
});
});
it('should call onRowClick with no cases and isSelectorView=true', async () => {
useGetCasesMock.mockReturnValue({
...defaultGetCases,

View file

@ -16,14 +16,8 @@ import {
FilterOptions,
SortFieldCase,
} from '../../../common/ui/types';
import {
CaseStatuses,
CommentRequestAlertType,
caseStatuses,
CommentType,
} from '../../../common/api';
import { CaseStatuses, caseStatuses } from '../../../common/api';
import { useGetCases } from '../../containers/use_get_cases';
import { usePostComment } from '../../containers/use_post_comment';
import { useAvailableCasesOwners } from '../app/use_available_owners';
import { useCasesColumns } from './columns';
@ -33,7 +27,6 @@ import { EuiBasicTableOnChange } from './types';
import { CasesTable } from './table';
import { useConnectors } from '../../containers/configure/use_connectors';
import { useCasesContext } from '../cases_context/use_cases_context';
import { CaseAttachments } from '../../types';
const ProgressLoader = styled(EuiProgress)`
${({ $isShow }: { $isShow: boolean }) =>
@ -52,28 +45,14 @@ const getSortField = (field: string): SortFieldCase =>
field === SortFieldCase.closedAt ? SortFieldCase.closedAt : SortFieldCase.createdAt;
export interface AllCasesListProps {
/**
* @deprecated Use the attachments prop instead
*/
alertData?: Omit<CommentRequestAlertType, 'type'>;
hiddenStatuses?: CaseStatusWithAllStatus[];
isSelectorView?: boolean;
onRowClick?: (theCase?: Case) => void;
updateCase?: (newCase: Case) => void;
doRefresh?: () => void;
attachments?: CaseAttachments;
}
export const AllCasesList = React.memo<AllCasesListProps>(
({
alertData,
attachments,
hiddenStatuses = [],
isSelectorView = false,
onRowClick,
updateCase,
doRefresh,
}) => {
({ hiddenStatuses = [], isSelectorView = false, onRowClick, doRefresh }) => {
const { owner, userCanCrud } = useCasesContext();
const hasOwner = !!owner.length;
const availableSolutions = useAvailableCasesOwners();
@ -97,8 +76,6 @@ export const AllCasesList = React.memo<AllCasesListProps>(
setSelectedCases,
} = useGetCases({ initialFilterOptions });
// Post Comment to Case
const { postComment, isLoading: isCommentUpdating } = usePostComment();
const { connectors } = useConnectors();
const sorting = useMemo(
@ -181,19 +158,6 @@ export const AllCasesList = React.memo<AllCasesListProps>(
const showActions = userCanCrud && !isSelectorView;
// TODO remove the deprecated alertData field when cleaning up
// code https://github.com/elastic/kibana/issues/123183
// This code is to support the deprecated alertData prop
const toAttach = useMemo((): CaseAttachments | undefined => {
if (attachments !== undefined || alertData !== undefined) {
const _toAttach = attachments ?? [];
if (alertData !== undefined) {
_toAttach.push({ ...alertData, type: CommentType.alert });
}
return _toAttach;
}
}, [alertData, attachments]);
const columns = useCasesColumns({
dispatchUpdateCaseProperty,
filterStatus: filterOptions.status,
@ -204,9 +168,6 @@ export const AllCasesList = React.memo<AllCasesListProps>(
userCanCrud,
connectors,
onRowClick,
attachments: toAttach,
postComment,
updateCase,
showSolutionColumn: !hasOwner && availableSolutions.length > 1,
});
@ -243,7 +204,7 @@ export const AllCasesList = React.memo<AllCasesListProps>(
size="xs"
color="accent"
className="essentialAnimation"
$isShow={(isCasesLoading || isLoading || isCommentUpdating) && !isDataEmpty}
$isShow={(isCasesLoading || isLoading) && !isDataEmpty}
/>
<CasesTableFilters
countClosedCases={data.countClosedCases}
@ -270,7 +231,7 @@ export const AllCasesList = React.memo<AllCasesListProps>(
goToCreateCase={onRowClick}
handleIsLoading={handleIsLoading}
isCasesLoading={isCasesLoading}
isCommentUpdating={isCommentUpdating}
isCommentUpdating={isCasesLoading}
isDataEmpty={isDataEmpty}
isSelectorView={isSelectorView}
onChange={tableOnChangeCallback}

View file

@ -38,8 +38,6 @@ import { useApplicationCapabilities, useKibana } from '../../common/lib/kibana';
import { StatusContextMenu } from '../case_action_bar/status_context_menu';
import { TruncatedText } from '../truncated_text';
import { getConnectorIcon } from '../utils';
import { PostComment } from '../../containers/use_post_comment';
import { CaseAttachments } from '../../types';
import type { CasesOwners } from '../../client/helpers/can_use_cases';
import { useCasesFeatures } from '../cases_context/use_cases_features';
@ -73,9 +71,6 @@ export interface GetCasesColumn {
userCanCrud: boolean;
connectors?: ActionConnector[];
onRowClick?: (theCase: Case) => void;
attachments?: CaseAttachments;
postComment?: (args: PostComment) => Promise<void>;
updateCase?: (newCase: Case) => void;
showSolutionColumn?: boolean;
}
@ -89,9 +84,6 @@ export const useCasesColumns = ({
userCanCrud,
connectors = [],
onRowClick,
attachments,
postComment,
updateCase,
showSolutionColumn,
}: GetCasesColumn): CasesColumns[] => {
// Delete case
@ -141,24 +133,11 @@ export const useCasesColumns = ({
const assignCaseAction = useCallback(
async (theCase: Case) => {
// TODO currently the API only supports to add a comment at the time
// once the API is updated we should use bulk post comment #124814
// this operation is intentionally made in sequence
if (attachments !== undefined && attachments.length > 0) {
for (const attachment of attachments) {
await postComment?.({
caseId: theCase.id,
data: attachment,
});
}
updateCase?.(theCase);
}
if (onRowClick) {
onRowClick(theCase);
}
},
[attachments, onRowClick, postComment, updateCase]
[onRowClick]
);
useEffect(() => {

View file

@ -11,7 +11,6 @@ import { mount } from 'enzyme';
import { AllCasesSelectorModal } from '.';
import { TestProviders } from '../../../common/mock';
import { AllCasesList } from '../all_cases_list';
import { SECURITY_SOLUTION_OWNER } from '../../../../common/constants';
jest.mock('../all_cases_list');
@ -19,7 +18,6 @@ const onRowClick = jest.fn();
const defaultProps = {
onRowClick,
};
const updateCase = jest.fn();
describe('AllCasesSelectorModal', () => {
beforeEach(() => {
@ -50,17 +48,7 @@ describe('AllCasesSelectorModal', () => {
it('pass the correct props to getAllCases method', () => {
const fullProps = {
...defaultProps,
alertData: {
rule: {
id: 'rule-id',
name: 'rule',
},
index: 'index-id',
alertId: 'alert-id',
owner: SECURITY_SOLUTION_OWNER,
},
hiddenStatuses: [],
updateCase,
};
mount(
@ -72,10 +60,8 @@ describe('AllCasesSelectorModal', () => {
// @ts-ignore idk what this mock style is but it works ¯\_(ツ)_/¯
expect(AllCasesList.type.mock.calls[0][0]).toEqual(
expect.objectContaining({
alertData: fullProps.alertData,
hiddenStatuses: fullProps.hiddenStatuses,
isSelectorView: true,
updateCase,
})
);
});

View file

@ -16,20 +16,13 @@ import {
} from '@elastic/eui';
import styled from 'styled-components';
import { Case, CaseStatusWithAllStatus } from '../../../../common/ui/types';
import { CommentRequestAlertType } from '../../../../common/api';
import * as i18n from '../../../common/translations';
import { AllCasesList } from '../all_cases_list';
import { CaseAttachments } from '../../../types';
export interface AllCasesSelectorModalProps {
/**
* @deprecated Use the attachments prop instead
*/
alertData?: Omit<CommentRequestAlertType, 'type'>;
hiddenStatuses?: CaseStatusWithAllStatus[];
onRowClick?: (theCase?: Case) => void;
updateCase?: (newCase: Case) => void;
onClose?: () => void;
attachments?: CaseAttachments;
}
const Modal = styled(EuiModal)`
@ -40,7 +33,7 @@ const Modal = styled(EuiModal)`
`;
export const AllCasesSelectorModal = React.memo<AllCasesSelectorModalProps>(
({ alertData, attachments, hiddenStatuses, onRowClick, updateCase, onClose }) => {
({ hiddenStatuses, onRowClick, onClose }) => {
const [isModalOpen, setIsModalOpen] = useState<boolean>(true);
const closeModal = useCallback(() => {
if (onClose) {
@ -66,12 +59,9 @@ export const AllCasesSelectorModal = React.memo<AllCasesSelectorModalProps>(
</EuiModalHeader>
<EuiModalBody>
<AllCasesList
alertData={alertData}
attachments={attachments}
hiddenStatuses={hiddenStatuses}
isSelectorView={true}
onRowClick={onClick}
updateCase={updateCase}
/>
</EuiModalBody>
<EuiModalFooter>

View file

@ -5,42 +5,82 @@
* 2.0.
*/
/* eslint-disable react/display-name */
import { renderHook } from '@testing-library/react-hooks';
import { waitFor } from '@testing-library/dom';
import { act, renderHook } from '@testing-library/react-hooks';
import userEvent from '@testing-library/user-event';
import React from 'react';
import { CaseStatuses, StatusAll } from '../../../../common';
import AllCasesSelectorModal from '.';
import { Case, CaseStatuses, StatusAll } from '../../../../common';
import { AppMockRenderer, createAppMockRenderer } from '../../../common/mock';
import { useCasesToast } from '../../../common/use_cases_toast';
import { alertComment } from '../../../containers/mock';
import { usePostComment } from '../../../containers/use_post_comment';
import { SupportedCaseAttachment } from '../../../types';
import { CasesContext } from '../../cases_context';
import { CasesContextStoreActionsList } from '../../cases_context/cases_context_reducer';
import { useCasesAddToExistingCaseModal } from './use_cases_add_to_existing_case_modal';
jest.mock('../../../common/use_cases_toast');
jest.mock('../../../containers/use_post_comment');
// dummy mock, will call onRowclick when rendering
jest.mock('./all_cases_selector_modal', () => {
return {
AllCasesSelectorModal: jest.fn(),
};
});
const useCasesToastMock = useCasesToast as jest.Mock;
const AllCasesSelectorModalMock = AllCasesSelectorModal as unknown as jest.Mock;
// test component to test the hook integration
const TestComponent: React.FC = () => {
const hook = useCasesAddToExistingCaseModal({
attachments: [alertComment as SupportedCaseAttachment],
});
const onClick = () => {
hook.open();
};
return <button type="button" data-test-subj="open-modal" onClick={onClick} />;
};
const usePostCommentMock = usePostComment as jest.Mock;
describe('use cases add to existing case modal hook', () => {
usePostCommentMock.mockReturnValue({
postComment: jest.fn(),
});
const dispatch = jest.fn();
let wrapper: React.FC;
let appMockRender: AppMockRenderer;
const wrapper: React.FC = ({ children }) => {
return (
<CasesContext.Provider
value={{
owner: ['test'],
userCanCrud: true,
appId: 'test',
appTitle: 'jest',
basePath: '/jest',
dispatch,
features: { alerts: { sync: true, enabled: true }, metrics: [] },
releasePhase: 'ga',
}}
>
{children}
</CasesContext.Provider>
);
};
const defaultParams = () => {
return { onRowClick: jest.fn() };
};
beforeEach(() => {
appMockRender = createAppMockRenderer();
dispatch.mockReset();
wrapper = ({ children }) => {
return (
<CasesContext.Provider
value={{
owner: ['test'],
userCanCrud: true,
appId: 'test',
appTitle: 'jest',
basePath: '/jest',
dispatch,
features: { alerts: { sync: true, enabled: true }, metrics: [] },
releasePhase: 'ga',
}}
>
{children}
</CasesContext.Provider>
);
};
AllCasesSelectorModalMock.mockReset();
});
it('should throw if called outside of a cases context', () => {
@ -89,4 +129,93 @@ describe('use cases add to existing case modal hook', () => {
})
);
});
it('should call postComment when a case is selected and show a toast message', async () => {
const mockedPostMessage = jest.fn();
usePostCommentMock.mockReturnValueOnce({
postComment: mockedPostMessage,
});
const mockedToastSuccess = jest.fn();
useCasesToastMock.mockReturnValue({
showSuccessAttach: mockedToastSuccess,
});
AllCasesSelectorModalMock.mockImplementation(({ onRowClick }) => {
onRowClick({ id: 'test' } as Case);
return null;
});
const result = appMockRender.render(<TestComponent />);
userEvent.click(result.getByTestId('open-modal'));
await waitFor(() => {
expect(mockedPostMessage).toHaveBeenCalledWith({
caseId: 'test',
data: alertComment,
throwOnError: true,
});
});
expect(mockedToastSuccess).toHaveBeenCalled();
});
it('should not call postComment nor show toast success when a case is not selected', async () => {
const mockedPostMessage = jest.fn();
usePostCommentMock.mockReturnValueOnce({
postComment: mockedPostMessage,
});
const mockedToastSuccess = jest.fn();
useCasesToastMock.mockReturnValue({
showSuccessAttach: mockedToastSuccess,
});
AllCasesSelectorModalMock.mockImplementation(({ onRowClick }) => {
onRowClick();
return null;
});
const result = appMockRender.render(<TestComponent />);
userEvent.click(result.getByTestId('open-modal'));
// give a small delay for the reducer to run
act(() => {
expect(mockedPostMessage).not.toHaveBeenCalled();
expect(mockedToastSuccess).not.toHaveBeenCalled();
});
});
it('should not show toast success when a case is selected with attachments and fails to update attachments', async () => {
const mockedPostMessage = jest.fn().mockRejectedValue(new Error('Impossible'));
usePostCommentMock.mockReturnValueOnce({
postComment: mockedPostMessage,
});
const mockedToast = jest.fn();
useCasesToastMock.mockReturnValue({
showSuccessAttach: mockedToast,
});
// simulate a case selected
AllCasesSelectorModalMock.mockImplementation(({ onRowClick }) => {
onRowClick({ id: 'test' } as Case);
return null;
});
const result = appMockRender.render(<TestComponent />);
userEvent.click(result.getByTestId('open-modal'));
await waitFor(() => {
expect(mockedPostMessage).toHaveBeenCalledWith({
caseId: 'test',
data: alertComment,
throwOnError: true,
});
});
act(() => {
expect(mockedPostMessage).toHaveBeenCalled();
expect(mockedToast).not.toHaveBeenCalled();
});
});
});

View file

@ -13,10 +13,13 @@ import { Case } from '../../../containers/types';
import { CasesContextStoreActionsList } from '../../cases_context/cases_context_reducer';
import { useCasesContext } from '../../cases_context/use_cases_context';
import { useCasesAddToNewCaseFlyout } from '../../create/flyout/use_cases_add_to_new_case_flyout';
import { CaseAttachments } from '../../../types';
import { usePostComment } from '../../../containers/use_post_comment';
type AddToExistingFlyoutProps = AllCasesSelectorModalProps & {
toastTitle?: string;
toastContent?: string;
attachments?: CaseAttachments;
};
export const useCasesAddToExistingCaseModal = (props: AddToExistingFlyoutProps) => {
@ -33,8 +36,10 @@ export const useCasesAddToExistingCaseModal = (props: AddToExistingFlyoutProps)
toastTitle: props.toastTitle,
toastContent: props.toastContent,
});
const { dispatch } = useCasesContext();
const casesToasts = useCasesToast();
const { postComment } = usePostComment();
const closeModal = useCallback(() => {
dispatch({
@ -47,45 +52,62 @@ export const useCasesAddToExistingCaseModal = (props: AddToExistingFlyoutProps)
});
}, [dispatch]);
const handleOnRowClick = useCallback(
async (theCase?: Case) => {
// when the case is undefined in the modal
// the user clicked "create new case"
if (theCase === undefined) {
closeModal();
createNewCaseFlyout.open();
return;
}
try {
// add attachments to the case
const attachments = props.attachments;
if (attachments !== undefined && attachments.length > 0) {
for (const attachment of attachments) {
await postComment({
caseId: theCase.id,
data: attachment,
throwOnError: true,
});
}
casesToasts.showSuccessAttach({
theCase,
attachments: props.attachments,
title: props.toastTitle,
content: props.toastContent,
});
}
} catch (error) {
// error toast is handled
// inside the postComment method
}
if (props.onRowClick) {
props.onRowClick(theCase);
}
},
[casesToasts, closeModal, createNewCaseFlyout, postComment, props]
);
const openModal = useCallback(() => {
dispatch({
type: CasesContextStoreActionsList.OPEN_ADD_TO_CASE_MODAL,
payload: {
...props,
hiddenStatuses: [CaseStatuses.closed, StatusAll],
onRowClick: (theCase?: Case) => {
// when the case is undefined in the modal
// the user clicked "create new case"
if (theCase === undefined) {
closeModal();
createNewCaseFlyout.open();
} else {
casesToasts.showSuccessAttach({
theCase,
attachments: props.attachments,
title: props.toastTitle,
content: props.toastContent,
});
if (props.onRowClick) {
props.onRowClick(theCase);
}
}
},
onRowClick: handleOnRowClick,
onClose: () => {
closeModal();
if (props.onClose) {
return props.onClose();
}
},
updateCase: async (...args) => {
closeModal();
if (props.updateCase) {
return props.updateCase(...args);
}
},
},
});
}, [casesToasts, closeModal, createNewCaseFlyout, dispatch, props]);
}, [closeModal, dispatch, handleOnRowClick, props]);
return {
open: openModal,
close: closeModal,

View file

@ -12,11 +12,19 @@ import { SECURITY_SOLUTION_OWNER } from '../../common/constants';
import { usePostComment, UsePostComment } from './use_post_comment';
import { basicCaseId } from './mock';
import * as api from './api';
import { useToasts } from '../common/lib/kibana';
jest.mock('./api');
jest.mock('../common/lib/kibana');
const useToastMock = useToasts as jest.Mock;
describe('usePostComment', () => {
const toastErrorMock = jest.fn();
useToastMock.mockReturnValue({
addError: toastErrorMock,
});
const abortCtrl = new AbortController();
const samplePost = {
comment: 'a comment',
@ -59,6 +67,7 @@ describe('usePostComment', () => {
});
await waitForNextUpdate();
expect(spyOnPostCase).toBeCalledWith(samplePost, basicCaseId, abortCtrl.signal);
expect(toastErrorMock).not.toHaveBeenCalled();
});
});
@ -98,7 +107,7 @@ describe('usePostComment', () => {
});
});
it('unhappy path', async () => {
it('set isError true and shows a toast error when an error occurs', async () => {
const spyOnPostCase = jest.spyOn(api, 'postComment');
spyOnPostCase.mockImplementation(() => {
throw new Error('Something went wrong');
@ -120,6 +129,33 @@ describe('usePostComment', () => {
isError: true,
postComment: result.current.postComment,
});
expect(toastErrorMock).toHaveBeenCalledWith(expect.any(Error), {
title: 'Error fetching data',
});
});
});
it('throws an error when invoked with throwOnError true', async () => {
const spyOnPostCase = jest.spyOn(api, 'postComment');
spyOnPostCase.mockImplementation(() => {
throw new Error('This is not possible');
});
await act(async () => {
const { result, waitForNextUpdate } = renderHook<string, UsePostComment>(() =>
usePostComment()
);
await waitForNextUpdate();
async function test() {
await result.current.postComment({
caseId: basicCaseId,
data: samplePost,
updateCase: updateCaseCallback,
throwOnError: true,
});
}
expect(test()).rejects.toThrowError('This is not possible');
});
});
});

View file

@ -45,6 +45,7 @@ export interface PostComment {
caseId: string;
data: CommentRequest;
updateCase?: (newCase: Case) => void;
throwOnError?: boolean;
}
export interface UsePostComment extends NewCommentState {
postComment: (args: PostComment) => Promise<void>;
@ -60,7 +61,7 @@ export const usePostComment = (): UsePostComment => {
const abortCtrlRef = useRef(new AbortController());
const postMyComment = useCallback(
async ({ caseId, data, updateCase }: PostComment) => {
async ({ caseId, data, updateCase, throwOnError }: PostComment) => {
try {
isCancelledRef.current = false;
abortCtrlRef.current.abort();
@ -84,6 +85,9 @@ export const usePostComment = (): UsePostComment => {
);
}
dispatch({ type: 'FETCH_FAILURE' });
if (throwOnError) {
throw error;
}
}
}
},