Fixed due to review

elastic · Feb 5, 2020 · 293d985 · 293d985
1 parent db7678b
commit 293d985
Show file tree

Hide file tree

Showing 17 changed files with 103 additions and 97 deletions.
diff --git a/...ns_ui/np_ready/public/application/components/builtin_alert_types/threshold/expression.tsx b/...ns_ui/np_ready/public/application/components/builtin_alert_types/threshold/expression.tsx
@@ -20,7 +20,7 @@ import {
   EuiFormRow,
   EuiCallOut,
 } from '@elastic/eui';
-import { COMPARATORS, buildinComparators } from '../../../../common/constants';
+import { COMPARATORS, builtInComparators } from '../../../../common/constants';
 import {
   getMatchingIndicesForThresholdAlertType,
   getThresholdAlertTypeFields,
@@ -36,7 +36,7 @@ import {
   ForLastExpression,
   GroupByExpression,
 } from '../../../../common';
-import { buildinAggregationTypes } from '../../../../common/constants';
+import { builtInAggregationTypes } from '../../../../common/constants';
 import { IndexThresholdAlertParams } from '../types';
 
 const DEFAULT_VALUES = {
@@ -381,7 +381,7 @@ export const IndexThresholdAlertTypeExpression: React.FunctionComponent<IndexThr
             }
           />
         </EuiFlexItem>
-        {aggType && buildinAggregationTypes[aggType].fieldRequired ? (
+        {aggType && builtInAggregationTypes[aggType].fieldRequired ? (
           <EuiFlexItem grow={false}>
             <OfExpression
               aggField={aggField}
@@ -396,10 +396,9 @@ export const IndexThresholdAlertTypeExpression: React.FunctionComponent<IndexThr
         ) : null}
         <EuiFlexItem grow={false}>
           <GroupByExpression
-            groupBy={groupBy}
+            groupBy={groupBy || DEFAULT_VALUES.GROUP_BY}
             termField={termField}
             termSize={termSize}
-            defaultGroupBy={DEFAULT_VALUES.GROUP_BY}
             errors={errors}
             fields={esFields}
             onChangeSelectedGroupBy={selectedGroupBy => setAlertParams('groupBy', selectedGroupBy)}
@@ -413,7 +412,7 @@ export const IndexThresholdAlertTypeExpression: React.FunctionComponent<IndexThr
         </EuiFlexItem>
         <EuiFlexItem grow={false}>
           <ThresholdExpression
-            thresholdComparator={thresholdComparator}
+            thresholdComparator={thresholdComparator || DEFAULT_VALUES.THRESHOLD_COMPARATOR}
             threshold={threshold}
             defaultThresholdComparator={DEFAULT_VALUES.THRESHOLD_COMPARATOR}
             errors={errors}
@@ -443,8 +442,8 @@ export const IndexThresholdAlertTypeExpression: React.FunctionComponent<IndexThr
         <Fragment>
           <ThresholdVisualization
             alertParams={alertParams}
-            aggregationTypes={buildinAggregationTypes}
-            comparators={buildinComparators}
+            aggregationTypes={builtInAggregationTypes}
+            comparators={builtInComparators}
           />
         </Fragment>
       )}

diff --git a/..._actions_ui/np_ready/public/application/components/builtin_alert_types/threshold/index.ts b/..._actions_ui/np_ready/public/application/components/builtin_alert_types/threshold/index.ts
@@ -7,7 +7,7 @@ import { i18n } from '@kbn/i18n';
 import { AlertTypeModel, ValidationResult } from '../../../../types';
 import { IndexThresholdAlertTypeExpression } from './expression';
 import { IndexThresholdAlertParams } from '../types';
-import { buildinGroupByTypes, buildinAggregationTypes } from '../../../../common/constants';
+import { builtInGroupByTypes, builtInAggregationTypes } from '../../../../common/constants';
 
 export function getAlertType(): AlertTypeModel {
   return {
@@ -53,7 +53,7 @@ export function getAlertType(): AlertTypeModel {
           })
         );
       }
