[Cases] Attaching files to cases #154436

adcoelho · 2023-04-05T09:19:43Z

Fixes #151595

Summary

In this PR we will be merging a feature branch into main.

This feature branch is a collection of several different PRs with file functionality for cases.

Most of the code was already reviewed so this will mainly be used for testing.

Files tab in the case detail view.
Attach files to a case.
View a list of all files attached to a case (with pagination).
Preview image files attached to a case.
Search for files attached to a case by file name.
Download files attached to a case.
Users are now able to see file activity in the case detail view.
Image files have a different icon and a clickable file name to preview.
Other files have a standard "document" icon and the name is not
clickable.
The file can be downloaded by clicking the download icon.

Release notes

Support file attachments in Cases.

Fixes #151723 This PR adds a way for users to: View a list of all files attached to a case(with pagination). Attach files to a case. Preview image files attached to a case. Search for files attached to a case by file name. Download files attached to a case.

**Merging to a feature branch** ## Summary In this PR we display the number of files attached to a case in the Files tab in the case detail view. https://user-images.githubusercontent.com/1533137/228211345-58ff8c16-7201-4192-9e2e-f8c854b47da9.mov

Fixes #152088 ## Summary Users are now able to see file activity in the case detail view. - Image files have a different icon and a clickable file name to preview - Other files have a standard "document" icon and the name is not clickable - The file can be downloaded by clicking the download icon <img width="1456" alt="Screenshot 2023-03-29 at 17 31 06" src="https://user-images.githubusercontent.com/1533137/228590244-c924bf2a-c5ac-47a0-ac1e-438cd089361b.png"> If the file has invalid metadata (not likely) we display the activity item below: <img width="808" alt="Screenshot 2023-03-29 at 17 36 25" src="https://user-images.githubusercontent.com/1533137/228591686-0bdf4077-ddbf-4da1-9d23-67c02e969c2c.png">

…iles-tab

In this PR we add the ability to delete files attached to a case. This can be done in the Case detail view either in the <details><summary>Files tab</summary> <img width="720" alt="Screenshot 2023-04-05 at 10 29 39" src="https://user-images.githubusercontent.com/1533137/230027093-072a622d-9fed-47d1-b2ac-bf4bcd14d5da.png"> </details> or the <details><summary>Activity tab</summary> <img width="1436" alt="Screenshot 2023-04-05 at 10 29 53" src="https://user-images.githubusercontent.com/1533137/230027406-509773bc-7aec-4c38-abeb-3487ca531e74.png"> </details> <details><summary>When a file is deleted the file activity item is replaced by a removed attachment activity item.</summary> <img width="1438" alt="Screenshot 2023-04-05 at 10 30 05" src="https://user-images.githubusercontent.com/1533137/230028060-a1804fd4-6484-4cb0-9147-a649c4a29ead.png"> </details>

…iles-tab

elasticmachine · 2023-04-11T16:20:10Z

Pinging @elastic/response-ops (Team:ResponseOps)

elasticmachine · 2023-04-11T16:20:13Z

Pinging @elastic/response-ops-cases (Feature:Cases)

…iles-tab

Fix failing ftr tests.

…iles-tab

jonathan-buttner

Great work @adcoelho ! I made it about half way through so I'll take a look at the rest later.

x-pack/plugins/cases/common/constants/mime_types.ts

x-pack/plugins/cases/public/common/mock/test_providers.tsx

x-pack/plugins/cases/public/components/files/add_file.test.tsx

x-pack/plugins/cases/public/components/files/add_file.tsx

x-pack/plugins/cases/public/components/files/add_file.test.tsx

x-pack/plugins/cases/public/components/files/file_delete_button_icon.tsx

x-pack/plugins/cases/public/components/files/file_type.tsx

jonathan-buttner · 2023-04-12T21:30:26Z

x-pack/plugins/cases/public/components/files/file_type.test.tsx

