[Security Solution] Rule bulk schedule fixes and test coverage expansion #141604

jpdjere · 2022-09-23T11:07:17Z

Summary

Fixes issues, nits and expands test coverage for PR: #140166

Extends definition of TimeUnit type and its tests
Adds e2e test to test default values of Bulk Schedule flyout
Corrects copy as reported by @elastic/security-docs
Corrects validation for Interval field when editing rule schedule individually

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

For maintainers

This was checked for breaking API changes and was labeled appropriately

elasticmachine · 2022-09-23T15:36:22Z

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

elasticmachine · 2022-09-23T15:36:23Z

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

banderror · 2022-09-26T09:56:24Z

Files by Code Owner

elastic/security-detections-response-alerts

x-pack/plugins/security_solution/cypress/e2e/detection_rules/bulk_edit_rules.cy.ts

elastic/security-detections-response-rules

x-pack/plugins/security_solution/cypress/e2e/detection_rules/bulk_edit_rules.cy.ts
x-pack/plugins/security_solution/public/detections/components/rules/step_schedule_rule/index.tsx
x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/bulk_actions/translations.tsx
x-pack/plugins/security_solution/server/lib/detection_engine/rules/bulk_actions/rule_params_modifier.test.ts

elastic/security-solution

x-pack/plugins/security_solution/cypress/e2e/detection_rules/bulk_edit_rules.cy.ts
x-pack/plugins/security_solution/cypress/screens/rules_bulk_edit.ts
x-pack/plugins/security_solution/cypress/tasks/rules_bulk_edit.ts
x-pack/plugins/security_solution/public/detections/components/rules/step_schedule_rule/index.tsx
x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/bulk_actions/translations.tsx
x-pack/plugins/security_solution/server/lib/detection_engine/rules/bulk_actions/rule_params_modifier.test.ts

elastic/security-solution-platform

packages/kbn-securitysolution-io-ts-types/src/time_duration/index.test.ts
packages/kbn-securitysolution-io-ts-types/src/time_duration/index.ts

e40pud

LGTM!

maximpn

LGTM

xcrzx · 2022-09-26T16:35:57Z

packages/kbn-securitysolution-io-ts-types/src/time_duration/index.test.ts

