[Detections][Alerts] fixes saved_query rule validation issue #142602

vitaliidm · 2022-10-04T13:15:35Z

Summary

addresses [Security Solution] Validation message is displaying for a second when rule is created with saved query then edit the rule #141758
loads saved query on rules edit page before form rendering, thus prevents validation run on uncompleted data(saved query wasn't fetched before QueryBar component renders). Then fetched saved query passed to QueryBarDefineRule component

Before

saved_query_bug.mp4

After

Screen.Recording.2022-10-04.at.18.15.43.mov

As side effect, it reduced number of requests for saved query:

Before

2 requests were sent

After

Only one request sent

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests were updated or added to match the most common scenarios

…/vitaliidm/kibana into alerts/saved_query_ux_validation

elasticmachine · 2022-10-05T08:42:11Z

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

elasticmachine · 2022-10-05T08:42:13Z

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

maximpn

@vitaliidm overall the changes look good, though there is a couple of comments worth to check.

maximpn · 2022-10-08T08:10:21Z

x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/edit/index.tsx

@@ -98,6 +100,11 @@ const EditRulePageComponent: FC = () => {
  const [ruleLoading, rule] = useRule(ruleId);
  const loading = ruleLoading || userInfoLoading || listsConfigLoading;

+  const { isSavedQueryLoading, savedQueryBar, savedQuery } = useGetSavedQuery(rule?.saved_id, {
+    ruleType: rule?.type,
+    onError: noop,


onError: noop, reduces straightness of the code since it requires knowledge what's going on under the hood and suppressing of that behaviour when necessary. I'd recommend to consider removing the default error handling functionality inside, exposing of showErrorToastHandler and passing it directly when necessary. Though it uses toasts then it makes sense to expose a hook like useShowErrorToastHandler() which can be composable and reusable.

@maximpn
Toast is not displayed in this case because error is displayed on edit form when saved query is not loaded. Thus it's prevents showing duplicated notifications

maximpn · 2022-10-08T08:12:56Z

x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/edit/index.tsx

-            <StepPanel loading={loading} title={ruleI18n.DEFINITION}>
-              {defineStep.data != null && (
+            <StepPanel loading={loading || isSavedQueryLoading} title={ruleI18n.DEFINITION}>
+              {defineStepDataWithSavedQuery != null && !isSavedQueryLoading && (


Interesting that defineStepDataWithSavedQuery is either a specific object or undefined but there is a non strict check for null

non strict check for null eliminates undefined, so the value of defineStepDataWithSavedQuery will be object of DefineStepRule type.

Fairly common pattern across Kibana codebase (example from previous implementation):

<StepPanel loading={loading} title={ruleI18n.DEFINITION}> {defineStep.data != null && (

vitaliidm · 2022-10-10T12:55:40Z

@elasticmachine merge upstream

maximpn

@vitaliidm LGTM

kibana-ci · 2022-10-10T17:42:26Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 45901f9

Failed CI Steps

Security Solution Tests #4

Test Failures

[job] [logs] Security Solution Tests #4 / Row renderers Selected renderer can be disabled and enabled

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
`securitySolution`	6.6MB	6.6MB	+746.0B

History

💔 Build #79110 failed 4fca1c3
💚 Build #78811 succeeded bdd051c
💚 Build #78037 succeeded a5bb487
💔 Build #77930 failed 6ec20f5

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

cc @vitaliidm

…#142602) ## Summary - addresses elastic#141758 - loads saved query on rules edit page before form rendering, thus prevents validation run on uncompleted data(saved query wasn't fetched before QueryBar component renders). Then fetched saved query passed to QueryBarDefineRule component ### Before https://user-images.githubusercontent.com/92328789/193884969-3647e06b-085a-4923-82e8-546bedabeb3e.mp4 ### After https://user-images.githubusercontent.com/92328789/193884323-629fb879-3a51-4412-8b63-2f36afc618cb.mov As side effect, it reduced number of requests for saved query: #### Before 2 requests were sent <img width="1995" alt="Screenshot 2022-10-05 at 09 39 09" src="https://user-images.githubusercontent.com/92328789/194018652-5612a032-e908-4e01-9858-2049c8e453e2.png"> #### After Only one request sent ### Checklist <img width="1996" alt="Screenshot 2022-10-05 at 09 37 12" src="https://user-images.githubusercontent.com/92328789/194018637-770a08df-832b-4838-a060-05058c436ed6.png"> Delete any items that are not applicable to this PR. - [x] [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 (cherry picked from commit 4feb397)

kibanamachine · 2022-10-10T17:46:43Z

💚 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

#143008) ## Summary - addresses #141758 - loads saved query on rules edit page before form rendering, thus prevents validation run on uncompleted data(saved query wasn't fetched before QueryBar component renders). Then fetched saved query passed to QueryBarDefineRule component ### Before https://user-images.githubusercontent.com/92328789/193884969-3647e06b-085a-4923-82e8-546bedabeb3e.mp4 ### After https://user-images.githubusercontent.com/92328789/193884323-629fb879-3a51-4412-8b63-2f36afc618cb.mov As side effect, it reduced number of requests for saved query: #### Before 2 requests were sent <img width="1995" alt="Screenshot 2022-10-05 at 09 39 09" src="https://user-images.githubusercontent.com/92328789/194018652-5612a032-e908-4e01-9858-2049c8e453e2.png"> #### After Only one request sent ### Checklist <img width="1996" alt="Screenshot 2022-10-05 at 09 37 12" src="https://user-images.githubusercontent.com/92328789/194018637-770a08df-832b-4838-a060-05058c436ed6.png"> Delete any items that are not applicable to this PR. - [x] [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 (cherry picked from commit 4feb397) Co-authored-by: Vitalii Dmyterko <[email protected]>

vitaliidm and others added 7 commits October 4, 2022 14:12

[Detections][Alerts] saved_query rule validaiton

d516e67

Update index.tsx

6ec20f5

refactroing

63a2201

Merge branch 'alerts/saved_query_ux_validation' of https://github.com…

698d33a

…/vitaliidm/kibana into alerts/saved_query_ux_validation

refactoring

386133e

fix test

a24362e

Merge branch 'elastic:main' into alerts/saved_query_ux_validation

a5bb487

vitaliidm self-assigned this Oct 4, 2022

vitaliidm marked this pull request as ready for review October 5, 2022 08:42

vitaliidm requested review from a team as code owners October 5, 2022 08:42

vitaliidm requested a review from maximpn October 5, 2022 08:42

Merge branch 'main' into alerts/saved_query_ux_validation

bdd051c

maximpn reviewed Oct 8, 2022

View reviewed changes

vitaliidm requested a review from maximpn October 10, 2022 11:06

Merge branch 'main' into alerts/saved_query_ux_validation

4fca1c3

maximpn approved these changes Oct 10, 2022

View reviewed changes

Merge branch 'main' into alerts/saved_query_ux_validation

45901f9

vitaliidm enabled auto-merge (squash) October 10, 2022 16:15

vitaliidm merged commit 4feb397 into elastic:main Oct 10, 2022

kibanamachine mentioned this pull request Oct 10, 2022

[8.5] [Detections][Alerts] fixes saved_query rule validation issue (#142602) #143008

Merged

vitaliidm mentioned this pull request Oct 11, 2022

[Security Solution] Validation message is displaying for a second when rule is created with saved query then edit the rule #141758

Closed

vitaliidm deleted the alerts/saved_query_ux_validation branch March 4, 2024 17:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Detections][Alerts] fixes saved_query rule validation issue #142602

[Detections][Alerts] fixes saved_query rule validation issue #142602

vitaliidm commented Oct 4, 2022 •

edited by kibanamachine

Loading

elasticmachine commented Oct 5, 2022

elasticmachine commented Oct 5, 2022

maximpn left a comment

maximpn Oct 8, 2022

vitaliidm Oct 10, 2022

maximpn Oct 8, 2022

vitaliidm Oct 10, 2022

vitaliidm commented Oct 10, 2022

maximpn left a comment

kibana-ci commented Oct 10, 2022

kibanamachine commented Oct 10, 2022

[Detections][Alerts] fixes saved_query rule validation issue #142602

[Detections][Alerts] fixes saved_query rule validation issue #142602

Conversation

vitaliidm commented Oct 4, 2022 • edited by kibanamachine Loading

Summary

Before

After

Before

After

Checklist

elasticmachine commented Oct 5, 2022

elasticmachine commented Oct 5, 2022

maximpn left a comment

Choose a reason for hiding this comment

maximpn Oct 8, 2022

Choose a reason for hiding this comment

vitaliidm Oct 10, 2022

Choose a reason for hiding this comment

maximpn Oct 8, 2022

Choose a reason for hiding this comment

vitaliidm Oct 10, 2022

Choose a reason for hiding this comment

vitaliidm commented Oct 10, 2022

maximpn left a comment

Choose a reason for hiding this comment

kibana-ci commented Oct 10, 2022

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Async chunks

History

kibanamachine commented Oct 10, 2022

💚 All backports created successfully

Questions ?

vitaliidm commented Oct 4, 2022 •

edited by kibanamachine

Loading