+    it('event renders a clickable name if the file is an image', async () => {
+      appMockRender = createAppMockRenderer();
+
+      // @ts-ignore


Are these ts-ignore because the event can be a string or component?

Just because event is possibly undefined.

This is a nit but if it's because it could be undefined and we know it won't in the test we could add the !. When I see ts-ignore it isn't immediately clear why we need it.

packages/kbn-optimizer/limits.yml

...ic/components/user_actions/property_actions/registered_attachments_property_actions.test.tsx

.../public/components/user_actions/property_actions/registered_attachments_property_actions.tsx

js-jankisalvi

Tested locally, feature looks great!! 🎉 👏

x-pack/plugins/cases/public/components/files/file_delete_button.tsx

js-jankisalvi · 2023-04-18T10:24:36Z

x-pack/plugins/cases/public/components/property_actions/index.tsx

+                    customDataTestSubj={customDataTestSubj}
+                  />
+                )) ||
+                  (action.type === AttachmentActionType.CUSTOM && action.render())}


nit: Maybe we should check whether action.render is not undefined?

mdefazio

@adcoelho This looks good! I just had two comments:

I don't think we need the button in the empty state of the table. We have the one by the search bar always there, so I think the one in the table isn't necessary.
Are we able to add a modal around the preview of the file? I think we need the close and it would be nice to have the download / delete options for this as well. (though not as necessary)

Update translations.

…iles-tab

Update page bundle limit.

kibana-ci · 2023-04-18T13:34:09Z

💚 Build Succeeded

Buildkite Build
Commit: f13f944

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`cases`	573	615	+42

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`cases`	364.0KB	374.3KB	+10.3KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`cases`	140.7KB	165.5KB	+24.8KB

Unknown metric groups

async chunk count

id	before	after	diff
`cases`	16	17	+1

ESLint disabled line counts

id	before	after	diff
`securitySolution`	432	435	+3

Total ESLint disabled count

id	before	after	diff
`securitySolution`	512	515	+3

History

💛 Build #120565 was flaky b5ecf14
💛 Build #120362 was flaky 8ee97d5
💔 Build #120271 failed 324c392
💔 Build #120200 failed bcee9cf
💔 Build #119807 failed ad172e7
💔 Build #119712 failed d2b4ecb

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @adcoelho

jonathan-buttner

## Summary One of the leftover issues from #154436 was adding a few e2e tests. This PR adds e2e tests for: - Attaching a file to a case - Deleting a file attached to a case - Opening and closing the file preview in the Cases detail view - Guarantee the File User Activity is rendered correctly ## Flaky Test Runner https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2177#0187bd93-edc9-4af0-85c4-5df3f55c418c Flaky tests: (`x-pack/test/functional_with_es_ssl/apps/cases/group1/config.ts` x 50) --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

## Summary PR #154436 changed the memoization function (my fault 🙂) and that caused the attachments to rerender each time a user does an action in cases. This PR fixes this issue. **Before:** https://user-images.githubusercontent.com/7871006/235171360-62be773f-8317-4762-9f14-0310d3825a1e.mov **After:** https://user-images.githubusercontent.com/7871006/235171429-5e06f9c4-9c0a-4148-8580-bde2360169af.mov ### 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)

## Summary PR elastic#154436 changed the memoization function (my fault 🙂) and that caused the attachments to rerender each time a user does an action in cases. This PR fixes this issue. **Before:** https://user-images.githubusercontent.com/7871006/235171360-62be773f-8317-4762-9f14-0310d3825a1e.mov **After:** https://user-images.githubusercontent.com/7871006/235171429-5e06f9c4-9c0a-4148-8580-bde2360169af.mov ### 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) (cherry picked from commit 60e3d98)

# Backport This will backport the following commits from `main` to `8.8`: - [[Cases] Fix attachment's renderer memoization (#156179)](#156179)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Christos Nasikas <christos.nasikas@elastic.co>