-      if (aggType && buildinAggregationTypes[aggType].fieldRequired && !aggField) {
+      if (aggType && builtInAggregationTypes[aggType].fieldRequired && !aggField) {
         errors.aggField.push(
           i18n.translate('xpack.triggersActionsUI.sections.addAlert.error.requiredAggFieldText', {
             defaultMessage: 'Aggregation field is required.',
@@ -67,7 +67,7 @@ export function getAlertType(): AlertTypeModel {
           })
         );
       }
-      if (groupBy && buildinGroupByTypes[groupBy].sizeRequired && !termField) {
+      if (!termField && groupBy && builtInGroupByTypes[groupBy].sizeRequired) {
         errors.termField.push(
           i18n.translate('xpack.triggersActionsUI.sections.addAlert.error.requiredtTermFieldText', {
             defaultMessage: 'Term field is required.',

diff --git a/...ui/np_ready/public/application/components/builtin_alert_types/threshold/visualization.tsx b/...ui/np_ready/public/application/components/builtin_alert_types/threshold/visualization.tsx
@@ -208,10 +208,9 @@ export const ThresholdVisualization: React.FunctionComponent<Props> = ({
   }
 
   const getThreshold = () => {
-    return alertParams.threshold.slice(
-      0,
-      comparators[alertParams.thresholdComparator].requiredValues
-    );
+    return thresholdComparator
+      ? threshold.slice(0, comparators[thresholdComparator].requiredValues)
+      : [];
   };
 
   if (visualizationData) {

diff --git a/...s/triggers_actions_ui/np_ready/public/application/components/builtin_alert_types/types.ts b/...s/triggers_actions_ui/np_ready/public/application/components/builtin_alert_types/types.ts
@@ -6,14 +6,14 @@
 
 export interface IndexThresholdAlertParams {
   index: string[];
-  timeField: string;
+  timeField?: string;
   aggType: string;
   aggField?: string;
-  groupBy: string;
-  termSize: number;
-  termField: string;
-  thresholdComparator: string;
+  groupBy?: string;
+  termSize?: number;
+  termField?: string;
+  thresholdComparator?: string;
   threshold: number[];
-  timeWindowSize?: number;
-  timeWindowUnit?: string;
+  timeWindowSize: number;
+  timeWindowUnit: string;
 }
diff --git a/.../plugins/triggers_actions_ui/np_ready/public/application/sections/alert_add/alert_add.tsx b/.../plugins/triggers_actions_ui/np_ready/public/application/sections/alert_add/alert_add.tsx
@@ -63,7 +63,7 @@ export const AlertAdd = () => {
 
   const alertType = alertTypeRegistry.get(alert.alertTypeId);
   const errors = {
-    ...(alertType ? alertType.validate(alert).errors : []),
+    ...(alertType ? alertType.validate(alert.params).errors : []),
     ...validateBaseProperties(alert).errors,
   } as IErrorObject;
   const hasErrors = !!Object.keys(errors).find(errorKey => errors[errorKey].length >= 1);

diff --git a/.../legacy/plugins/triggers_actions_ui/np_ready/public/common/constants/aggregation_types.ts b/.../legacy/plugins/triggers_actions_ui/np_ready/public/common/constants/aggregation_types.ts
@@ -14,7 +14,7 @@ export enum AGGREGATION_TYPES {
   MAX = 'max',
 }
 
-export const buildinAggregationTypes: { [key: string]: AggregationType } = {
+export const builtInAggregationTypes: { [key: string]: AggregationType } = {
   count: {
     text: 'count()',
     fieldRequired: false,

diff --git a/x-pack/legacy/plugins/triggers_actions_ui/np_ready/public/common/constants/comparators.ts b/x-pack/legacy/plugins/triggers_actions_ui/np_ready/public/common/constants/comparators.ts
@@ -15,7 +15,7 @@ export enum COMPARATORS {
   LESS_THAN_OR_EQUALS = '<=',
 }
 
-export const buildinComparators: { [key: string]: Comparator } = {
+export const builtInComparators: { [key: string]: Comparator } = {
   [COMPARATORS.GREATER_THAN]: {
     text: i18n.translate(
       'xpack.triggersActionsUI.sections.alertAdd.threshold.comparators.isAboveLabel',

diff --git a/x-pack/legacy/plugins/triggers_actions_ui/np_ready/public/common/constants/group_by_types.ts b/x-pack/legacy/plugins/triggers_actions_ui/np_ready/public/common/constants/group_by_types.ts
@@ -6,7 +6,7 @@
 import { i18n } from '@kbn/i18n';
 import { GroupByType } from '../types';
 
-export const buildinGroupByTypes: { [key: string]: GroupByType } = {
+export const builtInGroupByTypes: { [key: string]: GroupByType } = {
   all: {
     text: i18n.translate(
       'xpack.triggersActionsUI.sections.alertAdd.threshold.groupByLabel.allDocumentsLabel',

diff --git a/x-pack/legacy/plugins/triggers_actions_ui/np_ready/public/common/constants/index.ts b/x-pack/legacy/plugins/triggers_actions_ui/np_ready/public/common/constants/index.ts
@@ -4,6 +4,6 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-export { COMPARATORS, buildinComparators } from './comparators';
-export { AGGREGATION_TYPES, buildinAggregationTypes } from './aggregation_types';
-export { buildinGroupByTypes } from './group_by_types';
+export { COMPARATORS, builtInComparators } from './comparators';
+export { AGGREGATION_TYPES, builtInAggregationTypes } from './aggregation_types';
+export { builtInGroupByTypes } from './group_by_types';
diff --git a/...lugins/triggers_actions_ui/np_ready/public/common/expression_items/group_by_over.test.tsx b/...lugins/triggers_actions_ui/np_ready/public/common/expression_items/group_by_over.test.tsx
@@ -9,13 +9,12 @@ import { EuiPopoverTitle } from '@elastic/eui';
 import { GroupByExpression } from './group_by_over';
 
 describe('group by expression', () => {
-  it('renders with buildin group by types', () => {
+  it('renders with builtin group by types', () => {
     const onChangeSelectedTermField = jest.fn();
     const onChangeSelectedGroupBy = jest.fn();
     const onChangeSelectedTermSize = jest.fn();
     const wrapper = shallow(
       <GroupByExpression
-        defaultGroupBy={'all'}
         errors={{ termSize: [], termField: [] }}
         fields={[{}]}
         groupBy={'all'}
@@ -51,7 +50,6 @@ describe('group by expression', () => {
     const onChangeSelectedTermSize = jest.fn();
     const wrapper = shallow(
       <GroupByExpression
-        defaultGroupBy={'all'}
         errors={{ termSize: [], termField: [] }}
         fields={[{ normalizedType: 'number', name: 'test', text: 'test text' }]}
         groupBy={'top'}
@@ -89,7 +87,6 @@ describe('group by expression', () => {
     const onChangeSelectedTermSize = jest.fn();
     const wrapper = shallow(
       <GroupByExpression
-        defaultGroupBy={'all'}
         errors={{ termSize: [], termField: [] }}
         fields={[{}]}
         groupBy={'all'}

diff --git a/...acy/plugins/triggers_actions_ui/np_ready/public/common/expression_items/group_by_over.tsx b/...acy/plugins/triggers_actions_ui/np_ready/public/common/expression_items/group_by_over.tsx
@@ -16,12 +16,11 @@ import {
   EuiSelect,
   EuiFieldNumber,
 } from '@elastic/eui';
-import { buildinGroupByTypes } from '../constants';
+import { builtInGroupByTypes } from '../constants';
 import { GroupByType } from '../types';
 
 interface GroupByExpressionProps {
   groupBy: string;
-  defaultGroupBy: string;
   errors: { [key: string]: string[] };
   onChangeSelectedTermSize: (selectedTermSize?: number) => void;
   onChangeSelectedTermField: (selectedTermField?: string) => void;
@@ -36,19 +35,18 @@ interface GroupByExpressionProps {
 
 export const GroupByExpression = ({
   groupBy,
-  defaultGroupBy,
   errors,
   onChangeSelectedTermSize,
   onChangeSelectedTermField,
   onChangeSelectedGroupBy,
   fields,
-  termSize = 1,
+  termSize,
   termField,
   customGroupByTypes,
 }: GroupByExpressionProps) => {
-  const groupByTypes = customGroupByTypes ?? buildinGroupByTypes;
+  const groupByTypes = customGroupByTypes ?? builtInGroupByTypes;
   const [groupByPopoverOpen, setGroupByPopoverOpen] = useState(false);
-
+  const MIN_TERM_SIZE = 1;
   const firstFieldOption = {
     text: i18n.translate('xpack.triggersActionsUI.common.groupByType.timeFieldOptionLabel', {
       defaultMessage: 'Select a field',
@@ -62,16 +60,16 @@ export const GroupByExpression = ({
       button={
         <EuiExpression
           description={`${
-            groupByTypes[groupBy || defaultGroupBy].sizeRequired
+            groupByTypes[groupBy].sizeRequired
               ? i18n.translate('xpack.triggersActionsUI.common.groupByType.groupedOverLabel', {
                   defaultMessage: 'grouped over',
                 })
               : i18n.translate('xpack.triggersActionsUI.common.groupByType.overLabel', {
                   defaultMessage: 'over',
                 })
           }`}
-          value={`${groupByTypes[groupBy || defaultGroupBy].text} ${
-            groupByTypes[groupBy || defaultGroupBy].sizeRequired
+          value={`${groupByTypes[groupBy].text} ${
+            groupByTypes[groupBy].sizeRequired
               ? `${termSize} ${termField ? `'${termField}'` : ''}`
               : ''
           }`}
@@ -88,22 +86,27 @@ export const GroupByExpression = ({
       }}
       ownFocus
       withTitle
-      anchorPosition="downLeft"
+      anchorPosition="downRight"
     >
       <div>
         <EuiPopoverTitle>
           {i18n.translate('xpack.triggersActionsUI.common.groupByType.overButtonLabel', {
             defaultMessage: 'over',
           })}
         </EuiPopoverTitle>
-        <EuiFlexGroup>
+        <EuiFlexGroup justifyContent="spaceBetween">
           <EuiFlexItem grow={false}>
             <EuiSelect
               data-test-subj="overExpressionSelect"
-              value={groupBy || defaultGroupBy}
+              value={groupBy}
               onChange={e => {
-                onChangeSelectedTermSize(undefined);
-                onChangeSelectedTermField(undefined);
+                if (groupByTypes[e.target.value].sizeRequired) {
+                  onChangeSelectedTermSize(MIN_TERM_SIZE);
+                  onChangeSelectedTermField('');
+                } else {
+                  onChangeSelectedTermSize(undefined);
+                  onChangeSelectedTermField(undefined);
+                }
                 onChangeSelectedGroupBy(e.target.value);
               }}
               options={Object.values(groupByTypes).map(({ text, value }) => {
@@ -115,19 +118,22 @@ export const GroupByExpression = ({
             />
           </EuiFlexItem>
 
-          {groupByTypes[groupBy || defaultGroupBy].sizeRequired ? (
+          {groupByTypes[groupBy].sizeRequired ? (
             <Fragment>
               <EuiFlexItem grow={false}>
-                <EuiFormRow isInvalid={errors.termSize.length > 0} error={errors.termSize}>
+                <EuiFormRow
+                  isInvalid={errors.termSize.length > 0 && termSize !== undefined}
+                  error={errors.termSize}
+                >
                   <EuiFieldNumber
-                    isInvalid={errors.termSize.length > 0}
-                    value={termSize || 1}
+                    isInvalid={errors.termSize.length > 0 && termSize !== undefined}
+                    value={termSize}
                     onChange={e => {
                       const { value } = e.target;
-                      const termSizeVal = value !== '' ? parseFloat(value) : undefined;
+                      const termSizeVal = value !== '' ? parseFloat(value) : MIN_TERM_SIZE;
                       onChangeSelectedTermSize(termSizeVal);
                     }}
-                    min={1}
+                    min={MIN_TERM_SIZE}
                   />
                 </EuiFormRow>
               </EuiFlexItem>
@@ -138,17 +144,15 @@ export const GroupByExpression = ({
                 >
                   <EuiSelect
                     data-test-subj="fieldsExpressionSelect"
-                    value={termField || ''}
+                    value={termField}
                     isInvalid={errors.termField.length > 0 && termField !== undefined}
                     onChange={e => {
                       onChangeSelectedTermField(e.target.value);
                     }}
                     options={fields.reduce(
-                      (options: any, field: any) => {
+                      (options: any, field: { name: string; normalizedType: string }) => {
                         if (
-                          groupByTypes[groupBy || defaultGroupBy].validNormalizedTypes.includes(
-                            field.normalizedType
-                          )
+                          groupByTypes[groupBy].validNormalizedTypes.includes(field.normalizedType)
                         ) {
                           options.push({
                             text: field.name,
@@ -159,6 +163,11 @@ export const GroupByExpression = ({
                       },
                       [firstFieldOption]
                     )}
+                    onBlur={() => {
+                      if (termField === undefined) {
+                        onChangeSelectedTermField('');
+                      }
+                    }}
                   />
                 </EuiFormRow>
               </EuiFlexItem>

diff --git a/...ck/legacy/plugins/triggers_actions_ui/np_ready/public/common/expression_items/of.test.tsx b/...ck/legacy/plugins/triggers_actions_ui/np_ready/public/common/expression_items/of.test.tsx
@@ -9,7 +9,7 @@ import { EuiPopoverTitle } from '@elastic/eui';
 import { OfExpression } from './of';
 
 describe('of expression', () => {
-  it('renders of buildin aggregation types', () => {
+  it('renders of builtin aggregation types', () => {
     const onChangeSelectedAggField = jest.fn();
     const wrapper = shallow(
       <OfExpression