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

[Security Solution][Detections] adds confirmation modal window for bulk exporting action #136418

Merged
merged 94 commits into from
Jul 25, 2022

Conversation

vitaliidm
Copy link
Contributor

@vitaliidm vitaliidm commented Jul 14, 2022

Summary

Modal windows

no rules can be exported

Screenshot 2022-07-18 at 14 01 36

some rules can be exported

Screenshot 2022-07-18 at 14 02 30

Implementation details

  • we won't need dry run action here, because export doesn't mutate state
  • once user click on export, we download file in browser, read it, and display message to user how many rules can/can't be exported
  • since all failed export are immutable rules, we can safely display immutable message error to users in modal window
  • user can proceed with download of exported rules OR cancel export action, thus experience will become consistent with bulk edit

Checklist

Delete any items that are not applicable to this PR.

For maintainers

Release note

improves user experience for bulk export of security rules, by displaying confirmation modal, that show how many rules can be exported

vitaliidm and others added 30 commits June 17, 2022 13:43
@vitaliidm vitaliidm marked this pull request as ready for review July 20, 2022 10:55
@vitaliidm vitaliidm requested review from a team as code owners July 20, 2022 10:55
@vitaliidm vitaliidm requested a review from banderror July 20, 2022 10:55
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detections-response (Team:Detections and Resp)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@banderror
Copy link
Contributor

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

expected head sha didn’t match current head ref.

Copy link
Contributor

@banderror banderror left a comment

Choose a reason for hiding this comment

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

Checked out and reviewed the code changes. Leaving a bunch of comments related to quality improvements, but nothing critical.

Tested locally and it works as expected.

Thanks for working on this UX enhancement @vitaliidm!

@vitaliidm vitaliidm requested a review from banderror July 22, 2022 17:43
@vitaliidm vitaliidm enabled auto-merge (squash) July 25, 2022 08:26
Copy link
Contributor

@banderror banderror left a comment

Choose a reason for hiding this comment

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

Thank you for addressing the comments @vitaliidm. The remaining ones are minor, so I'll go ahead and approve the PR. Thanks!

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Security Solution Tests #1 / Alerts timeline Privileges: can crud "before each" hook for "should allow a user with crud privileges to attach alerts to cases"
  • [job] [logs] FTR Configs #7 / machine learning - data visualizer index based actions panel on trial license view in discover page action loads the source data in the data visualizer

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 3177 3179 +2

Async chunks

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

id before after diff
securitySolution 5.2MB 5.2MB +3.1KB
Unknown metric groups

ESLint disabled in files

id before after diff
securitySolution 76 77 +1

Total ESLint disabled count

id before after diff
securitySolution 496 497 +1

History

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

cc @vitaliidm

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:Rule Management Security Solution Detection Rule Management area impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. release_note:enhancement Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.4.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants