Solved the issue for a GROUP BY expression validation

x-pack/plugins/triggers_actions_ui/public/application/components/builtin_alert_types/threshold/index.ts

@@ -3,103 +3,16 @@
  * or more contributor license agreements. Licensed under the Elastic License;
  * you may not use this file except in compliance with the Elastic License.
  */
-import { i18n } from '@kbn/i18n';
-import { AlertTypeModel, ValidationResult } from '../../../../types';
+import { AlertTypeModel } from '../../../../types';
 import { IndexThresholdAlertTypeExpression } from './expression';
-import { IndexThresholdAlertParams } from './types';
-import { builtInGroupByTypes, builtInAggregationTypes } from '../../../../common/constants';
+import { validateExpression } from './validation';
 
 export function getAlertType(): AlertTypeModel {
   return {
     id: '.index-threshold',
     name: 'Index Threshold',
     iconClass: 'alert',
     alertParamsExpression: IndexThresholdAlertTypeExpression,
-    validate: (alertParams: IndexThresholdAlertParams): ValidationResult => {
-      const {
-        index,
-        timeField,
-        aggType,
-        aggField,
-        groupBy,
-        termSize,
-        termField,
-        threshold,
-        timeWindowSize,
-      } = alertParams;
-      const validationResult = { errors: {} };
-      const errors = {
-        aggField: new Array<string>(),
-        termSize: new Array<string>(),
-        termField: new Array<string>(),
-        timeWindowSize: new Array<string>(),
-        threshold0: new Array<string>(),
-        threshold1: new Array<string>(),
-        index: new Array<string>(),
-        timeField: new Array<string>(),
-      };
-      validationResult.errors = errors;
-      if (!index || index.length === 0) {
-        errors.index.push(
-          i18n.translate('xpack.triggersActionsUI.sections.addAlert.error.requiredIndexText', {
-            defaultMessage: 'Index is required.',
-          })
-        );
-      }
-      if (!timeField) {
-        errors.timeField.push(
-          i18n.translate('xpack.triggersActionsUI.sections.addAlert.error.requiredTimeFieldText', {
-            defaultMessage: 'Time field is required.',
-          })
-        );
-      }
-      if (aggType && builtInAggregationTypes[aggType].fieldRequired && !aggField) {
-        errors.aggField.push(
-          i18n.translate('xpack.triggersActionsUI.sections.addAlert.error.requiredAggFieldText', {
-            defaultMessage: 'Aggregation field is required.',
-          })
-        );
-      }
-      if (!termSize) {
-        errors.termSize.push(
-          i18n.translate('xpack.triggersActionsUI.sections.addAlert.error.requiredTermSizedText', {
-            defaultMessage: 'Term size is required.',
-          })
-        );
-      }
-      if (!termField && groupBy && builtInGroupByTypes[groupBy].sizeRequired) {
-        errors.termField.push(
-          i18n.translate('xpack.triggersActionsUI.sections.addAlert.error.requiredtTermFieldText', {
-            defaultMessage: 'Term field is required.',
-          })
-        );
-      }
-      if (!timeWindowSize) {
-        errors.timeWindowSize.push(
-          i18n.translate(
-            'xpack.triggersActionsUI.sections.addAlert.error.requiredTimeWindowSizeText',
-            {
-              defaultMessage: 'Time window size is required.',
-            }
-          )
-        );
-      }
-      if (threshold && threshold.length > 0 && !threshold[0]) {
-        errors.threshold0.push(
-          i18n.translate('xpack.triggersActionsUI.sections.addAlert.error.requiredThreshold0Text', {
-            defaultMessage: 'Threshold0, is required.',
-          })
-        );
-      }
-      if (threshold && threshold.length > 1 && !threshold[1]) {
-        errors.threshold1.push(
-          i18n.translate('xpack.triggersActionsUI.sections.addAlert.error.requiredThreshold1Text', {
-            defaultMessage: 'Threshold1 is required.',
-          })
-        );
-      }
-      return validationResult;
-    },
-    defaultActionMessage: 'Alert [{{ctx.metadata.name}}] has exceeded the threshold',
+    validate: validateExpression,
   };
 }

x-pack/plugins/triggers_actions_ui/public/application/components/builtin_alert_types/threshold/validation.test.ts

