Skip to content

Commit

Permalink
[Cases] Do not add already attached alerts to the case (#154322)
Browse files Browse the repository at this point in the history
## Summary

This PR filters out all alerts that are already attached to the selected
case. To avoid breaking changes and not confuse the users (trying to
find which alert is attached to which case) the UI will not produce any
error.

### Checklist

Delete any items that are not applicable to this PR.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

### For maintainers

- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
  • Loading branch information
cnasikas authored Apr 13, 2023
1 parent 70500d7 commit 9cc51bf
Show file tree
Hide file tree
Showing 30 changed files with 559 additions and 259 deletions.
5 changes: 3 additions & 2 deletions .buildkite/scripts/pipelines/pull_request/pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,9 @@ const uploadPipeline = (pipelineContent: string | object) => {
}

if (
(await doAnyChangesMatch([/^x-pack\/plugins\/osquery/, /^x-pack\/test\/osquery_cypress/])) ||
GITHUB_PR_LABELS.includes('ci:all-cypress-suites')
((await doAnyChangesMatch([/^x-pack\/plugins\/osquery/, /^x-pack\/test\/osquery_cypress/])) ||
GITHUB_PR_LABELS.includes('ci:all-cypress-suites')) &&
!GITHUB_PR_LABELS.includes('ci:skip-cypress-osquery')
) {
pipeline.push(getPipeline('.buildkite/pipelines/pull_request/osquery_cypress.yml'));
}
Expand Down
7 changes: 7 additions & 0 deletions x-pack/plugins/cases/public/common/use_cases_toast.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,13 @@ export const useCasesToast = () => {
showSuccessToast: (title: string) => {
toasts.addSuccess({ title, className: 'eui-textBreakWord' });
},
showInfoToast: (title: string, text?: string) => {
toasts.addInfo({
title,
text,
className: 'eui-textBreakWord',
});
},
};
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { useCreateAttachments } from '../../../containers/use_create_attachments
import { CasesContext } from '../../cases_context';
import { CasesContextStoreActionsList } from '../../cases_context/cases_context_reducer';
import { ExternalReferenceAttachmentTypeRegistry } from '../../../client/attachment_framework/external_reference_registry';
import type { AddToExistingCaseModalProps } from './use_cases_add_to_existing_case_modal';
import { useCasesAddToExistingCaseModal } from './use_cases_add_to_existing_case_modal';
import { PersistableStateAttachmentTypeRegistry } from '../../../client/attachment_framework/persistable_state_registry';

Expand All @@ -33,15 +34,18 @@ jest.mock('./all_cases_selector_modal', () => {
});

const onSuccess = jest.fn();
const getAttachments = jest.fn().mockReturnValue([alertComment]);
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({ onSuccess });
const TestComponent: React.FC<AddToExistingCaseModalProps> = (
props: AddToExistingCaseModalProps = {}
) => {
const hook = useCasesAddToExistingCaseModal({ onSuccess, ...props });

const onClick = () => {
hook.open({ attachments: [alertComment] });
hook.open({ getAttachments });
};

return <button type="button" data-test-subj="open-modal" onClick={onClick} />;
Expand Down Expand Up @@ -138,6 +142,65 @@ 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 Case);
return null;
});

const result = appMockRender.render(<TestComponent />);
userEvent.click(result.getByTestId('open-modal'));

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

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

getAttachments.mockReturnValueOnce([]);

const mockedToastInfo = jest.fn();
useCasesToastMock.mockReturnValue({
showInfoToast: mockedToastInfo,
});

const result = appMockRender.render(
<TestComponent noAttachmentsToaster={{ title: 'My title', content: 'My content' }} />
);
userEvent.click(result.getByTestId('open-modal'));

await waitFor(() => {
expect(mockedToastInfo).toHaveBeenCalledWith('My title', 'My content');
});
});

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

getAttachments.mockReturnValueOnce([]);

const mockedToastInfo = jest.fn();
useCasesToastMock.mockReturnValue({
showInfoToast: mockedToastInfo,
});

const result = appMockRender.render(<TestComponent />);
userEvent.click(result.getByTestId('open-modal'));

await waitFor(() => {
expect(mockedToastInfo).toHaveBeenCalledWith('No attachments added to the case', undefined);
});
});

