From 217c9b18e7e1b44840cc03e92a9c99484af604b4 Mon Sep 17 00:00:00 2001 From: Ievgen Sorokopud Date: Fri, 13 Sep 2024 10:37:39 +0200 Subject: [PATCH] [Security Solution] Add validation error description on prebuilt rule editing (#191832) (#192683) ## Summary Partially addressed https://github.com/elastic/kibana/issues/191832 With these changes: - We revert to the https://github.com/elastic/kibana/issues/180407#issuecomment-2312891214. Specifically, we return back the validation errors to the modal window. An example of this modal is in the ticket description. - Additionally, on the Rule Editing page and **only for prebuilt rules** we: 1) hide the callout that says "You have an invalid input in this tab: ...", and 2) we don't show the modal if there are any data validation errors. We shouldn't show this modal and this callout until we release the prebuilt rule customization feature. 3) We will only validate the Actions tab. - Fix MKI flaky cypress tests introduced in https://github.com/elastic/kibana/pull/191487 ([1](https://buildkite.com/organizations/elastic/analytics/suites/serverless-mki-cypress-detection-engine/tests/b1f442af-db44-8029-a9fb-7e3d988303b3?branch=main), [2](https://buildkite.com/organizations/elastic/analytics/suites/serverless-mki-cypress-detection-engine/tests/995655b6-ae70-86fd-b483-c65846cd8d66?branch=main), [3](https://buildkite.com/organizations/elastic/analytics/suites/serverless-mki-cypress-detection-engine/tests/02318f5c-6ca1-8779-a5a4-60f52a55b344?branch=main)). All three tests are failing due to missing `[data-test-subj="eqlRuleType"]` element. After checking and comparing my tests to other similar tests in the file, the only difference that I've found was extra `login();` call. Thus removing those. Here is the screen recording showing the new behaviour for prebuilt rules. The has missing data source query validation error, though we do not show it and allow user just to save the rule. Only Actions tab is validated on rule save action. https://github.com/user-attachments/assets/ce968f51-1a53-41b2-ad06-1b31dec085a6 ### Checklist Delete any items that are not applicable to this PR. - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed * [Detection Engine - Cypress](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/6925) (100 ESS & 100 Serverless) * [Rule Management - Cypress](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/6926) (100 ESS & 100 Serverless) * [Prebuilt Rules - Cypress](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/6927) (100 ESS & 100 Serverless) (cherry picked from commit c937e95e3137821b510fa480ee28f0cf3afb85ad) --- .../save_with_errors_confirmation/index.tsx | 16 ++++++++++++++-- .../pages/rule_editing/index.tsx | 14 ++++++++++++-- .../rule_creation/eql_rule.cy.ts | 3 --- 3 files changed, 26 insertions(+), 7 deletions(-) diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/save_with_errors_confirmation/index.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/save_with_errors_confirmation/index.tsx index 839513bf0e34c..3f14945bedadc 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/save_with_errors_confirmation/index.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/save_with_errors_confirmation/index.tsx @@ -7,7 +7,7 @@ import React from 'react'; -import { EuiConfirmModal } from '@elastic/eui'; +import { EuiConfirmModal, EuiSpacer, EuiText } from '@elastic/eui'; import * as i18n from './translations'; @@ -32,7 +32,19 @@ const SaveWithErrorsModalComponent = ({ confirmButtonText={i18n.SAVE_WITH_ERRORS_CONFIRM_BUTTON} defaultFocusedButton="confirm" > - <>{i18n.SAVE_WITH_ERRORS_MODAL_MESSAGE(errors.length)} + <> + {i18n.SAVE_WITH_ERRORS_MODAL_MESSAGE(errors.length)} + + + ); }; diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_editing/index.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_editing/index.tsx index 1ecbde5b00d7b..6c139acbbdbff 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_editing/index.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_editing/index.tsx @@ -436,11 +436,20 @@ const EditRulePageComponent: FC<{ rule: RuleResponse }> = ({ rule }) => { const onSubmit = useCallback(async () => { setNonBlockingRuleErrors([]); + const actionsStepFormValid = await actionsStepForm.validate(); + if (rule.immutable) { + // Since users cannot edit Define, About and Schedule tabs of the rule, we skip validation of those to avoid + // user confusion of seeing that those tabs have error and not being able to see or do anything about that. + // We will need to remove this condition once rule customization work is done. + if (actionsStepFormValid) { + await saveChanges(); + } + return; + } + const defineStepFormValid = await defineStepForm.validate(); const aboutStepFormValid = await aboutStepForm.validate(); const scheduleStepFormValid = await scheduleStepForm.validate(); - const actionsStepFormValid = await actionsStepForm.validate(); - if ( defineStepFormValid && aboutStepFormValid && @@ -465,6 +474,7 @@ const EditRulePageComponent: FC<{ rule: RuleResponse }> = ({ rule }) => { showSaveWithErrorsModal(); } }, [ + rule.immutable, defineStepForm, aboutStepForm, scheduleStepForm, diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_creation/eql_rule.cy.ts b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_creation/eql_rule.cy.ts index a14218fcdda56..dea2e554e8e4f 100644 --- a/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_creation/eql_rule.cy.ts +++ b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_creation/eql_rule.cy.ts @@ -231,7 +231,6 @@ describe('EQL rules', { tags: ['@ess', '@serverless'] }, () => { const rule = getEqlRule(); it('validates missing data source', () => { - login(); visit(CREATE_RULE_URL); selectEqlRuleType(); getIndexPatternClearButton().click(); @@ -258,7 +257,6 @@ describe('EQL rules', { tags: ['@ess', '@serverless'] }, () => { }); it('validates missing data fields', () => { - login(); visit(CREATE_RULE_URL); selectEqlRuleType(); @@ -282,7 +280,6 @@ describe('EQL rules', { tags: ['@ess', '@serverless'] }, () => { }); it('validates syntax errors', () => { - login(); visit(CREATE_RULE_URL); selectEqlRuleType();