## Summary Things got a bit out of control ever since we merged #154436. In this PR I managed to reduce the cases bundle size from 166.0k to 140.0k. This was done in several different steps: ### Removing the usage of `query-string` This package took over 0.6% of the size of all chunks. <img width="694" alt="Screenshot 2023-05-12 at 15 10 52" src="https://github.com/elastic/kibana/assets/1533137/4fd4f3e3-4f58-4160-8f95-919e78c23525"> All functionality it provides could be replaced by the native [URLSearchParans](https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams/URLSearchParams). I removed the package in 1ef0211. ### @kbn/cases-components The cases plugin used imports from `@kbn-cases-components` in several places, these took up 4.4% of the bundle size. <img width="757" alt="Screenshot 2023-05-12 at 15 13 48" src="https://github.com/elastic/kibana/assets/1533137/e007a239-5439-4ff2-9e5b-75fc942e86d9"> This would normally be fine but it seemed like `webpack`'s tree shaking wasn't working correctly when doing imports from the top like `import {...} from '@kbn/cases-components'`. The plugin bundle seemed to be getting too much unnecessary code so I changed the imports in 547755c to be more specific. Now `@kbn/cases-components` only takes up 0.1% of the bundle size. <img width="761" alt="Screenshot 2023-05-12 at 15 17 24" src="https://github.com/elastic/kibana/assets/1533137/99426547-de38-47f2-9f3c-aabf881a2854"> ### Removing unnecessary public exports > Code that is shared statically with other plugins will contribute to the page load bundle size of that plugin. This includes exports from the `public/index.ts` file and any file referenced by the `extraPublicDirs` manifest property. I read the above in the documentation and checked what we exported. I found a bunch of things that we were exporting but that actually weren't being used by other plugins. I removed those unnecessary exports in 6a0ff8c. ### Unnecessary exports part 2 After merging the multiple file-related branches we were exporting the `imageMimeTypes` variable(a quite large array) twice. 53135ec It belonged to `common/` that is referenced in `extraPublicDirs` so that saved us a few more Ks. ### Cases plugin > Code that is shared statically with other plugins will contribute to the page load bundle size of that plugin. In the cases plugin `start` we were returning a deprecated function that was not being used anywhere any more. I removed it -0.9K 🤷 42b30b3 ### Making the `clientAPI` lazy Once again, > Code that is shared statically with other plugins will contribute to the page load bundle size of that plugin. I was going over everything that our plugin exports to see if something was not used and noticed that the functions we return in `cases.api` are not used by everyone. They were already promises so I changed the implementation to lazy load the code. A good rule of thumb here is that if the chunk is not always needed at setup/start time it might benefit from being lazy loaded. <ins>Some</ins> plugins need <ins>some</ins> of this code so we can `async import()` and reduce the `page load bundle size` significantly. See e201ab1. ### Lazy loading components in the file attachment During the cases plugin setup step we register the file's internal attachment. This defines how the file user action will look on the Case Detail page so naturally it requires a bunch of components to be loaded. The mistake here was that we don't need those components to be loaded right away. I followed [React's guidelines](https://react.dev/reference/react/lazy#lazy) for lazy loading all components in the registered object greatly decreasing the bundle size in 1c9ede7.

adcoelho added 4 commits March 28, 2023 10:53

Merge remote-tracking branch 'upstream/main' into cases-detail-view-f…

93776cb

…iles-tab

adcoelho added release_note:enhancement enhancement New value added to drive a business result Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Feature:Cases Cases feature v8.8.0 labels Apr 5, 2023

adcoelho self-assigned this Apr 5, 2023

adcoelho added 5 commits April 6, 2023 13:00

Merge remote-tracking branch 'upstream/main' into cases-detail-view-f…

4a8ed59

…iles-tab

Merge remote-tracking branch 'upstream/main' into cases-detail-view-f…

d120883

…iles-tab

Change propertyActions type to be AttachmentAction

7577112

Merge remote-tracking branch 'upstream/main' into cases-detail-view-f…

e6a7d3a

…iles-tab

adcoelho requested a review from mdefazio April 11, 2023 16:17

adcoelho marked this pull request as ready for review April 11, 2023 16:20

adcoelho requested review from a team as code owners April 11, 2023 16:20

cnasikas added release_note:feature Makes this part of the condensed release notes and removed enhancement New value added to drive a business result release_note:enhancement labels Apr 11, 2023

adcoelho added 2 commits April 11, 2023 19:08

Add tests for new custom action logic.

a24d5f2

Merge remote-tracking branch 'upstream/main' into cases-detail-view-f…

b0d7a1d

…iles-tab

cnasikas mentioned this pull request Apr 12, 2023

[Cases][Meta]: File attachments #151595

Closed

5 tasks

adcoelho added 2 commits April 12, 2023 15:56

