[Security Solution][Detections] Implement "select all" rules#100554
Merged
xcrzx merged 1 commit intoelastic:masterfrom Jun 9, 2021
Merged
[Security Solution][Detections] Implement "select all" rules#100554xcrzx merged 1 commit intoelastic:masterfrom
xcrzx merged 1 commit intoelastic:masterfrom
Conversation
xcrzx
force-pushed
the
99440-select-all-rules
branch
3 times, most recently
from
7167ef6 to
0ca06c4
Compare
xcrzx
changed the title
xcrzx
force-pushed
the
99440-select-all-rules
branch
2 times, most recently
from
b6543a0 to
3bac34a
Compare
xcrzx
added
Team:Detections and Resp
xcrzx
force-pushed
the
99440-select-all-rules
branch
9 times, most recently
from
7445ab1 to
c00073c
Compare
Contributor
Author
|
@elasticmachine merge upstream |
1 similar comment
Contributor
Author
|
@elasticmachine merge upstream |
Contributor
|
Pinging @elastic/security-solution (Team: SecuritySolution) |
Contributor
|
Pinging @elastic/security-detections-response (Team:Detections and Resp) |
Contributor
|
Hi @xcrzx, thanks for implementing this long overdue feature! I did some testing and found a couple of issues that I think we should look into.
|
xcrzx
force-pushed
the
99440-select-all-rules
branch
from
afd419a to
7bc37f8
Compare
Contributor
Author
|
Hey @peluja1012, Thank you for reviewing this PR.
With the "select all" feature, there is no easy way to say if the user has selected prebuilt rules. So disabling the Export button doesn't seem like an option to me. But what about showing a notification to the user with something like: "Successfully exported X of Y rules. Prebuilt rules were excluded from the resulting file."?
Sure, no problem. I'll limit the maximum number of rows by 100.
Yea, I've also seen this. There is some strange logic implemented which resets the current selection on every refresh. I don't know whether it was implemented on purpose, but we could improve the rule table's UX a little bit by removing it. UPD: I think I've found out why the table's selection gets reset after every update. Fresh data could contain a different set of rows, so previously selected rows could become no longer visible on the current page. But in the case when all rows are selected, we can keep the selection. |
xcrzx
force-pushed
the
99440-select-all-rules
branch
3 times, most recently
from
baf2a2f to
775a6a8
Compare
Contributor
Hi @xcrzx, this sounds good to me. Thanks!
👍 |
xcrzx
force-pushed
the
99440-select-all-rules
branch
4 times, most recently
from
d898781 to
9db2f46
Compare
This was referenced Jun 8, 2021
Open
xcrzx
force-pushed
the
99440-select-all-rules
branch
from
9db2f46 to
18eb397
Compare
Contributor
💛 Build succeeded, but was flaky
Test FailuresKibana Pipeline / general / Chrome X-Pack UI Functional Tests.x-pack/test/functional/apps/monitoring/elasticsearch/nodes·js.Monitoring app Elasticsearch nodes listing with only online nodes should filter for non-existent indexStandard OutStack TraceMetrics [docs]Module Count
Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: cc @xcrzx |
| * Use this method to watch value for changes. | ||
| * | ||
| * CAUTION: you probably don't need this hook. Try to use useEffect first. | ||
| * It is only useful in rare cases when a value differs by reference but not by content between renders. |
FrankHassanabad
approved these changes
Jun 8, 2021
Contributor
FrankHassanabad
left a comment
FrankHassanabad
left a comment
There was a problem hiding this comment.
LGTM, thanks for the updates
peluja1012
approved these changes
Jun 9, 2021
Contributor
peluja1012
left a comment
peluja1012
left a comment
There was a problem hiding this comment.
LGTM. Thanks for your hard work on this feature @xcrzx!
Contributor
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
gmmorris
added a commit
to gmmorris/kibana
that referenced
this pull request
Jun 9, 2021
* master: (54 commits) Implement "select all" rules feature (elastic#100554) [ML] Remove script fields from the Anomaly detection alerting rule executor (elastic#101607) [Security solutions][Endpoint] Update event filtering texts (elastic#101563) [Enterprise Search] Mocks/tests tech debt - avoid hungry mocking (elastic#101107) [FTR] Updates esArchive paths [FTR] Updates esArchive paths [Security Solution][Detection Engine] Adds runtime field tests (elastic#101664) Added APM PHP agent to the list of agent names (elastic#101062) [CI] Restore old version_info behavior when .git directory is present (elastic#101642) [Fleet] Add fleet server telemetry (elastic#101400) [APM] Syncs agent config settings to APM Fleet policies (elastic#100744) [esArchiver] drop support for --dir, use repo-relative paths instead (elastic#101345) Revert "[xpack/test] restore incremental: false in ts project" [Security Solution] Remove Host Isolation feature flag (elastic#101655) [xpack/test] restore incremental: false in ts project [DOCS] Adds link to video landing page (elastic#101413) [ML] Move Index Data Visualizer into separate plugin (Part 1) (elastic#100922) Improve security plugin return types (elastic#101492) [ts] migrate `x-pack/test` to composite ts project (elastic#101441) [App Search] Updated Search UI to new URL (elastic#101320) ...
This was referenced Jun 9, 2021
Merged
kibanamachine
added a commit
that referenced
this pull request
Jun 9, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses: #99440
Summary
Screen.Recording.2021-06-01.at.17.42.25.mov
detection_engine/rules/_bulk_actionAPI endpoint. It accepts a filter query, and a bulks action:enable,disable,export,delete,duplicate. Under the hood, it queries detection rules using the provided filter string and applies the action to each of them. Currently, the number of rules is limited to 10,000.userHasNoPermissionstouserHasPermissionsto get rid of expressions likeuserHasNoPermissions={false}🤯.<GenericDownloader />component intodownloadBlobmethod. That simplifies file download logic and allows to download rules without knowing their ids.Checklist