From 02187ca2b44819d30cb9dd970af89b6fad31c5da Mon Sep 17 00:00:00 2001 From: Maxim Palenov Date: Fri, 6 Dec 2024 13:06:39 +0100 Subject: [PATCH] [Security Solution] Allow users to save rule query with non critical validation errors (#202544) **Addresses:** https://github.com/elastic/kibana/issues/171520 ## Summary This PR adds functionality to allow users save EQL and ES|QL queries in Prebuilt Rule Customization workflow by displaying a confirmation modal with non critical validation errors (a.k.a warnings). It also refactors confirmation modal usage in rule creation/editing forms for better reusability. ## Screenshots Screenshot 2024-12-03 at 14 01 36 https://github.com/user-attachments/assets/2a20fcfe-ffc0-4547-8621-7ac6873c8dc9 https://github.com/user-attachments/assets/50b5cf5a-ea3f-4c22-a443-b5d4056a92c8 Screenshot 2024-12-03 at 14 06 29 Screenshot 2024-12-03 at 14 06 51 Screenshot 2024-12-03 at 14 07 52 Screenshot 2024-12-03 at 14 08 18 --- .github/CODEOWNERS | 1 + .../public/common/hooks/eql/api.ts | 17 +- .../confirm_validation_errors_modal.tsx} | 27 +- .../index.tsx | 8 + .../translations.tsx | 36 +++ .../use_confirm_validation_errors_modal.tsx | 56 ++++ .../extract_validation_results.ts | 35 ++ .../form_hook_with_warnings.ts | 13 + .../hooks/use_form_with_warnings/index.ts | 9 + .../use_form_with_warnings.test.tsx | 241 ++++++++++++++ .../use_form_with_warnings.ts | 154 +++++++++ .../validation_results.ts | 13 + .../eql_query_edit/eql_query_bar.tsx | 23 +- .../eql_query_edit/eql_query_edit.tsx | 59 ++-- .../components/eql_query_edit/validators.ts | 29 -- .../eql_query_validator_factory.ts | 19 +- .../esql_query_edit/esql_query_edit.tsx | 10 +- .../esql_query_validator_factory.ts | 9 - .../constants/validation_warning_codes.ts | 38 +++ .../logic/extract_validation_messages.ts | 18 ++ .../translations.ts | 36 --- .../rule_creation_ui/pages/form.test.ts | 302 ------------------ .../rule_creation_ui/pages/form.tsx | 105 +----- .../pages/rule_creation/index.tsx | 148 ++++----- .../pages/rule_editing/index.tsx | 71 ++-- .../rule_creation_ui/pages/translations.ts | 6 - .../eql_query/eql_query_edit_adapter.tsx | 1 - .../esql_query/esql_query_edit_adapter.tsx | 1 - .../fields/rule_field_edit_form_wrapper.tsx | 37 ++- .../public/shared_imports.ts | 1 + .../translations/translations/fr-FR.json | 4 - .../translations/translations/ja-JP.json | 4 - .../translations/translations/zh-CN.json | 4 - 33 files changed, 827 insertions(+), 708 deletions(-) rename x-pack/plugins/security_solution/public/{detection_engine/rule_creation_ui/components/save_with_errors_confirmation/index.tsx => common/hooks/use_confirm_validation_errors_modal/confirm_validation_errors_modal.tsx} (59%) create mode 100644 x-pack/plugins/security_solution/public/common/hooks/use_confirm_validation_errors_modal/index.tsx create mode 100644 x-pack/plugins/security_solution/public/common/hooks/use_confirm_validation_errors_modal/translations.tsx create mode 100644 x-pack/plugins/security_solution/public/common/hooks/use_confirm_validation_errors_modal/use_confirm_validation_errors_modal.tsx create mode 100644 x-pack/plugins/security_solution/public/common/hooks/use_form_with_warnings/extract_validation_results.ts create mode 100644 x-pack/plugins/security_solution/public/common/hooks/use_form_with_warnings/form_hook_with_warnings.ts create mode 100644 x-pack/plugins/security_solution/public/common/hooks/use_form_with_warnings/index.ts create mode 100644 x-pack/plugins/security_solution/public/common/hooks/use_form_with_warnings/use_form_with_warnings.test.tsx create mode 100644 x-pack/plugins/security_solution/public/common/hooks/use_form_with_warnings/use_form_with_warnings.ts create mode 100644 x-pack/plugins/security_solution/public/common/hooks/use_form_with_warnings/validation_results.ts delete mode 100644 x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/eql_query_edit/validators.ts rename x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/eql_query_edit/{ => validators}/eql_query_validator_factory.ts (78%) create mode 100644 x-pack/plugins/security_solution/public/detection_engine/rule_creation/constants/validation_warning_codes.ts create mode 100644 x-pack/plugins/security_solution/public/detection_engine/rule_creation/logic/extract_validation_messages.ts delete mode 100644 x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/save_with_errors_confirmation/translations.ts delete mode 100644 x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/form.test.ts diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 210ef172b78ea..5dbb563fda702 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -2283,6 +2283,7 @@ x-pack/test/security_solution_cypress/cypress/tasks/expandable_flyout @elastic/ /x-pack/plugins/security_solution/public/common/components/with_hover_actions @elastic/security-threat-hunting-explore /x-pack/plugins/security_solution/public/common/containers/matrix_histogram @elastic/security-threat-hunting-explore /x-pack/plugins/security_solution/public/common/lib/cell_actions @elastic/security-threat-hunting-explore +/x-pack/plugins/security_solution/public/common/hooks/use_form_with_warn @elastic/security-detection-rule-management /x-pack/plugins/security_solution/public/cases @elastic/security-threat-hunting-explore /x-pack/plugins/security_solution/public/explore @elastic/security-threat-hunting-explore /x-pack/plugins/security_solution/public/overview @elastic/security-threat-hunting-explore 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 b586e0593ab6f..b26f935612755 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,11 +36,18 @@ interface Params { signal?: AbortSignal; } -export interface EqlResponseError { - code: EQL_ERROR_CODES; - messages?: string[]; - error?: Error; -} +export type EqlResponseError = + | { + code: + | EQL_ERROR_CODES.INVALID_SYNTAX + | EQL_ERROR_CODES.INVALID_EQL + | EQL_ERROR_CODES.MISSING_DATA_SOURCE; + messages: string[]; + } + | { + code: EQL_ERROR_CODES.FAILED_REQUEST; + error: Error; + }; export interface ValidateEqlResponse { valid: boolean; 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/common/hooks/use_confirm_validation_errors_modal/confirm_validation_errors_modal.tsx similarity index 59% rename from x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/save_with_errors_confirmation/index.tsx rename to x-pack/plugins/security_solution/public/common/hooks/use_confirm_validation_errors_modal/confirm_validation_errors_modal.tsx index 3f14945bedadc..52ef0465e39aa 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/common/hooks/use_confirm_validation_errors_modal/confirm_validation_errors_modal.tsx @@ -5,41 +5,39 @@ * 2.0. */ -import React from 'react'; - +import React, { memo } from 'react'; import { EuiConfirmModal, EuiSpacer, EuiText } from '@elastic/eui'; - import * as i18n from './translations'; -interface SaveWithErrorsModalProps { +interface ConfirmValidationErrorsModalProps { errors: string[]; onCancel: () => void; onConfirm: () => void; } -const SaveWithErrorsModalComponent = ({ +export const ConfirmValidationErrorsModal = memo(function ConfirmValidationErrorsModal({ errors, onCancel, onConfirm, -}: SaveWithErrorsModalProps) => { +}: ConfirmValidationErrorsModalProps): JSX.Element { return ( <> - {i18n.SAVE_WITH_ERRORS_MODAL_MESSAGE(errors.length)} + {i18n.SAVE_WITH_ERRORS_MESSAGE(errors.length)}
    - {errors.map((validationError, idx) => { + {errors.map((error) => { return ( -
  • - {validationError} +
  • + {error}
    • ); })} @@ -47,7 +45,4 @@ const SaveWithErrorsModalComponent = ({ ); -}; - -export const SaveWithErrorsModal = React.memo(SaveWithErrorsModalComponent); -SaveWithErrorsModal.displayName = 'SaveWithErrorsModal'; +}); diff --git a/x-pack/plugins/security_solution/public/common/hooks/use_confirm_validation_errors_modal/index.tsx b/x-pack/plugins/security_solution/public/common/hooks/use_confirm_validation_errors_modal/index.tsx new file mode 100644 index 0000000000000..505422ad807ae --- /dev/null +++ b/x-pack/plugins/security_solution/public/common/hooks/use_confirm_validation_errors_modal/index.tsx @@ -0,0 +1,8 @@ +/* + * 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. + */ + +export * from './use_confirm_validation_errors_modal'; diff --git a/x-pack/plugins/security_solution/public/common/hooks/use_confirm_validation_errors_modal/translations.tsx b/x-pack/plugins/security_solution/public/common/hooks/use_confirm_validation_errors_modal/translations.tsx new file mode 100644 index 0000000000000..de95f91e6ce73 --- /dev/null +++ b/x-pack/plugins/security_solution/public/common/hooks/use_confirm_validation_errors_modal/translations.tsx @@ -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.rules.upgradeRules.saveWithErrorsConfirmationModal.title', + { + defaultMessage: 'There are validation errors', + } +); + +export const CANCEL = i18n.translate( + 'xpack.securitySolution.detectionEngine.rules.upgradeRules.saveWithErrorsConfirmationModal.cancel', + { + defaultMessage: 'Cancel', + } +); + +export const CONFIRM = i18n.translate( + 'xpack.securitySolution.detectionEngine.rules.upgradeRules.saveWithErrorsConfirmationModal.confirm', + { + defaultMessage: 'Confirm', + } +); + +export const SAVE_WITH_ERRORS_MESSAGE = (errorsCount: number) => + i18n.translate('xpack.securitySolution.detectionEngine.createRule.saveWithErrorsModalMessage', { + defaultMessage: + 'There {errorsCount, plural, one {is} other {are}} {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/common/hooks/use_confirm_validation_errors_modal/use_confirm_validation_errors_modal.tsx b/x-pack/plugins/security_solution/public/common/hooks/use_confirm_validation_errors_modal/use_confirm_validation_errors_modal.tsx new file mode 100644 index 0000000000000..5dfdbfb969865 --- /dev/null +++ b/x-pack/plugins/security_solution/public/common/hooks/use_confirm_validation_errors_modal/use_confirm_validation_errors_modal.tsx @@ -0,0 +1,56 @@ +/* + * 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 type { ReactNode } from 'react'; +import React, { useCallback, useState, useMemo } from 'react'; +import { useBoolean } from '@kbn/react-hooks'; +import { useAsyncConfirmation } from '../../../detection_engine/rule_management_ui/components/rules_table/rules_table/use_async_confirmation'; +import { ConfirmValidationErrorsModal } from './confirm_validation_errors_modal'; + +interface UseFieldConfirmValidationErrorsModalResult { + modal: ReactNode; + confirmValidationErrors: (errorMessages: string[]) => Promise; +} + +export function useConfirmValidationErrorsModal(): UseFieldConfirmValidationErrorsModalResult { + const [visible, { on: showModal, off: hideModal }] = useBoolean(false); + const [initModal, confirm, cancel] = useAsyncConfirmation({ + onInit: showModal, + onFinish: hideModal, + }); + const [errorsToConfirm, setErrorsToConfirm] = useState([]); + + const confirmValidationErrors = useCallback( + (errorMessages: string[]) => { + if (errorMessages.length === 0) { + return Promise.resolve(true); + } + + setErrorsToConfirm(errorMessages); + + return initModal(); + }, + [initModal, setErrorsToConfirm] + ); + + const modal = useMemo( + () => + visible ? ( + + ) : null, + [visible, errorsToConfirm, confirm, cancel] + ); + + return { + modal, + confirmValidationErrors, + }; +} diff --git a/x-pack/plugins/security_solution/public/common/hooks/use_form_with_warnings/extract_validation_results.ts b/x-pack/plugins/security_solution/public/common/hooks/use_form_with_warnings/extract_validation_results.ts new file mode 100644 index 0000000000000..b3de9f58f1afb --- /dev/null +++ b/x-pack/plugins/security_solution/public/common/hooks/use_form_with_warnings/extract_validation_results.ts @@ -0,0 +1,35 @@ +/* + * 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 type { FieldHook, ValidationError } from '../../../shared_imports'; +import type { ValidationResults } from './validation_results'; + +export function extractValidationResults( + formFields: Readonly, + warningValidationCodes: Readonly +): ValidationResults { + const warningValidationCodesSet = new Set(warningValidationCodes); + const errors: ValidationError[] = []; + const warnings: ValidationError[] = []; + + for (const field of formFields) { + for (const error of field.errors) { + const path = error.path ?? field.path; + + if (!error.code || !warningValidationCodesSet.has(error.code)) { + errors.push({ ...error, path }); + } else { + warnings.push({ ...error, path }); + } + } + } + + return { + errors, + warnings, + }; +} diff --git a/x-pack/plugins/security_solution/public/common/hooks/use_form_with_warnings/form_hook_with_warnings.ts b/x-pack/plugins/security_solution/public/common/hooks/use_form_with_warnings/form_hook_with_warnings.ts new file mode 100644 index 0000000000000..eb823cd19d25d --- /dev/null +++ b/x-pack/plugins/security_solution/public/common/hooks/use_form_with_warnings/form_hook_with_warnings.ts @@ -0,0 +1,13 @@ +/* + * 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 type { FormHook, FormData, ValidationError } from '../../../shared_imports'; + +export interface FormHookWithWarnings + extends FormHook { + getValidationWarnings(): ValidationError[]; +} diff --git a/x-pack/plugins/security_solution/public/common/hooks/use_form_with_warnings/index.ts b/x-pack/plugins/security_solution/public/common/hooks/use_form_with_warnings/index.ts new file mode 100644 index 0000000000000..29e5c064b1a7a --- /dev/null +++ b/x-pack/plugins/security_solution/public/common/hooks/use_form_with_warnings/index.ts @@ -0,0 +1,9 @@ +/* + * 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. + */ + +export type * from './form_hook_with_warnings'; +export * from './use_form_with_warnings'; diff --git a/x-pack/plugins/security_solution/public/common/hooks/use_form_with_warnings/use_form_with_warnings.test.tsx b/x-pack/plugins/security_solution/public/common/hooks/use_form_with_warnings/use_form_with_warnings.test.tsx new file mode 100644 index 0000000000000..c9c9a939458e2 --- /dev/null +++ b/x-pack/plugins/security_solution/public/common/hooks/use_form_with_warnings/use_form_with_warnings.test.tsx @@ -0,0 +1,241 @@ +/* + * 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 { act, fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { TextField } from '@kbn/es-ui-shared-plugin/static/forms/components'; +import type { FieldConfig } from '../../../shared_imports'; +import { Form, UseField } from '../../../shared_imports'; +import type { FormWithWarningsSubmitHandler } from './use_form_with_warnings'; +import { useFormWithWarnings } from './use_form_with_warnings'; + +describe('useFormWithWarn', () => { + describe('isValid', () => { + it('is `undefined` initially', async () => { + render(); + + expect(screen.getByText('isValid: "undefined"')).toBeInTheDocument(); + }); + + it('is `true` when input is valid', async () => { + render(); + + typeText('someValue'); + await submitForm(); + + await waitFor(() => { + expect(screen.getByText('isValid: true')).toBeInTheDocument(); + }); + }); + + it('is `true` when input has warnings', async () => { + render(); + + typeText('warning'); + await submitForm(); + + expect(screen.getByText('isValid: true')).toBeInTheDocument(); + }); + + it('is `false` when input has error', async () => { + render(); + + typeText('error'); + await submitForm(); + + expect(screen.getByText('isValid: false')).toBeInTheDocument(); + }); + }); + + describe('isSubmitting', () => { + it('toggles upon form submission', async () => { + render(); + + expect(screen.getByText('isSubmitting: false')).toBeInTheDocument(); + + const finishAct = submitForm(); + + expect(screen.getByText('isSubmitting: true')).toBeInTheDocument(); + + await finishAct; + await waitFor(() => { + expect(screen.getByText('isSubmitting: false')).toBeInTheDocument(); + }); + }); + }); + + describe('isSubmitted', () => { + it('switched to true after form submission', async () => { + render(); + + expect(screen.getByText('isSubmitted: false')).toBeInTheDocument(); + + await submitForm(); + + await waitFor(() => { + expect(screen.getByText('isSubmitted: true')).toBeInTheDocument(); + }); + }); + }); + + describe('input w/o warnings', () => { + it('submits form successfully', async () => { + const handleSubmit = jest.fn(); + + render(); + typeText('someValue'); + + await submitForm(); + + await waitFor(() => { + expect(handleSubmit).toHaveBeenCalledWith({ testField: 'someValue' }, true, { + errors: [], + warnings: [], + }); + }); + }); + }); + + describe('w/ warnings', () => { + it('submits form successfully', async () => { + const handleSubmit = jest.fn(); + + render(); + typeText('warning'); + + await submitForm(); + + await waitFor(() => { + expect(handleSubmit).toHaveBeenCalledWith({ testField: 'warning' }, true, { + errors: [], + warnings: [ + expect.objectContaining({ + code: 'warning', + message: 'Validation warning', + path: 'testField', + }), + ], + }); + }); + }); + }); + + describe('w/ errors', () => { + it('passes validation errors to submit handler', async () => { + const handleSubmit = jest.fn(); + + render(); + typeText('error'); + + await submitForm(); + + await waitFor(() => { + expect(handleSubmit).toHaveBeenCalledWith({}, false, { + errors: [ + expect.objectContaining({ + code: 'error', + message: 'Validation error', + path: 'testField', + }), + ], + warnings: [], + }); + }); + }); + }); + + describe('w/ errors and warnings', () => { + it('passes validation errors and warnings to submit handler', async () => { + const handleSubmit = jest.fn(); + + render(); + typeText('error warning'); + + await submitForm(); + + await waitFor(() => { + expect(handleSubmit).toHaveBeenCalledWith({}, false, { + errors: [ + expect.objectContaining({ + code: 'error', + message: 'Validation error', + path: 'testField', + }), + ], + warnings: [], + }); + }); + }); + }); +}); + +interface TestFormProps { + onSubmit?: FormWithWarningsSubmitHandler; + warningValidationCodes: string[]; +} + +function TestForm({ onSubmit, warningValidationCodes }: TestFormProps): JSX.Element { + const { form } = useFormWithWarnings({ + onSubmit, + options: { + warningValidationCodes, + }, + }); + const textFieldConfig: FieldConfig = { + validations: [ + { + validator: (data) => { + if (data.value.includes('error')) { + return { + code: 'error', + message: 'Validation error', + }; + } + + if (data.value.includes('warning')) { + return { + code: 'warning', + message: 'Validation warning', + }; + } + }, + }, + ], + }; + + return ( +
    +
    + {'isValid:'} {JSON.stringify(form.isValid ?? 'undefined')} +
    +
    + {'isSubmitting:'} {JSON.stringify(form.isSubmitting)} +
    +
    + {'isSubmitted:'} {JSON.stringify(form.isSubmitted)} +
    + + + + ); +} + +function submitForm(): Promise { + return act(async () => { + fireEvent.click(screen.getByText('Submit')); + }); +} + +function typeText(value: string): void { + act(() => { + fireEvent.input(screen.getByRole('textbox'), { + target: { value }, + }); + }); +} diff --git a/x-pack/plugins/security_solution/public/common/hooks/use_form_with_warnings/use_form_with_warnings.ts b/x-pack/plugins/security_solution/public/common/hooks/use_form_with_warnings/use_form_with_warnings.ts new file mode 100644 index 0000000000000..191ba82f0943c --- /dev/null +++ b/x-pack/plugins/security_solution/public/common/hooks/use_form_with_warnings/use_form_with_warnings.ts @@ -0,0 +1,154 @@ +/* + * 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 { useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import { isEmpty } from 'lodash'; +import type { FormHook } from '../../../shared_imports'; +import { useForm, type FormConfig, type FormData } from '../../../shared_imports'; +import type { FormHookWithWarnings } from './form_hook_with_warnings'; +import { extractValidationResults } from './extract_validation_results'; +import type { ValidationResults } from './validation_results'; + +export type FormWithWarningsSubmitHandler = ( + formData: T, + isValid: boolean, + validationResults: ValidationResults +) => Promise; + +interface FormWithWarningsConfig + extends Omit, 'onSubmit'> { + onSubmit?: FormWithWarningsSubmitHandler; + options: FormConfig['options'] & { + warningValidationCodes: Readonly; + }; +} + +interface UseFormWithWarningsReturn { + form: FormHookWithWarnings; +} + +/** + * Form lib implements warning functionality via non blocking validators. `validations` allows to + * specify validation configuration with validator functions and extra parameters including + * `isBlocking`. Validators marked as `isBlocking` will produce non blocking validation errors + * a.k.a. warnings. + * + * The problem with the supported approach is lack of flexibility and necessary API like one for getting + * only blocking or non blocking errors. Flexibility requirement comes from complex async validators + * producing blocking and non blocking validation errors. There is no way to use `isBlocking` configuration + * option to separate errors. Separating such validating functions in two would lead to sending two + * HTTP requests and performing another async operations twice. + * + * On top of just having an ability to mark validation errors as non blocking via `isBlocking: false` + * configuration we require a way to return blocking and non blocking errors from a single validation + * function. It'd be possible by returning an error with `isBlocking` (or `isWarning`) flag along with + * `message` and `code` fields from a validator function. Attempts to reuse `__isBlocking__` internal + * field lead to inconsistent behavior. + * + * `useFormWithWarnings` implements warnings (non blocking errors) on top of `FormHook` using validation + * error codes as a flexible way to determine whether an error is a blocking error or it's a warning. + * It provides little interface extension to simplify errors and warnings consumption + * + * In some cases business logic requires implementing functionality to allow users perform an action + * despite non-critical validation errors a.k.a. warnings. Usually it's also required to inform users + * about warnings they got before proceeding for example via a modal. + * + * Since `FormHook` returned by `useForm` lacks of such functionality `useFormWithWarnings` is here to + * provide warnings functionality. It could be used and passed as `FormHook` when warnings functionality + * isn't required making absolutely transparent. + * + * **Important:** Validators use short circuiting by default. It means that any failed validation in + * `validations` configuration array will prevent the rest validators from running. When used with warnings + * it may lead to bugs when validator checks first for warnings. You have to make sure a value is validated + * for errors first and then for warnings. + * + * There is a ticket to move this functionality to Form lib https://github.com/elastic/kibana/issues/203097. + */ +export function useFormWithWarnings( + formConfig: FormWithWarningsConfig +): UseFormWithWarningsReturn { + const { + onSubmit, + options: { warningValidationCodes }, + } = formConfig; + const { form } = useForm(formConfig as FormConfig); + const { validate: originalValidate, getFormData, getFields } = form; + + const validationResultsRef = useRef({ + errors: [], + warnings: [], + }); + const [isSubmitted, setIsSubmitted] = useState(false); + const [isSubmitting, setSubmitting] = useState(false); + const [isValid, setIsValid] = useState(); + const isMounted = useRef(false); + + const validate: FormHook['validate'] = useCallback(async () => { + await originalValidate(); + + validationResultsRef.current = extractValidationResults( + Object.values(getFields()), + warningValidationCodes + ); + + const isFormValid = isEmpty(validationResultsRef.current.errors); + + setIsValid(isFormValid); + + return isFormValid; + }, [originalValidate, getFields, warningValidationCodes, validationResultsRef]); + + const submit: FormHook['submit'] = useCallback( + async (e) => { + if (e) { + e.preventDefault(); + } + + setIsSubmitted(true); + setSubmitting(true); + + const isFormValid = await validate(); + const formData = isFormValid ? getFormData() : ({} as T); + + if (onSubmit) { + await onSubmit(formData, isFormValid, validationResultsRef.current); + } + + if (isMounted.current) { + setSubmitting(false); + } + + return { data: formData, isValid: isFormValid }; + }, + [validate, getFormData, onSubmit, validationResultsRef] + ); + + // Track form's mounted state + useEffect(() => { + isMounted.current = true; + + return () => { + isMounted.current = false; + }; + }, []); + + return useMemo( + () => ({ + form: { + ...form, + isValid, + isSubmitted, + isSubmitting, + validate, + submit, + getErrors: () => validationResultsRef.current.errors.map((x) => x.message), + getValidationWarnings: () => validationResultsRef.current.warnings, + }, + }), + [form, validate, submit, isSubmitted, isSubmitting, isValid, validationResultsRef] + ); +} diff --git a/x-pack/plugins/security_solution/public/common/hooks/use_form_with_warnings/validation_results.ts b/x-pack/plugins/security_solution/public/common/hooks/use_form_with_warnings/validation_results.ts new file mode 100644 index 0000000000000..238abab2cf53c --- /dev/null +++ b/x-pack/plugins/security_solution/public/common/hooks/use_form_with_warnings/validation_results.ts @@ -0,0 +1,13 @@ +/* + * 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 type { ValidationError } from '../../../shared_imports'; + +export interface ValidationResults { + errors: ValidationError[]; + warnings: ValidationError[]; +} diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/eql_query_edit/eql_query_bar.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/eql_query_edit/eql_query_bar.tsx index c7ef28f5ed909..6111d2e1a2d3d 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/eql_query_edit/eql_query_bar.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/eql_query_edit/eql_query_bar.tsx @@ -6,7 +6,7 @@ */ import type { FC, ChangeEvent } from 'react'; -import React, { useCallback, useEffect, useState, useRef } from 'react'; +import React, { useCallback, useEffect, useRef, useMemo } from 'react'; import { Subscription } from 'rxjs'; import styled from 'styled-components'; import deepEqual from 'fast-deep-equal'; @@ -20,9 +20,9 @@ import { useAppToasts } from '../../../../common/hooks/use_app_toasts'; import type { EqlOptions } from '../../../../../common/search_strategy'; import type { FieldValueQueryBar } from '../../../rule_creation_ui/components/query_bar_field'; import { useKibana } from '../../../../common/lib/kibana'; +import { EQL_ERROR_CODES } from '../../../../common/hooks/eql/api'; import type { EqlQueryBarFooterProps } from './footer'; import { EqlQueryBarFooter } from './footer'; -import { getValidationResults } from './validators'; import * as i18n from './translations'; const TextArea = styled(EuiTextArea)` @@ -81,12 +81,10 @@ export const EqlQueryBar: FC = ({ onValidatingChange, }) => { const { addError } = useAppToasts(); - const [errorMessages, setErrorMessages] = useState([]); - const { isValidating, value: fieldValue, setValue: setFieldValue } = field; - const { isValid, message, messages, error } = getValidationResults(field); - const { uiSettings } = useKibana().services; const filterManager = useRef(new FilterManager(uiSettings)); + const { isValidating, value: fieldValue, setValue: setFieldValue, isValid, errors } = field; + const errorMessages = useMemo(() => errors.map((x) => x.message), [errors]); // Bubbles up field validity to parent. // Using something like form `getErrors` does @@ -98,14 +96,12 @@ export const EqlQueryBar: FC = ({ }, [isValid, onValidityChange]); useEffect(() => { - setErrorMessages(messages ?? []); - }, [messages]); + const requestError = errors.find((x) => x.code === EQL_ERROR_CODES.FAILED_REQUEST); - useEffect(() => { - if (error) { - addError(error, { title: i18n.EQL_VALIDATION_REQUEST_ERROR }); + if (requestError) { + addError(requestError.message, { title: i18n.EQL_VALIDATION_REQUEST_ERROR }); } - }, [error, addError]); + }, [errors, addError]); useEffect(() => { if (onValidatingChange) { @@ -152,7 +148,6 @@ export const EqlQueryBar: FC = ({ if (onValidatingChange) { onValidatingChange(true); } - setErrorMessages([]); setFieldValue({ filters: fieldValue.filters, query: { @@ -182,7 +177,7 @@ export const EqlQueryBar: FC = ({ label={field.label} labelAppend={field.labelAppend} helpText={field.helpText} - error={message} + error={errorMessages[0]} isInvalid={!isValid && !isValidating} fullWidth data-test-subj={dataTestSubj} diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/eql_query_edit/eql_query_edit.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/eql_query_edit/eql_query_edit.tsx index 75d3412705fde..36eef70b2be0f 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/eql_query_edit/eql_query_edit.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/eql_query_edit/eql_query_edit.tsx @@ -13,7 +13,7 @@ import { UseMultiFields } from '../../../../shared_imports'; import type { EqlFieldsComboBoxOptions, EqlOptions } from '../../../../../common/search_strategy'; import type { FieldValueQueryBar } from '../../../rule_creation_ui/components/query_bar_field'; import { queryRequiredValidatorFactory } from '../../../rule_creation_ui/validators/query_required_validator_factory'; -import { eqlQueryValidatorFactory } from './eql_query_validator_factory'; +import { eqlQueryValidatorFactory } from './validators/eql_query_validator_factory'; import { EqlQueryBar } from './eql_query_bar'; import * as i18n from './translations'; @@ -28,8 +28,6 @@ interface EqlQueryEditProps { required?: boolean; loading?: boolean; disabled?: boolean; - // This is a temporal solution for Prebuilt Customization workflow - skipEqlValidation?: boolean; onValidityChange?: (arg: boolean) => void; } @@ -43,7 +41,6 @@ export function EqlQueryEdit({ required, loading, disabled, - skipEqlValidation, onValidityChange, }: EqlQueryEditProps): JSX.Element { const componentProps = useMemo( @@ -73,43 +70,29 @@ export function EqlQueryEdit({ }, ] : []), - ...(!skipEqlValidation - ? [ - { - validator: debounceAsync( - (data: ValidationFuncArg) => { - const { formData } = data; - const eqlOptions = - eqlOptionsPath && formData[eqlOptionsPath] ? formData[eqlOptionsPath] : {}; + { + validator: debounceAsync((data: ValidationFuncArg) => { + const { formData } = data; + const eqlOptions = + eqlOptionsPath && formData[eqlOptionsPath] ? formData[eqlOptionsPath] : {}; - return eqlQueryValidatorFactory( - dataView.id - ? { - dataViewId: dataView.id, - eqlOptions, - } - : { - indexPatterns: dataView.title.split(','), - eqlOptions, - } - )(data); - }, - 300 - ), - }, - ] - : []), + return eqlQueryValidatorFactory( + dataView.id + ? { + dataViewId: dataView.id, + eqlOptions, + } + : { + indexPatterns: dataView.title.split(','), + eqlOptions, + } + )(data); + }, 300), + isAsync: true, + }, ], }), - [ - skipEqlValidation, - eqlOptionsPath, - required, - dataView.id, - dataView.title, - path, - fieldsToValidateOnChange, - ] + [eqlOptionsPath, required, dataView.id, dataView.title, path, fieldsToValidateOnChange] ); return ( diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/eql_query_edit/validators.ts b/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/eql_query_edit/validators.ts deleted file mode 100644 index 676a780d9daf5..0000000000000 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/eql_query_edit/validators.ts +++ /dev/null @@ -1,29 +0,0 @@ -/* - * 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 type { FieldHook } from '../../../../shared_imports'; -import { EQL_ERROR_CODES } from '../../../../common/hooks/eql/api'; - -export const getValidationResults = ( - field: FieldHook -): { isValid: boolean; message: string; messages?: string[]; error?: Error } => { - const hasErrors = field.errors.length > 0; - const isValid = !field.isChangingValue && !hasErrors; - - if (hasErrors) { - const [error] = field.errors; - const message = error.message; - - if (error.code === EQL_ERROR_CODES.FAILED_REQUEST) { - return { isValid, message, error: error.error }; - } else { - return { isValid, message, messages: error.messages }; - } - } else { - return { isValid, message: '' }; - } -}; diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/eql_query_edit/eql_query_validator_factory.ts b/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/eql_query_edit/validators/eql_query_validator_factory.ts similarity index 78% rename from x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/eql_query_edit/eql_query_validator_factory.ts rename to x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/eql_query_edit/validators/eql_query_validator_factory.ts index 284d0670dfbc3..4de9e713f7f02 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/eql_query_edit/eql_query_validator_factory.ts +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/eql_query_edit/validators/eql_query_validator_factory.ts @@ -6,13 +6,13 @@ */ import { isEmpty } from 'lodash'; -import type { FormData, ValidationError, ValidationFunc } from '../../../../shared_imports'; -import { KibanaServices } from '../../../../common/lib/kibana'; -import type { FieldValueQueryBar } from '../../../rule_creation_ui/components/query_bar_field'; -import type { EqlOptions } from '../../../../../common/search_strategy'; -import type { EqlResponseError } from '../../../../common/hooks/eql/api'; -import { EQL_ERROR_CODES, validateEql } from '../../../../common/hooks/eql/api'; -import { EQL_VALIDATION_REQUEST_ERROR } from './translations'; +import type { FormData, ValidationError, ValidationFunc } from '../../../../../shared_imports'; +import { KibanaServices } from '../../../../../common/lib/kibana'; +import type { FieldValueQueryBar } from '../../../../rule_creation_ui/components/query_bar_field'; +import type { EqlOptions } from '../../../../../../common/search_strategy'; +import type { EqlResponseError } from '../../../../../common/hooks/eql/api'; +import { EQL_ERROR_CODES, validateEql } from '../../../../../common/hooks/eql/api'; +import { EQL_VALIDATION_REQUEST_ERROR } from '../translations'; type EqlQueryValidatorFactoryParams = | { @@ -71,7 +71,7 @@ export function eqlQueryValidatorFactory({ function transformEqlResponseErrorToValidationError( responseError: EqlResponseError ): ValidationError { - if (responseError.error) { + if (responseError.code === EQL_ERROR_CODES.FAILED_REQUEST) { return { code: EQL_ERROR_CODES.FAILED_REQUEST, message: EQL_VALIDATION_REQUEST_ERROR, @@ -81,8 +81,7 @@ function transformEqlResponseErrorToValidationError( return { code: responseError.code, - message: '', - messages: responseError.messages, + message: responseError.messages.join(', '), }; } diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/esql_query_edit/esql_query_edit.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/esql_query_edit/esql_query_edit.tsx index 695a3d121c9a6..b8ee292081d36 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/esql_query_edit/esql_query_edit.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/esql_query_edit/esql_query_edit.tsx @@ -25,7 +25,6 @@ interface EsqlQueryEditProps { required?: boolean; loading?: boolean; disabled?: boolean; - skipIdColumnCheck?: boolean; onValidityChange?: (arg: boolean) => void; } @@ -36,7 +35,6 @@ export const EsqlQueryEdit = memo(function EsqlQueryEdit({ required = false, loading = false, disabled = false, - skipIdColumnCheck, onValidityChange, }: EsqlQueryEditProps): JSX.Element { const queryClient = useQueryClient(); @@ -67,14 +65,12 @@ export const EsqlQueryEdit = memo(function EsqlQueryEdit({ ] : []), { - validator: debounceAsync( - esqlQueryValidatorFactory({ queryClient, skipIdColumnCheck }), - 300 - ), + validator: debounceAsync(esqlQueryValidatorFactory({ queryClient }), 300), + isAsync: true, }, ], }), - [required, path, fieldsToValidateOnChange, queryClient, skipIdColumnCheck] + [required, path, fieldsToValidateOnChange, queryClient] ); return ( diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/esql_query_edit/validators/esql_query_validator_factory.ts b/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/esql_query_edit/validators/esql_query_validator_factory.ts index c5b54db172a18..a0420f51586aa 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/esql_query_edit/validators/esql_query_validator_factory.ts +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/esql_query_edit/validators/esql_query_validator_factory.ts @@ -16,15 +16,10 @@ import * as i18n from './translations'; interface EsqlQueryValidatorFactoryParams { queryClient: QueryClient; - /** - * This is a temporal fix to unlock prebuilt rule customization workflow - */ - skipIdColumnCheck?: boolean; } export function esqlQueryValidatorFactory({ queryClient, - skipIdColumnCheck, }: EsqlQueryValidatorFactoryParams): ValidationFunc { return async (...args) => { const [{ value }] = args; @@ -50,10 +45,6 @@ export function esqlQueryValidatorFactory({ }; } - if (skipIdColumnCheck) { - return; - } - const columns = await fetchEsqlQueryColumns({ esqlQuery, queryClient, diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation/constants/validation_warning_codes.ts b/x-pack/plugins/security_solution/public/detection_engine/rule_creation/constants/validation_warning_codes.ts new file mode 100644 index 0000000000000..9593324a9c224 --- /dev/null +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation/constants/validation_warning_codes.ts @@ -0,0 +1,38 @@ +/* + * 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'; +import { EQL_ERROR_CODES } from '../../../common/hooks/eql/api'; +import { ESQL_ERROR_CODES } from '../components/esql_query_edit'; + +const ESQL_FIELD_NAME = i18n.translate( + 'xpack.securitySolution.detectionEngine.createRule.nonBlockingErrorCodes.esqlFieldName', + { + defaultMessage: 'ES|QL Query', + } +); + +const EQL_FIELD_NAME = i18n.translate( + 'xpack.securitySolution.detectionEngine.createRule.nonBlockingErrorCodes.eqlFieldName', + { + defaultMessage: 'EQL Query', + } +); + +export const VALIDATION_WARNING_CODES = [ + ESQL_ERROR_CODES.INVALID_ESQL, + EQL_ERROR_CODES.FAILED_REQUEST, + EQL_ERROR_CODES.INVALID_EQL, + EQL_ERROR_CODES.MISSING_DATA_SOURCE, +] as const; + +export const VALIDATION_WARNING_CODE_FIELD_NAME_MAP: Readonly> = { + [ESQL_ERROR_CODES.INVALID_ESQL]: ESQL_FIELD_NAME, + [EQL_ERROR_CODES.FAILED_REQUEST]: EQL_FIELD_NAME, + [EQL_ERROR_CODES.INVALID_EQL]: EQL_FIELD_NAME, + [EQL_ERROR_CODES.MISSING_DATA_SOURCE]: EQL_FIELD_NAME, +}; diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation/logic/extract_validation_messages.ts b/x-pack/plugins/security_solution/public/detection_engine/rule_creation/logic/extract_validation_messages.ts new file mode 100644 index 0000000000000..77847a0b12231 --- /dev/null +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation/logic/extract_validation_messages.ts @@ -0,0 +1,18 @@ +/* + * 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 { capitalize } from 'lodash'; +import type { ValidationError } from '../../../shared_imports'; + +export function extractValidationMessages( + validationErrors: ValidationError[], + errorCodeFieldNameMap: Readonly> +): string[] { + return validationErrors.map( + (x) => `${errorCodeFieldNameMap[x.code ?? ''] ?? capitalize(x.path)}: ${x.message}` + ); +} 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 deleted file mode 100644 index e470b06c7e829..0000000000000 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/save_with_errors_confirmation/translations.ts +++ /dev/null @@ -1,36 +0,0 @@ -/* - * 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.test.ts b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/form.test.ts deleted file mode 100644 index 3210ac84b159a..0000000000000 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/form.test.ts +++ /dev/null @@ -1,302 +0,0 @@ -/* - * 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 { EQL_ERROR_CODES } from '../../../common/hooks/eql/api'; -import type { - AboutStepRule, - ActionsStepRule, - DefineStepRule, - ScheduleStepRule, -} from '../../../detections/pages/detection_engine/rules/types'; -import { ALERT_SUPPRESSION_FIELDS_FIELD_NAME } from '../../rule_creation/components/alert_suppression_edit'; -import { ESQL_ERROR_CODES } from '../../rule_creation/components/esql_query_edit'; -import { useRuleFormsErrors } from './form'; - -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 defineStepForm = getFormWithErrorsMock({ - queryBar: { - errors: [ - { - code: EQL_ERROR_CODES.INVALID_SYNTAX, - message: '', - messages: ["line 1:5: missing 'where' at 'demo'"], - }, - ], - }, - }); - - 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 defineStepForm = getFormWithErrorsMock({ - queryBar: { - errors: [ - { - code: EQL_ERROR_CODES.MISSING_DATA_SOURCE, - message: '', - messages: [ - 'index_not_found_exception Found 1 problem line -1:-1: Unknown index [*,-*]', - ], - }, - ], - }, - }); - - 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 defineStepForm = getFormWithErrorsMock({ - queryBar: { - errors: [ - { - code: EQL_ERROR_CODES.INVALID_EQL, - message: '', - messages: [ - 'Found 2 problems\nline 1:1: Unknown column [event.category]\nline 1:13: Unknown column [event.name]', - ], - }, - ], - }, - }); - - 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 defineStepForm = getFormWithErrorsMock({ - queryBar: { - errors: [ - { - code: EQL_ERROR_CODES.FAILED_REQUEST, - message: 'An error occurred while validating your EQL query', - error: new Error('Some internal error'), - }, - ], - }, - }); - - 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 defineStepForm = getFormWithErrorsMock({ - queryBar: { - errors: [ - { - code: EQL_ERROR_CODES.MISSING_DATA_SOURCE, - message: '', - messages: ['Missing data source'], - }, - ], - }, - }); - 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] }, - [ALERT_SUPPRESSION_FIELDS_FIELD_NAME]: { 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 f88d0c1449442..65ff07924c70f 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,8 +5,9 @@ * 2.0. */ -import { useState, useMemo, useEffect, useCallback } from 'react'; +import { useState, useMemo, useEffect } from 'react'; import type { DataViewBase } from '@kbn/es-query'; +import { useFormWithWarnings } from '../../../common/hooks/use_form_with_warnings'; import { isThreatMatchRule } from '../../../../common/detection_engine/utils'; import type { AboutStepRule, @@ -16,19 +17,17 @@ 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 type { FormHook } from '../../../shared_imports'; +import { useFormData } from '../../../shared_imports'; import { schema as defineRuleSchema } from '../components/step_define_rule/schema'; import { schema as aboutRuleSchema, threatMatchAboutSchema, } from '../components/step_about_rule/schema'; -import { ESQL_ERROR_CODES } from '../../rule_creation/components/esql_query_edit'; 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 { EQL_ERROR_CODES } from '../../../common/hooks/eql/api'; -import * as i18n from './translations'; +import { VALIDATION_WARNING_CODES } from '../../rule_creation/constants/validation_warning_codes'; export interface UseRuleFormsProps { defineStepDefault: DefineStepRule; @@ -47,9 +46,9 @@ export const useRuleForms = ({ triggersActionsUi: { actionTypeRegistry }, } = useKibana().services; // DEFINE STEP FORM - const { form: defineStepForm } = useForm({ + const { form: defineStepForm } = useFormWithWarnings({ defaultValue: defineStepDefault, - options: { stripEmptyFields: false }, + options: { stripEmptyFields: false, warningValidationCodes: VALIDATION_WARNING_CODES }, schema: defineRuleSchema, }); const [defineStepFormData] = useFormData({ @@ -67,9 +66,9 @@ export const useRuleForms = ({ () => (isThreatMatchRule(defineStepData.ruleType) ? threatMatchAboutSchema : aboutRuleSchema), [defineStepData.ruleType] ); - const { form: aboutStepForm } = useForm({ + const { form: aboutStepForm } = useFormWithWarnings({ defaultValue: aboutStepDefault, - options: { stripEmptyFields: false }, + options: { stripEmptyFields: false, warningValidationCodes: VALIDATION_WARNING_CODES }, schema: typeDependentAboutRuleSchema, }); const [aboutStepFormData] = useFormData({ @@ -78,9 +77,9 @@ export const useRuleForms = ({ const aboutStepData = 'name' in aboutStepFormData ? aboutStepFormData : aboutStepDefault; // SCHEDULE STEP FORM - const { form: scheduleStepForm } = useForm({ + const { form: scheduleStepForm } = useFormWithWarnings({ defaultValue: scheduleStepDefault, - options: { stripEmptyFields: false }, + options: { stripEmptyFields: false, warningValidationCodes: VALIDATION_WARNING_CODES }, schema: scheduleRuleSchema, }); const [scheduleStepFormData] = useFormData({ @@ -91,9 +90,9 @@ export const useRuleForms = ({ // ACTIONS STEP FORM const schema = useMemo(() => getActionsRuleSchema({ actionTypeRegistry }), [actionTypeRegistry]); - const { form: actionsStepForm } = useForm({ + const { form: actionsStepForm } = useFormWithWarnings({ defaultValue: actionsStepDefault, - options: { stripEmptyFields: false }, + options: { stripEmptyFields: false, warningValidationCodes: VALIDATION_WARNING_CODES }, schema, }); const [actionsStepFormData] = useFormData({ @@ -158,81 +157,3 @@ export interface UseRuleFormsErrors { scheduleStepForm?: FormHook; actionsStepForm?: FormHook; } - -const getFieldErrorMessages = (fieldError: ValidationError) => { - if (fieldError.message.length > 0) { - return [fieldError.message]; - } else if (Array.isArray(fieldError.messages)) { - // 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 []; -}; - -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 6019b696a089c..4bf634595a9db 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 @@ -56,6 +56,8 @@ import { } from '../../../../detections/pages/detection_engine/rules/helpers'; import type { DefineStepRule } from '../../../../detections/pages/detection_engine/rules/types'; import { RuleStep } from '../../../../detections/pages/detection_engine/rules/types'; +import { ALERT_SUPPRESSION_FIELDS_FIELD_NAME } from '../../../rule_creation/components/alert_suppression_edit'; +import { useConfirmValidationErrorsModal } from '../../../../common/hooks/use_confirm_validation_errors_modal'; import { formatRule } from './helpers'; import { useEsqlIndex, useEsqlQueryForAboutStep } from '../../hooks'; import * as i18n from './translations'; @@ -77,11 +79,11 @@ import { useKibana, useUiSetting$ } from '../../../../common/lib/kibana'; import { RulePreview } from '../../components/rule_preview'; import { getIsRulePreviewDisabled } from '../../components/rule_preview/helpers'; import { useStartMlJobs } from '../../../rule_management/logic/use_start_ml_jobs'; +import { VALIDATION_WARNING_CODE_FIELD_NAME_MAP } from '../../../rule_creation/constants/validation_warning_codes'; +import { extractValidationMessages } from '../../../rule_creation/logic/extract_validation_messages'; import { NextStep } from '../../components/next_step'; -import { useRuleForms, useRuleFormsErrors, useRuleIndexPattern } from '../form'; +import { useRuleForms, useRuleIndexPattern } from '../form'; import { CustomHeaderPageMemo } from '..'; -import { SaveWithErrorsModal } from '../../components/save_with_errors_confirmation'; -import { ALERT_SUPPRESSION_FIELDS_FIELD_NAME } from '../../../rule_creation/components/alert_suppression_edit'; const MyEuiPanel = styled(EuiPanel)<{ zindex?: number; @@ -178,6 +180,9 @@ const CreateRulePageComponent: React.FC = () => { actionsStepDefault, }); + const { modal: confirmSavingWithWarningModal, confirmValidationErrors } = + useConfirmValidationErrorsModal(); + const isThreatMatchRuleValue = useMemo( () => isThreatMatchRule(defineStepData.ruleType), [defineStepData.ruleType] @@ -203,12 +208,6 @@ 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); @@ -315,73 +314,73 @@ const CreateRulePageComponent: React.FC = () => { switch (step) { case RuleStep.defineRule: { const valid = await defineStepForm.validate(); - const { blockingErrors, nonBlockingErrors } = getRuleFormsErrors({ defineStepForm }); - return { valid, blockingErrors, nonBlockingErrors }; + + return { + valid, + warnings: defineStepForm.getValidationWarnings(), + }; } + case RuleStep.aboutRule: { const valid = await aboutStepForm.validate(); - const { blockingErrors, nonBlockingErrors } = getRuleFormsErrors({ aboutStepForm }); - return { valid, blockingErrors, nonBlockingErrors }; + + return { + valid, + warnings: aboutStepForm.getValidationWarnings(), + }; } case RuleStep.scheduleRule: { const valid = await scheduleStepForm.validate(); - const { blockingErrors, nonBlockingErrors } = getRuleFormsErrors({ scheduleStepForm }); - return { valid, blockingErrors, nonBlockingErrors }; + + return { + valid, + warnings: scheduleStepForm.getValidationWarnings(), + }; } case RuleStep.ruleActions: { const valid = await actionsStepForm.validate(); - const { blockingErrors, nonBlockingErrors } = getRuleFormsErrors({ actionsStepForm }); - return { valid, blockingErrors, nonBlockingErrors }; + + return { + valid, + warnings: actionsStepForm.getValidationWarnings(), + }; } } }, - [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); + [aboutStepForm, actionsStepForm, defineStepForm, scheduleStepForm] + ); + + const validateAllSteps = useCallback(async () => { + const { valid: defineStepFormValid, warnings: defineStepWarnings } = await validateStep( + RuleStep.defineRule + ); + const { valid: aboutStepFormValid, warnings: aboutStepWarnings } = await validateStep( + RuleStep.aboutRule + ); + const { valid: scheduleStepFormValid, warnings: scheduleStepWarnings } = await validateStep( + RuleStep.scheduleRule + ); + const { valid: actionsStepFormValid, warnings: actionsStepWarnings } = await validateStep( + RuleStep.ruleActions + ); const valid = defineStepFormValid && aboutStepFormValid && scheduleStepFormValid && actionsStepFormValid; - const blockingErrors = [ - ...defineStepBlockingErrors, - ...aboutStepBlockingErrors, - ...scheduleStepBlockingErrors, - ...actionsStepBlockingErrors, - ]; - const nonBlockingErrors = [ - ...defineStepNonBlockingErrors, - ...aboutStepNonBlockingErrors, - ...scheduleStepNonBlockingErrors, - ...actionsStepNonBlockingErrors, + const warnings = [ + ...defineStepWarnings, + ...aboutStepWarnings, + ...scheduleStepWarnings, + ...actionsStepWarnings, ]; - return { valid, blockingErrors, nonBlockingErrors }; + return { valid, warnings }; }, [validateStep]); const editStep = useCallback( async (step: RuleStep) => { - const { valid, blockingErrors } = await validateStep(activeStep); - if (valid || !blockingErrors.length) { + const { valid } = await validateStep(activeStep); + + if (valid) { goToStep(step); } }, @@ -440,34 +439,21 @@ 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 - await createRuleFromFormData(enabled); - return; - } + const { valid, warnings } = await validateAllSteps(); + const warningMessages = extractValidationMessages( + warnings, + VALIDATION_WARNING_CODE_FIELD_NAME_MAP + ); - if (blockingErrors.length > 0) { - // There are blocking validation errors, thus do not allow user to create a rule + if (!valid || !(await confirmValidationErrors(warningMessages))) { 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(); - } + + await createRuleFromFormData(enabled); }, - [createRuleFromFormData, showSaveWithErrorsModal, validateEachStep] + [createRuleFromFormData, validateAllSteps, confirmValidationErrors] ); const defineRuleButtonType = @@ -846,13 +832,7 @@ const CreateRulePageComponent: React.FC = () => { return ( <> - {isSaveWithErrorsModalVisible && ( - - )} + {confirmSavingWithWarningModal} {(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 594b9d2a35598..3327e45bd2bb7 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 @@ -21,6 +21,7 @@ import type { FC } from 'react'; import React, { memo, useCallback, useMemo, useRef, useState } from 'react'; import { useParams } from 'react-router-dom'; +import { useConfirmValidationErrorsModal } from '../../../../common/hooks/use_confirm_validation_errors_modal'; import { useAppToasts } from '../../../../common/hooks/use_app_toasts'; import { isEsqlRule } from '../../../../../common/detection_engine/utils'; import { RulePreview } from '../../components/rule_preview'; @@ -67,10 +68,11 @@ 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, useRuleFormsErrors, useRuleIndexPattern } from '../form'; +import { extractValidationMessages } from '../../../rule_creation/logic/extract_validation_messages'; +import { VALIDATION_WARNING_CODE_FIELD_NAME_MAP } from '../../../rule_creation/constants/validation_warning_codes'; +import { useRuleForms, useRuleIndexPattern } from '../form'; import { useEsqlIndex, useEsqlQueryForAboutStep } from '../../hooks'; import { CustomHeaderPageMemo } from '..'; -import { SaveWithErrorsModal } from '../../components/save_with_errors_confirmation'; import { useIsPrebuiltRulesCustomizationEnabled } from '../../../rule_management/hooks/use_is_prebuilt_rules_customization_enabled'; import { ALERT_SUPPRESSION_FIELDS_FIELD_NAME } from '../../../rule_creation/components/alert_suppression_edit'; @@ -104,9 +106,6 @@ 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([]); - const backOptions = useMemo( () => ({ path: getRuleDetailsUrl(ruleId ?? ''), @@ -140,7 +139,8 @@ const EditRulePageComponent: FC<{ rule: RuleResponse }> = ({ rule }) => { actionsStepDefault: ruleActionsData, }); - const { getRuleFormsErrors } = useRuleFormsErrors(); + const { modal: confirmSavingWithWarningModal, confirmValidationErrors } = + useConfirmValidationErrorsModal(); const esqlQueryForAboutStep = useEsqlQueryForAboutStep({ defineStepData, activeStep }); @@ -411,16 +411,7 @@ 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([]); - const actionsStepFormValid = await actionsStepForm.validate(); if (!isPrebuiltRulesCustomizationEnabled && rule.immutable) { // Since users cannot edit Define, About and Schedule tabs of the rule, we skip validation of those to avoid @@ -435,29 +426,36 @@ const EditRulePageComponent: FC<{ rule: RuleResponse }> = ({ rule }) => { const defineStepFormValid = await defineStepForm.validate(); const aboutStepFormValid = await aboutStepForm.validate(); const scheduleStepFormValid = await scheduleStepForm.validate(); + if ( - defineStepFormValid && - aboutStepFormValid && - scheduleStepFormValid && - actionsStepFormValid + !defineStepFormValid || + !aboutStepFormValid || + !scheduleStepFormValid || + !actionsStepFormValid ) { - await saveChanges(); return; } - const { blockingErrors, nonBlockingErrors } = getRuleFormsErrors({ - defineStepForm, - aboutStepForm, - scheduleStepForm, - actionsStepForm, - }); - if (blockingErrors.length > 0) { + const defineRuleWarnings = defineStepForm.getValidationWarnings(); + const aboutRuleWarnings = aboutStepForm.getValidationWarnings(); + const scheduleRuleWarnings = scheduleStepForm.getValidationWarnings(); + const ruleActionsWarnings = actionsStepForm.getValidationWarnings(); + + const warnings = extractValidationMessages( + [ + ...defineRuleWarnings, + ...aboutRuleWarnings, + ...scheduleRuleWarnings, + ...ruleActionsWarnings, + ], + VALIDATION_WARNING_CODE_FIELD_NAME_MAP + ); + + if (!(await confirmValidationErrors(warnings))) { return; } - if (nonBlockingErrors.length > 0) { - setNonBlockingRuleErrors(nonBlockingErrors); - showSaveWithErrorsModal(); - } + + await saveChanges(); }, [ actionsStepForm, isPrebuiltRulesCustomizationEnabled, @@ -465,9 +463,8 @@ const EditRulePageComponent: FC<{ rule: RuleResponse }> = ({ rule }) => { defineStepForm, aboutStepForm, scheduleStepForm, - getRuleFormsErrors, + confirmValidationErrors, saveChanges, - showSaveWithErrorsModal, ]); const onTabClick = useCallback(async (tab: EuiTabbedContentTab) => { @@ -523,13 +520,7 @@ const EditRulePageComponent: FC<{ rule: RuleResponse }> = ({ rule }) => { return ( <> - {isSaveWithErrorsModalVisible && ( - - )} + {confirmSavingWithWarningModal} {(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 77ea9438f66dc..e602b8be712c2 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,9 +13,3 @@ export const RULE_PREVIEW_TITLE = i18n.translate( defaultMessage: 'Rule preview', } ); - -export const QUERY_BAR_VALIDATION_ERROR = (validationError: string) => - i18n.translate('xpack.securitySolution.detectionEngine.createRule.validationError', { - values: { validationError }, - defaultMessage: 'Query bar: {validationError}', - }); diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/three_way_diff/final_edit/fields/eql_query/eql_query_edit_adapter.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/three_way_diff/final_edit/fields/eql_query/eql_query_edit_adapter.tsx index 787891452f1d7..cea9b9308c0df 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/three_way_diff/final_edit/fields/eql_query/eql_query_edit_adapter.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/three_way_diff/final_edit/fields/eql_query/eql_query_edit_adapter.tsx @@ -29,7 +29,6 @@ export function EqlQueryEditAdapter({ dataView={dataView ?? DEFAULT_DATA_VIEW_BASE} loading={isLoading} disabled={isLoading} - skipEqlValidation /> ); } diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/three_way_diff/final_edit/fields/esql_query/esql_query_edit_adapter.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/three_way_diff/final_edit/fields/esql_query/esql_query_edit_adapter.tsx index a9375b7316bb3..faf43d5b88b22 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/three_way_diff/final_edit/fields/esql_query/esql_query_edit_adapter.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/three_way_diff/final_edit/fields/esql_query/esql_query_edit_adapter.tsx @@ -23,7 +23,6 @@ export function EsqlQueryEditAdapter({ dataView={dataView ?? DEFAULT_DATA_VIEW_BASE} loading={isLoading} disabled={isLoading} - skipIdColumnCheck /> ); } diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/three_way_diff/final_edit/fields/rule_field_edit_form_wrapper.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/three_way_diff/final_edit/fields/rule_field_edit_form_wrapper.tsx index 1b45bea28880f..a5f7eedc6114c 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/three_way_diff/final_edit/fields/rule_field_edit_form_wrapper.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/three_way_diff/final_edit/fields/rule_field_edit_form_wrapper.tsx @@ -7,7 +7,10 @@ import React, { useCallback, useEffect } from 'react'; import { EuiButtonEmpty, EuiFlexGroup } from '@elastic/eui'; -import { useForm, Form } from '../../../../../../../shared_imports'; +import { extractValidationMessages } from '../../../../../../rule_creation/logic/extract_validation_messages'; +import type { FormWithWarningsSubmitHandler } from '../../../../../../../common/hooks/use_form_with_warnings'; +import { useFormWithWarnings } from '../../../../../../../common/hooks/use_form_with_warnings'; +import { Form } from '../../../../../../../shared_imports'; import type { FormSchema, FormData } from '../../../../../../../shared_imports'; import type { DiffableAllFields, @@ -17,6 +20,11 @@ import { useFinalSideContext } from '../../final_side/final_side_context'; import { useDiffableRuleContext } from '../../diffable_rule_context'; import * as i18n from '../../translations'; import type { RuleFieldEditComponentProps } from './rule_field_edit_component_props'; +import { useConfirmValidationErrorsModal } from '../../../../../../../common/hooks/use_confirm_validation_errors_modal'; +import { + VALIDATION_WARNING_CODE_FIELD_NAME_MAP, + VALIDATION_WARNING_CODES, +} from '../../../../../../rule_creation/constants/validation_warning_codes'; type RuleFieldEditComponent = React.ComponentType; @@ -56,9 +64,16 @@ export function RuleFieldEditFormWrapper({ [deserializer, finalDiffableRule] ); - const handleSubmit = useCallback( - async (formData: FormData, isValid: boolean) => { - if (!isValid) { + const { modal, confirmValidationErrors } = useConfirmValidationErrorsModal(); + + const handleSubmit = useCallback( + async (formData: FormData, isValid: boolean, { warnings }) => { + const warningMessages = extractValidationMessages( + warnings, + VALIDATION_WARNING_CODE_FIELD_NAME_MAP + ); + + if (!isValid || !(await confirmValidationErrors(warningMessages))) { return; } @@ -69,15 +84,24 @@ export function RuleFieldEditFormWrapper({ }); setReadOnlyMode(); }, - [fieldName, finalDiffableRule.rule_id, setReadOnlyMode, setRuleFieldResolvedValue] + [ + confirmValidationErrors, + fieldName, + finalDiffableRule.rule_id, + setReadOnlyMode, + setRuleFieldResolvedValue, + ] ); - const { form } = useForm({ + const { form } = useFormWithWarnings({ schema: ruleFieldFormSchema, defaultValue: getDefaultValue(fieldName, finalDiffableRule), deserializer: deserialize, serializer, onSubmit: handleSubmit, + options: { + warningValidationCodes: VALIDATION_WARNING_CODES, + }, }); // form.isValid has `undefined` value until all fields are dirty. @@ -96,6 +120,7 @@ export function RuleFieldEditFormWrapper({ {i18n.SAVE_BUTTON_LABEL} + {modal}