Fix type check errors.

102a6bb

Fix failing ftr tests.

Merge remote-tracking branch 'upstream/main' into cases-detail-view-f…

8c88727

…iles-tab

jonathan-buttner reviewed Apr 12, 2023

View reviewed changes

adcoelho reopened this Apr 17, 2023

jbudz reviewed Apr 17, 2023

View reviewed changes

packages/kbn-optimizer/limits.yml Outdated Show resolved Hide resolved

jbudz approved these changes Apr 17, 2023

View reviewed changes

Fix jest tests.

8ee97d5

jonathan-buttner reviewed Apr 17, 2023

View reviewed changes

...ic/components/user_actions/property_actions/registered_attachments_property_actions.test.tsx Outdated Show resolved Hide resolved

.../public/components/user_actions/property_actions/registered_attachments_property_actions.tsx Show resolved Hide resolved

Addressing PR comments.

b5ecf14

js-jankisalvi reviewed Apr 18, 2023

View reviewed changes

mdefazio reviewed Apr 18, 2023

View reviewed changes

adcoelho added 3 commits April 18, 2023 13:34

Display better names for mime types.

39d70ff

Update translations.

Merge remote-tracking branch 'upstream/main' into cases-detail-view-f…

51ad455

…iles-tab

Rename mime type.

f13f944

Update page bundle limit.

nastasha-solomon mentioned this pull request Apr 18, 2023

Changes to Security cases in 8.8 elastic/security-docs#3171

Closed

19 tasks

jonathan-buttner approved these changes Apr 18, 2023

View reviewed changes

adcoelho merged commit 0a38f85 into main Apr 18, 2023

adcoelho deleted the cases-detail-view-files-tab branch April 18, 2023 14:02

kibanamachine added the backport:skip This commit does not require backporting label Apr 18, 2023

cnasikas mentioned this pull request Apr 20, 2023

[Security Solution][Cases] Allow us to paste screenshots / images from clipboard #81109

Closed

adcoelho mentioned this pull request Apr 24, 2023

[Cases ]Add e2e tests for case files view #155634

Merged

cnasikas mentioned this pull request Apr 28, 2023

[Cases] Fix attachment's renderer memoization #156179

Merged

1 task

adcoelho mentioned this pull request May 12, 2023

[Cases] Reducing the page load bundle size #157492

Merged

nastasha-solomon mentioned this pull request May 15, 2023

Move case features to the correct section in kibana release notes #157804

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Cases] Attaching files to cases #154436

[Cases] Attaching files to cases #154436

adcoelho commented Apr 5, 2023 •

edited by cnasikas

Loading

elasticmachine commented Apr 11, 2023

elasticmachine commented Apr 11, 2023

jonathan-buttner left a comment

jonathan-buttner Apr 12, 2023

adcoelho Apr 13, 2023

jonathan-buttner Apr 13, 2023

js-jankisalvi left a comment

js-jankisalvi Apr 18, 2023

mdefazio left a comment

kibana-ci commented Apr 18, 2023

async chunk count

ESLint disabled line counts

Total ESLint disabled count

jonathan-buttner left a comment

[Cases] Attaching files to cases #154436

[Cases] Attaching files to cases #154436

Conversation

adcoelho commented Apr 5, 2023 • edited by cnasikas Loading

Summary

Release notes

elasticmachine commented Apr 11, 2023

elasticmachine commented Apr 11, 2023

jonathan-buttner left a comment

Choose a reason for hiding this comment

jonathan-buttner Apr 12, 2023

Choose a reason for hiding this comment

adcoelho Apr 13, 2023

Choose a reason for hiding this comment

jonathan-buttner Apr 13, 2023

Choose a reason for hiding this comment

js-jankisalvi left a comment

Choose a reason for hiding this comment

js-jankisalvi Apr 18, 2023

Choose a reason for hiding this comment

mdefazio left a comment

Choose a reason for hiding this comment

kibana-ci commented Apr 18, 2023

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

async chunk count

ESLint disabled line counts

Total ESLint disabled count

History

jonathan-buttner left a comment

Choose a reason for hiding this comment

adcoelho commented Apr 5, 2023 •

edited by cnasikas

Loading