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] Refactor: AllCasesList component in selection mode should only select #128915

Merged

Conversation

academo
Copy link
Contributor

@academo academo commented Mar 30, 2022

Summary

Partially refactors the AllCasesList component to not accept attachments. This PR doesn't touch any business logic or change any UX or UI.

This will keep the component as a listing and selection only component with no side effects.

The logic before available inside the component was moved to the existing hook to select and attach to a case.

In addition it removes a deprecated prop alertData and unused prop updateCase

It also fixes: #128723

Checklist

Delete any items that are not applicable to this PR.

@academo academo force-pushed the refactor/remove-postcomment-from-allcases-list branch from ffc6d22 to 3183d44 Compare March 30, 2022 14:52
@academo academo self-assigned this Mar 31, 2022
@academo academo added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Feature:Cases Cases feature v8.3.0 labels Mar 31, 2022
@academo academo marked this pull request as ready for review March 31, 2022 12:28
@academo academo requested a review from a team as a code owner March 31, 2022 12:28
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

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

Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

Tested and LGTM

@academo academo requested a review from cnasikas April 1, 2022 12:29
@cnasikas
Copy link
Member

cnasikas commented Apr 1, 2022

@elasticmachine merge upstream

Copy link
Member

@cnasikas cnasikas left a comment

Choose a reason for hiding this comment

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

Tested and LGTM!

@academo academo enabled auto-merge (squash) April 1, 2022 15:24
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Test Failures

  • [job] [logs] Default CI Group #27 / Cases View case properties "before all" hook for "edits a case title from the case view page"

Metrics [docs]

Async chunks

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

id before after diff
cases 292.9KB 283.8KB -9.1KB

Page load bundle

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

id before after diff
cases 86.3KB 90.6KB +4.3KB
Unknown metric groups

ESLint disabled in files

id before after diff
cases 17 16 -1

Total ESLint disabled count

id before after diff
cases 93 92 -1

History

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

cc @academo

@academo academo merged commit 0c2ec49 into elastic:main Apr 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Cases Cases feature release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Cases] Attachment hooks shows a success toast on an error.
6 participants