From 18a86beb216b76aad4079bf4447821bb4a1dc9bc Mon Sep 17 00:00:00 2001 From: Dzmitry Tamashevich Date: Thu, 24 Nov 2022 20:15:13 +0300 Subject: [PATCH 1/5] [Discover] validate if data view time field exists on alert creation --- .../explore_matching_button.tsx | 1 + .../public/rule_types/es_query/constants.ts | 30 ++-- .../expression/es_query_expression.tsx | 24 ++- .../es_query/expression/expression.tsx | 7 +- .../expression/search_source_expression.tsx | 4 +- .../search_source_expression_form.tsx | 7 +- .../public/rule_types/es_query/types.ts | 17 +- .../public/rule_types/es_query/util.ts | 6 +- .../public/rule_types/es_query/validation.ts | 153 ++++++++++++------ .../sections/rule_form/rule_add.tsx | 21 ++- .../sections/rule_form/rule_edit.tsx | 22 ++- .../sections/rule_form/rule_errors.ts | 8 +- .../triggers_actions_ui/public/types.ts | 6 +- .../apps/discover/search_source_alert.ts | 31 +++- 14 files changed, 227 insertions(+), 110 deletions(-) diff --git a/src/plugins/unified_search/public/dataview_picker/explore_matching_button.tsx b/src/plugins/unified_search/public/dataview_picker/explore_matching_button.tsx index db98916b1a080..2d25196c119f5 100644 --- a/src/plugins/unified_search/public/dataview_picker/explore_matching_button.tsx +++ b/src/plugins/unified_search/public/dataview_picker/explore_matching_button.tsx @@ -44,6 +44,7 @@ export const ExploreMatchingButton = ({ { setPopoverIsOpen(false); onCreateDefaultAdHocDataView(dataViewSearchString); diff --git a/x-pack/plugins/stack_alerts/public/rule_types/es_query/constants.ts b/x-pack/plugins/stack_alerts/public/rule_types/es_query/constants.ts index efb8144c2a840..dae63a7e1ad4a 100644 --- a/x-pack/plugins/stack_alerts/public/rule_types/es_query/constants.ts +++ b/x-pack/plugins/stack_alerts/public/rule_types/es_query/constants.ts @@ -6,7 +6,6 @@ */ import { COMPARATORS } from '@kbn/triggers-actions-ui-plugin/public'; -import { ErrorKey } from './types'; export const DEFAULT_VALUES = { THRESHOLD_COMPARATOR: COMPARATORS.GREATER_THAN, @@ -22,17 +21,30 @@ export const DEFAULT_VALUES = { EXCLUDE_PREVIOUS_HITS: true, }; -export const EXPRESSION_ERRORS = { - index: new Array(), - size: new Array(), - timeField: new Array(), +export const COMMON_EXPRESSION_ERRORS = { + searchType: new Array(), threshold0: new Array(), threshold1: new Array(), - esQuery: new Array(), - thresholdComparator: new Array(), timeWindowSize: new Array(), + size: new Array(), +}; + +export const SEARCH_SOURCE_ONLY_EXPRESSION_ERRORS = { searchConfiguration: new Array(), - searchType: new Array(), }; -export const EXPRESSION_ERROR_KEYS = Object.keys(EXPRESSION_ERRORS) as ErrorKey[]; +export const ONLY_ES_QUERY_EXPRESSION_ERRORS = { + index: new Array(), + timeField: new Array(), + esQuery: new Array(), +}; + +const ALL_EXPRESSION_ERROR_ENTRIES = { + ...COMMON_EXPRESSION_ERRORS, + ...SEARCH_SOURCE_ONLY_EXPRESSION_ERRORS, + ...ONLY_ES_QUERY_EXPRESSION_ERRORS, +}; + +export const ALL_EXPRESSION_ERROR_KEYS = Object.keys(ALL_EXPRESSION_ERROR_ENTRIES) as Array< + keyof typeof ALL_EXPRESSION_ERROR_ENTRIES +>; diff --git a/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/es_query_expression.tsx b/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/es_query_expression.tsx index 359faa935346b..c07153c77146f 100644 --- a/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/es_query_expression.tsx +++ b/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/es_query_expression.tsx @@ -11,10 +11,9 @@ import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n-react'; import { EuiFormRow, EuiLink, EuiSpacer, EuiTitle } from '@elastic/eui'; -import { DocLinksStart, HttpSetup } from '@kbn/core/public'; import { XJson } from '@kbn/es-ui-shared-plugin/public'; -import { CodeEditor, useKibana } from '@kbn/kibana-react-plugin/public'; +import { CodeEditor } from '@kbn/kibana-react-plugin/public'; import { getFields, RuleTypeParamsExpressionProps } from '@kbn/triggers-actions-ui-plugin/public'; import { parseDuration } from '@kbn/alerting-plugin/common'; import { hasExpressionValidationErrors } from '../validation'; @@ -24,14 +23,10 @@ import { IndexSelectPopover } from '../../components/index_select_popover'; import { DEFAULT_VALUES } from '../constants'; import { RuleCommonExpressions } from '../rule_common_expressions'; import { totalHitsToNumber } from '../test_query_row'; +import { useTriggerUiActionServices } from '../util'; const { useXJsonMode } = XJson; -interface KibanaDeps { - http: HttpSetup; - docLinks: DocLinksStart; -} - export const EsQueryExpression: React.FC< RuleTypeParamsExpressionProps, EsQueryRuleMetaData> > = ({ ruleParams, setRuleParams, setRuleProperty, errors, data }) => { @@ -73,7 +68,8 @@ export const EsQueryExpression: React.FC< [setRuleParams] ); - const { http, docLinks } = useKibana().services; + const services = useTriggerUiActionServices(); + const { http, docLinks } = services; const [esFields, setEsFields] = useState< Array<{ @@ -107,13 +103,15 @@ export const EsQueryExpression: React.FC< } }; - const hasValidationErrors = useCallback(() => { - return hasExpressionValidationErrors(currentRuleParams); - }, [currentRuleParams]); + const [hasValidationErrors, setHasValidationErrors] = useState(false); + + useEffect(() => { + hasExpressionValidationErrors(currentRuleParams, services).then(setHasValidationErrors); + }, [currentRuleParams, services]); const onTestQuery = useCallback(async () => { const window = `${timeWindowSize}${timeWindowUnit}`; - if (hasValidationErrors()) { + if (hasValidationErrors) { return { nrOfDocs: 0, timeWindow: window }; } const timeWindow = parseDuration(window); @@ -251,7 +249,7 @@ export const EsQueryExpression: React.FC< setParam('size', updatedValue); }} errors={errors} - hasValidationErrors={hasValidationErrors()} + hasValidationErrors={hasValidationErrors} onTestFetch={onTestQuery} excludeHitsFromPreviousRun={excludeHitsFromPreviousRun} onChangeExcludeHitsFromPreviousRun={(exclude) => { diff --git a/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/expression.tsx b/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/expression.tsx index 65cd4b8e1a65c..2998ebb66083a 100644 --- a/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/expression.tsx +++ b/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/expression.tsx @@ -16,7 +16,7 @@ import { SearchSourceExpression, SearchSourceExpressionProps } from './search_so import { EsQueryExpression } from './es_query_expression'; import { QueryFormTypeChooser } from './query_form_type_chooser'; import { isSearchSourceRule } from '../util'; -import { EXPRESSION_ERROR_KEYS } from '../constants'; +import { ALL_EXPRESSION_ERROR_KEYS } from '../constants'; function areSearchSourceExpressionPropsEqual( prevProps: Readonly>, @@ -59,7 +59,7 @@ export const EsQueryRuleTypeExpression: React.FunctionComponent< } ); - const errorParam = EXPRESSION_ERROR_KEYS.find((errorKey) => { + const errorParam = ALL_EXPRESSION_ERROR_KEYS.find((errorKey) => { return errors[errorKey]?.length >= 1 && ruleParams[errorKey] !== undefined; }); @@ -68,8 +68,9 @@ export const EsQueryRuleTypeExpression: React.FunctionComponent< (); const setParam = useCallback( - (paramField: string, paramValue: unknown) => setRuleParams(paramField, paramValue), + (paramField: string, paramValue: unknown) => { + setRuleParams(paramField, paramValue); + }, [setRuleParams] ); diff --git a/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/search_source_expression_form.tsx b/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/search_source_expression_form.tsx index a439748500a48..4b8ad477d468f 100644 --- a/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/search_source_expression_form.tsx +++ b/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/search_source_expression_form.tsx @@ -217,6 +217,11 @@ export const SearchSourceExpressionForm = (props: SearchSourceExpressionFormProp return { nrOfDocs: totalHitsToNumber(rawResponse.hits.total), timeWindow }; }, [timeWindow, createTestSearchSource]); + const [hasValidationErrors, setHasValidationErrors] = useState(false); + useEffect(() => { + hasExpressionValidationErrors(ruleParams, services).then(setHasValidationErrors); + }, [ruleParams, services]); + return ( @@ -290,7 +295,7 @@ export const SearchSourceExpressionForm = (props: SearchSourceExpressionFormProp onChangeWindowUnit={onChangeWindowUnit} onChangeSizeValue={onChangeSizeValue} errors={errors} - hasValidationErrors={hasExpressionValidationErrors(ruleParams) || !dataView} + hasValidationErrors={hasValidationErrors} onTestFetch={onTestFetch} onCopyQuery={onCopyQuery} excludeHitsFromPreviousRun={ruleConfiguration.excludeHitsFromPreviousRun} diff --git a/x-pack/plugins/stack_alerts/public/rule_types/es_query/types.ts b/x-pack/plugins/stack_alerts/public/rule_types/es_query/types.ts index 23cb6d8fc6e82..49bacc84beb72 100644 --- a/x-pack/plugins/stack_alerts/public/rule_types/es_query/types.ts +++ b/x-pack/plugins/stack_alerts/public/rule_types/es_query/types.ts @@ -8,11 +8,7 @@ import { RuleTypeParams } from '@kbn/alerting-plugin/common'; import { SerializedSearchSourceFields } from '@kbn/data-plugin/common'; import { EuiComboBoxOptionOption } from '@elastic/eui'; -import type { DataPublicPluginStart } from '@kbn/data-plugin/public'; -import type { DataViewEditorStart } from '@kbn/data-view-editor-plugin/public'; -import type { UnifiedSearchPublicPluginStart } from '@kbn/unified-search-plugin/public'; -import type { DataViewsPublicPluginStart, DataView } from '@kbn/data-views-plugin/public'; -import { EXPRESSION_ERRORS } from './constants'; +import type { DataView } from '@kbn/data-views-plugin/public'; export interface Comparator { text: string; @@ -56,14 +52,3 @@ export interface OnlySearchSourceRuleParams { } export type DataViewOption = EuiComboBoxOptionOption; - -export type ExpressionErrors = typeof EXPRESSION_ERRORS; - -export type ErrorKey = keyof ExpressionErrors & unknown; - -export interface TriggersAndActionsUiDeps { - dataViews: DataViewsPublicPluginStart; - unifiedSearch: UnifiedSearchPublicPluginStart; - data: DataPublicPluginStart; - dataViewEditor: DataViewEditorStart; -} diff --git a/x-pack/plugins/stack_alerts/public/rule_types/es_query/util.ts b/x-pack/plugins/stack_alerts/public/rule_types/es_query/util.ts index cc4cfb2313d69..dfb833ef93a0f 100644 --- a/x-pack/plugins/stack_alerts/public/rule_types/es_query/util.ts +++ b/x-pack/plugins/stack_alerts/public/rule_types/es_query/util.ts @@ -5,8 +5,8 @@ * 2.0. */ -import { useKibana } from '@kbn/kibana-react-plugin/public'; -import { EsQueryRuleParams, SearchType, TriggersAndActionsUiDeps } from './types'; +import { useKibana } from '@kbn/triggers-actions-ui-plugin/public'; +import { EsQueryRuleParams, SearchType } from './types'; export const isSearchSourceRule = ( ruleParams: EsQueryRuleParams @@ -14,4 +14,4 @@ export const isSearchSourceRule = ( return ruleParams.searchType === 'searchSource'; }; -export const useTriggerUiActionServices = () => useKibana().services; +export const useTriggerUiActionServices = () => useKibana().services; diff --git a/x-pack/plugins/stack_alerts/public/rule_types/es_query/validation.ts b/x-pack/plugins/stack_alerts/public/rule_types/es_query/validation.ts index bbfd8cdd8e698..d86a1595848c5 100644 --- a/x-pack/plugins/stack_alerts/public/rule_types/es_query/validation.ts +++ b/x-pack/plugins/stack_alerts/public/rule_types/es_query/validation.ts @@ -7,16 +7,22 @@ import { defaultsDeep, isNil } from 'lodash'; import { i18n } from '@kbn/i18n'; -import { ValidationResult, builtInComparators } from '@kbn/triggers-actions-ui-plugin/public'; -import { EsQueryRuleParams, ExpressionErrors } from './types'; +import { + ValidationResult, + builtInComparators, + TriggersAndActionsUiServices, +} from '@kbn/triggers-actions-ui-plugin/public'; +import { EsQueryRuleParams, OnlySearchSourceRuleParams, OnlyEsQueryRuleParams } from './types'; import { isSearchSourceRule } from './util'; -import { EXPRESSION_ERRORS } from './constants'; +import { + COMMON_EXPRESSION_ERRORS, + ONLY_ES_QUERY_EXPRESSION_ERRORS, + SEARCH_SOURCE_ONLY_EXPRESSION_ERRORS, +} from './constants'; -export const validateExpression = (ruleParams: EsQueryRuleParams): ValidationResult => { +const validateCommonParams = (ruleParams: EsQueryRuleParams) => { const { size, threshold, timeWindowSize, thresholdComparator } = ruleParams; - const validationResult = { errors: {} }; - const errors: ExpressionErrors = defaultsDeep({}, EXPRESSION_ERRORS); - validationResult.errors = errors; + const errors: typeof COMMON_EXPRESSION_ERRORS = defaultsDeep({}, COMMON_EXPRESSION_ERRORS); if (!('index' in ruleParams) && !ruleParams.searchType) { errors.searchType.push( @@ -25,7 +31,7 @@ export const validateExpression = (ruleParams: EsQueryRuleParams): ValidationRes }) ); - return validationResult; + return errors; } if (!threshold || threshold.length === 0 || threshold[0] === undefined) { @@ -79,43 +85,70 @@ export const validateExpression = (ruleParams: EsQueryRuleParams): ValidationRes ); } - /** - * Skip esQuery and index params check if it is search source rule, - * since it should contain searchConfiguration instead of esQuery and index. - */ - const isSearchSource = isSearchSourceRule(ruleParams); - if (isSearchSource) { - if (!ruleParams.searchConfiguration) { - errors.searchConfiguration.push( - i18n.translate( - 'xpack.stackAlerts.esQuery.ui.validation.error.requiredSearchConfiguration', - { - defaultMessage: 'Search source configuration is required.', - } - ) - ); - } else if (!ruleParams.searchConfiguration.index) { - errors.index.push( - i18n.translate('xpack.stackAlerts.esQuery.ui.validation.error.requiredDataViewText', { - defaultMessage: 'Data view is required.', - }) - ); - } else if ( - typeof ruleParams.searchConfiguration.index === 'object' && - !Object.hasOwn(ruleParams.searchConfiguration.index, 'timeFieldName') - ) { - errors.index.push( - i18n.translate( - 'xpack.stackAlerts.esQuery.ui.validation.error.requiredDataViewTimeFieldText', - { - defaultMessage: 'Data view should have a time field.', - } - ) - ); - } - return validationResult; + return errors; +}; + +const validateSearchSourceParams = async ( + ruleParams: OnlySearchSourceRuleParams, + services: TriggersAndActionsUiServices +) => { + const errors: typeof SEARCH_SOURCE_ONLY_EXPRESSION_ERRORS = defaultsDeep( + {}, + SEARCH_SOURCE_ONLY_EXPRESSION_ERRORS + ); + + if (!ruleParams.searchConfiguration) { + errors.searchConfiguration.push( + i18n.translate('xpack.stackAlerts.esQuery.ui.validation.error.requiredSearchConfiguration', { + defaultMessage: 'Search source configuration is required.', + }) + ); + return errors; + } + + let searchSource; + try { + searchSource = await services.data.search.searchSource.create(ruleParams.searchConfiguration); + } catch (e) { + errors.searchConfiguration.push( + i18n.translate('xpack.stackAlerts.esQuery.ui.validation.error.searchConfigError', { + defaultMessage: 'Search source configuration is invalid.', + }) + ); + return errors; } + const dataView = searchSource?.getField('index'); + if (!dataView) { + errors.searchConfiguration.push( + i18n.translate('xpack.stackAlerts.esQuery.ui.validation.error.requiredDataViewText', { + defaultMessage: 'Data view is required.', + }) + ); + return errors; + } + + if (!dataView?.isTimeBased()) { + errors.searchConfiguration.push( + i18n.translate( + 'xpack.stackAlerts.esQuery.ui.validation.error.requiredDataViewTimeFieldText', + { + defaultMessage: 'Data view should have a time field.', + } + ) + ); + return errors; + } + + return errors; +}; + +const validateEsQueryParams = (ruleParams: OnlyEsQueryRuleParams) => { + const errors: typeof ONLY_ES_QUERY_EXPRESSION_ERRORS = defaultsDeep( + {}, + ONLY_ES_QUERY_EXPRESSION_ERRORS + ); + if (!ruleParams.index || ruleParams.index.length === 0) { errors.index.push( i18n.translate('xpack.stackAlerts.esQuery.ui.validation.error.requiredIndexText', { @@ -156,12 +189,42 @@ export const validateExpression = (ruleParams: EsQueryRuleParams): ValidationRes ); } } + return errors; +}; + +export const validateExpression = async ( + ruleParams: EsQueryRuleParams, + services: TriggersAndActionsUiServices +): Promise => { + const validationResult = { errors: {} }; + + const commonErrors = validateCommonParams(ruleParams); + validationResult.errors = commonErrors; + + /** + * Skip esQuery and index params check if it is search source rule, + * since it should contain searchConfiguration instead of esQuery and index. + * + * It's important to report searchSource rule related errors only into errors.searchConfiguration prop. + * For example errors.index is a mistake to report searchSource rule related errors. It will lead to issues. + */ + const isSearchSource = isSearchSourceRule(ruleParams); + if (isSearchSource) { + const searchSourceParamsErrors = await validateSearchSourceParams(ruleParams, services); + validationResult.errors = { ...validationResult.errors, ...searchSourceParamsErrors }; + return validationResult; + } + const esQueryErrors = validateEsQueryParams(ruleParams as OnlyEsQueryRuleParams); + validationResult.errors = { ...validationResult.errors, ...esQueryErrors }; return validationResult; }; -export const hasExpressionValidationErrors = (ruleParams: EsQueryRuleParams) => { - const { errors: validationErrors } = validateExpression(ruleParams); +export const hasExpressionValidationErrors = async ( + ruleParams: EsQueryRuleParams, + services: TriggersAndActionsUiServices +) => { + const { errors: validationErrors } = await validateExpression(ruleParams, services); return Object.keys(validationErrors).some( (key) => validationErrors[key] && validationErrors[key].length ); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_add.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_add.tsx index b12a3c07c7875..010e434c32a9a 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_add.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_add.tsx @@ -91,11 +91,12 @@ const RuleAdd = ({ dispatch({ command: { type: 'setProperty' }, payload: { key, value } }); }; + const services = useKibana().services; const { http, notifications: { toasts }, application: { capabilities }, - } = useKibana().services; + } = services; const canShowActions = hasShowActionsCapability(capabilities); @@ -196,11 +197,19 @@ const RuleAdd = ({ const ruleType = rule.ruleTypeId ? ruleTypeRegistry.get(rule.ruleTypeId) : null; - const { ruleBaseErrors, ruleErrors, ruleParamsErrors } = getRuleErrors( - rule as Rule, - ruleType, - config - ); + const [{ ruleBaseErrors, ruleErrors, ruleParamsErrors }, setErrors] = useState<{ + ruleBaseErrors: IErrorObject; + ruleErrors: IErrorObject; + ruleParamsErrors: IErrorObject; + }>({ + ruleBaseErrors: {}, + ruleErrors: {}, + ruleParamsErrors: {}, + }); + + useEffect(() => { + getRuleErrors(rule as Rule, ruleType, config, services).then(setErrors); + }, [config, rule, ruleType, services]); // Confirm before saving if user is able to add actions but hasn't added any to this rule const shouldConfirmSave = canShowActions && rule.actions?.length === 0; diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_edit.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_edit.tsx index a03ba8dc92620..74121689a3f47 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_edit.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_edit.tsx @@ -74,10 +74,12 @@ export const RuleEdit = ({ const [metadata, setMetadata] = useState(initialMetadata); const onChangeMetaData = useCallback((newMetadata) => setMetadata(newMetadata), []); + const services = useKibana().services; const { http, notifications: { toasts }, - } = useKibana().services; + } = services; + const setRule = (value: Rule) => { dispatch({ command: { type: 'setRule' }, payload: { key: 'rule', value } }); }; @@ -112,11 +114,19 @@ export const RuleEdit = ({ } }, [props.ruleType, ruleType.id, serverRuleType, http]); - const { ruleBaseErrors, ruleErrors, ruleParamsErrors } = getRuleErrors( - rule as Rule, - ruleType, - config - ); + const [{ ruleBaseErrors, ruleErrors, ruleParamsErrors }, setErrors] = useState<{ + ruleBaseErrors: IErrorObject; + ruleErrors: IErrorObject; + ruleParamsErrors: IErrorObject; + }>({ + ruleBaseErrors: {}, + ruleErrors: {}, + ruleParamsErrors: {}, + }); + + useEffect(() => { + getRuleErrors(rule as Rule, ruleType, config, services).then(setErrors); + }, [config, rule, ruleType, services]); const checkForChangesAndCloseFlyout = () => { if (hasRuleChanged(rule, initialRule, true)) { diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_errors.ts b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_errors.ts index 73769d8d9af10..24ca2d4e8c3f1 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_errors.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_errors.ts @@ -17,6 +17,7 @@ import { TriggersActionsUiConfig, } from '../../../types'; import { InitialRule } from './rule_reducer'; +import { TriggersAndActionsUiServices } from '../../connectors_app'; export function validateBaseProperties( ruleObject: InitialRule, @@ -79,13 +80,14 @@ export function validateBaseProperties( return validationResult; } -export function getRuleErrors( +export async function getRuleErrors( rule: Rule, ruleTypeModel: RuleTypeModel | null, - config: TriggersActionsUiConfig + config: TriggersActionsUiConfig, + services: TriggersAndActionsUiServices ) { const ruleParamsErrors: IErrorObject = ruleTypeModel - ? ruleTypeModel.validate(rule.params).errors + ? (await ruleTypeModel.validate(rule.params, services)).errors : []; const ruleBaseErrors = validateBaseProperties(rule, config).errors as IErrorObject; const ruleErrors = { diff --git a/x-pack/plugins/triggers_actions_ui/public/types.ts b/x-pack/plugins/triggers_actions_ui/public/types.ts index 415672324e648..bde1d26ac2005 100644 --- a/x-pack/plugins/triggers_actions_ui/public/types.ts +++ b/x-pack/plugins/triggers_actions_ui/public/types.ts @@ -73,6 +73,7 @@ import type { } from './application/sections/field_browser/types'; import { RulesListVisibleColumns } from './application/sections/rules_list/components/rules_list_column_selector'; import { TimelineItem } from './application/sections/alerts_table/bulk_actions/components/toolbar'; +import { TriggersAndActionsUiServices } from './application/app'; // In Triggers and Actions we treat all `Alert`s as `SanitizedRule` // so the `Params` is a black-box of Record @@ -345,7 +346,10 @@ export interface RuleTypeModel { description: string; iconClass: string; documentationUrl: string | ((docLinks: DocLinksStart) => string) | null; - validate: (ruleParams: Params) => ValidationResult; + validate: ( + ruleParams: Params, + services: TriggersAndActionsUiServices + ) => Promise | ValidationResult; ruleParamsExpression: | React.FunctionComponent | React.LazyExoticComponent>>; diff --git a/x-pack/test/functional_with_es_ssl/apps/discover/search_source_alert.ts b/x-pack/test/functional_with_es_ssl/apps/discover/search_source_alert.ts index 22518669681ed..7efc5a0ca1ab8 100644 --- a/x-pack/test/functional_with_es_ssl/apps/discover/search_source_alert.ts +++ b/x-pack/test/functional_with_es_ssl/apps/discover/search_source_alert.ts @@ -169,7 +169,6 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { "alert_id": "{{alertId}}", "context_message": "{{context.message}}" }`); - await testSubjects.click('saveRuleButton'); }; const openDiscoverAlertFlyout = async () => { @@ -280,15 +279,40 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { outputDataViewId = outputDataViewResponse.body.data_view.id; }); - it('should navigate to alert results via view in app link', async () => { + it('should show time field validation error', async () => { await PageObjects.common.navigateToApp('discover'); await PageObjects.discover.waitUntilSearchingHasFinished(); await PageObjects.discover.selectIndexPattern(SOURCE_DATA_INDEX); await PageObjects.timePicker.setCommonlyUsedTime('Last_15 minutes'); - // create an alert await openDiscoverAlertFlyout(); await defineSearchSourceAlert(RULE_NAME); + await testSubjects.click('selectDataViewExpression'); + + await testSubjects.click('indexPattern-switcher--input'); + const input = await find.activeElement(); + // search-source-alert-output index does not have time field + await input.type('search-source-alert-o*'); + await testSubjects.click('explore-matching-indices-button'); + + await testSubjects.click('saveRuleButton'); + + const errorElem = await testSubjects.find('esQueryAlertExpressionError'); + const errorText = await errorElem.getVisibleText(); + expect(errorText).to.eql('Data view should have a time field.'); + }); + + it('should navigate to alert results via view in app link', async () => { + await testSubjects.click('selectDataViewExpression'); + await testSubjects.click('indexPattern-switcher--input'); + const dataViewsElem = await testSubjects.find('euiSelectableList'); + const sourceDataViewOption = await dataViewsElem.findByCssSelector( + `[title="${SOURCE_DATA_INDEX}"]` + ); + await sourceDataViewOption.click(); + + await testSubjects.click('saveRuleButton'); + await PageObjects.header.waitUntilLoadingHasFinished(); await openAlertRuleInManagement(RULE_NAME); @@ -403,6 +427,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { // create an alert await openDiscoverAlertFlyout(); await defineSearchSourceAlert('test-adhoc-alert'); + await testSubjects.click('saveRuleButton'); await PageObjects.header.waitUntilLoadingHasFinished(); sourceAdHocDataViewId = await PageObjects.discover.getCurrentDataViewId(); From 6f926fd3091344565760dd1988f24bf01e791104 Mon Sep 17 00:00:00 2001 From: Dzmitry Tamashevich Date: Thu, 1 Dec 2022 17:00:51 +0300 Subject: [PATCH 2/5] [Discover] keep validation as synchronous operation --- .../components/data_view_select_popover.tsx | 15 ++++-- .../expression/es_query_expression.tsx | 21 ++++---- .../es_query/expression/expression.tsx | 8 ++- .../expression/search_source_expression.tsx | 25 ++++++++-- .../search_source_expression_form.tsx | 10 ++-- .../public/rule_types/es_query/types.ts | 4 +- .../public/rule_types/es_query/validation.ts | 49 +++++-------------- .../sections/rule_form/rule_add.tsx | 19 +++---- .../sections/rule_form/rule_edit.tsx | 19 +++---- .../sections/rule_form/rule_errors.ts | 12 +++-- .../triggers_actions_ui/public/types.ts | 8 ++- 11 files changed, 90 insertions(+), 100 deletions(-) diff --git a/x-pack/plugins/stack_alerts/public/rule_types/components/data_view_select_popover.tsx b/x-pack/plugins/stack_alerts/public/rule_types/components/data_view_select_popover.tsx index 7180db27afdc6..9dbe204451aac 100644 --- a/x-pack/plugins/stack_alerts/public/rule_types/components/data_view_select_popover.tsx +++ b/x-pack/plugins/stack_alerts/public/rule_types/components/data_view_select_popover.tsx @@ -41,7 +41,7 @@ const toDataViewListItem = (dataView: DataView): DataViewListItem => { }; export const DataViewSelectPopover: React.FunctionComponent = ({ - metadata = { adHocDataViewList: [], isManagementPage: true }, + metadata, dataView, onSelectDataView, onChangeMetaData, @@ -50,11 +50,16 @@ export const DataViewSelectPopover: React.FunctionComponent([]); const [dataViewPopoverOpen, setDataViewPopoverOpen] = useState(false); + const adhocDataViewList = useMemo( + () => metadata?.adHocDataViewList || [], + [metadata?.adHocDataViewList] + ); + const closeDataViewEditor = useRef<() => void | undefined>(); const allDataViewItems = useMemo( - () => [...dataViewItems, ...metadata.adHocDataViewList.map(toDataViewListItem)], - [dataViewItems, metadata.adHocDataViewList] + () => [...dataViewItems, ...adhocDataViewList.map(toDataViewListItem)], + [dataViewItems, adhocDataViewList] ); const closeDataViewPopover = useCallback(() => setDataViewPopoverOpen(false), []); @@ -79,10 +84,10 @@ export const DataViewSelectPopover: React.FunctionComponent { onChangeMetaData({ ...metadata, - adHocDataViewList: [...metadata.adHocDataViewList, adHocDataView], + adHocDataViewList: [...adhocDataViewList, adHocDataView], }); }, - [metadata, onChangeMetaData] + [adhocDataViewList, metadata, onChangeMetaData] ); const createDataView = useMemo( diff --git a/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/es_query_expression.tsx b/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/es_query_expression.tsx index c07153c77146f..7d2ffda9b920f 100644 --- a/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/es_query_expression.tsx +++ b/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/es_query_expression.tsx @@ -16,7 +16,7 @@ import { XJson } from '@kbn/es-ui-shared-plugin/public'; import { CodeEditor } from '@kbn/kibana-react-plugin/public'; import { getFields, RuleTypeParamsExpressionProps } from '@kbn/triggers-actions-ui-plugin/public'; import { parseDuration } from '@kbn/alerting-plugin/common'; -import { hasExpressionValidationErrors } from '../validation'; +import { validateExpression } from '../validation'; import { buildSortedEventsQuery } from '../../../../common/build_sorted_events_query'; import { EsQueryRuleParams, EsQueryRuleMetaData, SearchType } from '../types'; import { IndexSelectPopover } from '../../components/index_select_popover'; @@ -27,6 +27,13 @@ import { useTriggerUiActionServices } from '../util'; const { useXJsonMode } = XJson; +const hasExpressionValidationErrors = (ruleParams: EsQueryRuleParams) => { + const { errors: validationErrors } = validateExpression(ruleParams); + return Object.keys(validationErrors).some( + (key) => validationErrors[key] && validationErrors[key].length + ); +}; + export const EsQueryExpression: React.FC< RuleTypeParamsExpressionProps, EsQueryRuleMetaData> > = ({ ruleParams, setRuleParams, setRuleProperty, errors, data }) => { @@ -103,15 +110,9 @@ export const EsQueryExpression: React.FC< } }; - const [hasValidationErrors, setHasValidationErrors] = useState(false); - - useEffect(() => { - hasExpressionValidationErrors(currentRuleParams, services).then(setHasValidationErrors); - }, [currentRuleParams, services]); - const onTestQuery = useCallback(async () => { const window = `${timeWindowSize}${timeWindowUnit}`; - if (hasValidationErrors) { + if (hasExpressionValidationErrors(currentRuleParams)) { return { nrOfDocs: 0, timeWindow: window }; } const timeWindow = parseDuration(window); @@ -134,7 +135,7 @@ export const EsQueryExpression: React.FC< const hits = rawResponse.hits; return { nrOfDocs: totalHitsToNumber(hits.total), timeWindow: window }; - }, [data.search, esQuery, index, timeField, timeWindowSize, timeWindowUnit, hasValidationErrors]); + }, [timeWindowSize, timeWindowUnit, currentRuleParams, esQuery, data.search, index, timeField]); return ( @@ -249,7 +250,7 @@ export const EsQueryExpression: React.FC< setParam('size', updatedValue); }} errors={errors} - hasValidationErrors={hasValidationErrors} + hasValidationErrors={hasExpressionValidationErrors(currentRuleParams)} onTestFetch={onTestQuery} excludeHitsFromPreviousRun={excludeHitsFromPreviousRun} onChangeExcludeHitsFromPreviousRun={(exclude) => { diff --git a/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/expression.tsx b/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/expression.tsx index 2998ebb66083a..865f5ab2f1497 100644 --- a/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/expression.tsx +++ b/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/expression.tsx @@ -63,6 +63,8 @@ export const EsQueryRuleTypeExpression: React.FunctionComponent< return errors[errorKey]?.length >= 1 && ruleParams[errorKey] !== undefined; }); + const hasEsExpressionErrors = !!errorParam; + const expressionError = !!errorParam && ( <> + )} {ruleParams.searchType && !isSearchSource && ( diff --git a/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/search_source_expression.tsx b/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/search_source_expression.tsx index e254805fc137b..3270cbd49767d 100644 --- a/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/search_source_expression.tsx +++ b/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/search_source_expression.tsx @@ -11,15 +11,23 @@ import { EuiSpacer, EuiLoadingSpinner, EuiEmptyPrompt, EuiCallOut } from '@elast import { ISearchSource } from '@kbn/data-plugin/common'; import { RuleTypeParamsExpressionProps } from '@kbn/triggers-actions-ui-plugin/public'; import { SavedQuery } from '@kbn/data-plugin/public'; -import { EsQueryRuleMetaData, EsQueryRuleParams, SearchType } from '../types'; +import { + EsQueryRuleMetaData, + EsQueryRuleParams, + OnlySearchSourceRuleParams, + SearchType, +} from '../types'; import { SearchSourceExpressionForm } from './search_source_expression_form'; import { DEFAULT_VALUES } from '../constants'; import { useTriggerUiActionServices } from '../util'; +import { validateSearchSourceParams } from '../validation'; export type SearchSourceExpressionProps = RuleTypeParamsExpressionProps< EsQueryRuleParams, EsQueryRuleMetaData ->; +> & { + hasValidationErrors: boolean; +}; export const SearchSourceExpression = ({ ruleParams, @@ -27,6 +35,7 @@ export const SearchSourceExpression = ({ setRuleParams, setRuleProperty, metadata, + hasValidationErrors, onChangeMetaData, }: SearchSourceExpressionProps) => { const { @@ -47,9 +56,18 @@ export const SearchSourceExpression = ({ const setParam = useCallback( (paramField: string, paramValue: unknown) => { + if (paramField === 'searchConfiguration') { + onChangeMetaData({ + ...metadata, + moreParamsErrors: validateSearchSourceParams( + ruleParams as OnlySearchSourceRuleParams, + searchSource + ), + }); + } setRuleParams(paramField, paramValue); }, - [setRuleParams] + [metadata, onChangeMetaData, ruleParams, searchSource, setRuleParams] ); useEffect(() => { @@ -119,6 +137,7 @@ export const SearchSourceExpression = ({ setParam={setParam} metadata={metadata} onChangeMetaData={onChangeMetaData} + hasValidationErrors={hasValidationErrors} /> ); }; diff --git a/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/search_source_expression_form.tsx b/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/search_source_expression_form.tsx index 4b8ad477d468f..baa4d0b2c5b4b 100644 --- a/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/search_source_expression_form.tsx +++ b/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/search_source_expression_form.tsx @@ -26,7 +26,6 @@ import { DEFAULT_VALUES } from '../constants'; import { DataViewSelectPopover } from '../../components/data_view_select_popover'; import { RuleCommonExpressions } from '../rule_common_expressions'; import { totalHitsToNumber } from '../test_query_row'; -import { hasExpressionValidationErrors } from '../validation'; import { useTriggerUiActionServices } from '../util'; const HIDDEN_FILTER_PANEL_OPTIONS: SearchBarProps['hiddenFilterPanelOptions'] = [ @@ -71,6 +70,7 @@ interface SearchSourceExpressionFormProps { searchSource: ISearchSource; ruleParams: EsQueryRuleParams; errors: IErrorObject; + hasValidationErrors: boolean; metadata?: EsQueryRuleMetaData; initialSavedQuery?: SavedQuery; setParam: (paramField: string, paramValue: unknown) => void; @@ -112,6 +112,7 @@ export const SearchSourceExpressionForm = (props: SearchSourceExpressionFormProp ruleParams.excludeHitsFromPreviousRun ?? DEFAULT_VALUES.EXCLUDE_PREVIOUS_HITS, } ); + const { index: dataView, query, filter: filters } = ruleConfiguration; const dataViews = useMemo(() => (dataView ? [dataView] : []), [dataView]); @@ -217,11 +218,6 @@ export const SearchSourceExpressionForm = (props: SearchSourceExpressionFormProp return { nrOfDocs: totalHitsToNumber(rawResponse.hits.total), timeWindow }; }, [timeWindow, createTestSearchSource]); - const [hasValidationErrors, setHasValidationErrors] = useState(false); - useEffect(() => { - hasExpressionValidationErrors(ruleParams, services).then(setHasValidationErrors); - }, [ruleParams, services]); - return ( @@ -295,7 +291,7 @@ export const SearchSourceExpressionForm = (props: SearchSourceExpressionFormProp onChangeWindowUnit={onChangeWindowUnit} onChangeSizeValue={onChangeSizeValue} errors={errors} - hasValidationErrors={hasValidationErrors} + hasValidationErrors={props.hasValidationErrors} onTestFetch={onTestFetch} onCopyQuery={onCopyQuery} excludeHitsFromPreviousRun={ruleConfiguration.excludeHitsFromPreviousRun} diff --git a/x-pack/plugins/stack_alerts/public/rule_types/es_query/types.ts b/x-pack/plugins/stack_alerts/public/rule_types/es_query/types.ts index 49bacc84beb72..7415dae3ac4af 100644 --- a/x-pack/plugins/stack_alerts/public/rule_types/es_query/types.ts +++ b/x-pack/plugins/stack_alerts/public/rule_types/es_query/types.ts @@ -9,6 +9,7 @@ import { RuleTypeParams } from '@kbn/alerting-plugin/common'; import { SerializedSearchSourceFields } from '@kbn/data-plugin/common'; import { EuiComboBoxOptionOption } from '@elastic/eui'; import type { DataView } from '@kbn/data-views-plugin/public'; +import { ValidationResult } from '@kbn/triggers-actions-ui-plugin/public'; export interface Comparator { text: string; @@ -31,8 +32,9 @@ export interface CommonRuleParams extends RuleTypeParams { } export interface EsQueryRuleMetaData { - adHocDataViewList: DataView[]; + adHocDataViewList?: DataView[]; isManagementPage?: boolean; + moreParamsErrors?: ValidationResult['errors']; } export type EsQueryRuleParams = T extends SearchType.searchSource diff --git a/x-pack/plugins/stack_alerts/public/rule_types/es_query/validation.ts b/x-pack/plugins/stack_alerts/public/rule_types/es_query/validation.ts index d86a1595848c5..7d3fcb38f1902 100644 --- a/x-pack/plugins/stack_alerts/public/rule_types/es_query/validation.ts +++ b/x-pack/plugins/stack_alerts/public/rule_types/es_query/validation.ts @@ -7,11 +7,8 @@ import { defaultsDeep, isNil } from 'lodash'; import { i18n } from '@kbn/i18n'; -import { - ValidationResult, - builtInComparators, - TriggersAndActionsUiServices, -} from '@kbn/triggers-actions-ui-plugin/public'; +import { ValidationResult, builtInComparators } from '@kbn/triggers-actions-ui-plugin/public'; +import { ISearchSource } from '@kbn/data-plugin/public'; import { EsQueryRuleParams, OnlySearchSourceRuleParams, OnlyEsQueryRuleParams } from './types'; import { isSearchSourceRule } from './util'; import { @@ -88,16 +85,16 @@ const validateCommonParams = (ruleParams: EsQueryRuleParams) => { return errors; }; -const validateSearchSourceParams = async ( +export const validateSearchSourceParams = ( ruleParams: OnlySearchSourceRuleParams, - services: TriggersAndActionsUiServices + searchSource?: ISearchSource ) => { const errors: typeof SEARCH_SOURCE_ONLY_EXPRESSION_ERRORS = defaultsDeep( {}, SEARCH_SOURCE_ONLY_EXPRESSION_ERRORS ); - if (!ruleParams.searchConfiguration) { + if (!ruleParams.searchConfiguration || !searchSource) { errors.searchConfiguration.push( i18n.translate('xpack.stackAlerts.esQuery.ui.validation.error.requiredSearchConfiguration', { defaultMessage: 'Search source configuration is required.', @@ -106,19 +103,7 @@ const validateSearchSourceParams = async ( return errors; } - let searchSource; - try { - searchSource = await services.data.search.searchSource.create(ruleParams.searchConfiguration); - } catch (e) { - errors.searchConfiguration.push( - i18n.translate('xpack.stackAlerts.esQuery.ui.validation.error.searchConfigError', { - defaultMessage: 'Search source configuration is invalid.', - }) - ); - return errors; - } - - const dataView = searchSource?.getField('index'); + const dataView = searchSource.getField('index'); if (!dataView) { errors.searchConfiguration.push( i18n.translate('xpack.stackAlerts.esQuery.ui.validation.error.requiredDataViewText', { @@ -192,10 +177,7 @@ const validateEsQueryParams = (ruleParams: OnlyEsQueryRuleParams) => { return errors; }; -export const validateExpression = async ( - ruleParams: EsQueryRuleParams, - services: TriggersAndActionsUiServices -): Promise => { +export const validateExpression = (ruleParams: EsQueryRuleParams): ValidationResult => { const validationResult = { errors: {} }; const commonErrors = validateCommonParams(ruleParams); @@ -210,8 +192,11 @@ export const validateExpression = async ( */ const isSearchSource = isSearchSourceRule(ruleParams); if (isSearchSource) { - const searchSourceParamsErrors = await validateSearchSourceParams(ruleParams, services); - validationResult.errors = { ...validationResult.errors, ...searchSourceParamsErrors }; + validationResult.errors = { + ...validationResult.errors, + ...defaultsDeep({}, SEARCH_SOURCE_ONLY_EXPRESSION_ERRORS), + }; + // skip searchConfiguration check here, since it will be checked in by updating metadata return validationResult; } @@ -219,13 +204,3 @@ export const validateExpression = async ( validationResult.errors = { ...validationResult.errors, ...esQueryErrors }; return validationResult; }; - -export const hasExpressionValidationErrors = async ( - ruleParams: EsQueryRuleParams, - services: TriggersAndActionsUiServices -) => { - const { errors: validationErrors } = await validateExpression(ruleParams, services); - return Object.keys(validationErrors).some( - (key) => validationErrors[key] && validationErrors[key].length - ); -}; diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_add.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_add.tsx index 010e434c32a9a..a210b98fa7559 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_add.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_add.tsx @@ -197,19 +197,12 @@ const RuleAdd = ({ const ruleType = rule.ruleTypeId ? ruleTypeRegistry.get(rule.ruleTypeId) : null; - const [{ ruleBaseErrors, ruleErrors, ruleParamsErrors }, setErrors] = useState<{ - ruleBaseErrors: IErrorObject; - ruleErrors: IErrorObject; - ruleParamsErrors: IErrorObject; - }>({ - ruleBaseErrors: {}, - ruleErrors: {}, - ruleParamsErrors: {}, - }); - - useEffect(() => { - getRuleErrors(rule as Rule, ruleType, config, services).then(setErrors); - }, [config, rule, ruleType, services]); + const { ruleBaseErrors, ruleErrors, ruleParamsErrors } = getRuleErrors( + rule as Rule, + ruleType, + config, + metadata + ); // Confirm before saving if user is able to add actions but hasn't added any to this rule const shouldConfirmSave = canShowActions && rule.actions?.length === 0; diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_edit.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_edit.tsx index 74121689a3f47..479d3521ffdc2 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_edit.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_edit.tsx @@ -114,19 +114,12 @@ export const RuleEdit = ({ } }, [props.ruleType, ruleType.id, serverRuleType, http]); - const [{ ruleBaseErrors, ruleErrors, ruleParamsErrors }, setErrors] = useState<{ - ruleBaseErrors: IErrorObject; - ruleErrors: IErrorObject; - ruleParamsErrors: IErrorObject; - }>({ - ruleBaseErrors: {}, - ruleErrors: {}, - ruleParamsErrors: {}, - }); - - useEffect(() => { - getRuleErrors(rule as Rule, ruleType, config, services).then(setErrors); - }, [config, rule, ruleType, services]); + const { ruleBaseErrors, ruleErrors, ruleParamsErrors } = getRuleErrors( + rule as Rule, + ruleType, + config, + metadata + ); const checkForChangesAndCloseFlyout = () => { if (hasRuleChanged(rule, initialRule, true)) { diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_errors.ts b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_errors.ts index 24ca2d4e8c3f1..01ae4822c6308 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_errors.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_errors.ts @@ -17,7 +17,6 @@ import { TriggersActionsUiConfig, } from '../../../types'; import { InitialRule } from './rule_reducer'; -import { TriggersAndActionsUiServices } from '../../connectors_app'; export function validateBaseProperties( ruleObject: InitialRule, @@ -80,15 +79,18 @@ export function validateBaseProperties( return validationResult; } -export async function getRuleErrors( +export function getRuleErrors( rule: Rule, ruleTypeModel: RuleTypeModel | null, config: TriggersActionsUiConfig, - services: TriggersAndActionsUiServices + metadata?: Record ) { - const ruleParamsErrors: IErrorObject = ruleTypeModel - ? (await ruleTypeModel.validate(rule.params, services)).errors + let ruleParamsErrors: IErrorObject = ruleTypeModel + ? ruleTypeModel.validate(rule.params).errors : []; + if (metadata && metadata.moreParamsErrors) { + ruleParamsErrors = { ...ruleParamsErrors, ...(metadata.moreParamsErrors as IErrorObject) }; + } const ruleBaseErrors = validateBaseProperties(rule, config).errors as IErrorObject; const ruleErrors = { ...ruleParamsErrors, diff --git a/x-pack/plugins/triggers_actions_ui/public/types.ts b/x-pack/plugins/triggers_actions_ui/public/types.ts index 9d0fcdb603bc7..0d3e357191e4c 100644 --- a/x-pack/plugins/triggers_actions_ui/public/types.ts +++ b/x-pack/plugins/triggers_actions_ui/public/types.ts @@ -73,7 +73,6 @@ import type { } from './application/sections/field_browser/types'; import { RulesListVisibleColumns } from './application/sections/rules_list/components/rules_list_column_selector'; import { TimelineItem } from './application/sections/alerts_table/bulk_actions/components/toolbar'; -import { TriggersAndActionsUiServices } from './application/app'; // In Triggers and Actions we treat all `Alert`s as `SanitizedRule` // so the `Params` is a black-box of Record @@ -358,10 +357,7 @@ export interface RuleTypeModel { description: string; iconClass: string; documentationUrl: string | ((docLinks: DocLinksStart) => string) | null; - validate: ( - ruleParams: Params, - services: TriggersAndActionsUiServices - ) => Promise | ValidationResult; + validate: (ruleParams: Params) => ValidationResult; ruleParamsExpression: | React.FunctionComponent | React.LazyExoticComponent>>; @@ -402,6 +398,8 @@ export interface RuleAddProps> { /** @deprecated use `onSave` as a callback after an alert is saved*/ reloadRules?: () => Promise; onSave?: (metadata?: MetaData) => Promise; + // be aware, `moreParamsErrors` prop of metadata is used to pass + // errors which can't be validated syncronously within `validate` function metadata?: MetaData; ruleTypeIndex?: RuleTypeIndex; filteredRuleTypes?: string[]; From 865d79c5b3395ab796683b35b01194e67248a611 Mon Sep 17 00:00:00 2001 From: Dzmitry Tamashevich Date: Fri, 2 Dec 2022 17:12:33 +0300 Subject: [PATCH 3/5] [Discover] add optional timeField for search source alert --- .../components/data_view_select_popover.tsx | 15 ++--- .../public/rule_types/es_query/constants.ts | 3 +- .../expression/es_query_expression.tsx | 9 +-- .../es_query/expression/expression.tsx | 10 +-- .../expression/search_source_expression.tsx | 61 +++++++------------ .../search_source_expression_form.tsx | 8 ++- .../public/rule_types/es_query/types.ts | 5 +- .../public/rule_types/es_query/validation.ts | 31 +++++----- .../lib/fetch_search_source_query.test.ts | 1 + .../es_query/lib/fetch_search_source_query.ts | 7 +-- .../rule_types/es_query/rule_type_params.ts | 12 ++-- .../server/rule_types/es_query/types.ts | 7 +-- .../sections/rule_form/rule_add.tsx | 6 +- .../sections/rule_form/rule_edit.tsx | 6 +- .../sections/rule_form/rule_errors.ts | 8 +-- .../triggers_actions_ui/public/types.ts | 2 - 16 files changed, 76 insertions(+), 115 deletions(-) diff --git a/x-pack/plugins/stack_alerts/public/rule_types/components/data_view_select_popover.tsx b/x-pack/plugins/stack_alerts/public/rule_types/components/data_view_select_popover.tsx index 9dbe204451aac..7180db27afdc6 100644 --- a/x-pack/plugins/stack_alerts/public/rule_types/components/data_view_select_popover.tsx +++ b/x-pack/plugins/stack_alerts/public/rule_types/components/data_view_select_popover.tsx @@ -41,7 +41,7 @@ const toDataViewListItem = (dataView: DataView): DataViewListItem => { }; export const DataViewSelectPopover: React.FunctionComponent = ({ - metadata, + metadata = { adHocDataViewList: [], isManagementPage: true }, dataView, onSelectDataView, onChangeMetaData, @@ -50,16 +50,11 @@ export const DataViewSelectPopover: React.FunctionComponent([]); const [dataViewPopoverOpen, setDataViewPopoverOpen] = useState(false); - const adhocDataViewList = useMemo( - () => metadata?.adHocDataViewList || [], - [metadata?.adHocDataViewList] - ); - const closeDataViewEditor = useRef<() => void | undefined>(); const allDataViewItems = useMemo( - () => [...dataViewItems, ...adhocDataViewList.map(toDataViewListItem)], - [dataViewItems, adhocDataViewList] + () => [...dataViewItems, ...metadata.adHocDataViewList.map(toDataViewListItem)], + [dataViewItems, metadata.adHocDataViewList] ); const closeDataViewPopover = useCallback(() => setDataViewPopoverOpen(false), []); @@ -84,10 +79,10 @@ export const DataViewSelectPopover: React.FunctionComponent { onChangeMetaData({ ...metadata, - adHocDataViewList: [...adhocDataViewList, adHocDataView], + adHocDataViewList: [...metadata.adHocDataViewList, adHocDataView], }); }, - [adhocDataViewList, metadata, onChangeMetaData] + [metadata, onChangeMetaData] ); const createDataView = useMemo( diff --git a/x-pack/plugins/stack_alerts/public/rule_types/es_query/constants.ts b/x-pack/plugins/stack_alerts/public/rule_types/es_query/constants.ts index dae63a7e1ad4a..7013f2099b8b4 100644 --- a/x-pack/plugins/stack_alerts/public/rule_types/es_query/constants.ts +++ b/x-pack/plugins/stack_alerts/public/rule_types/es_query/constants.ts @@ -31,12 +31,13 @@ export const COMMON_EXPRESSION_ERRORS = { export const SEARCH_SOURCE_ONLY_EXPRESSION_ERRORS = { searchConfiguration: new Array(), + timeField: new Array(), }; export const ONLY_ES_QUERY_EXPRESSION_ERRORS = { index: new Array(), - timeField: new Array(), esQuery: new Array(), + timeField: new Array(), }; const ALL_EXPRESSION_ERROR_ENTRIES = { diff --git a/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/es_query_expression.tsx b/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/es_query_expression.tsx index 7d2ffda9b920f..7c0c5a6d2b5bb 100644 --- a/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/es_query_expression.tsx +++ b/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/es_query_expression.tsx @@ -16,7 +16,7 @@ import { XJson } from '@kbn/es-ui-shared-plugin/public'; import { CodeEditor } from '@kbn/kibana-react-plugin/public'; import { getFields, RuleTypeParamsExpressionProps } from '@kbn/triggers-actions-ui-plugin/public'; import { parseDuration } from '@kbn/alerting-plugin/common'; -import { validateExpression } from '../validation'; +import { hasExpressionValidationErrors } from '../validation'; import { buildSortedEventsQuery } from '../../../../common/build_sorted_events_query'; import { EsQueryRuleParams, EsQueryRuleMetaData, SearchType } from '../types'; import { IndexSelectPopover } from '../../components/index_select_popover'; @@ -27,13 +27,6 @@ import { useTriggerUiActionServices } from '../util'; const { useXJsonMode } = XJson; -const hasExpressionValidationErrors = (ruleParams: EsQueryRuleParams) => { - const { errors: validationErrors } = validateExpression(ruleParams); - return Object.keys(validationErrors).some( - (key) => validationErrors[key] && validationErrors[key].length - ); -}; - export const EsQueryExpression: React.FC< RuleTypeParamsExpressionProps, EsQueryRuleMetaData> > = ({ ruleParams, setRuleParams, setRuleProperty, errors, data }) => { diff --git a/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/expression.tsx b/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/expression.tsx index 865f5ab2f1497..30045fee81a29 100644 --- a/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/expression.tsx +++ b/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/expression.tsx @@ -63,8 +63,6 @@ export const EsQueryRuleTypeExpression: React.FunctionComponent< return errors[errorKey]?.length >= 1 && ruleParams[errorKey] !== undefined; }); - const hasEsExpressionErrors = !!errorParam; - const expressionError = !!errorParam && ( <> + )} {ruleParams.searchType && !isSearchSource && ( diff --git a/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/search_source_expression.tsx b/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/search_source_expression.tsx index 3270cbd49767d..d92f4639aa9a8 100644 --- a/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/search_source_expression.tsx +++ b/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/search_source_expression.tsx @@ -11,23 +11,15 @@ import { EuiSpacer, EuiLoadingSpinner, EuiEmptyPrompt, EuiCallOut } from '@elast import { ISearchSource } from '@kbn/data-plugin/common'; import { RuleTypeParamsExpressionProps } from '@kbn/triggers-actions-ui-plugin/public'; import { SavedQuery } from '@kbn/data-plugin/public'; -import { - EsQueryRuleMetaData, - EsQueryRuleParams, - OnlySearchSourceRuleParams, - SearchType, -} from '../types'; +import { EsQueryRuleMetaData, EsQueryRuleParams, SearchType } from '../types'; import { SearchSourceExpressionForm } from './search_source_expression_form'; import { DEFAULT_VALUES } from '../constants'; import { useTriggerUiActionServices } from '../util'; -import { validateSearchSourceParams } from '../validation'; export type SearchSourceExpressionProps = RuleTypeParamsExpressionProps< EsQueryRuleParams, EsQueryRuleMetaData -> & { - hasValidationErrors: boolean; -}; +>; export const SearchSourceExpression = ({ ruleParams, @@ -35,7 +27,6 @@ export const SearchSourceExpression = ({ setRuleParams, setRuleProperty, metadata, - hasValidationErrors, onChangeMetaData, }: SearchSourceExpressionProps) => { const { @@ -56,18 +47,9 @@ export const SearchSourceExpression = ({ const setParam = useCallback( (paramField: string, paramValue: unknown) => { - if (paramField === 'searchConfiguration') { - onChangeMetaData({ - ...metadata, - moreParamsErrors: validateSearchSourceParams( - ruleParams as OnlySearchSourceRuleParams, - searchSource - ), - }); - } setRuleParams(paramField, paramValue); }, - [metadata, onChangeMetaData, ruleParams, searchSource, setRuleParams] + [setRuleParams] ); useEffect(() => { @@ -85,22 +67,26 @@ export const SearchSourceExpression = ({ initialSearchConfiguration = newSearchSource.getSerializedFields(); } - setRuleProperty('params', { - searchConfiguration: initialSearchConfiguration, - searchType: SearchType.searchSource, - timeWindowSize: timeWindowSize ?? DEFAULT_VALUES.TIME_WINDOW_SIZE, - timeWindowUnit: timeWindowUnit ?? DEFAULT_VALUES.TIME_WINDOW_UNIT, - threshold: threshold ?? DEFAULT_VALUES.THRESHOLD, - thresholdComparator: thresholdComparator ?? DEFAULT_VALUES.THRESHOLD_COMPARATOR, - size: size ?? DEFAULT_VALUES.SIZE, - excludeHitsFromPreviousRun: - excludeHitsFromPreviousRun ?? DEFAULT_VALUES.EXCLUDE_PREVIOUS_HITS, - }); - - data.search.searchSource - .create(initialSearchConfiguration) - .then(setSearchSource) - .catch(setParamsError); + try { + const createdSearchSource = await data.search.searchSource.create( + initialSearchConfiguration + ); + setRuleProperty('params', { + searchConfiguration: initialSearchConfiguration, + timeField: createdSearchSource.getField('index')?.timeFieldName, + searchType: SearchType.searchSource, + timeWindowSize: timeWindowSize ?? DEFAULT_VALUES.TIME_WINDOW_SIZE, + timeWindowUnit: timeWindowUnit ?? DEFAULT_VALUES.TIME_WINDOW_UNIT, + threshold: threshold ?? DEFAULT_VALUES.THRESHOLD, + thresholdComparator: thresholdComparator ?? DEFAULT_VALUES.THRESHOLD_COMPARATOR, + size: size ?? DEFAULT_VALUES.SIZE, + excludeHitsFromPreviousRun: + excludeHitsFromPreviousRun ?? DEFAULT_VALUES.EXCLUDE_PREVIOUS_HITS, + }); + setSearchSource(createdSearchSource); + } catch (error) { + setParamsError(error); + } }; initSearchSource(); @@ -137,7 +123,6 @@ export const SearchSourceExpression = ({ setParam={setParam} metadata={metadata} onChangeMetaData={onChangeMetaData} - hasValidationErrors={hasValidationErrors} /> ); }; diff --git a/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/search_source_expression_form.tsx b/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/search_source_expression_form.tsx index baa4d0b2c5b4b..97edd294df28a 100644 --- a/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/search_source_expression_form.tsx +++ b/x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/search_source_expression_form.tsx @@ -27,6 +27,7 @@ import { DataViewSelectPopover } from '../../components/data_view_select_popover import { RuleCommonExpressions } from '../rule_common_expressions'; import { totalHitsToNumber } from '../test_query_row'; import { useTriggerUiActionServices } from '../util'; +import { hasExpressionValidationErrors } from '../validation'; const HIDDEN_FILTER_PANEL_OPTIONS: SearchBarProps['hiddenFilterPanelOptions'] = [ 'pinFilter', @@ -70,7 +71,6 @@ interface SearchSourceExpressionFormProps { searchSource: ISearchSource; ruleParams: EsQueryRuleParams; errors: IErrorObject; - hasValidationErrors: boolean; metadata?: EsQueryRuleMetaData; initialSavedQuery?: SavedQuery; setParam: (paramField: string, paramValue: unknown) => void; @@ -94,6 +94,10 @@ export const SearchSourceExpressionForm = (props: SearchSourceExpressionFormProp if (isSearchSourceParam(action)) { searchSource.setParent(undefined).setField(action.type, action.payload); setParam('searchConfiguration', searchSource.getSerializedFields()); + + if (action.type === 'index') { + setParam('timeField', searchSource.getField('index')?.timeFieldName); + } } else { setParam(action.type, action.payload); } @@ -291,7 +295,7 @@ export const SearchSourceExpressionForm = (props: SearchSourceExpressionFormProp onChangeWindowUnit={onChangeWindowUnit} onChangeSizeValue={onChangeSizeValue} errors={errors} - hasValidationErrors={props.hasValidationErrors} + hasValidationErrors={hasExpressionValidationErrors(props.ruleParams)} onTestFetch={onTestFetch} onCopyQuery={onCopyQuery} excludeHitsFromPreviousRun={ruleConfiguration.excludeHitsFromPreviousRun} diff --git a/x-pack/plugins/stack_alerts/public/rule_types/es_query/types.ts b/x-pack/plugins/stack_alerts/public/rule_types/es_query/types.ts index 7415dae3ac4af..ab096a8d0fe68 100644 --- a/x-pack/plugins/stack_alerts/public/rule_types/es_query/types.ts +++ b/x-pack/plugins/stack_alerts/public/rule_types/es_query/types.ts @@ -9,7 +9,6 @@ import { RuleTypeParams } from '@kbn/alerting-plugin/common'; import { SerializedSearchSourceFields } from '@kbn/data-plugin/common'; import { EuiComboBoxOptionOption } from '@elastic/eui'; import type { DataView } from '@kbn/data-views-plugin/public'; -import { ValidationResult } from '@kbn/triggers-actions-ui-plugin/public'; export interface Comparator { text: string; @@ -32,9 +31,8 @@ export interface CommonRuleParams extends RuleTypeParams { } export interface EsQueryRuleMetaData { - adHocDataViewList?: DataView[]; + adHocDataViewList: DataView[]; isManagementPage?: boolean; - moreParamsErrors?: ValidationResult['errors']; } export type EsQueryRuleParams = T extends SearchType.searchSource @@ -48,6 +46,7 @@ export interface OnlyEsQueryRuleParams { } export interface OnlySearchSourceRuleParams { + timeField?: string; searchType?: 'searchSource'; searchConfiguration?: SerializedSearchSourceFields; savedQueryId?: string; diff --git a/x-pack/plugins/stack_alerts/public/rule_types/es_query/validation.ts b/x-pack/plugins/stack_alerts/public/rule_types/es_query/validation.ts index 7d3fcb38f1902..e287d30fdc034 100644 --- a/x-pack/plugins/stack_alerts/public/rule_types/es_query/validation.ts +++ b/x-pack/plugins/stack_alerts/public/rule_types/es_query/validation.ts @@ -8,8 +8,7 @@ import { defaultsDeep, isNil } from 'lodash'; import { i18n } from '@kbn/i18n'; import { ValidationResult, builtInComparators } from '@kbn/triggers-actions-ui-plugin/public'; -import { ISearchSource } from '@kbn/data-plugin/public'; -import { EsQueryRuleParams, OnlySearchSourceRuleParams, OnlyEsQueryRuleParams } from './types'; +import { EsQueryRuleParams, SearchType } from './types'; import { isSearchSourceRule } from './util'; import { COMMON_EXPRESSION_ERRORS, @@ -85,16 +84,13 @@ const validateCommonParams = (ruleParams: EsQueryRuleParams) => { return errors; }; -export const validateSearchSourceParams = ( - ruleParams: OnlySearchSourceRuleParams, - searchSource?: ISearchSource -) => { +const validateSearchSourceParams = (ruleParams: EsQueryRuleParams) => { const errors: typeof SEARCH_SOURCE_ONLY_EXPRESSION_ERRORS = defaultsDeep( {}, SEARCH_SOURCE_ONLY_EXPRESSION_ERRORS ); - if (!ruleParams.searchConfiguration || !searchSource) { + if (!ruleParams.searchConfiguration) { errors.searchConfiguration.push( i18n.translate('xpack.stackAlerts.esQuery.ui.validation.error.requiredSearchConfiguration', { defaultMessage: 'Search source configuration is required.', @@ -103,8 +99,7 @@ export const validateSearchSourceParams = ( return errors; } - const dataView = searchSource.getField('index'); - if (!dataView) { + if (!ruleParams.searchConfiguration.index) { errors.searchConfiguration.push( i18n.translate('xpack.stackAlerts.esQuery.ui.validation.error.requiredDataViewText', { defaultMessage: 'Data view is required.', @@ -113,8 +108,8 @@ export const validateSearchSourceParams = ( return errors; } - if (!dataView?.isTimeBased()) { - errors.searchConfiguration.push( + if (!ruleParams.timeField) { + errors.timeField.push( i18n.translate( 'xpack.stackAlerts.esQuery.ui.validation.error.requiredDataViewTimeFieldText', { @@ -128,7 +123,7 @@ export const validateSearchSourceParams = ( return errors; }; -const validateEsQueryParams = (ruleParams: OnlyEsQueryRuleParams) => { +const validateEsQueryParams = (ruleParams: EsQueryRuleParams) => { const errors: typeof ONLY_ES_QUERY_EXPRESSION_ERRORS = defaultsDeep( {}, ONLY_ES_QUERY_EXPRESSION_ERRORS @@ -194,13 +189,19 @@ export const validateExpression = (ruleParams: EsQueryRuleParams): ValidationRes if (isSearchSource) { validationResult.errors = { ...validationResult.errors, - ...defaultsDeep({}, SEARCH_SOURCE_ONLY_EXPRESSION_ERRORS), + ...validateSearchSourceParams(ruleParams), }; - // skip searchConfiguration check here, since it will be checked in by updating metadata return validationResult; } - const esQueryErrors = validateEsQueryParams(ruleParams as OnlyEsQueryRuleParams); + const esQueryErrors = validateEsQueryParams(ruleParams as EsQueryRuleParams); validationResult.errors = { ...validationResult.errors, ...esQueryErrors }; return validationResult; }; + +export const hasExpressionValidationErrors = (ruleParams: EsQueryRuleParams) => { + const { errors: validationErrors } = validateExpression(ruleParams); + return Object.keys(validationErrors).some( + (key) => validationErrors[key] && validationErrors[key].length + ); +}; diff --git a/x-pack/plugins/stack_alerts/server/rule_types/es_query/lib/fetch_search_source_query.test.ts b/x-pack/plugins/stack_alerts/server/rule_types/es_query/lib/fetch_search_source_query.test.ts index 127224f1bf1b6..5b6d12ee1222a 100644 --- a/x-pack/plugins/stack_alerts/server/rule_types/es_query/lib/fetch_search_source_query.test.ts +++ b/x-pack/plugins/stack_alerts/server/rule_types/es_query/lib/fetch_search_source_query.test.ts @@ -38,6 +38,7 @@ const defaultParams: OnlySearchSourceRuleParams = { searchConfiguration: {}, searchType: 'searchSource', excludeHitsFromPreviousRun: true, + timeField: 'time', }; describe('fetchSearchSourceQuery', () => { diff --git a/x-pack/plugins/stack_alerts/server/rule_types/es_query/lib/fetch_search_source_query.ts b/x-pack/plugins/stack_alerts/server/rule_types/es_query/lib/fetch_search_source_query.ts index 81f6248d906ca..f16a443fb41a9 100644 --- a/x-pack/plugins/stack_alerts/server/rule_types/es_query/lib/fetch_search_source_query.ts +++ b/x-pack/plugins/stack_alerts/server/rule_types/es_query/lib/fetch_search_source_query.ts @@ -54,9 +54,8 @@ export function updateSearchSource( params: OnlySearchSourceRuleParams, latestTimestamp: string | undefined ) { - const index = searchSource.getField('index'); - - const timeFieldName = index?.timeFieldName; + const index = searchSource.getField('index')!; + const timeFieldName = params.timeField || index.timeFieldName; if (!timeFieldName) { throw new Error('Invalid data view without timeFieldName.'); } @@ -75,7 +74,7 @@ export function updateSearchSource( if (latestTimestamp && latestTimestamp > dateStart) { // add additional filter for documents with a timestamp greater then // the timestamp of the previous run, so that those documents are not counted twice - const field = index.fields.find((f) => f.name === timeFieldName); + const field = index.fields.find((f) => f.name === params.timeField); const addTimeRangeField = buildRangeFilter(field!, { gt: latestTimestamp }, index); filters.push(addTimeRangeField); } diff --git a/x-pack/plugins/stack_alerts/server/rule_types/es_query/rule_type_params.ts b/x-pack/plugins/stack_alerts/server/rule_types/es_query/rule_type_params.ts index 941a45d3a9a1e..d62afa9abd7f7 100644 --- a/x-pack/plugins/stack_alerts/server/rule_types/es_query/rule_type_params.ts +++ b/x-pack/plugins/stack_alerts/server/rule_types/es_query/rule_type_params.ts @@ -38,6 +38,12 @@ const EsQueryRuleParamsSchemaProperties = { searchType: schema.oneOf([schema.literal('searchSource'), schema.literal('esQuery')], { defaultValue: 'esQuery', }), + timeField: schema.conditional( + schema.siblingRef('searchType'), + schema.literal('esQuery'), + schema.string({ minLength: 1 }), + schema.maybe(schema.string({ minLength: 1 })) + ), // searchSource rule param only searchConfiguration: schema.conditional( schema.siblingRef('searchType'), @@ -58,12 +64,6 @@ const EsQueryRuleParamsSchemaProperties = { schema.arrayOf(schema.string({ minLength: 1 }), { minSize: 1 }), schema.never() ), - timeField: schema.conditional( - schema.siblingRef('searchType'), - schema.literal('esQuery'), - schema.string({ minLength: 1 }), - schema.never() - ), }; export const EsQueryRuleParamsSchema = schema.object(EsQueryRuleParamsSchemaProperties, { diff --git a/x-pack/plugins/stack_alerts/server/rule_types/es_query/types.ts b/x-pack/plugins/stack_alerts/server/rule_types/es_query/types.ts index 2b0f0f7a7407c..d8eccab1ba404 100644 --- a/x-pack/plugins/stack_alerts/server/rule_types/es_query/types.ts +++ b/x-pack/plugins/stack_alerts/server/rule_types/es_query/types.ts @@ -12,13 +12,12 @@ import { ActionGroupId } from './constants'; export type OnlyEsQueryRuleParams = Omit & { searchType: 'esQuery'; + timeField: string; }; -export type OnlySearchSourceRuleParams = Omit< - EsQueryRuleParams, - 'esQuery' | 'index' | 'timeField' -> & { +export type OnlySearchSourceRuleParams = Omit & { searchType: 'searchSource'; + timeField?: string; }; export type ExecutorOptions

= RuleExecutorOptions< diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_add.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_add.tsx index a210b98fa7559..b12a3c07c7875 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_add.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_add.tsx @@ -91,12 +91,11 @@ const RuleAdd = ({ dispatch({ command: { type: 'setProperty' }, payload: { key, value } }); }; - const services = useKibana().services; const { http, notifications: { toasts }, application: { capabilities }, - } = services; + } = useKibana().services; const canShowActions = hasShowActionsCapability(capabilities); @@ -200,8 +199,7 @@ const RuleAdd = ({ const { ruleBaseErrors, ruleErrors, ruleParamsErrors } = getRuleErrors( rule as Rule, ruleType, - config, - metadata + config ); // Confirm before saving if user is able to add actions but hasn't added any to this rule diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_edit.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_edit.tsx index 479d3521ffdc2..feec862ab804f 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_edit.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_edit.tsx @@ -74,11 +74,10 @@ export const RuleEdit = ({ const [metadata, setMetadata] = useState(initialMetadata); const onChangeMetaData = useCallback((newMetadata) => setMetadata(newMetadata), []); - const services = useKibana().services; const { http, notifications: { toasts }, - } = services; + } = useKibana().services; const setRule = (value: Rule) => { dispatch({ command: { type: 'setRule' }, payload: { key: 'rule', value } }); @@ -117,8 +116,7 @@ export const RuleEdit = ({ const { ruleBaseErrors, ruleErrors, ruleParamsErrors } = getRuleErrors( rule as Rule, ruleType, - config, - metadata + config ); const checkForChangesAndCloseFlyout = () => { diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_errors.ts b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_errors.ts index 01ae4822c6308..73769d8d9af10 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_errors.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_errors.ts @@ -82,15 +82,11 @@ export function validateBaseProperties( export function getRuleErrors( rule: Rule, ruleTypeModel: RuleTypeModel | null, - config: TriggersActionsUiConfig, - metadata?: Record + config: TriggersActionsUiConfig ) { - let ruleParamsErrors: IErrorObject = ruleTypeModel + const ruleParamsErrors: IErrorObject = ruleTypeModel ? ruleTypeModel.validate(rule.params).errors : []; - if (metadata && metadata.moreParamsErrors) { - ruleParamsErrors = { ...ruleParamsErrors, ...(metadata.moreParamsErrors as IErrorObject) }; - } const ruleBaseErrors = validateBaseProperties(rule, config).errors as IErrorObject; const ruleErrors = { ...ruleParamsErrors, diff --git a/x-pack/plugins/triggers_actions_ui/public/types.ts b/x-pack/plugins/triggers_actions_ui/public/types.ts index fc07467c2415c..f0d2b8e2e2aea 100644 --- a/x-pack/plugins/triggers_actions_ui/public/types.ts +++ b/x-pack/plugins/triggers_actions_ui/public/types.ts @@ -401,8 +401,6 @@ export interface RuleAddProps> { /** @deprecated use `onSave` as a callback after an alert is saved*/ reloadRules?: () => Promise; onSave?: (metadata?: MetaData) => Promise; - // be aware, `moreParamsErrors` prop of metadata is used to pass - // errors which can't be validated syncronously within `validate` function metadata?: MetaData; ruleTypeIndex?: RuleTypeIndex; filteredRuleTypes?: string[]; From ace937b3b2bea8eb0b836440cb1fc35002504e41 Mon Sep 17 00:00:00 2001 From: Dzmitry Tamashevich Date: Mon, 5 Dec 2022 13:47:54 +0300 Subject: [PATCH 4/5] [Discover] fix types --- .../server/rule_types/es_query/action_context.test.ts | 9 ++++----- .../server/rule_types/es_query/action_context.ts | 4 ++-- .../stack_alerts/server/rule_types/es_query/types.ts | 1 - 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/x-pack/plugins/stack_alerts/server/rule_types/es_query/action_context.test.ts b/x-pack/plugins/stack_alerts/server/rule_types/es_query/action_context.test.ts index 5aa40290c2f44..c9aa81223c6f3 100644 --- a/x-pack/plugins/stack_alerts/server/rule_types/es_query/action_context.test.ts +++ b/x-pack/plugins/stack_alerts/server/rule_types/es_query/action_context.test.ts @@ -6,8 +6,7 @@ */ import { EsQueryRuleActionContext, addMessages } from './action_context'; -import { EsQueryRuleParamsSchema } from './rule_type_params'; -import { OnlyEsQueryRuleParams } from './types'; +import { EsQueryRuleParams, EsQueryRuleParamsSchema } from './rule_type_params'; describe('ActionContext', () => { it('generates expected properties', async () => { @@ -21,7 +20,7 @@ describe('ActionContext', () => { thresholdComparator: '>', threshold: [4], searchType: 'esQuery', - }) as OnlyEsQueryRuleParams; + }) as EsQueryRuleParams; const base: EsQueryRuleActionContext = { date: '2020-01-01T00:00:00.000Z', value: 42, @@ -52,7 +51,7 @@ describe('ActionContext', () => { thresholdComparator: '>', threshold: [4], searchType: 'esQuery', - }) as OnlyEsQueryRuleParams; + }) as EsQueryRuleParams; const base: EsQueryRuleActionContext = { date: '2020-01-01T00:00:00.000Z', value: 42, @@ -83,7 +82,7 @@ describe('ActionContext', () => { thresholdComparator: 'between', threshold: [4, 5], searchType: 'esQuery', - }) as OnlyEsQueryRuleParams; + }) as EsQueryRuleParams; const base: EsQueryRuleActionContext = { date: '2020-01-01T00:00:00.000Z', value: 4, diff --git a/x-pack/plugins/stack_alerts/server/rule_types/es_query/action_context.ts b/x-pack/plugins/stack_alerts/server/rule_types/es_query/action_context.ts index 1dc5b5422e121..38a16dacf9c53 100644 --- a/x-pack/plugins/stack_alerts/server/rule_types/es_query/action_context.ts +++ b/x-pack/plugins/stack_alerts/server/rule_types/es_query/action_context.ts @@ -8,7 +8,7 @@ import { i18n } from '@kbn/i18n'; import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey'; import { AlertInstanceContext } from '@kbn/alerting-plugin/server'; -import { OnlyEsQueryRuleParams, OnlySearchSourceRuleParams } from './types'; +import { EsQueryRuleParams } from './rule_type_params'; // rule type context provided to actions export interface ActionContext extends EsQueryRuleActionContext { @@ -35,7 +35,7 @@ export interface EsQueryRuleActionContext extends AlertInstanceContext { export function addMessages( ruleName: string, baseContext: EsQueryRuleActionContext, - params: OnlyEsQueryRuleParams | OnlySearchSourceRuleParams, + params: EsQueryRuleParams, isRecovered: boolean = false ): ActionContext { const title = i18n.translate('xpack.stackAlerts.esQuery.alertTypeContextSubjectTitle', { diff --git a/x-pack/plugins/stack_alerts/server/rule_types/es_query/types.ts b/x-pack/plugins/stack_alerts/server/rule_types/es_query/types.ts index d8eccab1ba404..c8844b19a678a 100644 --- a/x-pack/plugins/stack_alerts/server/rule_types/es_query/types.ts +++ b/x-pack/plugins/stack_alerts/server/rule_types/es_query/types.ts @@ -17,7 +17,6 @@ export type OnlyEsQueryRuleParams = Omit & { searchType: 'searchSource'; - timeField?: string; }; export type ExecutorOptions

= RuleExecutorOptions< From 97f66018784bfb19ad0f2d7f33fa94336a2131be Mon Sep 17 00:00:00 2001 From: Dzmitry Tamashevich Date: Wed, 14 Dec 2022 13:21:11 +0300 Subject: [PATCH 5/5] [Discover] apply suggestions --- .../server/rule_types/es_query/lib/fetch_search_source_query.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/stack_alerts/server/rule_types/es_query/lib/fetch_search_source_query.ts b/x-pack/plugins/stack_alerts/server/rule_types/es_query/lib/fetch_search_source_query.ts index f16a443fb41a9..c14e57c5414d3 100644 --- a/x-pack/plugins/stack_alerts/server/rule_types/es_query/lib/fetch_search_source_query.ts +++ b/x-pack/plugins/stack_alerts/server/rule_types/es_query/lib/fetch_search_source_query.ts @@ -74,7 +74,7 @@ export function updateSearchSource( if (latestTimestamp && latestTimestamp > dateStart) { // add additional filter for documents with a timestamp greater then // the timestamp of the previous run, so that those documents are not counted twice - const field = index.fields.find((f) => f.name === params.timeField); + const field = index.fields.find((f) => f.name === timeFieldName); const addTimeRangeField = buildRangeFilter(field!, { gt: latestTimestamp }, index); filters.push(addTimeRangeField); }