@@ -0,0 +1,96 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import { IndexThresholdAlertParams } from './types';
+import { validateExpression } from './validation';
+
+describe('expression params validation', () => {
+  test('if index property is invalid should return proper error message', () => {
+    const initialParams: IndexThresholdAlertParams = {
+      index: [],
+      aggType: 'count',
+      threshold: [],
+      timeWindowSize: 1,
+      timeWindowUnit: 's',
+    };
+    expect(validateExpression(initialParams).errors.index.length).toBeGreaterThan(0);
+    expect(validateExpression(initialParams).errors.index[0]).toBe('Index is required.');
+  });
+  test('if timeField property is not defined should return proper error message', () => {
+    const initialParams: IndexThresholdAlertParams = {
+      index: ['test'],
+      aggType: 'count',
+      threshold: [],
+      timeWindowSize: 1,
+      timeWindowUnit: 's',
+    };
+    expect(validateExpression(initialParams).errors.timeField.length).toBeGreaterThan(0);
+    expect(validateExpression(initialParams).errors.timeField[0]).toBe('Time field is required.');
+  });
+  test('if aggField property is invalid should return proper error message', () => {
+    const initialParams: IndexThresholdAlertParams = {
+      index: ['test'],
+      aggType: 'avg',
+      threshold: [],
+      timeWindowSize: 1,
+      timeWindowUnit: 's',
+    };
+    expect(validateExpression(initialParams).errors.aggField.length).toBeGreaterThan(0);
+    expect(validateExpression(initialParams).errors.aggField[0]).toBe(
+      'Aggregation field is required.'
+    );
+  });
+  test('if termSize property is not set should return proper error message', () => {
+    const initialParams: IndexThresholdAlertParams = {
+      index: ['test'],
+      aggType: 'count',
+      groupBy: 'top',
+      threshold: [],
+      timeWindowSize: 1,
+      timeWindowUnit: 's',
+    };
+    expect(validateExpression(initialParams).errors.termSize.length).toBeGreaterThan(0);
+    expect(validateExpression(initialParams).errors.termSize[0]).toBe('Term size is required.');
+  });
+  test('if termField property is not set should return proper error message', () => {
+    const initialParams: IndexThresholdAlertParams = {
+      index: ['test'],
+      aggType: 'count',
+      groupBy: 'top',
+      threshold: [],
+      timeWindowSize: 1,
+      timeWindowUnit: 's',
+    };
+    expect(validateExpression(initialParams).errors.termField.length).toBeGreaterThan(0);
+    expect(validateExpression(initialParams).errors.termField[0]).toBe('Term field is required.');
+  });
+  test('if threshold0 property is not set should return proper error message', () => {
+    const initialParams: IndexThresholdAlertParams = {
+      index: ['test'],
+      aggType: 'count',
+      groupBy: 'top',
+      threshold: [],
+      timeWindowSize: 1,
+      timeWindowUnit: 's',
+      thresholdComparator: '<',
+    };
+    expect(validateExpression(initialParams).errors.threshold0.length).toBeGreaterThan(0);
+    expect(validateExpression(initialParams).errors.threshold0[0]).toBe('Threshold0 is required.');
+  });
+  test('if threshold1 property is not set should return proper error message', () => {
+    const initialParams: IndexThresholdAlertParams = {
+      index: ['test'],
+      aggType: 'count',
+      groupBy: 'top',
+      threshold: [1],
+      timeWindowSize: 1,
+      timeWindowUnit: 's',
+      thresholdComparator: 'between',
+    };
+    expect(validateExpression(initialParams).errors.threshold1.length).toBeGreaterThan(0);
+    expect(validateExpression(initialParams).errors.threshold1[0]).toBe('Threshold1 is required.');
+  });
+});

x-pack/plugins/triggers_actions_ui/public/application/components/builtin_alert_types/threshold/validation.ts

