Fixed a bug that displayed all alerts for a monitor on individual tri…

…ggers' flyouts. Fixed a bug that displayed incorrect source for the condition field on the alerts flyout. Fixed a bug that displayed incorrect severity on the alerts flyout. Fixed a bug that prevented selecting query-level monitor alerts 1 by 1. Fixed bug relating to validation of popovers when defining monitor queries. (opensearch-project#123) * Create opensearch-alerting-dashboards-plugin.release-notes-1.1.0.0.md (opensearch-project#101) * Create opensearch-alerting-dashboards-plugin.release-notes-1.1.0.0.md * Create opensearch-alerting-dashboards-plugin.release-notes-1.1.0.0.md Signed-off-by: Annie Lee <[email protected]> * Update opensearch-alerting-dashboards-plugin.release-notes-1.1.0.0.md Signed-off-by: Annie Lee <[email protected]> * Update comments Signed-off-by: Annie Lee <[email protected]> * Update version in package.json (opensearch-project#102) * Update package.json * Update opensearch-alerting-dashboards-plugin.release-notes-1.1.0.0.md Signed-off-by: Annie Lee <[email protected]> * Update opensearch-alerting-dashboards-plugin.release-notes-1.1.0.0.md Signed-off-by: Annie Lee <[email protected]> * Text updates (opensearch-project#105) * Add icon tooltip * Update text and rename MonitorDefinitionCard directory * Update Schedule.js * Update Schedule.js Signed-off-by: Annie Lee <[email protected]> * Update opensearch-alerting-dashboards-plugin.release-notes-1.1.0.0.md * Remove license Signed-off-by: Annie Lee <[email protected]> * Added badges to the package README, and the Uploads coverage job to the unit tests workflow. (opensearch-project#104) * Added badges to the package README, and the Uploads coverage job to the unit tests workflow. * Removing code coverage upload token. * Update jest unit tests (opensearch-project#112) * Update .opensearch_dashboards-plugin-helpers.json * Update snapshots * Update whereExpression.test and some snapshots * Update whereExpression.test and some snapshots Signed-off-by: Annie Lee <[email protected]> * Update snapshots * Update formikToTrigger.test.js * Update formikToTrigger.test.js * Update formikToTrigger.test.js * Add icon tooltip * Add test * Update tests * remove license * Update TriggerExpressions.test.js * Update Triggers.test.js * Update validation test * Update getOverviewStats.test.js * Update validate.test.js * Update helpers.test.js and remove unused import * Update Triggers.test.js * Update helpers.js * Update CreateMonitor test and clean up code * Update CreateMonitor test and clean up code Signed-off-by: Annie Lee <[email protected]> * Update release note and adding more tests * Add test and modify cypress common-utils branch * Update MonitorDefinitionCard.test.js.snap * Update cypress-workflow.yml * Update jest unit tests (opensearch-project#112) * Update .opensearch_dashboards-plugin-helpers.json * Update snapshots * Update whereExpression.test and some snapshots * Update whereExpression.test and some snapshots Signed-off-by: Annie Lee <[email protected]> * Update snapshots * Update formikToTrigger.test.js * Update formikToTrigger.test.js * Update formikToTrigger.test.js * Add icon tooltip * Add test * Update tests * remove license * Update TriggerExpressions.test.js * Update Triggers.test.js * Update validation test * Update getOverviewStats.test.js * Update validate.test.js * Update helpers.test.js and remove unused import * Update Triggers.test.js * Update helpers.js * Update CreateMonitor test and clean up code * Update CreateMonitor test and clean up code Signed-off-by: Annie Lee <[email protected]> * Update release note and adding more tests * Add test and modify cypress common-utils branch * Update MonitorDefinitionCard.test.js.snap * Update cypress-workflow.yml Signed-off-by: Annie Lee <[email protected]> * Fixed a few bugs * Update opensearch-alerting-dashboards-plugin.release-notes-1.1.0.0.md Signed-off-by: Annie Lee <[email protected]> * Fixed a bug that displayed all alerts for a monitor on individual trigger flyouts. Fixed a bug that diplayed incorrect source for the condition field on the alerts flyout. Fixed a bug that diplaying incorrect severity on the alerts flyout. * Updated release notes to reflect PR 122 bug fix. * Fixing number of alerts displayed on Monitors tab. * Update opensearch-alerting-dashboards-plugin.release-notes-1.1.0.0.md Signed-off-by: Annie Lee <[email protected]> * More bug fix Signed-off-by: Annie Lee <[email protected]> * Skip test based on modification Signed-off-by: Annie Lee <[email protected]> * Skip test based on modification Signed-off-by: Annie Lee <[email protected]> * Update popover windows to remove item when filed is not defined * Update field validation * Fixed a bug that prevented selecting query-level monitor alerts 1 by 1. Removed experimental code and comments. * Merge remote-tracking branch 'thomas/alertFlyoutBugFix' into bug-fix * Update Dashboard.js * Support data filter when using null operator * Update WhereExpression.js * Fixed a bug that was causing incorrect pagination display on alerts flyout. * Removed redundant validation from filter values that was generating error messages that prevented preview graphs from displaying data. * Removed redundant validation from filter values that was generating error messages that prevented preview graphs from displaying data. * Update metric error for query monitors * Update MetricExpression.js * Removed experimental dev code. * Updated release notes. Co-authored-by: Annie Lee <[email protected]> Co-authored-by: Annie Lee <[email protected]>
annie3431 · Oct 12, 2021 · 8dbe8cf · 8dbe8cf
1 parent 725c2d7
commit 8dbe8cf
Show file tree

