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.

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] Deleting file attachments from a case #154432

Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,10 @@ describe('AddFile', () => {
userEvent.click(await screen.findByTestId('testMetadata'));

await waitFor(() =>
expect(validateMetadata).toHaveBeenCalledWith({ caseId, owner: mockedTestProvidersOwner[0] })
expect(validateMetadata).toHaveBeenCalledWith({
caseIds: [caseId],
owner: mockedTestProvidersOwner[0],
})
);
});

Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/cases/public/components/files/add_file.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ const AddFileComponent: React.FC<AddFileProps> = ({ caseId }) => {
kind={constructFileKindIdByOwner(owner[0] as Owner)}
onDone={onUploadDone}
onError={onError}
meta={{ caseId, owner: owner[0] }}
meta={{ caseIds: [caseId], owner: owner[0] }}
/>
</EuiModalBody>
</EuiModal>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import React from 'react';

import { screen, waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';

import type { AppMockRenderer } from '../../common/mock';

import { createAppMockRenderer } from '../../common/mock';
import { basicCaseId, basicFileMock } from '../../containers/mock';
import { useDeleteFileAttachment } from '../../containers/use_delete_file_attachment';
import { FileDeleteButtonIcon } from './file_delete_button_icon';

jest.mock('../../containers/use_delete_file_attachment');

const useDeleteFileAttachmentMock = useDeleteFileAttachment as jest.Mock;

describe('FileDeleteButtonIcon', () => {
let appMockRender: AppMockRenderer;
const mutate = jest.fn();

useDeleteFileAttachmentMock.mockReturnValue({ isLoading: false, mutate });

beforeEach(() => {
jest.clearAllMocks();
appMockRender = createAppMockRenderer();
});

it('renders delete button correctly', async () => {
appMockRender.render(<FileDeleteButtonIcon caseId={basicCaseId} fileId={basicFileMock.id} />);

expect(await screen.findByTestId('cases-files-delete-button')).toBeInTheDocument();

expect(useDeleteFileAttachmentMock).toBeCalledTimes(1);
});

it('clicking delete button opens the confirmation modal', async () => {
appMockRender.render(<FileDeleteButtonIcon caseId={basicCaseId} fileId={basicFileMock.id} />);

const deleteButton = await screen.findByTestId('cases-files-delete-button');

expect(deleteButton).toBeInTheDocument();

userEvent.click(deleteButton);

expect(await screen.findAllByTestId('property-actions-confirm-modal'));
});

it('clicking delete button in the confirmation modal calls deleteFileAttachment with proper params', async () => {
appMockRender.render(<FileDeleteButtonIcon caseId={basicCaseId} fileId={basicFileMock.id} />);

const deleteButton = await screen.findByTestId('cases-files-delete-button');

expect(deleteButton).toBeInTheDocument();

userEvent.click(deleteButton);

expect(await screen.findAllByTestId('property-actions-confirm-modal'));

userEvent.click(await screen.findByTestId('confirmModalConfirmButton'));

await waitFor(() => {
expect(mutate).toHaveBeenCalledTimes(1);
expect(mutate).toHaveBeenCalledWith({
caseId: basicCaseId,
fileId: basicFileMock.id,
});
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import React from 'react';

import { EuiButtonIcon } from '@elastic/eui';
import * as i18n from './translations';
import { useDeleteFileAttachment } from '../../containers/use_delete_file_attachment';
import { useDeletePropertyAction } from '../user_actions/property_actions/use_delete_property_action';
import { DeleteAttachmentConfirmationModal } from '../user_actions/delete_attachment_confirmation_modal';

interface FileDeleteButtonIconProps {
caseId: string;
fileId: string;
}

const FileDeleteButtonIconComponent: React.FC<FileDeleteButtonIconProps> = ({ caseId, fileId }) => {
const { isLoading, mutate: deleteFileAttachment } = useDeleteFileAttachment();

const { showDeletionModal, onModalOpen, onConfirm, onCancel } = useDeletePropertyAction({
onDelete: () => deleteFileAttachment({ caseId, fileId }),
});

return (
<>
<EuiButtonIcon
iconType={'trash'}
aria-label={i18n.DELETE_FILE}
color={'danger'}
isDisabled={isLoading}
onClick={onModalOpen}
data-test-subj={'cases-files-delete-button'}
/>
{showDeletionModal ? (
<DeleteAttachmentConfirmationModal
title={i18n.DELETE_FILE_TITLE}
confirmButtonText={i18n.DELETE}
onCancel={onCancel}
onConfirm={onConfirm}
/>
) : null}
</>
);
};
FileDeleteButtonIconComponent.displayName = 'FileDeleteButtonIcon';

export const FileDeleteButtonIcon = React.memo(FileDeleteButtonIconComponent);
13 changes: 12 additions & 1 deletion x-pack/plugins/cases/public/components/files/file_type.tsx
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a test where we delete a file from the user activity?

Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { FilePreview } from './file_preview';
import * as i18n from './translations';
import { isImage, isValidFileExternalReferenceMetadata } from './utils';
import { useFilePreview } from './use_file_preview';
import { FileDeleteButtonIcon } from './file_delete_button_icon';

interface FileAttachmentEventProps {
file: FileJSON;
Expand All @@ -39,6 +40,15 @@ const FileAttachmentEvent = ({ file }: FileAttachmentEventProps) => {

FileAttachmentEvent.displayName = 'FileAttachmentEvent';

const FileAttachmentActions = ({ caseId, fileId }: { caseId: string; fileId: string }) => (
<>
<FileDownloadButtonIcon fileId={fileId} />
<FileDeleteButtonIcon caseId={caseId} fileId={fileId} />
</>
);

FileAttachmentActions.displayName = 'FileAttachmentActions';

const getFileAttachmentViewObject = (props: ExternalReferenceAttachmentViewProps) => {
if (!isValidFileExternalReferenceMetadata(props.externalReferenceMetadata)) {
return {
Expand All @@ -50,6 +60,7 @@ const getFileAttachmentViewObject = (props: ExternalReferenceAttachmentViewProps
}

const fileId = props.externalReferenceId;
const caseId = props.caseData.id;

// @ts-ignore
const partialFileJSON = props.externalReferenceMetadata?.files[0] as Partial<FileJSON>;
Expand All @@ -62,7 +73,7 @@ const getFileAttachmentViewObject = (props: ExternalReferenceAttachmentViewProps
return {
event: <FileAttachmentEvent file={file} />,
timelineAvatar: isImage(file) ? 'image' : 'document',
actions: <FileDownloadButtonIcon fileId={fileId} />,
actions: <FileAttachmentActions caseId={caseId} fileId={fileId} />,
hideDefaultActions: true,
};
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ describe('FilesTable', () => {
expect(await screen.findByTestId('cases-files-table-filetype')).toBeInTheDocument();
expect(await screen.findByTestId('cases-files-table-date-added')).toBeInTheDocument();
expect(await screen.findByTestId('cases-files-download-button')).toBeInTheDocument();
expect(await screen.findByTestId('cases-files-table-action-delete')).toBeInTheDocument();
expect(await screen.findByTestId('cases-files-delete-button')).toBeInTheDocument();
});

it('renders loading state', async () => {
Expand Down
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a test where we delete a file from the files table?

Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export const FilesTable = ({ caseId, items, pagination, onChange, isLoading }: F
showPreview();
};

const columns = useFilesTableColumns({ showPreview: displayPreview });
const columns = useFilesTableColumns({ caseId, showPreview: displayPreview });

return isLoading ? (
<>
Expand Down
8 changes: 8 additions & 0 deletions x-pack/plugins/cases/public/components/files/translations.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -90,3 +90,11 @@ export const ADDED_UNKNOWN_FILE = i18n.translate('xpack.cases.caseView.files.add
export const DOWNLOAD = i18n.translate('xpack.cases.caseView.files.download', {
defaultMessage: 'download',
});

export const DELETE = i18n.translate('xpack.cases.caseView.files.delete', {
defaultMessage: 'Delete',
});

export const DELETE_FILE_TITLE = i18n.translate('xpack.cases.caseView.files.deleteThisFile', {
defaultMessage: 'Delete this file?',
});
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,13 @@ import { useFilesTableColumns } from './use_files_table_columns';
import type { AppMockRenderer } from '../../common/mock';
import { createAppMockRenderer } from '../../common/mock';
import { renderHook } from '@testing-library/react-hooks';
import { basicCase } from '../../containers/mock';

describe('useCasesColumns ', () => {
describe('useFilesTableColumns', () => {
let appMockRender: AppMockRenderer;

const useCasesColumnsProps: FilesTableColumnsProps = {
const useFilesTableColumnsProps: FilesTableColumnsProps = {
caseId: basicCase.id,
showPreview: () => {},
};

Expand All @@ -24,7 +26,7 @@ describe('useCasesColumns ', () => {
});

it('return all files table columns correctly', async () => {
const { result } = renderHook(() => useFilesTableColumns(useCasesColumnsProps), {
const { result } = renderHook(() => useFilesTableColumns(useFilesTableColumnsProps), {
wrapper: appMockRender.AppWrapper,
});

Expand Down Expand Up @@ -56,14 +58,10 @@ describe('useCasesColumns ', () => {
"render": [Function],
},
Object {
"color": "danger",
"data-test-subj": "cases-files-table-action-delete",
"description": "Delete File",
"icon": "trash",
"isPrimary": true,
"name": "Delete",
"onClick": [Function],
"type": "icon",
"render": [Function],
},
],
"name": "Actions",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,15 @@ import * as i18n from './translations';
import { parseMimeType } from './utils';
import { FileNameLink } from './file_name_link';
import { FileDownloadButtonIcon } from './file_download_button_icon';
import { FileDeleteButtonIcon } from './file_delete_button_icon';

export interface FilesTableColumnsProps {
caseId: string;
showPreview: (file: FileJSON) => void;
}

export const useFilesTableColumns = ({
caseId,
showPreview,
}: FilesTableColumnsProps): Array<EuiBasicTableColumn<FileJSON>> => {
return [
Expand Down Expand Up @@ -58,11 +61,7 @@ export const useFilesTableColumns = ({
name: 'Delete',
isPrimary: true,
description: i18n.DELETE_FILE,
color: 'danger',
icon: 'trash',
type: 'icon',
onClick: () => {},
'data-test-subj': 'cases-files-table-action-delete',
render: (file: FileJSON) => <FileDeleteButtonIcon caseId={caseId} fileId={file.id} />,
},
],
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const UserActionContentToolbarComponent: React.FC<UserActionContentToolbarProps>
withCopyLinkAction = true,
children,
}) => (
<EuiFlexGroup responsive={false} alignItems="center">
<EuiFlexGroup responsive={false} alignItems="center" gutterSize="m">
{withCopyLinkAction ? (
<EuiFlexItem grow={false}>
<UserActionCopyLink id={id} />
Expand Down
10 changes: 10 additions & 0 deletions x-pack/plugins/cases/public/containers/__mocks__/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,3 +162,13 @@ export const getCaseConnectors = async (

export const getCaseUsers = async (caseId: string, signal: AbortSignal): Promise<CaseUsers> =>
Promise.resolve(getCaseUsersMockResponse());

export const deleteFileAttachments = async ({
caseId,
fileIds,
signal,
}: {
caseId: string;
fileIds: string[];
signal: AbortSignal;
}): Promise<void> => Promise.resolve(undefined);
27 changes: 27 additions & 0 deletions x-pack/plugins/cases/public/containers/api.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
INTERNAL_BULK_CREATE_ATTACHMENTS_URL,
SECURITY_SOLUTION_OWNER,
INTERNAL_GET_CASE_USER_ACTIONS_STATS_URL,
INTERNAL_DELETE_FILE_ATTACHMENTS_URL,
} from '../../common/constants';

import {
Expand All @@ -37,6 +38,7 @@ import {
postComment,
getCaseConnectors,
getCaseUserActionsStats,
deleteFileAttachments,
} from './api';

import {
Expand All @@ -59,6 +61,7 @@ import {
caseUserActionsWithRegisteredAttachmentsSnake,
basicPushSnake,
getCaseUserActionsStatsResponse,
basicFileMock,
} from './mock';

import { DEFAULT_FILTER_OPTIONS, DEFAULT_QUERY_PARAMS } from './use_get_cases';
Expand Down Expand Up @@ -820,6 +823,30 @@ describe('Cases API', () => {
});
});

describe('deleteFileAttachments', () => {
beforeEach(() => {
fetchMock.mockClear();
fetchMock.mockResolvedValue(null);
});

it('should be called with correct url, method, signal and body', async () => {
const resp = await deleteFileAttachments({
caseId: basicCaseId,
fileIds: [basicFileMock.id],
signal: abortCtrl.signal,
});
expect(fetchMock).toHaveBeenCalledWith(
INTERNAL_DELETE_FILE_ATTACHMENTS_URL.replace('{case_id}', basicCaseId),
{
method: 'POST',
body: JSON.stringify({ ids: [basicFileMock.id] }),
signal: abortCtrl.signal,
}
);
expect(resp).toBe(undefined);
});
});

describe('pushCase', () => {
const connectorId = 'connectorId';

Expand Down
Loading