-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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 bulk edit rule actions #138900
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vitaliidm Thank you for addressing the comments and creating the follow-up tickets. I have 2 comments left but none of them seem to be blocking.
Before you merge this PR please assist @elastic/security-docs with anything they might need for reviewing the wording used in the new bulk editing form.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool new feature, thanks for adding docs team for review! Added a few line edits.
Also, overall we might want to reconsider the name "Add rule actions" (in the "Bulk actions" menu, the flyout title, etc.), because a user might also use this UI to edit or remove actions, not just add. Maybe just the name "Rule actions" would cover all those use cases.
.../public/detections/pages/detection_engine/rules/all/bulk_actions/forms/rule_actions_form.tsx
Outdated
Show resolved
Hide resolved
.../public/detections/pages/detection_engine/rules/all/bulk_actions/forms/rule_actions_form.tsx
Outdated
Show resolved
Hide resolved
.../public/detections/pages/detection_engine/rules/all/bulk_actions/forms/rule_actions_form.tsx
Outdated
Show resolved
Hide resolved
.../public/detections/pages/detection_engine/rules/all/bulk_actions/forms/rule_actions_form.tsx
Outdated
Show resolved
Hide resolved
...ty_solution/public/detections/pages/detection_engine/rules/all/bulk_actions/translations.tsx
Outdated
Show resolved
Hide resolved
…tion_engine/rules/all/bulk_actions/forms/rule_actions_form.tsx Co-authored-by: Joe Peeples <joe.peeples@elastic.co>
…tion_engine/rules/all/bulk_actions/forms/rule_actions_form.tsx Co-authored-by: Joe Peeples <joe.peeples@elastic.co>
…tion_engine/rules/all/bulk_actions/forms/rule_actions_form.tsx Co-authored-by: Joe Peeples <joe.peeples@elastic.co>
…tion_engine/rules/all/bulk_actions/forms/rule_actions_form.tsx Co-authored-by: Joe Peeples <joe.peeples@elastic.co>
…tion_engine/rules/all/bulk_actions/translations.tsx Co-authored-by: Joe Peeples <joe.peeples@elastic.co>
...ty_solution/public/detections/pages/detection_engine/rules/all/bulk_actions/translations.tsx
Outdated
Show resolved
Hide resolved
Thanks for your help, @joepeeples Here is the final version:
This name is in line with other bulk edit actions menu: |
@vitaliidm Thanks for the updated screenshot! Actually I think it's OK to use a bulleted list for the tips, and they probably work better that way; I was just saying that it wasn't necessary to introduce the list the way the original heading was doing.
That makes sense to me, thanks for the extra context. |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you! 🚀
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: cc @vitaliidm |
Summary
add_rule_actions
,set_rule_actions
validateMutatedParams
to action validator. Because, rule immutability depends on actions performed on it, not only onimmutable
propertyno_actions
Feature recording
Note: callouts on recording are not up to date
Screen.Recording.2022-08-18.at.12.11.12.mov
Screen
Checklist
Delete any items that are not applicable to this PR.
For maintainers
Release note
Adding bulk edit of rule actions