Hide file tree

Showing 17 changed files with 90 additions and 43 deletions.
diff --git a/public/components/Flyout/flyouts/alertsDashboard.js b/public/components/Flyout/flyouts/alertsDashboard.js
@@ -80,6 +80,7 @@ const getBucketLevelGraphFilter = (trigger) => {
 
 const alertsDashboard = (payload) => {
   const {
+    alerts,
     history,
     httpClient,
     last_notification_time,
@@ -90,9 +91,8 @@ const alertsDashboard = (payload) => {
     monitor_name,
     notifications,
     setFlyout,
-    severity,
     start_time,
-    triggerId,
+    triggerID,
     trigger_name,
   } = payload;
   const monitor = _.get(_.find(monitors, { _id: monitor_id }), '_source');
@@ -104,10 +104,11 @@ const alertsDashboard = (payload) => {
     monitorType === MONITOR_TYPE.QUERY_LEVEL ? TRIGGER_TYPE.QUERY_LEVEL : TRIGGER_TYPE.BUCKET_LEVEL;
 
   let trigger = _.get(monitor, 'triggers', []).find((trigger) => {
-    return trigger[triggerType].triggerId === triggerId;
+    return trigger[triggerType].id === triggerID;
   });
   trigger = _.get(trigger, triggerType);
 
+  const severity = _.get(trigger, 'severity');
   const groupBy = _.get(monitor, MONITOR_GROUP_BY);
 
   const condition =
@@ -260,6 +261,7 @@ const alertsDashboard = (payload) => {
               monitorType={monitorType}
               perAlertView={true}
               groupBy={groupBy}
+              flyoutAlerts={alerts}
             />
           </EuiFlexItem>
         </EuiFlexGroup>

diff --git a/public/pages/CreateMonitor/components/MonitorExpressions/expressions/GroupByExpression.js b/public/pages/CreateMonitor/components/MonitorExpressions/expressions/GroupByExpression.js
@@ -30,7 +30,7 @@ import { EuiText, EuiButtonEmpty, EuiSpacer } from '@elastic/eui';
 import { getIndexFields } from './utils/dataTypes';
 import { getGroupByExpressionAllowedTypes } from './utils/helpers';
 import GroupByItem from './GroupByItem';
-import { GROUP_BY_ERROR } from './utils/constants';
+import { GROUP_BY_ERROR, QUERY_TYPE_GROUP_BY_ERROR } from './utils/constants';
 import { MONITOR_TYPE } from '../../../../../utils/constants';
 import { inputLimitText } from '../../../../../utils/helpers';
 import {
@@ -86,6 +86,8 @@ class GroupByExpression extends Component {
     const isBucketLevelMonitor = values.monitor_type === MONITOR_TYPE.BUCKET_LEVEL;
     if (!values.groupBy.length && isBucketLevelMonitor) {
       errors.groupBy = GROUP_BY_ERROR;
+    } else if (!isBucketLevelMonitor && values.groupBy.length > MAX_NUM_QUERY_LEVEL_GROUP_BYS) {
+      errors.groupBy = QUERY_TYPE_GROUP_BY_ERROR;
     } else {
       delete errors.groupBy;
     }

diff --git a/public/pages/CreateMonitor/components/MonitorExpressions/expressions/GroupByItem.js b/public/pages/CreateMonitor/components/MonitorExpressions/expressions/GroupByItem.js
@@ -10,6 +10,7 @@
  */
 
 import React, { useState } from 'react';
+import _ from 'lodash';
 import { EuiPopover, EuiBadge, EuiPopoverTitle } from '@elastic/eui';
 import { GroupByPopover } from './index';
 
@@ -18,6 +19,7 @@ export default function GroupByItem(
 ) {
   const [isPopoverOpen, setIsPopoverOpen] = useState(groupByItem === '');
   const closePopover = () => {
+    if (_.isEmpty(groupByItem)) arrayHelpers.remove(index);
     setIsPopoverOpen(false);
   };
 

diff --git a/public/pages/CreateMonitor/components/MonitorExpressions/expressions/MetricExpression.js b/public/pages/CreateMonitor/components/MonitorExpressions/expressions/MetricExpression.js
@@ -27,9 +27,9 @@
 import React, { Component } from 'react';
 import { connect } from 'formik';
 import { EuiText, EuiButtonEmpty, EuiSpacer, EuiBadge, EuiToolTip, EuiIcon } from '@elastic/eui';
+import _ from 'lodash';
 import { getIndexFields } from './utils/dataTypes';
 import { getMetricExpressionAllowedTypes, validateAggregationsDuplicates } from './utils/helpers';
-import _ from 'lodash';
 import {
   FORMIK_INITIAL_AGG_VALUES,
   METRIC_TOOLTIP_TEXT,
@@ -38,6 +38,7 @@ import { MetricItem } from './index';
 import { MONITOR_TYPE } from '../../../../../utils/constants';
 import { inputLimitText } from '../../../../../utils/helpers';
 import IconToolTip from '../../../../../components/IconToolTip';
+import { QUERY_TYPE_METRIC_ERROR } from './utils/constants';
 
 export const MAX_NUM_QUERY_LEVEL_METRICS = 1;
 export const MAX_NUM_BUCKET_LEVEL_METRICS = 5;
@@ -107,6 +108,11 @@ class MetricExpression extends Component {
 
     if (validateAggregationsDuplicates(aggregations)) {
       errors.aggregations = `You have defined duplicated metrics.`;
+    } else if (
+      MONITOR_TYPE.QUERY_LEVEL === monitorType &&
+      aggregations.length > MAX_NUM_QUERY_LEVEL_METRICS
+    ) {
+      errors.aggregations = QUERY_TYPE_METRIC_ERROR;
     } else {
       delete errors.aggregations;
     }

diff --git a/public/pages/CreateMonitor/components/MonitorExpressions/expressions/MetricItem.js b/public/pages/CreateMonitor/components/MonitorExpressions/expressions/MetricItem.js
@@ -10,6 +10,7 @@
  */
 
 import React, { useState } from 'react';
+import _ from 'lodash';
 import { EuiPopover, EuiBadge, EuiPopoverTitle } from '@elastic/eui';
 import MetricPopover from './MetricPopover';
 
@@ -18,6 +19,7 @@ export default function MetricItem(
 ) {
   const [isPopoverOpen, setIsPopoverOpen] = useState(aggregation.fieldName === '');
   const closePopover = () => {
+    if (_.isEmpty(aggregation.fieldName)) arrayHelpers.remove(index);
     setIsPopoverOpen(false);
   };
 

diff --git a/public/pages/CreateMonitor/components/MonitorExpressions/expressions/WhereExpression.js b/public/pages/CreateMonitor/components/MonitorExpressions/expressions/WhereExpression.js
@@ -50,12 +50,7 @@ import {
   isNullOperator,
   isRangeOperator,
 } from './utils/whereHelpers';
-import {
-  hasError,
-  isInvalid,
-  required,
-  validateRequiredNumber,
-} from '../../../../../utils/validate';
+import { hasError, isInvalid } from '../../../../../utils/validate';
 import {
   FormikComboBox,
   FormikSelect,
@@ -66,7 +61,6 @@ import { getFilteredIndexFields, getIndexFields } from './utils/dataTypes';
 import {
   FILTERS_TOOLTIP_TEXT,
   FORMIK_INITIAL_VALUES,
-  TIME_RANGE_TOOLTIP_TEXT,
 } from '../../../containers/CreateMonitor/utils/constants';
 import { DATA_TYPES } from '../../../../../utils/constants';
 import {
@@ -105,9 +99,10 @@ class WhereExpression extends Component {
     }
   };
 
-  handleOperatorChange = (e, field) => {
+  handleOperatorChange = (e, field, form) => {
     this.props.onMadeChanges();
     field.onChange(e);
+    form.setFieldError('where', undefined);
   };
 
   handleChangeWrapper = (e, field) => {
@@ -123,9 +118,16 @@ class WhereExpression extends Component {
     } = this.props;
     // Explicitly invoking validation, this component unmount after it closes.
     const fieldName = _.get(values, `${fieldPath}where.fieldName`, '');
+    const fieldOperator = _.get(values, `${fieldPath}where.operator`, 'is');
+    const fieldValue = _.get(values, `${fieldPath}where.fieldValue`, '');
     if (fieldName > 0) {
       await this.props.formik.validateForm();
     }
+    if (
+      _.isEmpty(fieldName) ||
+      (!isNullOperator(fieldOperator) && _.isEmpty(fieldValue.toString()))
+    )
+      this.resetValues();
     closeExpression(Expressions.WHERE);
   };
 
@@ -182,7 +184,6 @@ class WhereExpression extends Component {
       ) : (
         <FormikFieldNumber
           name={`${fieldPath}where.fieldValue`}
-          fieldProps={{ validate: validateRequiredNumber }}
           inputProps={{ onChange: this.handleChangeWrapper }}
           formRow
           rowProps={{ isInvalid, error: hasError }}
@@ -192,7 +193,6 @@ class WhereExpression extends Component {
       return (
         <FormikSelect
           name={`${fieldPath}where.fieldValue`}
-          fieldProps={{ validate: required }}
           inputProps={{
             onChange: this.handleChangeWrapper,
             options: WHERE_BOOLEAN_FILTERS,
@@ -204,7 +204,6 @@ class WhereExpression extends Component {
       return (
         <FormikFieldText
           name={`${fieldPath}where.fieldValue`}
-          fieldProps={{ validate: required }}
           inputProps={{ onChange: this.handleChangeWrapper, isInvalid }}
         />
       );
@@ -267,7 +266,7 @@ class WhereExpression extends Component {
                   iconSide="right"
                   iconType="cross"
                   iconOnClick={() => this.resetValues()}
-                  iconOnClickAriaLabel="Remove where filter"
+                  iconOnClickAriaLabel="Remove filter"
                   onClick={() => {
                     openExpression(Expressions.WHERE);
                   }}

diff --git a/public/pages/CreateMonitor/components/MonitorExpressions/expressions/utils/constants.js b/public/pages/CreateMonitor/components/MonitorExpressions/expressions/utils/constants.js
@@ -114,3 +114,6 @@ export const AGGREGATION_TYPES = [
 ];
 
 export const GROUP_BY_ERROR = 'Must specify at least 1 group by expression.';
+export const QUERY_TYPE_GROUP_BY_ERROR = 'Can have a maximum of 1 group by selections.';
+
+export const QUERY_TYPE_METRIC_ERROR = 'Can have a maximum of 1 metric selections.';
diff --git a/public/pages/CreateMonitor/components/VisualGraph/VisualGraph.js b/public/pages/CreateMonitor/components/VisualGraph/VisualGraph.js
@@ -128,7 +128,7 @@ export default class VisualGraph extends Component {
   renderAggregationXYPlot = (data, groupedData) => {
     const { annotation, thresholdValue, values, fieldName, aggregationType } = this.props;
     const { hint } = this.state;
-    const xDomain = getBufferedXDomain(data);
+    const xDomain = getBufferedXDomain(data, values);
     const yDomain = getYDomain(data);
     const annotations = getAnnotationData(xDomain, yDomain, thresholdValue);
     const xTitle = values.timeField;

diff --git a/public/pages/CreateMonitor/components/VisualGraph/utils/helpers.js b/public/pages/CreateMonitor/components/VisualGraph/utils/helpers.js
@@ -25,6 +25,7 @@
  */
 
 import _ from 'lodash';
+import moment from 'moment';
 
 import { selectOptionValueToText } from '../../MonitorExpressions/expressions/utils/helpers';
 import {
@@ -61,10 +62,14 @@ export function getXDomain(data) {
   return [minDate.x, maxDate.x];
 }
 
-export function getBufferedXDomain(data) {
+export function getBufferedXDomain(data, values) {
+  const { bucketValue, bucketUnitOfTime } = values;
   const minDate = data[0].x;
   const maxDate = data[data.length - 1].x;
-  const timeRange = maxDate - minDate;
+  // If minDate equals to maxDate, then use bucketValue and bucketUnitOfTime as timeRange.
+  let timeRange = maxDate - minDate;
+  if (!timeRange) timeRange = moment.duration(bucketValue, bucketUnitOfTime);
+
   const minDateBuffer = minDate - timeRange * X_DOMAIN_BUFFER;
   const maxDateBuffer = maxDate.getTime() + timeRange * X_DOMAIN_BUFFER;
   return [minDateBuffer, maxDateBuffer];
@@ -111,13 +116,13 @@ export function getDataFromResponse(response, fieldName, monitorType) {
   }
 }
 
-// Function for aggregation type monitors to get Map of data. 
-// The current response gives a large number of data aggregated in buckets, and this function returns the top n results with highest count of data points. 
-// The number n is based on the constant BAY_KEY_COUNT.  
+// Function for aggregation type monitors to get Map of data.
+// The current response gives a large number of data aggregated in buckets, and this function returns the top n results with highest count of data points.
+// The number n is based on the constant BAY_KEY_COUNT.
 export function getMapDataFromResponse(response, fieldName, groupByFields) {
   if (!response) return [];
   const buckets = _.get(response, 'aggregations.composite_agg.buckets', []);
-  let allData = new Map();
+  const allData = new Map();
   buckets.map((bucket) => {
     const dataPoint = getXYValuesByFieldName(bucket, fieldName);
     // Key of object is the string concat by group by field values
@@ -126,7 +131,7 @@ export function getMapDataFromResponse(response, fieldName, groupByFields) {
       ? allData.set(key, [dataPoint, ...allData.get(key)])
       : allData.set(key, [dataPoint]);
   });
-  let entryLength = [];
+  const entryLength = [];
   for (const [key, value] of allData.entries()) {
     allData.set(key, _.filter(value, filterInvalidYValues));
     entryLength.push({ key, length: value.length });
@@ -141,7 +146,9 @@ export function getMapDataFromResponse(response, fieldName, groupByFields) {
 
 export function getXYValuesByFieldName(bucket, fieldName) {
   const x = new Date(bucket.key.date);
-  const path = bucket[fieldName] ? `${fieldName}.value` : 'doc_count';
+  // Parse the fieldName containing "." to "_"
+  const parsedFieldName = fieldName.replace(/\./g, '_');
+  const path = bucket[parsedFieldName] ? `${parsedFieldName}.value` : 'doc_count';
   const y = _.get(bucket, path, null);
   return { x, y };
 }

diff --git a/public/pages/CreateMonitor/containers/DefineMonitor/DefineMonitor.js b/public/pages/CreateMonitor/containers/DefineMonitor/DefineMonitor.js
@@ -182,7 +182,7 @@ class DefineMonitor extends Component {
     // Default `count of documents` graph when using Bucket-level monitor
     let graphs = [
       <Fragment key={`multi-visual-graph-0`}>
-        <VisualGraph values={formikSnapshot} fieldName="doc_count" response={response} />,
+        <VisualGraph values={formikSnapshot} fieldName="doc_count" response={response} />
       </Fragment>,
     ];
 

diff --git a/public/pages/CreateTrigger/containers/DefineTrigger/utils/validation.js b/public/pages/CreateTrigger/containers/DefineTrigger/utils/validation.js
@@ -28,7 +28,7 @@ import _ from 'lodash';
 import { FORMIK_INITIAL_TRIGGER_VALUES, TRIGGER_TYPE } from '../../CreateTrigger/utils/constants';
 
 export const validateTriggerName = (triggers, triggerToEdit, fieldPath) => (value) => {
-  if (!value) return 'Required';
+  if (!value) return 'Required.';
   const nameExists = triggers.filter((trigger) => {
     const triggerId = _.get(
       trigger,
@@ -44,7 +44,7 @@ export const validateTriggerName = (triggers, triggerToEdit, fieldPath) => (valu
     return triggerToEditId !== triggerId && triggerName.toLowerCase() === value.toLowerCase();
   });
   if (nameExists.length > 0) {
-    return 'Trigger name already used';
+    return 'Trigger name already used.';
   }
   // TODO: character restrictions
   // TODO: character limits

diff --git a/public/pages/CreateTrigger/containers/DefineTrigger/utils/validation.test.js b/public/pages/CreateTrigger/containers/DefineTrigger/utils/validation.test.js
@@ -33,13 +33,13 @@ describe('validateTriggerName', () => {
   });
 
   test('returns Required string if falsy value', () => {
-    expect(validateTriggerName([], {})()).toBe('Required');
-    expect(validateTriggerName([], {})('')).toBe('Required');
+    expect(validateTriggerName([], {})()).toBe('Required.');
+    expect(validateTriggerName([], {})('')).toBe('Required.');
   });
   test('returns false if name already exists in monitor while creates new trigger', () => {
     const triggers = [{ [TRIGGER_TYPE.QUERY_LEVEL]: { id: '123', name: 'Test' } }];
     expect(validateTriggerName(triggers, { [TRIGGER_TYPE.QUERY_LEVEL]: {} })('Test')).toBe(
-      'Trigger name already used'
+      'Trigger name already used.'
     );
   });