it('should call createAttachments when a case is selected and show a toast message', async () => {
const mockBulkCreateAttachments = jest.fn();
useCreateAttachmentsMock.mockReturnValueOnce({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,23 +16,30 @@ import { useCasesAddToNewCaseFlyout } from '../../create/flyout/use_cases_add_to
import type { CaseAttachmentsWithoutOwner } from '../../../types';
import { useCreateAttachments } from '../../../containers/use_create_attachments';
import { useAddAttachmentToExistingCaseTransaction } from '../../../common/apm/use_cases_transactions';
import { NO_ATTACHMENTS_ADDED } from '../translations';

type AddToExistingFlyoutProps = Omit<AllCasesSelectorModalProps, 'onRowClick'> & {
toastTitle?: string;
toastContent?: string;
export type AddToExistingCaseModalProps = Omit<AllCasesSelectorModalProps, 'onRowClick'> & {
successToaster?: {
title?: string;
content?: string;
};
noAttachmentsToaster?: {
title?: string;
content?: string;
};
onSuccess?: (theCase: Case) => void;
};

export const useCasesAddToExistingCaseModal = (props: AddToExistingFlyoutProps = {}) => {
export const useCasesAddToExistingCaseModal = (props: AddToExistingCaseModalProps = {}) => {
const createNewCaseFlyout = useCasesAddToNewCaseFlyout({
onClose: props.onClose,
onSuccess: (theCase?: Case) => {
if (props.onSuccess && theCase) {
return props.onSuccess(theCase);
}
},
toastTitle: props.toastTitle,
toastContent: props.toastContent,
toastTitle: props.successToaster?.title,
toastContent: props.successToaster?.content,
});

const { dispatch, appId } = useCasesContext();
Expand All @@ -52,7 +59,11 @@ export const useCasesAddToExistingCaseModal = (props: AddToExistingFlyoutProps =
}, [dispatch]);

const handleOnRowClick = useCallback(
async (theCase: Case | undefined, attachments: CaseAttachmentsWithoutOwner) => {
async (
theCase: Case | undefined,
getAttachments?: ({ theCase }: { theCase?: Case }) => CaseAttachmentsWithoutOwner
) => {
const attachments = getAttachments?.({ theCase }) ?? [];
// when the case is undefined in the modal
// the user clicked "create new case"
if (theCase === undefined) {
Expand All @@ -63,27 +74,33 @@ export const useCasesAddToExistingCaseModal = (props: AddToExistingFlyoutProps =

try {
// add attachments to the case
if (attachments !== undefined && attachments.length > 0) {
startTransaction({ appId, attachments });
if (attachments === undefined || attachments.length === 0) {
const title = props.noAttachmentsToaster?.title ?? NO_ATTACHMENTS_ADDED;
const content = props.noAttachmentsToaster?.content;
casesToasts.showInfoToast(title, content);

await createAttachments({
caseId: theCase.id,
caseOwner: theCase.owner,
data: attachments,
throwOnError: true,
});
return;
}

if (props.onSuccess) {
props.onSuccess(theCase);
}
startTransaction({ appId, attachments });

casesToasts.showSuccessAttach({
theCase,
attachments,
title: props.toastTitle,
content: props.toastContent,
});
await createAttachments({
caseId: theCase.id,
caseOwner: theCase.owner,
data: attachments,
throwOnError: true,
});

if (props.onSuccess) {
props.onSuccess(theCase);
}

casesToasts.showSuccessAttach({
theCase,
attachments,
title: props.successToaster?.title,
content: props.successToaster?.content,
});
} catch (error) {
// error toast is handled
// inside the createAttachments method
Expand All @@ -101,15 +118,18 @@ export const useCasesAddToExistingCaseModal = (props: AddToExistingFlyoutProps =
);

const openModal = useCallback(
({ attachments }: { attachments?: CaseAttachmentsWithoutOwner } = {}) => {
({
getAttachments,
}: {
getAttachments?: ({ theCase }: { theCase?: Case }) => CaseAttachmentsWithoutOwner;
} = {}) => {
dispatch({
type: CasesContextStoreActionsList.OPEN_ADD_TO_CASE_MODAL,
payload: {
...props,
hiddenStatuses: [CaseStatuses.closed, StatusAll],
onRowClick: (theCase?: Case) => {
const caseAttachments = attachments ?? [];
handleOnRowClick(theCase, caseAttachments);
handleOnRowClick(theCase, getAttachments);
},
onClose: () => {
closeModal();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,3 +147,10 @@ export const SHOW_MORE = (count: number) =>
defaultMessage: '+{count} more',
values: { count },
});

export const NO_ATTACHMENTS_ADDED = i18n.translate(
'xpack.cases.modal.attachments.noAttachmentsTitle',
{
defaultMessage: 'No attachments added to the case',
}
);
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,9 @@ describe('use cases add to new case flyout hook', () => {
},
{ wrapper }
);

result.current.open({ attachments: [alertComment] });

expect(dispatch).toHaveBeenCalledWith(
expect.objectContaining({
type: CasesContextStoreActionsList.OPEN_CREATE_CASE_FLYOUT,
Expand Down
Loading

0 comments on commit 9cc51bf

Please sign in to comment.