Document level monitor UX bug fixes (opensearch-project#226)

* Refactored updateMonitor API to no longer require seqNo or primaryTerm. Signed-off-by: AWSHurneyt <[email protected]> * Fixed bug that was preventing acknowledge modal from refreshing when acknowledging alerts. Fixed bug that was causing the acknowledge modal to render on the monitor details page instead of immediately acknowledging selected alerts. Signed-off-by: AWSHurneyt <[email protected]> * Fixed alerts flyout to render correct pluralization for conditions. Adjusted spacing on action component. Signed-off-by: AWSHurneyt <[email protected]> * Implemented logic to hide anomaly detection option when defining a doc level monitor. Signed-off-by: AWSHurneyt <[email protected]> * Fixed a bug impacting sending test notifications for doc level monitors. Signed-off-by: AWSHurneyt <[email protected]> * Refactored visual editor to no longer support spaces in tags and query names. Fixed a bug that preventing creating a doc level monitor with a notification channel. Signed-off-by: AWSHurneyt <[email protected]> * Updated snapshots. Signed-off-by: AWSHurneyt <[email protected]>
lezzago · Apr 26, 2022 · 3da1a0a · 3da1a0a
1 parent de11dd8
commit 3da1a0a
Show file tree

Hide file tree

Showing 21 changed files with 310 additions and 171 deletions.
diff --git a/public/components/Flyout/flyouts/components/AlertsDashboardFlyoutComponent.js b/public/components/Flyout/flyouts/components/AlertsDashboardFlyoutComponent.js
@@ -148,7 +148,7 @@ export default class AlertsDashboardFlyoutComponent extends Component {
       this.setState({ tabContent: this.renderAlertsTable() });
   }
 
-  getBucketLevelGraphConditions = (trigger) => {
+  getMultipleGraphConditions = (trigger) => {
     let conditions = _.get(trigger, 'condition.script.source');
     if (_.isEmpty(conditions)) {
       return '-';
@@ -514,11 +514,21 @@ export default class AlertsDashboardFlyoutComponent extends Component {
     const groupBy = _.get(monitor, MONITOR_GROUP_BY);
 
     const condition =
-      (searchType === SEARCH_TYPE.GRAPH && monitorType === MONITOR_TYPE.BUCKET_LEVEL) ||
-      MONITOR_TYPE.DOC_LEVEL
-        ? this.getBucketLevelGraphConditions(trigger)
+      searchType === SEARCH_TYPE.GRAPH &&
+      (monitorType === MONITOR_TYPE.BUCKET_LEVEL || monitorType === MONITOR_TYPE.DOC_LEVEL)
+        ? this.getMultipleGraphConditions(trigger)
         : _.get(trigger, 'condition.script.source', '-');
 
+    let displayMultipleConditions;
+    switch (monitorType) {
+      case MONITOR_TYPE.BUCKET_LEVEL:
+      case MONITOR_TYPE.DOC_LEVEL:
+        displayMultipleConditions = true;
+        break;
+      default:
+        displayMultipleConditions = false;
+    }
+
     const filters =
       monitorType === MONITOR_TYPE.BUCKET_LEVEL && searchType === SEARCH_TYPE.GRAPH
         ? this.getBucketLevelGraphFilter(trigger)
@@ -534,7 +544,15 @@ export default class AlertsDashboardFlyoutComponent extends Component {
         ? `${bucketValue} ${bucketUnitOfTime}`
         : '-';
 
-    const displayTableTabs = monitorType === MONITOR_TYPE.DOC_LEVEL;
+    let displayTableTabs;
+    switch (monitorType) {
+      case MONITOR_TYPE.DOC_LEVEL:
+        displayTableTabs = true;
+        break;
+      default:
+        displayTableTabs = false;
+        break;
+    }
     return (
       <div>
         <EuiFlexGroup>
@@ -590,9 +608,7 @@ export default class AlertsDashboardFlyoutComponent extends Component {
         <EuiFlexGroup>
           <EuiFlexItem>
             <EuiText size={'m'} data-test-subj={`alertsDashboardFlyout_conditions_${trigger_name}`}>
-              <strong>
-                {monitorType === MONITOR_TYPE.BUCKET_LEVEL ? 'Conditions' : 'Condition'}
-              </strong>
+              <strong>{displayMultipleConditions ? 'Conditions' : 'Condition'}</strong>
               <p style={{ whiteSpace: 'pre-wrap' }}>
                 {loadingMonitors || loading ? 'Loading conditions...' : condition}
               </p>

diff --git a/...ges/CreateMonitor/components/DocumentLevelMonitorQueries/ConfigureDocumentLevelQueries.js b/...ges/CreateMonitor/components/DocumentLevelMonitorQueries/ConfigureDocumentLevelQueries.js
@@ -8,7 +8,8 @@ import _ from 'lodash';
 import { connect, FieldArray } from 'formik';
 import { EuiButton, EuiSpacer } from '@elastic/eui';
 import { inputLimitText } from '../../../../utils/helpers';
-import DocumentLevelQuery, { getInitialQueryValues } from './DocumentLevelQuery';
+import DocumentLevelQuery from './DocumentLevelQuery';
+import { FORMIK_INITIAL_DOCUMENT_LEVEL_QUERY_VALUES } from '../../containers/CreateMonitor/utils/constants';
 
 export const MAX_QUERIES = 10; // TODO DRAFT: Placeholder limit
 
@@ -23,7 +24,8 @@ class ConfigureDocumentLevelQueries extends Component {
       dataTypes,
       formik: { values },
     } = this.props;
-    if (_.isEmpty(values.queries)) arrayHelpers.push(_.cloneDeep(getInitialQueryValues()));
+    if (_.isEmpty(values.queries))
+      arrayHelpers.push(_.cloneDeep(FORMIK_INITIAL_DOCUMENT_LEVEL_QUERY_VALUES));
     const numOfQueries = values.queries.length;
     return (
       <div>
@@ -44,7 +46,9 @@ class ConfigureDocumentLevelQueries extends Component {
           <EuiButton
             fill={false}
             size={'s'}
-            onClick={() => arrayHelpers.push(_.cloneDeep(getInitialQueryValues(numOfQueries)))}
+            onClick={() =>
+              arrayHelpers.push(_.cloneDeep(FORMIK_INITIAL_DOCUMENT_LEVEL_QUERY_VALUES))
+            }
             disabled={numOfQueries >= MAX_QUERIES}
           >
             {numOfQueries === 0 ? 'Add query' : 'Add another query'}

diff --git a/...s/CreateMonitor/components/DocumentLevelMonitorQueries/ConfigureDocumentLevelQueryTags.js b/...s/CreateMonitor/components/DocumentLevelMonitorQueries/ConfigureDocumentLevelQueryTags.js
@@ -5,9 +5,13 @@
 
 import React, { Component } from 'react';
 import { connect, FieldArray } from 'formik';
-import { EuiButtonEmpty } from '@elastic/eui';
+import { EuiButtonEmpty, EuiSpacer, EuiText } from '@elastic/eui';
 import { inputLimitText } from '../../../../utils/helpers';
-import DocumentLevelQueryTag from './DocumentLevelQueryTag';
+import DocumentLevelQueryTag, { DOC_LEVEL_TAG_TOOLTIP } from './DocumentLevelQueryTag';
+import IconToolTip from '../../../../components/IconToolTip';
+import _ from 'lodash';
+import { FormikFormRow } from '../../../../components/FormControls';
+import { hasError, isInvalid } from '../../../../utils/validate';
 
 export const MAX_TAGS = 10; // TODO DRAFT: Placeholder limit
 
@@ -19,45 +23,67 @@ class ConfigureDocumentLevelQueryTags extends Component {
 
   renderTags(arrayHelpers) {
     const {
-      formik: { values },
+      formik: { errors, values },
       formFieldName = '',
       query,
       queryIndex,
     } = this.props;
     const numOfTags = query.tags.length;
+    const tagsErrors = _.get(errors, `${formFieldName}.tags`, []);
+    const containsErrors = !_.isEmpty(tagsErrors);
     return (
       <div>
-        {values.queries[queryIndex].tags.map((tag, index) => {
-          return (
-            <span style={{ paddingRight: '5px' }} key={`${formFieldName}.tags.${index}`}>
-              <DocumentLevelQueryTag
-                tag={tag}
-                tagIndex={index}
-                arrayHelpers={arrayHelpers}
-                formFieldName={`${formFieldName}.tags.${index}`}
-              />
-            </span>
-          );
-        })}
-        <div>
-          <EuiButtonEmpty
-            size={'xs'}
-            onClick={() => arrayHelpers.push('')}
-            disabled={numOfTags >= MAX_TAGS}
-            style={{ paddingTop: '5px' }}
-          >
-            + Add tag
-          </EuiButtonEmpty>
-          {inputLimitText(numOfTags, MAX_TAGS, 'tag', 'tags')}
-        </div>
+        <EuiText size={'xs'} color={containsErrors ? 'danger' : undefined}>
+          <strong>Tags</strong>
+          <i> - optional </i>
+          <IconToolTip content={DOC_LEVEL_TAG_TOOLTIP} iconType={'questionInCircle'} />
+        </EuiText>
+        <EuiSpacer size={'s'} />
+
+        {_.isEmpty(query.tags) && (
+          <div>
+            <EuiText size={'xs'}>No tags defined.</EuiText>
+          </div>
+        )}
+
+        <FormikFormRow
+          form={this.props.formik}
+          name={`${formFieldName}.tags`}
+          rowProps={{ error: hasError, isInvalid }}
+        >
+          <div>
+            {values.queries[queryIndex].tags.map((tag, index) => {
+              return (
+                <span style={{ paddingRight: '5px' }} key={`${formFieldName}.tags.${index}`}>
+                  <DocumentLevelQueryTag
+                    tag={tag}
+                    tagIndex={index}
+                    arrayHelpers={arrayHelpers}
+                    formFieldName={`${formFieldName}.tags.${index}`}
+                  />
+                </span>
+              );
+            })}
+          </div>
+        </FormikFormRow>
+
+        <EuiButtonEmpty
+          size={'xs'}
+          onClick={() => arrayHelpers.push('')}
+          disabled={numOfTags >= MAX_TAGS}
+          style={{ paddingTop: '5px' }}
+        >
+          + Add tag
+        </EuiButtonEmpty>
+        {inputLimitText(numOfTags, MAX_TAGS, 'tag', 'tags')}
       </div>
     );
   }
 
   render() {
     const { formFieldName } = this.props;
     return (
-      <FieldArray name={`${formFieldName}.tags`} validateOnChange={false}>
+      <FieldArray name={`${formFieldName}.tags`} validateOnChange={true}>
         {(arrayHelpers) => this.renderTags(arrayHelpers)}
       </FieldArray>
     );

diff --git a/public/pages/CreateMonitor/components/DocumentLevelMonitorQueries/DocumentLevelQuery.js b/public/pages/CreateMonitor/components/DocumentLevelMonitorQueries/DocumentLevelQuery.js
@@ -4,32 +4,23 @@
  */
 
 import React, { Component } from 'react';
-import _ from 'lodash';
 import { connect } from 'formik';
-import {
-  EuiButton,
-  EuiFlexGroup,
-  EuiFlexItem,
-  EuiHorizontalRule,
-  EuiSpacer,
-  EuiText,
-} from '@elastic/eui';
+import { EuiButton, EuiFlexGroup, EuiFlexItem, EuiHorizontalRule, EuiSpacer } from '@elastic/eui';
 import { FormikComboBox, FormikFieldText, FormikSelect } from '../../../../components/FormControls';
-import { hasError, isInvalid, required } from '../../../../utils/validate';
-import { FORMIK_INITIAL_DOCUMENT_LEVEL_QUERY_VALUES } from '../../containers/CreateMonitor/utils/constants';
-import { DOC_LEVEL_TAG_TOOLTIP } from './DocumentLevelQueryTag';
-import IconToolTip from '../../../../components/IconToolTip';
+import {
+  hasError,
+  isInvalid,
+  required,
+  validateIllegalCharacters,
+} from '../../../../utils/validate';
 import ConfigureDocumentLevelQueryTags from './ConfigureDocumentLevelQueryTags';
 import { getIndexFields } from '../MonitorExpressions/expressions/utils/dataTypes';
 import { QUERY_OPERATORS } from '../../../Dashboard/components/FindingsDashboard/utils';
 
 const ALLOWED_DATA_TYPES = ['number', 'text', 'keyword', 'boolean'];
 
-export const getInitialQueryValues = (queryIndexNum = 0) =>
-  _.cloneDeep({
-    ...FORMIK_INITIAL_DOCUMENT_LEVEL_QUERY_VALUES,
-    queryName: `Query ${queryIndexNum + 1}`,
-  });
+// TODO DRAFT: implement validation
+export const ILLEGAL_QUERY_NAME_CHARACTERS = [' '];
 
 class DocumentLevelQuery extends Component {
   constructor(props) {
@@ -46,14 +37,19 @@ class DocumentLevelQuery extends Component {
             <FormikFieldText
               name={`${formFieldName}.queryName`}
               formRow
-              fieldProps={{ validate: required }} // TODO DRAFT: implement validation
+              fieldProps={{
+                validate: validateIllegalCharacters(ILLEGAL_QUERY_NAME_CHARACTERS),
+              }}
               rowProps={{
                 label: 'Query name',
                 style: { width: '300px' },
                 isInvalid,
                 error: hasError,
               }}
-              inputProps={{ isInvalid }}
+              inputProps={{
+                placeholder: 'Enter a name for the query',
+                isInvalid,
+              }}
             />
           </EuiFlexItem>
 
@@ -124,19 +120,6 @@ class DocumentLevelQuery extends Component {
 
         <EuiSpacer size={'m'} />
 
-        <EuiText size={'xs'}>
-          <strong>Tags</strong>
-          <i> - optional </i>
-          <IconToolTip content={DOC_LEVEL_TAG_TOOLTIP} iconType={'questionInCircle'} />
-        </EuiText>
-        <EuiSpacer size={'s'} />
-
-        {_.isEmpty(query.tags) && (
-          <div>
-            <EuiText size={'xs'}>No tags defined.</EuiText>
-          </div>
-        )}
-
         <ConfigureDocumentLevelQueryTags
           formFieldName={formFieldName}
           query={query}

diff --git a/public/pages/CreateMonitor/components/DocumentLevelMonitorQueries/DocumentLevelQueryTag.js b/public/pages/CreateMonitor/components/DocumentLevelMonitorQueries/DocumentLevelQueryTag.js
@@ -17,12 +17,15 @@ import {
   EuiSpacer,
 } from '@elastic/eui';
 import { FormikFieldText } from '../../../../components/FormControls';
-import { hasError, isInvalid, required } from '../../../../utils/validate';
+import { hasError, isInvalid, validateIllegalCharacters } from '../../../../utils/validate';
 import { EXPRESSION_STYLE, POPOVER_STYLE } from '../MonitorExpressions/expressions/utils/constants';
 
 export const DOC_LEVEL_TAG_TOOLTIP = 'Tags to associate with your queries.'; // TODO DRAFT: Placeholder wording
 export const TAG_PLACEHOLDER_TEXT = 'Enter the search term'; // TODO DRAFT: Placeholder wording
 
+// TODO DRAFT: What constraints should we implement?
+export const ILLEGAL_TAG_CHARACTERS = [' '];
+
 class DocumentLevelQueryTag extends Component {
   constructor(props) {
     super(props);
@@ -58,7 +61,9 @@ class DocumentLevelQueryTag extends Component {
         <FormikFieldText
           name={formFieldName}
           formRow
-          fieldProps={{ validate: required }} // TODO DRAFT: What constraints should we implement?
+          fieldProps={{
+            validate: validateIllegalCharacters(ILLEGAL_TAG_CHARACTERS),
+          }}
           rowProps={{
             style: { maxWidth: '100%' },
             isInvalid,
@@ -86,8 +91,21 @@ class DocumentLevelQueryTag extends Component {
   }
 
   render() {
-    const { arrayHelpers, tag = '', tagIndex = 0 } = this.props;
+    const {
+      formik: { errors },
+      arrayHelpers,
+      formFieldName,
+      tag = '',
+      tagIndex = 0,
+    } = this.props;
     const { isPopoverOpen } = this.state;
+
+    let errorText;
+    if (!_.isEmpty(tag)) errorText = validateIllegalCharacters(ILLEGAL_TAG_CHARACTERS)(tag);
+
+    if (_.isEmpty(errorText)) delete errors[formFieldName];
+    else _.set(errors, formFieldName, validateIllegalCharacters(ILLEGAL_TAG_CHARACTERS)(tag));
+
     return (
       <EuiPopover
         id={'tag-badge-popover'}

diff --git a/public/pages/CreateMonitor/components/MonitorDefinitionCard/MonitorDefinitionCard.js b/public/pages/CreateMonitor/components/MonitorDefinitionCard/MonitorDefinitionCard.js
@@ -18,8 +18,14 @@ const onChangeDefinition = (e, form) => {
 
 const MonitorDefinitionCard = ({ values, plugins }) => {
   const hasADPlugin = plugins.indexOf(OS_AD_PLUGIN) !== -1;
-  const isBucketLevelMonitor = values.monitor_type === MONITOR_TYPE.BUCKET_LEVEL;
-
+  let supportsADOption;
+  switch (values.monitor_type) {
+    case MONITOR_TYPE.QUERY_LEVEL:
+      supportsADOption = true;
+      break;
+    default:
+      supportsADOption = false;
+  }
   return (
     <div>
       <EuiText size={'xs'} style={{ paddingBottom: '0px', marginBottom: '0px' }}>
@@ -68,8 +74,8 @@ const MonitorDefinitionCard = ({ values, plugins }) => {
             }}
           />
         </EuiFlexItem>
-        {/*// Only show the anomaly detector option when anomaly detection plugin is present, but not for bucket-level monitors.*/}
-        {hasADPlugin && !isBucketLevelMonitor && (
+        {/*// Only show the anomaly detector option when anomaly detection plugin is present, and for supporting monitors.*/}
+        {hasADPlugin && supportsADOption && (
           <EuiFlexItem grow={false} style={{ width: `${MONITOR_DEFINITION_CARD_WIDTH}px` }}>
             <FormikCheckableCard
               name="searchTypeAD"