From 010088d0a5b19cd78d3be43287dbc2f6f8bdd324 Mon Sep 17 00:00:00 2001 From: Ievgen Sorokopud Date: Mon, 26 Aug 2024 11:11:16 +0200 Subject: [PATCH 1/6] [Security Solution] Editing rules independently of source data (#180407) --- .../save_with_errors_confirmation/index.tsx | 53 ++++ .../translations.ts | 36 +++ .../rule_creation_ui/pages/form.tsx | 87 +++++- .../pages/rule_creation/index.tsx | 250 ++++++++++++------ .../pages/rule_editing/index.tsx | 103 ++++++-- .../rule_creation_ui/pages/translations.ts | 6 + .../rule_creation/eql_rule.cy.ts | 20 +- .../rule_creation/esql_rule.cy.ts | 16 ++ .../rule_edit/eql_query_rule.cy.ts | 47 ++++ .../rule_edit/esql_rule.cy.ts | 30 ++- .../cypress/screens/create_new_rule.ts | 4 + .../cypress/tasks/create_new_rule.ts | 10 + .../cypress/tasks/edit_rule.ts | 12 + 13 files changed, 566 insertions(+), 108 deletions(-) create mode 100644 x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/save_with_errors_confirmation/index.tsx create mode 100644 x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/save_with_errors_confirmation/translations.ts create mode 100644 x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_edit/eql_query_rule.cy.ts 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 new file mode 100644 index 0000000000000..3f14945bedadc --- /dev/null +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/save_with_errors_confirmation/index.tsx @@ -0,0 +1,53 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import React from 'react'; + +import { EuiConfirmModal, EuiSpacer, EuiText } from '@elastic/eui'; + +import * as i18n from './translations'; + +interface SaveWithErrorsModalProps { + errors: string[]; + onCancel: () => void; + onConfirm: () => void; +} + +const SaveWithErrorsModalComponent = ({ + errors, + onCancel, + onConfirm, +}: SaveWithErrorsModalProps) => { + return ( + + <> + {i18n.SAVE_WITH_ERRORS_MODAL_MESSAGE(errors.length)} + +
    + {errors.map((validationError, idx) => { + return ( +
  • + {validationError} +
    • + ); + })} +
+ + + ); +}; + +export const SaveWithErrorsModal = React.memo(SaveWithErrorsModalComponent); +SaveWithErrorsModal.displayName = 'SaveWithErrorsModal'; diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/save_with_errors_confirmation/translations.ts b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/save_with_errors_confirmation/translations.ts new file mode 100644 index 0000000000000..e470b06c7e829 --- /dev/null +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/save_with_errors_confirmation/translations.ts @@ -0,0 +1,36 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { i18n } from '@kbn/i18n'; + +export const SAVE_WITH_ERRORS_MODAL_TITLE = i18n.translate( + 'xpack.securitySolution.detectionEngine.createRule.saveWithErrorsModalTitle', + { + defaultMessage: 'This rule has validation errors', + } +); + +export const SAVE_WITH_ERRORS_CANCEL_BUTTON = i18n.translate( + 'xpack.securitySolution.detectionEngine.createRule.saveWithErrorsCancelButton', + { + defaultMessage: 'Cancel', + } +); + +export const SAVE_WITH_ERRORS_CONFIRM_BUTTON = i18n.translate( + 'xpack.securitySolution.detectionEngine.createRule.saveWithErrorsConfirmButton', + { + defaultMessage: 'Confirm', + } +); + +export const SAVE_WITH_ERRORS_MODAL_MESSAGE = (errorsCount: number) => + i18n.translate('xpack.securitySolution.detectionEngine.createRule.saveWithErrorsModalMessage', { + defaultMessage: + 'This rule has {errorsCount} validation {errorsCount, plural, one {error} other {errors}} which can lead to failed rule executions, save anyway?', + values: { errorsCount }, + }); diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/form.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/form.tsx index 690dedafaf852..d7e3f4566a887 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/form.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/form.tsx @@ -5,7 +5,7 @@ * 2.0. */ -import { useState, useMemo, useEffect } from 'react'; +import { useState, useMemo, useEffect, useCallback } from 'react'; import type { DataViewBase } from '@kbn/es-query'; import { isThreatMatchRule } from '../../../../common/detection_engine/utils'; import type { @@ -16,6 +16,7 @@ import type { } from '../../../detections/pages/detection_engine/rules/types'; import { DataSourceType } from '../../../detections/pages/detection_engine/rules/types'; import { useKibana } from '../../../common/lib/kibana'; +import type { FormHook, ValidationError } from '../../../shared_imports'; import { useForm, useFormData } from '../../../shared_imports'; import { schema as defineRuleSchema } from '../components/step_define_rule/schema'; import type { EqlOptionsSelected } from '../../../../common/search_strategy'; @@ -26,6 +27,9 @@ import { import { schema as scheduleRuleSchema } from '../components/step_schedule_rule/schema'; import { getSchema as getActionsRuleSchema } from '../../rule_creation/components/step_rule_actions/get_schema'; import { useFetchIndex } from '../../../common/containers/source'; +import { ERROR_CODES as ESQL_ERROR_CODES } from '../../rule_creation/logic/esql_validator'; +import { EQL_ERROR_CODES } from '../../../common/hooks/eql/api'; +import * as i18n from './translations'; export interface UseRuleFormsProps { defineStepDefault: DefineStepRule; @@ -156,3 +160,84 @@ export const useRuleIndexPattern = ({ }, [dataSourceType, isIndexPatternLoading, data, dataViewId, initIndexPattern]); return { indexPattern, isIndexPatternLoading, browserFields }; }; + +export interface UseRuleFormsErrors { + defineStepForm?: FormHook; + aboutStepForm?: FormHook; + scheduleStepForm?: FormHook; + actionsStepForm?: FormHook; +} + +const getFieldErrorMessages = (fieldError: ValidationError) => { + if (fieldError.message.length > 0) { + return [fieldError.message]; + } else if (Array.isArray(fieldError.messages)) { + return fieldError.messages.map((message) => JSON.stringify(message)); + } + return []; +}; + +const NON_BLOCKING_QUERY_BAR_ERROR_CODES = [ + ESQL_ERROR_CODES.INVALID_ESQL, + EQL_ERROR_CODES.FAILED_REQUEST, + EQL_ERROR_CODES.INVALID_EQL, + EQL_ERROR_CODES.MISSING_DATA_SOURCE, +]; + +const isNonBlockingQueryBarErrorCode = (errorCode?: string) => { + return !!NON_BLOCKING_QUERY_BAR_ERROR_CODES.find((code) => code === errorCode); +}; + +const NON_BLOCKING_ERROR_CODES = [...NON_BLOCKING_QUERY_BAR_ERROR_CODES]; + +const isNonBlockingErrorCode = (errorCode?: string) => { + return !!NON_BLOCKING_ERROR_CODES.find((code) => code === errorCode); +}; + +const transformValidationError = ({ + errorCode, + errorMessage, +}: { + errorCode?: string; + errorMessage: string; +}) => { + if (isNonBlockingQueryBarErrorCode(errorCode)) { + return i18n.QUERY_BAR_VALIDATION_ERROR(errorMessage); + } + return errorMessage; +}; + +export const useRuleFormsErrors = () => { + const getRuleFormsErrors = useCallback( + ({ defineStepForm, aboutStepForm, scheduleStepForm, actionsStepForm }: UseRuleFormsErrors) => { + const blockingErrors: string[] = []; + const nonBlockingErrors: string[] = []; + + for (const [_, fieldHook] of Object.entries(defineStepForm?.getFields() ?? {})) { + fieldHook.errors.forEach((fieldError) => { + const messages = getFieldErrorMessages(fieldError); + if (isNonBlockingErrorCode(fieldError.code)) { + nonBlockingErrors.push( + ...messages.map((message) => + transformValidationError({ errorCode: fieldError.code, errorMessage: message }) + ) + ); + } else { + blockingErrors.push(...messages); + } + }); + } + + const blockingForms = [aboutStepForm, scheduleStepForm, actionsStepForm]; + blockingForms.forEach((form) => { + for (const [_, fieldHook] of Object.entries(form?.getFields() ?? {})) { + blockingErrors.push(...fieldHook.errors.map((fieldError) => fieldError.message)); + } + }); + return { blockingErrors, nonBlockingErrors }; + }, + [] + ); + + return { getRuleFormsErrors }; +}; diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/index.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/index.tsx index 6e93d6927b6be..97b89630392d8 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/index.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/index.tsx @@ -80,8 +80,9 @@ import { RulePreview } from '../../components/rule_preview'; import { getIsRulePreviewDisabled } from '../../components/rule_preview/helpers'; import { useStartMlJobs } from '../../../rule_management/logic/use_start_ml_jobs'; import { NextStep } from '../../components/next_step'; -import { useRuleForms, useRuleIndexPattern } from '../form'; +import { useRuleForms, useRuleFormsErrors, useRuleIndexPattern } from '../form'; import { CustomHeaderPageMemo } from '..'; +import { SaveWithErrorsModal } from '../../components/save_with_errors_confirmation'; const MyEuiPanel = styled(EuiPanel)<{ zindex?: number; @@ -210,6 +211,12 @@ const CreateRulePageComponent: React.FC = () => { const [isQueryBarValid, setIsQueryBarValid] = useState(false); const [isThreatQueryBarValid, setIsThreatQueryBarValid] = useState(false); + const [isSaveWithErrorsModalVisible, setIsSaveWithErrorsModalVisible] = useState(false); + const [enableRuleAfterConfirmation, setEnableRuleAfterConfirmation] = useState(false); + const [nonBlockingRuleErrors, setNonBlockingRuleErrors] = useState([]); + + const { getRuleFormsErrors } = useRuleFormsErrors(); + const esqlQueryForAboutStep = useEsqlQueryForAboutStep({ defineStepData, activeStep }); const esqlIndex = useEsqlIndex(defineStepData.queryBar.query.query, ruleType); @@ -328,89 +335,173 @@ const CreateRulePageComponent: React.FC = () => { const validateStep = useCallback( async (step: RuleStep) => { switch (step) { - case RuleStep.defineRule: - return defineStepForm.validate(); - case RuleStep.aboutRule: - return aboutStepForm.validate(); - case RuleStep.scheduleRule: - return scheduleStepForm.validate(); - case RuleStep.ruleActions: - return actionsStepForm.validate(); + case RuleStep.defineRule: { + const valid = await defineStepForm.validate(); + const { blockingErrors, nonBlockingErrors } = getRuleFormsErrors({ defineStepForm }); + return { valid, blockingErrors, nonBlockingErrors }; + } + case RuleStep.aboutRule: { + const valid = await aboutStepForm.validate(); + const { blockingErrors, nonBlockingErrors } = getRuleFormsErrors({ aboutStepForm }); + return { valid, blockingErrors, nonBlockingErrors }; + } + case RuleStep.scheduleRule: { + const valid = await scheduleStepForm.validate(); + const { blockingErrors, nonBlockingErrors } = getRuleFormsErrors({ scheduleStepForm }); + return { valid, blockingErrors, nonBlockingErrors }; + } + case RuleStep.ruleActions: { + const valid = await actionsStepForm.validate(); + const { blockingErrors, nonBlockingErrors } = getRuleFormsErrors({ actionsStepForm }); + return { valid, blockingErrors, nonBlockingErrors }; + } } }, - [aboutStepForm, actionsStepForm, defineStepForm, scheduleStepForm] + [aboutStepForm, actionsStepForm, defineStepForm, getRuleFormsErrors, scheduleStepForm] + ); + + const validateEachStep = useCallback(async () => { + const { + valid: defineStepFormValid, + blockingErrors: defineStepBlockingErrors, + nonBlockingErrors: defineStepNonBlockingErrors, + } = await validateStep(RuleStep.defineRule); + const { + valid: aboutStepFormValid, + blockingErrors: aboutStepBlockingErrors, + nonBlockingErrors: aboutStepNonBlockingErrors, + } = await validateStep(RuleStep.aboutRule); + const { + valid: scheduleStepFormValid, + blockingErrors: scheduleStepBlockingErrors, + nonBlockingErrors: scheduleStepNonBlockingErrors, + } = await validateStep(RuleStep.scheduleRule); + const { + valid: actionsStepFormValid, + blockingErrors: actionsStepBlockingErrors, + nonBlockingErrors: actionsStepNonBlockingErrors, + } = await validateStep(RuleStep.ruleActions); + const valid = + defineStepFormValid && aboutStepFormValid && scheduleStepFormValid && actionsStepFormValid; + + const blockingErrors = [ + ...defineStepBlockingErrors, + ...aboutStepBlockingErrors, + ...scheduleStepBlockingErrors, + ...actionsStepBlockingErrors, + ]; + const nonBlockingErrors = [ + ...defineStepNonBlockingErrors, + ...aboutStepNonBlockingErrors, + ...scheduleStepNonBlockingErrors, + ...actionsStepNonBlockingErrors, + ]; + + return { valid, blockingErrors, nonBlockingErrors }; + }, [validateStep]); + + const isValidStep = useCallback( + async (step: RuleStep) => { + const { valid, blockingErrors } = await validateStep(step); + return valid || !blockingErrors.length; + }, + [validateStep] ); const editStep = useCallback( async (step: RuleStep) => { - const valid = await validateStep(activeStep); - + const valid = await isValidStep(activeStep); if (valid) { goToStep(step); } }, - [activeStep, validateStep, goToStep] + [isValidStep, activeStep, goToStep] ); - const submitRule = useCallback( - async (step: RuleStep, enabled: boolean) => { - const valid = await validateStep(step); - - if (valid) { - const localDefineStepData: DefineStepRule = defineFieldsTransform({ - ...defineStepForm.getFormData(), - eqlOptions: eqlOptionsSelected, - }); - const localAboutStepData = aboutStepForm.getFormData(); - const localScheduleStepData = scheduleStepForm.getFormData(); - const localActionsStepData = actionsStepForm.getFormData(); - const startMlJobsIfNeeded = async () => { - if (!isMlRule(ruleType) || !enabled) { - return; - } - await startMlJobs(localDefineStepData.machineLearningJobId); - }; - const [, createdRule] = await Promise.all([ - startMlJobsIfNeeded(), - createRule( - formatRule( - localDefineStepData, - localAboutStepData, - localScheduleStepData, - { - ...localActionsStepData, - enabled, - }, - triggersActionsUi.actionTypeRegistry - ) - ), - ]); - - addSuccess(i18n.SUCCESSFULLY_CREATED_RULES(createdRule.name)); - - navigateToApp(APP_UI_ID, { - deepLinkId: SecurityPageName.rules, - path: getRuleDetailsUrl(createdRule.id), - }); - } + const createRuleFromFormData = useCallback( + async (enabled: boolean) => { + const localDefineStepData: DefineStepRule = defineFieldsTransform({ + ...defineStepForm.getFormData(), + eqlOptions: eqlOptionsSelected, + }); + const localAboutStepData = aboutStepForm.getFormData(); + const localScheduleStepData = scheduleStepForm.getFormData(); + const localActionsStepData = actionsStepForm.getFormData(); + const startMlJobsIfNeeded = async () => { + if (!isMlRule(ruleType) || !enabled) { + return; + } + await startMlJobs(localDefineStepData.machineLearningJobId); + }; + const [, createdRule] = await Promise.all([ + startMlJobsIfNeeded(), + createRule( + formatRule( + localDefineStepData, + localAboutStepData, + localScheduleStepData, + { + ...localActionsStepData, + enabled, + }, + triggersActionsUi.actionTypeRegistry + ) + ), + ]); + + addSuccess(i18n.SUCCESSFULLY_CREATED_RULES(createdRule.name)); + + navigateToApp(APP_UI_ID, { + deepLinkId: SecurityPageName.rules, + path: getRuleDetailsUrl(createdRule.id), + }); }, [ - validateStep, - defineStepForm, - eqlOptionsSelected, aboutStepForm, - scheduleStepForm, actionsStepForm, - createRule, addSuccess, + createRule, + defineFieldsTransform, + defineStepForm, + eqlOptionsSelected, navigateToApp, ruleType, + scheduleStepForm, startMlJobs, - defineFieldsTransform, triggersActionsUi.actionTypeRegistry, ] ); + const submitRule = useCallback( + async (enabled: boolean) => { + const { valid, blockingErrors, nonBlockingErrors } = await validateEachStep(); + if (valid) { + // There are no validation errors, thus proceed to rule creation + createRuleFromFormData(enabled); + return; + } + + if (blockingErrors.length > 0) { + // There are blocking validation errors, thus do not allow user to create a rule + return; + } + if (nonBlockingErrors.length > 0) { + // There are non-blocking validation errors, thus confirm that user understand that this can cause rule failures + setEnableRuleAfterConfirmation(enabled); + setNonBlockingRuleErrors(nonBlockingErrors); + showSaveWithErrorsModal(); + } + }, + [createRuleFromFormData, validateEachStep] + ); + + const showSaveWithErrorsModal = () => setIsSaveWithErrorsModalVisible(true); + const closeSaveWithErrorsModal = () => setIsSaveWithErrorsModalVisible(false); + const onConfirmSaveWithErrors = useCallback(async () => { + closeSaveWithErrorsModal(); + await createRuleFromFormData(enableRuleAfterConfirmation); + }, [createRuleFromFormData, enableRuleAfterConfirmation]); + const defineRuleButtonType = activeStep === RuleStep.defineRule ? 'active' : defineStepForm.isValid ? 'valid' : 'passive'; const defineRuleButton = useMemo( @@ -418,14 +509,14 @@ const CreateRulePageComponent: React.FC = () => { [defineRuleButtonType] ); const defineRuleNextStep = useCallback(async () => { - const valid = await defineStepForm.validate(); + const valid = await isValidStep(RuleStep.defineRule); if (valid) { const nextStep = getNextStep(RuleStep.defineRule); if (nextStep) { goToStep(nextStep); } } - }, [defineStepForm, goToStep]); + }, [goToStep, isValidStep]); const aboutRuleButtonType = activeStep === RuleStep.aboutRule ? 'active' : aboutStepForm.isValid ? 'valid' : 'passive'; @@ -434,14 +525,14 @@ const CreateRulePageComponent: React.FC = () => { [aboutRuleButtonType] ); const aboutRuleNextStep = useCallback(async () => { - const valid = await aboutStepForm.validate(); + const valid = await isValidStep(RuleStep.aboutRule); if (valid) { const nextStep = getNextStep(RuleStep.aboutRule); if (nextStep) { goToStep(nextStep); } } - }, [aboutStepForm, goToStep]); + }, [goToStep, isValidStep]); const scheduleRuleButtonType = activeStep === RuleStep.scheduleRule @@ -454,14 +545,14 @@ const CreateRulePageComponent: React.FC = () => { [scheduleRuleButtonType] ); const scheduleRuleNextStep = useCallback(async () => { - const valid = await scheduleStepForm.validate(); + const valid = await isValidStep(RuleStep.scheduleRule); if (valid) { const nextStep = getNextStep(RuleStep.scheduleRule); if (nextStep) { goToStep(nextStep); } } - }, [scheduleStepForm, goToStep]); + }, [isValidStep, goToStep]); const actionsRuleButtonType = activeStep === RuleStep.ruleActions ? 'active' : actionsStepForm.isValid ? 'valid' : 'passive'; @@ -470,10 +561,10 @@ const CreateRulePageComponent: React.FC = () => { [actionsRuleButtonType] ); const submitRuleDisabled = useCallback(() => { - submitRule(RuleStep.ruleActions, false); + submitRule(false); }, [submitRule]); const submitRuleEnabled = useCallback(() => { - submitRule(RuleStep.ruleActions, true); + submitRule(true); }, [submitRule]); const memoDefineStepReadOnly = useMemo( @@ -559,7 +650,7 @@ const CreateRulePageComponent: React.FC = () => { ); const memoDefineStepExtraAction = useMemo( () => - defineStepForm.isValid && ( + activeStep !== RuleStep.defineRule && ( { {i18n.EDIT_RULE} ), - [defineStepForm.isValid, editStep] + [activeStep, editStep] ); const memoAboutStepReadOnly = useMemo( @@ -629,7 +720,7 @@ const CreateRulePageComponent: React.FC = () => { ); const memoAboutStepExtraAction = useMemo( () => - aboutStepForm.isValid && ( + activeStep !== RuleStep.aboutRule && ( { {i18n.EDIT_RULE} ), - [aboutStepForm.isValid, editStep] + [activeStep, editStep] ); const memoStepScheduleRule = useMemo( @@ -682,12 +773,12 @@ const CreateRulePageComponent: React.FC = () => { ); const memoScheduleStepExtraAction = useMemo( () => - scheduleStepForm.isValid && ( + activeStep !== RuleStep.scheduleRule && ( editStep(RuleStep.scheduleRule)}> {i18n.EDIT_RULE} ), - [editStep, scheduleStepForm.isValid] + [activeStep, editStep] ); const memoStepRuleActions = useMemo( @@ -762,12 +853,12 @@ const CreateRulePageComponent: React.FC = () => { ); const memoActionsStepExtraAction = useMemo( () => - actionsStepForm.isValid && ( + activeStep !== RuleStep.ruleActions && ( editStep(RuleStep.ruleActions)}> {i18n.EDIT_RULE} ), - [actionsStepForm.isValid, editStep] + [activeStep, editStep] ); const onToggleCollapsedMemo = useCallback( @@ -798,6 +889,13 @@ const CreateRulePageComponent: React.FC = () => { return ( <> + {isSaveWithErrorsModalVisible && ( + + )} {(EuiResizablePanel, EuiResizableButton, { togglePanel }) => { 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 5e6ed40d4d282..0544f01bb1b7a 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 @@ -67,9 +67,10 @@ import { import { useStartTransaction } from '../../../../common/lib/apm/use_start_transaction'; import { SINGLE_RULE_ACTIONS } from '../../../../common/lib/apm/user_actions'; import { useGetSavedQuery } from '../../../../detections/pages/detection_engine/rules/use_get_saved_query'; -import { useRuleForms, useRuleIndexPattern } from '../form'; +import { useRuleForms, useRuleFormsErrors, useRuleIndexPattern } from '../form'; import { useEsqlIndex, useEsqlQueryForAboutStep } from '../../hooks'; import { CustomHeaderPageMemo } from '..'; +import { SaveWithErrorsModal } from '../../components/save_with_errors_confirmation'; const EditRulePageComponent: FC<{ rule: RuleResponse }> = ({ rule }) => { const [, dispatchToaster] = useStateToaster(); @@ -99,6 +100,9 @@ const EditRulePageComponent: FC<{ rule: RuleResponse }> = ({ rule }) => { const [isQueryBarValid, setIsQueryBarValid] = useState(false); const [isThreatQueryBarValid, setIsThreatQueryBarValid] = useState(false); + const [isSaveWithErrorsModalVisible, setIsSaveWithErrorsModalVisible] = useState(false); + const [nonBlockingRuleErrors, setNonBlockingRuleErrors] = useState([]); + useEffect(() => { const fetchDataViews = async () => { const dataViewsRefs = await dataServices.dataViews.getIdsWithTitle(); @@ -149,6 +153,8 @@ const EditRulePageComponent: FC<{ rule: RuleResponse }> = ({ rule }) => { actionsStepDefault: ruleActionsData, }); + const { getRuleFormsErrors } = useRuleFormsErrors(); + const esqlQueryForAboutStep = useEsqlQueryForAboutStep({ defineStepData, activeStep }); const esqlIndex = useEsqlIndex(defineStepData.queryBar.query.query, defineStepData.ruleType); @@ -386,7 +392,43 @@ const EditRulePageComponent: FC<{ rule: RuleResponse }> = ({ rule }) => { const { startTransaction } = useStartTransaction(); + const saveChanges = useCallback(async () => { + startTransaction({ name: SINGLE_RULE_ACTIONS.SAVE }); + await updateRule({ + ...formatRule( + defineStepData, + aboutStepData, + scheduleStepData, + actionsStepData, + triggersActionsUi.actionTypeRegistry, + rule?.exceptions_list + ), + ...(ruleId ? { id: ruleId } : {}), + }); + + displaySuccessToast(i18n.SUCCESSFULLY_SAVED_RULE(rule?.name ?? ''), dispatchToaster); + navigateToApp(APP_UI_ID, { + deepLinkId: SecurityPageName.rules, + path: getRuleDetailsUrl(ruleId ?? ''), + }); + }, [ + aboutStepData, + actionsStepData, + defineStepData, + dispatchToaster, + navigateToApp, + rule?.exceptions_list, + rule?.name, + ruleId, + scheduleStepData, + startTransaction, + triggersActionsUi.actionTypeRegistry, + updateRule, + ]); + const onSubmit = useCallback(async () => { + setNonBlockingRuleErrors([]); + const defineStepFormValid = await defineStepForm.validate(); const aboutStepFormValid = await aboutStepForm.validate(); const scheduleStepFormValid = await scheduleStepForm.validate(); @@ -398,43 +440,39 @@ const EditRulePageComponent: FC<{ rule: RuleResponse }> = ({ rule }) => { scheduleStepFormValid && actionsStepFormValid ) { - startTransaction({ name: SINGLE_RULE_ACTIONS.SAVE }); - await updateRule({ - ...formatRule( - defineStepData, - aboutStepData, - scheduleStepData, - actionsStepData, - triggersActionsUi.actionTypeRegistry, - rule?.exceptions_list - ), - ...(ruleId ? { id: ruleId } : {}), - }); + await saveChanges(); + return; + } - displaySuccessToast(i18n.SUCCESSFULLY_SAVED_RULE(rule?.name ?? ''), dispatchToaster); - navigateToApp(APP_UI_ID, { - deepLinkId: SecurityPageName.rules, - path: getRuleDetailsUrl(ruleId ?? ''), - }); + const { blockingErrors, nonBlockingErrors } = getRuleFormsErrors({ + defineStepForm, + aboutStepForm, + scheduleStepForm, + actionsStepForm, + }); + if (blockingErrors.length > 0) { + return; + } + if (nonBlockingErrors.length > 0) { + setNonBlockingRuleErrors(nonBlockingErrors); + showSaveWithErrorsModal(); } }, [ defineStepForm, aboutStepForm, scheduleStepForm, actionsStepForm, - startTransaction, - updateRule, - defineStepData, - aboutStepData, - scheduleStepData, - actionsStepData, - rule, - ruleId, - dispatchToaster, - navigateToApp, - triggersActionsUi.actionTypeRegistry, + getRuleFormsErrors, + saveChanges, ]); + const showSaveWithErrorsModal = () => setIsSaveWithErrorsModalVisible(true); + const closeSaveWithErrorsModal = () => setIsSaveWithErrorsModalVisible(false); + const onConfirmSaveWithErrors = useCallback(async () => { + closeSaveWithErrorsModal(); + await saveChanges(); + }, [saveChanges]); + const onTabClick = useCallback(async (tab: EuiTabbedContentTab) => { const targetStep = tab.id as RuleStep; setActiveStep(targetStep); @@ -488,6 +526,13 @@ const EditRulePageComponent: FC<{ rule: RuleResponse }> = ({ rule }) => { return ( <> + {isSaveWithErrorsModalVisible && ( + + )} {(EuiResizablePanel, EuiResizableButton, { togglePanel }) => { diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/translations.ts b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/translations.ts index e602b8be712c2..7a2064dd4f35c 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/translations.ts +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/translations.ts @@ -13,3 +13,9 @@ export const RULE_PREVIEW_TITLE = i18n.translate( defaultMessage: 'Rule preview', } ); + +export const QUERY_BAR_VALIDATION_ERROR = (vlidationError: string) => + i18n.translate('xpack.securitySolution.detectionEngine.createRule.validationError', { + values: { vlidationError }, + defaultMessage: 'Query bar: {vlidationError}', + }); 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 2b0b9f3fdab61..a14218fcdda56 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 @@ -46,10 +46,13 @@ import { getDetails } from '../../../../tasks/rule_details'; import { expectNumberOfRules, goToRuleDetailsOf } from '../../../../tasks/alerts_detection_rules'; import { deleteAlertsAndRules } from '../../../../tasks/api_calls/common'; import { + continueFromDefineStep, createAndEnableRule, + createRuleWithNonBlockingErrors, fillAboutRuleAndContinue, fillDefineEqlRuleAndContinue, fillScheduleRuleAndContinue, + getDefineContinueButton, getIndexPatternClearButton, getRuleIndexInput, selectEqlRuleType, @@ -225,6 +228,8 @@ describe('EQL rules', { tags: ['@ess', '@serverless'] }, () => { }); describe('EQL query validation', () => { + const rule = getEqlRule(); + it('validates missing data source', () => { login(); visit(CREATE_RULE_URL); @@ -236,14 +241,20 @@ describe('EQL rules', { tags: ['@ess', '@serverless'] }, () => { cy.get(RULES_CREATION_FORM).find(EQL_QUERY_INPUT).should('be.visible'); cy.get(RULES_CREATION_FORM).find(EQL_QUERY_INPUT).type('any where true'); + const expectedValidationError = `index_not_found_exception\n\tCaused by:\n\t\tverification_exception: Found 1 problem\nline -1:-1: Unknown index [*,-*]\n\tRoot causes:\n\t\tverification_exception: Found 1 problem\nline -1:-1: Unknown index [*,-*]`; cy.get(EQL_QUERY_VALIDATION_ERROR).should('be.visible'); cy.get(EQL_QUERY_VALIDATION_ERROR).should('have.text', '1'); cy.get(EQL_QUERY_VALIDATION_ERROR).click(); cy.get(EQL_QUERY_VALIDATION_ERROR_CONTENT).should('be.visible'); cy.get(EQL_QUERY_VALIDATION_ERROR_CONTENT).should( 'have.text', - `EQL Validation Errorsindex_not_found_exception\n\tCaused by:\n\t\tverification_exception: Found 1 problem\nline -1:-1: Unknown index [*,-*]\n\tRoot causes:\n\t\tverification_exception: Found 1 problem\nline -1:-1: Unknown index [*,-*]` + `EQL Validation Errors${expectedValidationError}` ); + continueFromDefineStep(); + + fillAboutRuleAndContinue(rule); + fillScheduleRuleAndContinue(rule); + createRuleWithNonBlockingErrors(); }); it('validates missing data fields', () => { @@ -263,6 +274,11 @@ describe('EQL rules', { tags: ['@ess', '@serverless'] }, () => { 'have.text', 'EQL Validation ErrorsFound 1 problem\nline 1:11: Unknown column [field1]' ); + continueFromDefineStep(); + + fillAboutRuleAndContinue(rule); + fillScheduleRuleAndContinue(rule); + createRuleWithNonBlockingErrors(); }); it('validates syntax errors', () => { @@ -282,6 +298,8 @@ describe('EQL rules', { tags: ['@ess', '@serverless'] }, () => { 'have.text', `EQL Validation Errorsline 1:6: extraneous input 'any' expecting 'where'` ); + continueFromDefineStep(); + getDefineContinueButton().should('exist'); }); }); }); diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_creation/esql_rule.cy.ts b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_creation/esql_rule.cy.ts index 348133b1d2802..3727c2ccbd501 100644 --- a/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_creation/esql_rule.cy.ts +++ b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_creation/esql_rule.cy.ts @@ -49,6 +49,7 @@ import { fillAboutRuleMinimumAndContinue, skipScheduleRuleAction, interceptEsqlQueryFieldsRequest, + createRuleWithNonBlockingErrors, } from '../../../../tasks/create_new_rule'; import { login } from '../../../../tasks/login'; import { visit } from '../../../../tasks/navigation'; @@ -191,6 +192,21 @@ describe( `Error validating ES|QL: "SyntaxError: extraneous input 'test' expecting "` ); }); + + it('shows confirmation modal about existing non-blocking validation errors', function () { + const nonExistingDataSourceQuery = 'from fake* metadata _id, _version, _index | limit 5'; + selectEsqlRuleType(); + fillEsqlQueryBar(nonExistingDataSourceQuery); + getDefineContinueButton().click(); + + fillRuleName(); + fillDescription(); + getAboutContinueButton().click(); + + fillScheduleRuleAndContinue(rule); + + createRuleWithNonBlockingErrors(); + }); }); describe('ES|QL investigation fields', () => { diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_edit/eql_query_rule.cy.ts b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_edit/eql_query_rule.cy.ts new file mode 100644 index 0000000000000..994dbb2eb8ce8 --- /dev/null +++ b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_edit/eql_query_rule.cy.ts @@ -0,0 +1,47 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { getEqlRule } from '../../../../objects/rule'; + +import { createRule } from '../../../../tasks/api_calls/rules'; +import { deleteAlertsAndRules } from '../../../../tasks/api_calls/common'; +import { + saveEditedRuleWithNonBlockingErrors, + visitEditRulePage, +} from '../../../../tasks/edit_rule'; +import { login } from '../../../../tasks/login'; + +describe('EQL query rules', { tags: ['@ess', '@serverless'] }, () => { + context('Editing rule with non-blocking query validation errors', () => { + beforeEach(() => { + login(); + deleteAlertsAndRules(); + }); + + it('should allow user to save a rule and show confirmation modal when data source does not exist', () => { + const rule = { + ...getEqlRule(), + index: ['fake*'], + }; + createRule(rule).then((createdRule) => { + visitEditRulePage(createdRule.body.id); + saveEditedRuleWithNonBlockingErrors(); + }); + }); + + it('should allow user to save a rule and show confirmation modal when data field does not exist', () => { + const rule = { + ...getEqlRule(), + query: 'any where hello.world', + }; + createRule(rule).then((createdRule) => { + visitEditRulePage(createdRule.body.id); + saveEditedRuleWithNonBlockingErrors(); + }); + }); + }); +}); diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_edit/esql_rule.cy.ts b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_edit/esql_rule.cy.ts index 43655b1358b29..a180e1a0b50c2 100644 --- a/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_edit/esql_rule.cy.ts +++ b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_edit/esql_rule.cy.ts @@ -43,7 +43,11 @@ import { login } from '../../../../tasks/login'; import { editFirstRule } from '../../../../tasks/alerts_detection_rules'; -import { saveEditedRule } from '../../../../tasks/edit_rule'; +import { + saveEditedRule, + saveEditedRuleWithNonBlockingErrors, + visitEditRulePage, +} from '../../../../tasks/edit_rule'; import { visit } from '../../../../tasks/navigation'; const rule = getEsqlRule(); @@ -192,5 +196,29 @@ describe( }); }); }); + + describe('Editing rule with non-blocking query validation errors', () => { + it('should allow user to save a rule and show confirmation modal when data source does not exist', () => { + const esqlRule = { + ...rule, + query: 'from fake-* metadata _id, _version, _index | keep agent.*,_id | eval test_id=_id', + }; + createRule(esqlRule).then((createdRule) => { + visitEditRulePage(createdRule.body.id); + saveEditedRuleWithNonBlockingErrors(); + }); + }); + + it('should allow user to save a rule and show confirmation modal when data field does not exist', () => { + const esqlRule = { + ...rule, + query: 'from auditbeat-* metadata _id, _version, _index | keep hello.world', + }; + createRule(esqlRule).then((createdRule) => { + visitEditRulePage(createdRule.body.id); + saveEditedRuleWithNonBlockingErrors(); + }); + }); + }); } ); diff --git a/x-pack/test/security_solution_cypress/cypress/screens/create_new_rule.ts b/x-pack/test/security_solution_cypress/cypress/screens/create_new_rule.ts index 4e6df1ed3a750..72d1104985d77 100644 --- a/x-pack/test/security_solution_cypress/cypress/screens/create_new_rule.ts +++ b/x-pack/test/security_solution_cypress/cypress/screens/create_new_rule.ts @@ -56,6 +56,10 @@ export const CREATE_AND_ENABLE_BTN = '[data-test-subj="create-enable"]'; export const CREATE_WITHOUT_ENABLING_BTN = '[data-test-subj="create-enabled-false"]'; +export const SAVE_WITH_ERRORS_MODAL = '[data-test-subj="save-with-errors-confirmation-modal"]'; + +export const SAVE_WITH_ERRORS_MODAL_CONFIRM_BTN = '[data-test-subj="confirmModalConfirmButton"]'; + export const CUSTOM_QUERY_INPUT = '[data-test-subj="queryInput"]'; export const CUSTOM_QUERY_BAR = '[data-test-subj="detectionEngineStepDefineRuleQueryBar"]'; diff --git a/x-pack/test/security_solution_cypress/cypress/tasks/create_new_rule.ts b/x-pack/test/security_solution_cypress/cypress/tasks/create_new_rule.ts index 4251ef8ee0ec8..045c40da4b2cb 100644 --- a/x-pack/test/security_solution_cypress/cypress/tasks/create_new_rule.ts +++ b/x-pack/test/security_solution_cypress/cypress/tasks/create_new_rule.ts @@ -128,6 +128,8 @@ import { MAX_SIGNALS_INPUT, SETUP_GUIDE_TEXTAREA, RELATED_INTEGRATION_COMBO_BOX_INPUT, + SAVE_WITH_ERRORS_MODAL, + SAVE_WITH_ERRORS_MODAL_CONFIRM_BTN, } from '../screens/create_new_rule'; import { INDEX_SELECTOR, @@ -162,6 +164,14 @@ export const createRuleWithoutEnabling = () => { cy.get(CREATE_WITHOUT_ENABLING_BTN).should('not.exist'); }; +export const createRuleWithNonBlockingErrors = () => { + cy.get(CREATE_AND_ENABLE_BTN).click(); + cy.get(SAVE_WITH_ERRORS_MODAL).should('exist'); + cy.get(SAVE_WITH_ERRORS_MODAL_CONFIRM_BTN).first().click(); + cy.get(SAVE_WITH_ERRORS_MODAL).should('not.exist'); + cy.get(CREATE_AND_ENABLE_BTN).should('not.exist'); +}; + export const fillAboutRule = (rule: RuleCreateProps) => { cy.get(RULE_NAME_INPUT).clear({ force: true }); cy.get(RULE_NAME_INPUT).type(rule.name, { force: true }); diff --git a/x-pack/test/security_solution_cypress/cypress/tasks/edit_rule.ts b/x-pack/test/security_solution_cypress/cypress/tasks/edit_rule.ts index 14c9ef05aa878..4ea919231e08f 100644 --- a/x-pack/test/security_solution_cypress/cypress/tasks/edit_rule.ts +++ b/x-pack/test/security_solution_cypress/cypress/tasks/edit_rule.ts @@ -5,6 +5,10 @@ * 2.0. */ +import { + SAVE_WITH_ERRORS_MODAL, + SAVE_WITH_ERRORS_MODAL_CONFIRM_BTN, +} from '../screens/create_new_rule'; import { BACK_TO_RULE_DETAILS, EDIT_SUBMIT_BUTTON } from '../screens/edit_rule'; import { editRuleUrl } from '../urls/edit_rule'; import { visit } from './navigation'; @@ -18,6 +22,14 @@ export const saveEditedRule = () => { cy.get(EDIT_SUBMIT_BUTTON).should('not.exist'); }; +export const saveEditedRuleWithNonBlockingErrors = () => { + cy.get(EDIT_SUBMIT_BUTTON).click(); + cy.get(SAVE_WITH_ERRORS_MODAL).should('exist'); + cy.get(SAVE_WITH_ERRORS_MODAL_CONFIRM_BTN).first().click(); + cy.get(SAVE_WITH_ERRORS_MODAL).should('not.exist'); + cy.get(EDIT_SUBMIT_BUTTON).should('not.exist'); +}; + export const goBackToRuleDetails = () => { cy.get(BACK_TO_RULE_DETAILS).should('exist').click(); cy.get(BACK_TO_RULE_DETAILS).should('not.exist'); From 4ee535f47f6fc6dd7b01a94074a6f73825dd731d Mon Sep 17 00:00:00 2001 From: Ievgen Sorokopud Date: Tue, 27 Aug 2024 16:04:05 +0200 Subject: [PATCH 2/6] Fix tests --- .../pages/rule_creation/index.tsx | 59 ++++++++----------- .../rule_creation/common_flows.cy.ts | 1 - 2 files changed, 23 insertions(+), 37 deletions(-) diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/index.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/index.tsx index 97b89630392d8..d4d0b304fe1b0 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/index.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/index.tsx @@ -400,22 +400,14 @@ const CreateRulePageComponent: React.FC = () => { return { valid, blockingErrors, nonBlockingErrors }; }, [validateStep]); - const isValidStep = useCallback( - async (step: RuleStep) => { - const { valid, blockingErrors } = await validateStep(step); - return valid || !blockingErrors.length; - }, - [validateStep] - ); - const editStep = useCallback( async (step: RuleStep) => { - const valid = await isValidStep(activeStep); - if (valid) { + const { valid, blockingErrors } = await validateStep(activeStep); + if (valid || !blockingErrors.length) { goToStep(step); } }, - [isValidStep, activeStep, goToStep] + [validateStep, activeStep, goToStep] ); const createRuleFromFormData = useCallback( @@ -509,14 +501,11 @@ const CreateRulePageComponent: React.FC = () => { [defineRuleButtonType] ); const defineRuleNextStep = useCallback(async () => { - const valid = await isValidStep(RuleStep.defineRule); - if (valid) { - const nextStep = getNextStep(RuleStep.defineRule); - if (nextStep) { - goToStep(nextStep); - } + const nextStep = getNextStep(RuleStep.defineRule); + if (nextStep) { + await editStep(nextStep); } - }, [goToStep, isValidStep]); + }, [editStep]); const aboutRuleButtonType = activeStep === RuleStep.aboutRule ? 'active' : aboutStepForm.isValid ? 'valid' : 'passive'; @@ -525,14 +514,11 @@ const CreateRulePageComponent: React.FC = () => { [aboutRuleButtonType] ); const aboutRuleNextStep = useCallback(async () => { - const valid = await isValidStep(RuleStep.aboutRule); - if (valid) { - const nextStep = getNextStep(RuleStep.aboutRule); - if (nextStep) { - goToStep(nextStep); - } + const nextStep = getNextStep(RuleStep.aboutRule); + if (nextStep) { + await editStep(nextStep); } - }, [goToStep, isValidStep]); + }, [editStep]); const scheduleRuleButtonType = activeStep === RuleStep.scheduleRule @@ -545,14 +531,11 @@ const CreateRulePageComponent: React.FC = () => { [scheduleRuleButtonType] ); const scheduleRuleNextStep = useCallback(async () => { - const valid = await isValidStep(RuleStep.scheduleRule); - if (valid) { - const nextStep = getNextStep(RuleStep.scheduleRule); - if (nextStep) { - goToStep(nextStep); - } + const nextStep = getNextStep(RuleStep.scheduleRule); + if (nextStep) { + await editStep(nextStep); } - }, [isValidStep, goToStep]); + }, [editStep]); const actionsRuleButtonType = activeStep === RuleStep.ruleActions ? 'active' : actionsStepForm.isValid ? 'valid' : 'passive'; @@ -650,6 +633,7 @@ const CreateRulePageComponent: React.FC = () => { ); const memoDefineStepExtraAction = useMemo( () => + defineStepForm.isValid !== undefined && activeStep !== RuleStep.defineRule && ( { {i18n.EDIT_RULE} ), - [activeStep, editStep] + [activeStep, defineStepForm.isValid, editStep] ); const memoAboutStepReadOnly = useMemo( @@ -720,6 +704,7 @@ const CreateRulePageComponent: React.FC = () => { ); const memoAboutStepExtraAction = useMemo( () => + aboutStepForm.isValid !== undefined && activeStep !== RuleStep.aboutRule && ( { {i18n.EDIT_RULE} ), - [activeStep, editStep] + [aboutStepForm.isValid, activeStep, editStep] ); const memoStepScheduleRule = useMemo( @@ -773,12 +758,13 @@ const CreateRulePageComponent: React.FC = () => { ); const memoScheduleStepExtraAction = useMemo( () => + scheduleStepForm.isValid !== undefined && activeStep !== RuleStep.scheduleRule && ( editStep(RuleStep.scheduleRule)}> {i18n.EDIT_RULE} ), - [activeStep, editStep] + [activeStep, editStep, scheduleStepForm.isValid] ); const memoStepRuleActions = useMemo( @@ -853,12 +839,13 @@ const CreateRulePageComponent: React.FC = () => { ); const memoActionsStepExtraAction = useMemo( () => + actionsStepForm.isValid !== undefined && activeStep !== RuleStep.ruleActions && ( editStep(RuleStep.ruleActions)}> {i18n.EDIT_RULE} ), - [activeStep, editStep] + [actionsStepForm.isValid, activeStep, editStep] ); const onToggleCollapsedMemo = useCallback( diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_creation/common_flows.cy.ts b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_creation/common_flows.cy.ts index 0c4885ad35352..5e10a3895b10c 100644 --- a/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_creation/common_flows.cy.ts +++ b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_creation/common_flows.cy.ts @@ -100,7 +100,6 @@ describe('Common rule creation flows', { tags: ['@ess', '@serverless'] }, () => cy.get(DEFINE_CONTINUE_BUTTON).should('exist').click(); // expect about step to populate - cy.get(ABOUT_EDIT_BUTTON).click(); cy.get(RULE_NAME_INPUT).invoke('val').should('eql', ruleFields.ruleName); cy.get(ABOUT_CONTINUE_BTN).should('exist').click(); cy.get(SCHEDULE_CONTINUE_BUTTON).click(); From 7b2616aa07a932418f7035c8218afeab7497242f Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Tue, 27 Aug 2024 16:16:05 +0000 Subject: [PATCH 3/6] [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix' --- .../detection_engine/rule_creation/common_flows.cy.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_creation/common_flows.cy.ts b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_creation/common_flows.cy.ts index 5e10a3895b10c..438743dd01a70 100644 --- a/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_creation/common_flows.cy.ts +++ b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_creation/common_flows.cy.ts @@ -8,7 +8,6 @@ import { ruleFields } from '../../../../data/detection_engine'; import { ABOUT_CONTINUE_BTN, - ABOUT_EDIT_BUTTON, CUSTOM_QUERY_INPUT, DEFINE_CONTINUE_BUTTON, DEFINE_EDIT_BUTTON, From b90708729613168b3933b4420d39d48b0fa231c5 Mon Sep 17 00:00:00 2001 From: Ievgen Sorokopud Date: Wed, 28 Aug 2024 12:53:13 +0200 Subject: [PATCH 4/6] Simplify confirmation modal description --- .../save_with_errors_confirmation/index.tsx | 16 ++-------------- 1 file changed, 2 insertions(+), 14 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 3f14945bedadc..839513bf0e34c 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, EuiSpacer, EuiText } from '@elastic/eui'; +import { EuiConfirmModal } from '@elastic/eui'; import * as i18n from './translations'; @@ -32,19 +32,7 @@ const SaveWithErrorsModalComponent = ({ confirmButtonText={i18n.SAVE_WITH_ERRORS_CONFIRM_BUTTON} defaultFocusedButton="confirm" > - <> - {i18n.SAVE_WITH_ERRORS_MODAL_MESSAGE(errors.length)} - -
    - {errors.map((validationError, idx) => { - return ( -
  • - {validationError} -
    • - ); - })} -
- + <>{i18n.SAVE_WITH_ERRORS_MODAL_MESSAGE(errors.length)} ); }; From a76369e05c500f0ff35ae872569e0d77bc9bcb1c Mon Sep 17 00:00:00 2001 From: Ievgen Sorokopud Date: Thu, 29 Aug 2024 12:02:33 +0200 Subject: [PATCH 5/6] Update x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/translations.ts Co-authored-by: Vitalii Dmyterko <92328789+vitaliidm@users.noreply.github.com> --- .../detection_engine/rule_creation_ui/pages/translations.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/translations.ts b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/translations.ts index 7a2064dd4f35c..77ea9438f66dc 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/translations.ts +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/translations.ts @@ -14,8 +14,8 @@ export const RULE_PREVIEW_TITLE = i18n.translate( } ); -export const QUERY_BAR_VALIDATION_ERROR = (vlidationError: string) => +export const QUERY_BAR_VALIDATION_ERROR = (validationError: string) => i18n.translate('xpack.securitySolution.detectionEngine.createRule.validationError', { - values: { vlidationError }, - defaultMessage: 'Query bar: {vlidationError}', + values: { validationError }, + defaultMessage: 'Query bar: {validationError}', }); From 2cfbf7251ef5d18c3ba303543b14da2b3c1b329f Mon Sep 17 00:00:00 2001 From: Ievgen Sorokopud Date: Thu, 29 Aug 2024 13:26:17 +0200 Subject: [PATCH 6/6] Review feedback - memoize callbacks - clarify multiple EQL validation errors functionality - comments to clarify step editing button visibility conditions - add unit tests for `useRuleFormsErrors` --- .../public/common/hooks/eql/api.ts | 16 +- .../components/eql_query_bar/validators.ts | 27 +- .../rule_creation_ui/pages/form.test.ts | 291 ++++++++++++++++++ .../rule_creation_ui/pages/form.tsx | 6 +- .../pages/rule_creation/index.tsx | 30 +- .../pages/rule_editing/index.tsx | 15 +- 6 files changed, 357 insertions(+), 28 deletions(-) create mode 100644 x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/form.test.ts diff --git a/x-pack/plugins/security_solution/public/common/hooks/eql/api.ts b/x-pack/plugins/security_solution/public/common/hooks/eql/api.ts index 921d4fde4bfd0..569344297f319 100644 --- a/x-pack/plugins/security_solution/public/common/hooks/eql/api.ts +++ b/x-pack/plugins/security_solution/public/common/hooks/eql/api.ts @@ -36,6 +36,17 @@ interface Params { options: Omit | undefined; } +export interface EqlResponseError { + code: EQL_ERROR_CODES; + messages?: string[]; + error?: Error; +} + +export interface ValidateEqlResponse { + valid: boolean; + error?: EqlResponseError; +} + export const validateEql = async ({ data, dataViewTitle, @@ -43,10 +54,7 @@ export const validateEql = async ({ signal, runtimeMappings, options, -}: Params): Promise<{ - valid: boolean; - error?: { code: EQL_ERROR_CODES; messages?: string[]; error?: Error }; -}> => { +}: Params): Promise => { try { const { rawResponse: response } = await firstValueFrom( data.search.search( diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/eql_query_bar/validators.ts b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/eql_query_bar/validators.ts index 04ea9bbc43356..8cd9a4d60745e 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/eql_query_bar/validators.ts +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/eql_query_bar/validators.ts @@ -12,6 +12,7 @@ import { isEqlRule } from '../../../../../common/detection_engine/utils'; import { KibanaServices } from '../../../../common/lib/kibana'; import type { DefineStepRule } from '../../../../detections/pages/detection_engine/rules/types'; import { DataSourceType } from '../../../../detections/pages/detection_engine/rules/types'; +import type { EqlResponseError } from '../../../../common/hooks/eql/api'; import { validateEql, EQL_ERROR_CODES } from '../../../../common/hooks/eql/api'; import type { FieldValueQueryBar } from '../query_bar'; import * as i18n from './translations'; @@ -47,6 +48,23 @@ export const debounceAsync = => { + if (responseError.error) { + return { + code: EQL_ERROR_CODES.FAILED_REQUEST, + message: i18n.EQL_VALIDATION_REQUEST_ERROR, + error: responseError.error, + }; + } + return { + code: responseError.code, + message: '', + messages: responseError.messages, + }; +}; + export const eqlValidator = async ( ...args: Parameters ): Promise | void | undefined> => { @@ -86,13 +104,8 @@ export const eqlValidator = async ( options: eqlOptions, }); - if (response?.valid === false) { - return { - code: response.error?.code, - message: '', - messages: response.error?.messages, - error: response.error?.error, - }; + if (response?.valid === false && response.error) { + return transformEqlResponseErrorToValidationError(response.error); } } catch (error) { return { diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/form.test.ts b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/form.test.ts new file mode 100644 index 0000000000000..ca1e5042eac80 --- /dev/null +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/form.test.ts @@ -0,0 +1,291 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ +import { renderHook } from '@testing-library/react-hooks'; + +import type { FormData, FormHook, ValidationError } from '../../../shared_imports'; +import { ERROR_CODES as ESQL_ERROR_CODES } from '../../rule_creation/logic/esql_validator'; +import { EQL_ERROR_CODES } from '../../../common/hooks/eql/api'; +import type { + AboutStepRule, + ActionsStepRule, + DefineStepRule, + ScheduleStepRule, +} from '../../../detections/pages/detection_engine/rules/types'; + +import { useRuleFormsErrors } from './form'; +import { transformEqlResponseErrorToValidationError } from '../components/eql_query_bar/validators'; + +const getFormWithErrorsMock = (fields: { + [key: string]: { errors: Array> }; +}) => { + return { + getFields: () => fields, + } as unknown as FormHook; +}; + +describe('useRuleFormsErrors', () => { + describe('EQL query validation errors', () => { + it('should return blocking error in case of syntax validation error', async () => { + const { result } = renderHook(() => useRuleFormsErrors()); + + const validationError = transformEqlResponseErrorToValidationError({ + code: EQL_ERROR_CODES.INVALID_SYNTAX, + messages: ["line 1:5: missing 'where' at 'demo'"], + }); + const defineStepForm = getFormWithErrorsMock({ + queryBar: { + errors: [validationError], + }, + }); + + const { getRuleFormsErrors } = result.current; + const { blockingErrors, nonBlockingErrors } = getRuleFormsErrors({ defineStepForm }); + + expect(blockingErrors).toEqual(["line 1:5: missing 'where' at 'demo'"]); + expect(nonBlockingErrors).toEqual([]); + }); + + it('should return non-blocking error in case of missing data source validation error', async () => { + const { result } = renderHook(() => useRuleFormsErrors()); + + const validationError = transformEqlResponseErrorToValidationError({ + code: EQL_ERROR_CODES.MISSING_DATA_SOURCE, + messages: ['index_not_found_exception Found 1 problem line -1:-1: Unknown index [*,-*]'], + }); + const defineStepForm = getFormWithErrorsMock({ + queryBar: { + errors: [validationError], + }, + }); + + const { getRuleFormsErrors } = result.current; + const { blockingErrors, nonBlockingErrors } = getRuleFormsErrors({ defineStepForm }); + + expect(blockingErrors).toEqual([]); + expect(nonBlockingErrors).toEqual([ + 'Query bar: index_not_found_exception Found 1 problem line -1:-1: Unknown index [*,-*]', + ]); + }); + + it('should return non-blocking error in case of missing data field validation error', async () => { + const { result } = renderHook(() => useRuleFormsErrors()); + + const validationError = transformEqlResponseErrorToValidationError({ + code: EQL_ERROR_CODES.INVALID_EQL, + messages: [ + 'Found 2 problems\nline 1:1: Unknown column [event.category]\nline 1:13: Unknown column [event.name]', + ], + }); + const defineStepForm = getFormWithErrorsMock({ + queryBar: { + errors: [validationError], + }, + }); + + const { getRuleFormsErrors } = result.current; + const { blockingErrors, nonBlockingErrors } = getRuleFormsErrors({ defineStepForm }); + + expect(blockingErrors).toEqual([]); + expect(nonBlockingErrors).toEqual([ + 'Query bar: Found 2 problems\nline 1:1: Unknown column [event.category]\nline 1:13: Unknown column [event.name]', + ]); + }); + + it('should return non-blocking error in case of failed request error', async () => { + const { result } = renderHook(() => useRuleFormsErrors()); + + const validationError = transformEqlResponseErrorToValidationError({ + code: EQL_ERROR_CODES.FAILED_REQUEST, + error: new Error('Some internal error'), + }); + const defineStepForm = getFormWithErrorsMock({ + queryBar: { + errors: [validationError], + }, + }); + + const { getRuleFormsErrors } = result.current; + const { blockingErrors, nonBlockingErrors } = getRuleFormsErrors({ defineStepForm }); + + expect(blockingErrors).toEqual([]); + expect(nonBlockingErrors).toEqual([ + 'Query bar: An error occurred while validating your EQL query', + ]); + }); + + it('should return blocking and non-blocking errors', async () => { + const { result } = renderHook(() => useRuleFormsErrors()); + + const validationError = transformEqlResponseErrorToValidationError({ + code: EQL_ERROR_CODES.MISSING_DATA_SOURCE, + messages: ['Missing data source'], + }); + const defineStepForm = getFormWithErrorsMock({ + queryBar: { + errors: [validationError], + }, + }); + const aboutStepForm = getFormWithErrorsMock({ + name: { + errors: [ + { + message: 'Required field', + }, + ], + }, + }); + + const { getRuleFormsErrors } = result.current; + const { blockingErrors, nonBlockingErrors } = getRuleFormsErrors({ + defineStepForm, + aboutStepForm, + }); + + expect(blockingErrors).toEqual(['Required field']); + expect(nonBlockingErrors).toEqual(['Query bar: Missing data source']); + }); + }); + + describe('ES|QL query validation errors', () => { + it('should return blocking error in case of syntax validation error', async () => { + const { result } = renderHook(() => useRuleFormsErrors()); + + const validationError = { + code: ESQL_ERROR_CODES.INVALID_SYNTAX, + message: 'Broken ES|QL syntax', + }; + const defineStepForm = getFormWithErrorsMock({ + queryBar: { + errors: [validationError], + }, + }); + + const { getRuleFormsErrors } = result.current; + const { blockingErrors, nonBlockingErrors } = getRuleFormsErrors({ defineStepForm }); + + expect(blockingErrors).toEqual(['Broken ES|QL syntax']); + expect(nonBlockingErrors).toEqual([]); + }); + + it('should return blocking error in case of missed ES|QL metadata validation error', async () => { + const { result } = renderHook(() => useRuleFormsErrors()); + + const validationError = { + code: ESQL_ERROR_CODES.ERR_MISSING_ID_FIELD_FROM_RESULT, + message: 'Metadata is missing', + }; + const defineStepForm = getFormWithErrorsMock({ + queryBar: { + errors: [validationError], + }, + }); + + const { getRuleFormsErrors } = result.current; + const { blockingErrors, nonBlockingErrors } = getRuleFormsErrors({ defineStepForm }); + + expect(blockingErrors).toEqual(['Metadata is missing']); + expect(nonBlockingErrors).toEqual([]); + }); + + it('should return non-blocking error in case of missing data field validation error', async () => { + const { result } = renderHook(() => useRuleFormsErrors()); + + const validationError = { + code: ESQL_ERROR_CODES.INVALID_ESQL, + message: 'Unknown column [hello.world]', + }; + const defineStepForm = getFormWithErrorsMock({ + queryBar: { + errors: [validationError], + }, + }); + + const { getRuleFormsErrors } = result.current; + const { blockingErrors, nonBlockingErrors } = getRuleFormsErrors({ defineStepForm }); + + expect(blockingErrors).toEqual([]); + expect(nonBlockingErrors).toEqual(['Query bar: Unknown column [hello.world]']); + }); + }); + + describe('general cases', () => { + it('should not return blocking and non-blocking errors in case there are none exist', async () => { + const { result } = renderHook(() => useRuleFormsErrors()); + + const defineStepForm = getFormWithErrorsMock({ queryBar: { errors: [] } }); + const aboutStepForm = getFormWithErrorsMock({ name: { errors: [] } }); + const scheduleStepForm = getFormWithErrorsMock({ + interval: { errors: [] }, + }); + const actionsStepForm = getFormWithErrorsMock({ actions: { errors: [] } }); + + const { getRuleFormsErrors } = result.current; + const { blockingErrors, nonBlockingErrors } = getRuleFormsErrors({ + defineStepForm, + aboutStepForm, + scheduleStepForm, + actionsStepForm, + }); + + expect(blockingErrors).toEqual([]); + expect(nonBlockingErrors).toEqual([]); + }); + + it('should not return all errors', async () => { + const { result } = renderHook(() => useRuleFormsErrors()); + + const esqlValidationError = { + code: ESQL_ERROR_CODES.INVALID_ESQL, + message: 'Missing index [logs*]', + }; + const groupByValidationError = { + message: 'Number of grouping fields must be at most 3', + }; + + const defineStepForm = getFormWithErrorsMock({ + queryBar: { errors: [esqlValidationError] }, + groupByFields: { errors: [groupByValidationError] }, + }); + const aboutStepForm = getFormWithErrorsMock({ + name: { + errors: [ + { + message: 'Required field', + }, + ], + }, + }); + const scheduleStepForm = getFormWithErrorsMock({ + interval: { errors: [] }, + }); + const actionsStepForm = getFormWithErrorsMock({ + actions: { + errors: [ + { + message: 'Missing webhook connector', + }, + ], + }, + }); + + const { getRuleFormsErrors } = result.current; + const { blockingErrors, nonBlockingErrors } = getRuleFormsErrors({ + defineStepForm, + aboutStepForm, + scheduleStepForm, + actionsStepForm, + }); + + expect(blockingErrors).toEqual([ + 'Number of grouping fields must be at most 3', + 'Required field', + 'Missing webhook connector', + ]); + expect(nonBlockingErrors).toEqual(['Query bar: Missing index [logs*]']); + }); + }); +}); diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/form.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/form.tsx index d7e3f4566a887..90b302c3bc904 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/form.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/form.tsx @@ -172,7 +172,11 @@ const getFieldErrorMessages = (fieldError: ValidationError) => { if (fieldError.message.length > 0) { return [fieldError.message]; } else if (Array.isArray(fieldError.messages)) { - return fieldError.messages.map((message) => JSON.stringify(message)); + // EQL validation can return multiple errors and thus we store them in a custom `messages` field on `ValidationError` object. + // Here we double check that `messages` is in fact an array and the content is of type `string`, otherwise we stringify it. + return fieldError.messages.map((message) => + typeof message === 'string' ? message : JSON.stringify(message) + ); } return []; }; diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/index.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/index.tsx index d4d0b304fe1b0..500fedb4d0005 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/index.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/index.tsx @@ -464,12 +464,19 @@ const CreateRulePageComponent: React.FC = () => { ] ); + const showSaveWithErrorsModal = useCallback(() => setIsSaveWithErrorsModalVisible(true), []); + const closeSaveWithErrorsModal = useCallback(() => setIsSaveWithErrorsModalVisible(false), []); + const onConfirmSaveWithErrors = useCallback(async () => { + closeSaveWithErrorsModal(); + await createRuleFromFormData(enableRuleAfterConfirmation); + }, [closeSaveWithErrorsModal, createRuleFromFormData, enableRuleAfterConfirmation]); + const submitRule = useCallback( async (enabled: boolean) => { const { valid, blockingErrors, nonBlockingErrors } = await validateEachStep(); if (valid) { // There are no validation errors, thus proceed to rule creation - createRuleFromFormData(enabled); + await createRuleFromFormData(enabled); return; } @@ -484,16 +491,9 @@ const CreateRulePageComponent: React.FC = () => { showSaveWithErrorsModal(); } }, - [createRuleFromFormData, validateEachStep] + [createRuleFromFormData, showSaveWithErrorsModal, validateEachStep] ); - const showSaveWithErrorsModal = () => setIsSaveWithErrorsModalVisible(true); - const closeSaveWithErrorsModal = () => setIsSaveWithErrorsModalVisible(false); - const onConfirmSaveWithErrors = useCallback(async () => { - closeSaveWithErrorsModal(); - await createRuleFromFormData(enableRuleAfterConfirmation); - }, [createRuleFromFormData, enableRuleAfterConfirmation]); - const defineRuleButtonType = activeStep === RuleStep.defineRule ? 'active' : defineStepForm.isValid ? 'valid' : 'passive'; const defineRuleButton = useMemo( @@ -633,6 +633,9 @@ const CreateRulePageComponent: React.FC = () => { ); const memoDefineStepExtraAction = useMemo( () => + // During rule creation we would like to hide the edit button if user did not reach current step yet, + // thus we do `defineStepForm.isValid !== undefined` check which that the form validation has not been checked yet. + // Otherwise, we would like to show step edit button if user is currently at another step. defineStepForm.isValid !== undefined && activeStep !== RuleStep.defineRule && ( { ); const memoAboutStepExtraAction = useMemo( () => + // During rule creation we would like to hide the edit button if user did not reach current step yet, + // thus we do `defineStepForm.isValid !== undefined` check which that the form validation has not been checked yet. + // Otherwise, we would like to show step edit button if user is currently at another step. aboutStepForm.isValid !== undefined && activeStep !== RuleStep.aboutRule && ( { ); const memoScheduleStepExtraAction = useMemo( () => + // During rule creation we would like to hide the edit button if user did not reach current step yet, + // thus we do `defineStepForm.isValid !== undefined` check which that the form validation has not been checked yet. + // Otherwise, we would like to show step edit button if user is currently at another step. scheduleStepForm.isValid !== undefined && activeStep !== RuleStep.scheduleRule && ( editStep(RuleStep.scheduleRule)}> @@ -839,6 +848,9 @@ const CreateRulePageComponent: React.FC = () => { ); const memoActionsStepExtraAction = useMemo( () => + // During rule creation we would like to hide the edit button if user did not reach current step yet, + // thus we do `defineStepForm.isValid !== undefined` check which that the form validation has not been checked yet. + // Otherwise, we would like to show step edit button if user is currently at another step. actionsStepForm.isValid !== undefined && activeStep !== RuleStep.ruleActions && ( editStep(RuleStep.ruleActions)}> 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 6b5bd2d33c574..1ecbde5b00d7b 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 @@ -426,6 +426,13 @@ const EditRulePageComponent: FC<{ rule: RuleResponse }> = ({ rule }) => { updateRule, ]); + const showSaveWithErrorsModal = useCallback(() => setIsSaveWithErrorsModalVisible(true), []); + const closeSaveWithErrorsModal = useCallback(() => setIsSaveWithErrorsModalVisible(false), []); + const onConfirmSaveWithErrors = useCallback(async () => { + closeSaveWithErrorsModal(); + await saveChanges(); + }, [closeSaveWithErrorsModal, saveChanges]); + const onSubmit = useCallback(async () => { setNonBlockingRuleErrors([]); @@ -464,15 +471,9 @@ const EditRulePageComponent: FC<{ rule: RuleResponse }> = ({ rule }) => { actionsStepForm, getRuleFormsErrors, saveChanges, + showSaveWithErrorsModal, ]); - const showSaveWithErrorsModal = () => setIsSaveWithErrorsModalVisible(true); - const closeSaveWithErrorsModal = () => setIsSaveWithErrorsModalVisible(false); - const onConfirmSaveWithErrors = useCallback(async () => { - closeSaveWithErrorsModal(); - await saveChanges(); - }, [saveChanges]); - const onTabClick = useCallback(async (tab: EuiTabbedContentTab) => { const targetStep = tab.id as RuleStep; setActiveStep(targetStep);