@@ -0,0 +1,119 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+import { i18n } from '@kbn/i18n';
+import { ValidationResult } from '../../../../types';
+import { IndexThresholdAlertParams } from './types';
+import {
+  builtInGroupByTypes,
+  builtInAggregationTypes,
+  builtInComparators,
+} from '../../../../common/constants';
+
+export const validateExpression = (alertParams: IndexThresholdAlertParams): ValidationResult => {
+  const {
+    index,
+    timeField,
+    aggType,
+    aggField,
+    groupBy,
+    termSize,
+    termField,
+    threshold,
+    timeWindowSize,
+    thresholdComparator,
+  } = alertParams;
+  const validationResult = { errors: {} };
+  const errors = {
+    aggField: new Array<string>(),
+    termSize: new Array<string>(),
+    termField: new Array<string>(),
+    timeWindowSize: new Array<string>(),
+    threshold0: new Array<string>(),
+    threshold1: new Array<string>(),
+    index: new Array<string>(),
+    timeField: new Array<string>(),
+  };
+  validationResult.errors = errors;
+  if (!index || index.length === 0) {
+    errors.index.push(
+      i18n.translate('xpack.triggersActionsUI.sections.addAlert.error.requiredIndexText', {
+        defaultMessage: 'Index is required.',
+      })
+    );
+  }
+  if (!timeField) {
+    errors.timeField.push(
+      i18n.translate('xpack.triggersActionsUI.sections.addAlert.error.requiredTimeFieldText', {
+        defaultMessage: 'Time field is required.',
+      })
+    );
+  }
+  if (aggType && builtInAggregationTypes[aggType].fieldRequired && !aggField) {
+    errors.aggField.push(
+      i18n.translate('xpack.triggersActionsUI.sections.addAlert.error.requiredAggFieldText', {
+        defaultMessage: 'Aggregation field is required.',
+      })
+    );
+  }
+  if (
+    groupBy &&
+    builtInGroupByTypes[groupBy] &&
+    builtInGroupByTypes[groupBy].sizeRequired &&
+    !termSize
+  ) {
+    errors.termSize.push(
+      i18n.translate('xpack.triggersActionsUI.sections.addAlert.error.requiredTermSizedText', {
+        defaultMessage: 'Term size is required.',
+      })
+    );
+  }
+  if (
+    groupBy &&
+    builtInGroupByTypes[groupBy].validNormalizedTypes &&
+    builtInGroupByTypes[groupBy].validNormalizedTypes.length > 0 &&
+    !termField
+  ) {
+    errors.termField.push(
+      i18n.translate('xpack.triggersActionsUI.sections.addAlert.error.requiredtTermFieldText', {
+        defaultMessage: 'Term field is required.',
+      })
+    );
+  }
+  if (!timeWindowSize) {
+    errors.timeWindowSize.push(
+      i18n.translate('xpack.triggersActionsUI.sections.addAlert.error.requiredTimeWindowSizeText', {
+        defaultMessage: 'Time window size is required.',
+      })
+    );
+  }
+  if (!threshold || threshold.length === 0 || (threshold.length === 1 && !threshold[0])) {
+    errors.threshold0.push(
+      i18n.translate('xpack.triggersActionsUI.sections.addAlert.error.requiredThreshold0Text', {
+        defaultMessage: 'Threshold0 is required.',
+      })
+    );
+  }
+  if (
+    thresholdComparator &&
+    builtInComparators[thresholdComparator].requiredValues > 1 &&
+    (!threshold ||
+      (threshold && threshold.length < builtInComparators[thresholdComparator!].requiredValues))
+  ) {
+    errors.threshold1.push(
+      i18n.translate('xpack.triggersActionsUI.sections.addAlert.error.requiredThreshold1Text', {
+        defaultMessage: 'Threshold1 is required.',
+      })
+    );
+  }
+  if (threshold && threshold.length === 2 && threshold[0] > threshold[1]) {
+    errors.threshold1.push(
+      i18n.translate('xpack.triggersActionsUI.sections.addAlert.error.greaterThenThreshold0Text', {
+        defaultMessage: 'Threshold1 should be > Threshold0.',
+      })
+    );
+  }
+  return validationResult;
+};

x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.tsx

@@ -117,16 +117,14 @@ export const ActionForm = ({
       const actionsResponse = await loadAllActions({ http });
       setConnectors(actionsResponse.data);
     } catch (e) {
-      if (toastNotifications) {
-        toastNotifications.addDanger({
-          title: i18n.translate(
-            'xpack.triggersActionsUI.sections.alertForm.unableToLoadActionsMessage',
-            {
-              defaultMessage: 'Unable to load connectors',
-            }
-          ),
-        });
-      }
+      toastNotifications.addDanger({
+        title: i18n.translate(
+          'xpack.triggersActionsUI.sections.alertForm.unableToLoadActionsMessage',
+          {
+            defaultMessage: 'Unable to load connectors',
+          }
+        ),
+      });
     }
   }