[Composite monitor] UX updates (#679)

* updated composite monitors ux Signed-off-by: Amardeepsingh Siglani <amardeep7194@gmail.com> * updated tests Signed-off-by: Amardeepsingh Siglani <amardeep7194@gmail.com> * remove expression when monitor unselected in list Signed-off-by: Amardeepsingh Siglani <amardeep7194@gmail.com> * addressed PR comments Signed-off-by: Amardeepsingh Siglani <amardeep7194@gmail.com> * fixed cypress test Signed-off-by: Amardeepsingh Siglani <amardeep7194@gmail.com> --------- Signed-off-by: Amardeepsingh Siglani <amardeep7194@gmail.com>
opensearch-project · Aug 9, 2023 · 5025ae3 · 5025ae3
1 parent 7f6171c
commit 5025ae3
Show file tree

Hide file tree

Showing 19 changed files with 431 additions and 157 deletions.
diff --git a/cypress/integration/composite_level_monitor_spec.js b/cypress/integration/composite_level_monitor_spec.js
@@ -61,12 +61,11 @@ describe('CompositeLevelMonitor', () => {
       cy.get('input[name="name"]').type(SAMPLE_VISUAL_EDITOR_MONITOR);
 
       // Select associated monitors
-      cy.get('[data-test-subj="monitors_list_0"]')
-        .type('monitorOne', { delay: 50 })
-        .type('{enter}');
-      cy.get('[data-test-subj="monitors_list_1"]')
-        .type('monitorTwo', { delay: 50 })
-        .type('{enter}');
+      cy.get('[data-test-subj="monitors_list_0"]').type('monitorOne', { delay: 50 });
+      cy.get('[title="monitorOne"]').click({ force: true });
+
+      cy.get('[data-test-subj="monitors_list_1"]').type('monitorTwo', { delay: 50 });
+      cy.get('[title="monitorTwo"]').click({ force: true });
 
       cy.get('button').contains('Add trigger').click({ force: true });
 
@@ -138,9 +137,8 @@ describe('CompositeLevelMonitor', () => {
 
       cy.get('button').contains('Add another monitor').click({ force: true });
 
-      cy.get('[data-test-subj="monitors_list_2"]')
-        .type('monitorThree', { delay: 50 })
-        .type('{enter}');
+      cy.get('[data-test-subj="monitors_list_2"]').type('monitorThree', { delay: 50 });
+      cy.get('[title="monitorThree"]').click({ force: true });
 
       cy.get('button').contains('Composite trigger').click({ force: true });
 

diff --git a/public/pages/CreateMonitor/components/AssociateMonitors/AssociateMonitors.js b/public/pages/CreateMonitor/components/AssociateMonitors/AssociateMonitors.js
@@ -29,7 +29,11 @@ export const getMonitors = async (httpClient) => {
           monitor.monitor?.type === 'monitor' &&
           monitorTypesForComposition.has(monitor.monitor?.monitor_type)
       )
-      .map((monitor) => ({ monitor_id: monitor.id, monitor_name: monitor.name }));
+      .map((monitor) => ({
+        monitor_id: monitor.id,
+        monitor_name: monitor.name,
+        monitor_type: monitor.monitor?.monitor_type,
+      }));
   } else {
     console.log('error getting monitors:', response);
     return [];

diff --git a/public/pages/CreateMonitor/components/AssociateMonitors/components/MonitorsList.js b/public/pages/CreateMonitor/components/AssociateMonitors/components/MonitorsList.js
@@ -3,7 +3,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import React, { Fragment, useState, useEffect } from 'react';
+import React, { Fragment, useState, useEffect, useCallback } from 'react';
 import * as _ from 'lodash';
 import {
   EuiButton,
@@ -22,6 +22,7 @@ import {
 import { DEFAULT_ASSOCIATED_MONITORS_VALUE } from '../../../containers/CreateMonitor/utils/constants';
 import { getMonitors } from '../AssociateMonitors';
 import { required } from '../../../../../utils/validate';
+import { getItemLevelType } from '../../../../Monitors/containers/Monitors/utils/helpers';
 
 const MonitorsList = ({ values, httpClient }) => {
   const formikFieldName = 'associatedMonitorsList';
@@ -81,9 +82,10 @@ const MonitorsList = ({ values, httpClient }) => {
   };
 
   const monitorsToOptions = (monitors) =>
-    monitors.map((monitor) => ({
-      label: monitor.monitor_name,
-      value: monitor.monitor_id,
+    monitors.map(({ monitor_name, monitor_id, monitor_type }) => ({
+      label: monitor_name,
+      value: monitor_id,
+      monitor_type,
     }));
 
   const onChange = (options, monitorIdx, form) => {
@@ -173,6 +175,22 @@ const MonitorsList = ({ values, httpClient }) => {
 
   const isValid = () => Object.keys(selection).length > 1;
 
+  const getGroupedOptions = useCallback(() => {
+    const monitorsByType = {};
+    options.forEach((option) => {
+      const { monitor_type } = option;
+      monitorsByType[monitor_type] = monitorsByType[monitor_type] || [];
+      monitorsByType[monitor_type].push(option);
+    });
+
+    return Object.entries(monitorsByType).map(([monitorType, monitors]) => {
+      return {
+        label: `${getItemLevelType(monitorType)} monitor(s)`,
+        options: monitors,
+      };
+    });
+  }, [options]);
+
   return (
     <FormikInputWrapper
       name={formikFieldName}
@@ -206,7 +224,7 @@ const MonitorsList = ({ values, httpClient }) => {
                       placeholder: 'Select a monitor',
                       onChange: (options, field, form) => onChange(options, monitorIdx, form),
                       onBlur: (e, field, form) => onBlur(e, field, form),
-                      options: options,
+                      options: getGroupedOptions(),
                       singleSelection: { asPlainText: true },
                       selectedOptions: selection[monitorIdx] ? [selection[monitorIdx]] : undefined,
                       'data-test-subj': `monitors_list_${monitorIdx}`,

diff --git a/public/pages/CreateMonitor/components/MonitorDefinitionCard/MonitorDefinitionCard.js b/public/pages/CreateMonitor/components/MonitorDefinitionCard/MonitorDefinitionCard.js
@@ -9,7 +9,7 @@ import FormikCheckableCard from '../../../../components/FormControls/FormikCheck
 import { OS_AD_PLUGIN, MONITOR_TYPE, SEARCH_TYPE } from '../../../../utils/constants';
 import { URL } from '../../../../../utils/constants';
 import _ from 'lodash';
-import { conditionToExpressions } from '../../../CreateTrigger/components/CompositeTriggerCondition/ExpressionBuilder';
+import { conditionToExpressions } from '../../../CreateTrigger/utils/helper';
 
 const MONITOR_DEFINITION_CARD_WIDTH = '275';
 

diff --git a/public/pages/CreateMonitor/containers/CreateMonitor/utils/monitorToFormik.js b/public/pages/CreateMonitor/containers/CreateMonitor/utils/monitorToFormik.js
@@ -11,7 +11,7 @@ import {
   DOC_LEVEL_INPUT_FIELD,
   QUERY_STRING_QUERY_OPERATORS,
 } from '../../../components/DocumentLevelMonitorQueries/utils/constants';
-import { conditionToExpressions } from '../../../../CreateTrigger/components/CompositeTriggerCondition/ExpressionBuilder';
+import { conditionToExpressions } from '../../../../CreateTrigger/utils/helper';
 
 // Convert Monitor JSON to Formik values used in UI forms
 export default function monitorToFormik(monitor) {

diff --git a/public/pages/CreateTrigger/components/CompositeTriggerCondition/ExpressionBuilder.js b/public/pages/CreateTrigger/components/CompositeTriggerCondition/ExpressionBuilder.js
@@ -12,43 +12,7 @@ import {
 import * as _ from 'lodash';
 import { FormikFormRow, FormikInputWrapper } from '../../../../components/FormControls';
 import { getMonitors } from '../../../CreateMonitor/components/AssociateMonitors/AssociateMonitors';
-
-export const conditionToExpressions = (condition = '', monitors) => {
-  if (!condition.length) return [];
-
-  const conditionMap = {
-    '&&': 'AND',
-    '||': 'OR',
-    '!': 'NOT',
-    '': '',
-    '&& !': 'AND NOT',
-    '|| !': 'OR NOT',
-  };
-  const queryToExpressionRegex = new RegExp(
-    /(!|| && || \|\| || && \!|| \|\| \!)?(monitor\[id=(.*?)\])/,
-    'gm'
-  );
-  const matcher = condition.matchAll(queryToExpressionRegex);
-  let match;
-  let expressions = [];
-  let counter = 0;
-  while ((match = matcher.next().value)) {
-    if (counter && !match[1]) return []; // Didn't find condition after the first match
-
-    const monitorId = match[3]?.trim(); // match [3] is the monitor_id
-    const monitor = monitors.filter((mon) => mon.monitor_id === monitorId);
-    expressions.push({
-      description: conditionMap[match[1]?.trim()] || '', // match [1] is the description/condition
-      isOpen: false,
-      monitor_name: monitor[0]?.monitor_name,
-      monitor_id: monitorId,
-    });
-
-    counter++;
-  }
-
-  return expressions;
-};
+import { conditionToExpressions } from '../../utils/helper';
 
 const ExpressionBuilder = ({
   formikFieldPath = '',
@@ -121,12 +85,7 @@ const ExpressionBuilder = ({
     const condition = _.get(values, formikFullFieldName, '');
 
     let expressions = conditionToExpressions(condition, monitors);
-    if (
-      !edit &&
-      !_.get(touched, formikFullFieldValue, false) &&
-      triggerIndex === 0 &&
-      expressions.length === 0
-    ) {
+    if (!edit && !_.get(touched, formikFullFieldValue, false) && expressions.length === 0) {
       expressions = [];
       monitorOptions.forEach((monitor, index) => {
         expressions.push({
@@ -135,11 +94,28 @@ const ExpressionBuilder = ({
           monitor_name: monitor.label,
         });
       });
+    } else {
+      // verify that expressions has only selected monitors
+      for (let i = expressions.length - 1; i > -1; i--) {
+        const exp = expressions[i];
+        if (!monitorOptions.find((monitor) => monitor.monitor_id === exp.monitor_id)) {
+          if (expressions.length > 2) {
+            expressions.splice(i, 1);
+          } else {
+            expressions[i] = { ...(i === 0 ? DEFAULT_EXPRESSION : DEFAULT_NEXT_EXPRESSION) };
+          }
+        }
+      }
+    }
 
-      _.set(values, formikFullFieldName, expressionsToCondition(expressions));
+    if (expressions.length === 1) {
+      expressions.push({ ...DEFAULT_NEXT_EXPRESSION });
     }
 
-    setUsedExpressions(expressions?.length ? expressions : [DEFAULT_EXPRESSION]);
+    _.set(values, formikFullFieldName, expressionsToCondition(expressions));
+    setUsedExpressions(
+      expressions.length ? expressions : [{ ...DEFAULT_EXPRESSION }, { ...DEFAULT_NEXT_EXPRESSION }]
+    );
   };
 
   const expressionsToCondition = (expressions) => {
@@ -243,7 +219,7 @@ const ExpressionBuilder = ({
     if (hasInvalidExpression()) return 'Invalid expressions.';
   };
 
-  const renderOptions = (expression, idx = 0, form) => (
+  const renderOptions = (expression, hideDeleteButton, idx = 0, form) => (
     <EuiFlexGroup
       gutterSize="s"
       data-test-subj={`${formikFullFieldName}_${triggerIndex}_${idx}_options`}
@@ -262,18 +238,20 @@ const ExpressionBuilder = ({
         />
       </EuiFlexItem>
       <EuiFlexItem grow={false}>{renderMonitorOptions(expression, idx, form)}</EuiFlexItem>
-      <EuiFlexItem grow={false}>
-        <EuiToolTip content={'Remove monitor'}>
-          <EuiButtonIcon
-            data-test-subj={`selection-exp-field-item-remove-${triggerIndex}-${idx}`}
-            onClick={() => onRemoveExpression(form, idx)}
-            iconType={'trash'}
-            color="danger"
-            aria-label={'Remove condition'}
-            style={{ marginTop: '4px' }}
-          />
-        </EuiToolTip>
-      </EuiFlexItem>
+      {!hideDeleteButton && (
+        <EuiFlexItem grow={false}>
+          <EuiToolTip content={'Remove monitor'}>
+            <EuiButtonIcon
+              data-test-subj={`selection-exp-field-item-remove-${triggerIndex}-${idx}`}
+              onClick={() => onRemoveExpression(form, idx)}
+              iconType={'trash'}
+              color="danger"
+              aria-label={'Remove condition'}
+              style={{ marginTop: '4px' }}
+            />
+          </EuiToolTip>
+        </EuiFlexItem>
+      )}
     </EuiFlexGroup>
   );
 
@@ -355,7 +333,7 @@ const ExpressionBuilder = ({
                   panelPaddingSize="s"
                   anchorPosition="upCenter"
                 >
-                  {renderOptions(expression, idx, form)}
+                  {renderOptions(expression, usedExpressions.length <= 2, idx, form)}
                 </EuiPopover>
               </EuiFlexItem>
             ))}

diff --git a/public/pages/CreateTrigger/utils/helper.js b/public/pages/CreateTrigger/utils/helper.js
@@ -65,3 +65,40 @@ export const getTriggerContext = (executeResponse, monitor, values, triggerIndex
     monitor: monitor,
   };
 };
+
+export const conditionToExpressions = (condition = '', monitors) => {
+  if (!condition.length) return [];
+
+  const conditionMap = {
+    '&&': 'AND',
+    '||': 'OR',
+    '!': 'NOT',
+    '': '',
+    '&& !': 'AND NOT',
+    '|| !': 'OR NOT',
+  };
+  const queryToExpressionRegex = new RegExp(
+    /(!|| && || \|\| || && \!|| \|\| \!)?(monitor\[id=(.*?)\])/,
+    'gm'
+  );
+  const matcher = condition.matchAll(queryToExpressionRegex);
+  let match;
+  let expressions = [];
+  let counter = 0;
+  while ((match = matcher.next().value)) {
+    if (counter && !match[1]) return []; // Didn't find condition after the first match
+
+    const monitorId = match[3]?.trim(); // match [3] is the monitor_id
+    const monitor = monitors.filter((mon) => mon.monitor_id === monitorId);
+    expressions.push({
+      description: conditionMap[match[1]?.trim()] || '', // match [1] is the description/condition
+      isOpen: false,
+      monitor_name: monitor[0]?.monitor_name,
+      monitor_id: monitorId,
+    });
+
+    counter++;
+  }
+
+  return expressions;
+};
diff --git a/public/pages/Dashboard/components/ChainedAlertDetailsFlyout/ChainedAlertDetailsFlyout.tsx b/public/pages/Dashboard/components/ChainedAlertDetailsFlyout/ChainedAlertDetailsFlyout.tsx
@@ -19,7 +19,20 @@ export const ChainedAlertDetailsFlyout = ({ closeFlyout, alert, httpClient }) =>
   const [associatedAlerts, setAssociatedAlerts] = useState([]);
 
   useEffect(() => {
-    httpClient.get('../api/alerting/workflows/alerts', { query: { workflowIds: alert.workflow_id, getAssociatedAlerts: true }})
+    httpClient.get('../api/alerting/workflows/alerts', { 
+      query: { 
+        workflowIds: alert.workflow_id,
+        getAssociatedAlerts: true,
+        sortString: 'start_time',
+        sortOrder: 'desc',
+        startIndex: 0,
+        size: 1000,
+        alertIds: alert.id,
+        severityLevel: 'ALL',
+        alertState: 'ALL',
+        searchString: ''
+      }
+    })
     .then((response: any) => {
       if (response.ok) {
         const associatedAlertIds = new Set(alert.associated_alert_ids);