+  test('it should NOT validate a TimeDuration of 0 length', () => {
+    const payload = '0s';
+    const decoded = TimeDuration.decode(payload);
+    const message = pipe(decoded, foldLeftRight);


Nit: why use pipe to invoke a single function? Calling directly foldLeftRight(decoded) looks much simpler.

I will merge this PR as is and fix this in #140626, which greatly expands on this testing cases

xcrzx · 2022-09-26T16:40:15Z

x-pack/plugins/security_solution/cypress/tasks/rules_bulk_edit.ts

    'have.text',
-    `You're about to apply changes to ${expectedNumberOfNotMLRules} selected rules. The changes you made will be overwritten to the existing Rule schedules and additional look-back time (if any).`
+    `You're about to apply changes to ${expectedNumberOfNotMLRules} selected rules. The changes you make will overwrite the existing rule schedules and additional look-back time (if any).`


Nit: would be nice to reduce the amount of text in this check by switching from have.text to contain.text.

yctercero

Codeowners changes for platform LGTM 👍🏽

kibana-ci · 2022-09-27T13:50:02Z

💚 Build Succeeded

Buildkite Build
Commit: 91861e4

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
`lists`	152.9KB	152.9KB	+20.0B
`securitySolution`	6.6MB	6.6MB	+22.0B
total			+42.0B

Page load bundle

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

id	before	after	diff
`securitySolution`	263.0KB	263.1KB	+20.0B

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`securitySolution`	410	411	+1

Total ESLint disabled count

id	before	after	diff
`securitySolution`	482	483	+1

History

💛 Build #75542 was flaky fd1540146e599b4400124d85f9a549a2ba21e84a
💛 Build #75342 was flaky 8517970deb69cc7b8d3a87d1eae6179787929c97
💚 Build #75235 succeeded 840489139764a8ac5e96baf3c3c599999c5119f9
💔 Build #75228 failed 5a39e3cd5f5d2b5cfda9af6453e599e77bc95828
💔 Build #75216 failed cc9d3cea6b22e1fbc54e7417ae1947373271e9cf

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

cc @jpdjere

…ion (elastic#141604) ## Summary Fixes issues, nits and [expands test coverage](https://docs.google.com/document/d/116x7ITTTJQ6cTiwaGK831_f6Ox7XB3qyLiHxC3Cmf8w/edit#) for PR: elastic#140166 - Extends definition of `TimeUnit` type and its tests - Adds e2e test to test default values of Bulk Schedule flyout - Corrects copy as reported by @elastic/security-docs - Corrects validation for Interval field when editing rule schedule individually ### Checklist Delete any items that are not applicable to this PR. - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [ ] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [ ] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [ ] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### For maintainers - [ ] 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 9738b04)

kibanamachine · 2022-09-27T14:05:54Z

💚 All backports created successfully

Status	Branch	Result
✅	8.5

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

…ion (#141604) (#141946) ## Summary Fixes issues, nits and [expands test coverage](https://docs.google.com/document/d/116x7ITTTJQ6cTiwaGK831_f6Ox7XB3qyLiHxC3Cmf8w/edit#) for PR: #140166 - Extends definition of `TimeUnit` type and its tests - Adds e2e test to test default values of Bulk Schedule flyout - Corrects copy as reported by @elastic/security-docs - Corrects validation for Interval field when editing rule schedule individually ### Checklist Delete any items that are not applicable to this PR. - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [ ] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [ ] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [ ] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### For maintainers - [ ] 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 9738b04) Co-authored-by: Juan Pablo Djeredjian <[email protected]>

jpdjere force-pushed the bulk-schedule-corrections branch 2 times, most recently from 1898c8f to 8404891 Compare September 23, 2022 12:02

jpdjere self-assigned this Sep 23, 2022

jpdjere marked this pull request as ready for review September 23, 2022 15:36

jpdjere requested review from a team as code owners September 23, 2022 15:36

jpdjere requested a review from maximpn September 23, 2022 15:36

jpdjere force-pushed the bulk-schedule-corrections branch 2 times, most recently from 8517970 to fd15401 Compare September 26, 2022 09:33

e40pud approved these changes Sep 26, 2022

View reviewed changes

maximpn approved these changes Sep 26, 2022

View reviewed changes

xcrzx reviewed Sep 26, 2022

View reviewed changes

banderror mentioned this pull request Sep 26, 2022

[Security Solution][Detections] Add Bulk Scheduling for rules #140166

Merged

6 tasks

banderror added Feature:Rule Management Security Solution Detection Rule Management area bug Fixes for quality problems that affect the customer experience labels Sep 26, 2022

yctercero approved these changes Sep 27, 2022

View reviewed changes

jpdjere force-pushed the bulk-schedule-corrections branch from fd15401 to f215b62 Compare September 27, 2022 12:31

jpdjere added 2 commits September 27, 2022 14:39

Address comments

f05255d

Copy fixes and additional e2e testing case

74f2196

jpdjere added 2 commits September 27, 2022 14:39

Correct test

0525e0c

Address comments

91861e4

jpdjere force-pushed the bulk-schedule-corrections branch from 8fdfff5 to 91861e4 Compare September 27, 2022 12:39

jpdjere merged commit 9738b04 into elastic:main Sep 27, 2022

jpdjere deleted the bulk-schedule-corrections branch September 27, 2022 14:01

kibanamachine added the v8.6.0 label Sep 27, 2022

kibanamachine mentioned this pull request Sep 27, 2022

[8.5] [Security Solution] Rule bulk schedule fixes and test coverage expansion (#141604) #141946

Merged

kibanamachine added the v8.5.0 label Sep 27, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution] Rule bulk schedule fixes and test coverage expansion #141604

[Security Solution] Rule bulk schedule fixes and test coverage expansion #141604

jpdjere commented Sep 23, 2022 •

edited by kibanamachine

Loading

elasticmachine commented Sep 23, 2022

elasticmachine commented Sep 23, 2022

banderror commented Sep 26, 2022

e40pud left a comment

maximpn left a comment

xcrzx Sep 26, 2022

jpdjere Sep 27, 2022

xcrzx Sep 26, 2022

yctercero left a comment

kibana-ci commented Sep 27, 2022

ESLint disabled line counts

Total ESLint disabled count

kibanamachine commented Sep 27, 2022

[Security Solution] Rule bulk schedule fixes and test coverage expansion #141604

[Security Solution] Rule bulk schedule fixes and test coverage expansion #141604

Conversation

jpdjere commented Sep 23, 2022 • edited by kibanamachine Loading

Summary

Checklist

For maintainers

elasticmachine commented Sep 23, 2022

elasticmachine commented Sep 23, 2022

banderror commented Sep 26, 2022

Files by Code Owner

elastic/security-detections-response-alerts

elastic/security-detections-response-rules

elastic/security-solution

elastic/security-solution-platform

e40pud left a comment

Choose a reason for hiding this comment

maximpn left a comment

Choose a reason for hiding this comment

xcrzx Sep 26, 2022

Choose a reason for hiding this comment

jpdjere Sep 27, 2022

Choose a reason for hiding this comment

xcrzx Sep 26, 2022

Choose a reason for hiding this comment

yctercero left a comment

Choose a reason for hiding this comment

kibana-ci commented Sep 27, 2022

💚 Build Succeeded

Metrics [docs]

Async chunks

Page load bundle

ESLint disabled line counts

Total ESLint disabled count

History

kibanamachine commented Sep 27, 2022

💚 All backports created successfully

Questions ?

jpdjere commented Sep 23, 2022 •

edited by kibanamachine

Loading