updated composite monitors UX (#685) (#687)

(cherry picked from commit 8e1d0b9) Signed-off-by: Amardeepsingh Siglani <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
opensearch-project · Aug 17, 2023 · 83a2507 · 83a2507
1 parent 8f34066
commit 83a2507
Show file tree

Hide file tree

Showing 17 changed files with 118 additions and 147 deletions.
diff --git a/package.json b/package.json
@@ -36,8 +36,7 @@
     "@elastic/eslint-import-resolver-kibana": "link:../../packages/osd-eslint-import-resolver-opensearch-dashboards",
     "cypress": "^6.0.0",
     "husky": "^3.0.0",
-    "lint-staged": "^10.2.0",
-    "@babel/plugin-transform-modules-commonjs": "^7.16.5"
+    "lint-staged": "^10.2.0"
   },
   "dependencies": {
     "brace": "0.11.1",

diff --git a/public/components/DeleteModal/DeleteMonitorModal.tsx b/public/components/DeleteModal/DeleteMonitorModal.tsx
@@ -28,49 +28,51 @@ export const DeleteMonitorModal = ({
   }: DeleteModalProps) => {
   const [associatedWorkflows, setAssociatedWorkflows] = useState<undefined | any[]>(undefined);
   const monitorNames = monitors.map(monitor => monitor.name);
-  let warningHeading = `Delete monitor ${monitorNames[0]}?`;
-  let warningBody: React.ReactNode = 'This action cannot be undone.';
-  let allowDelete = true;
+  const [warningHeading, setWarningHeading] = useState<string>(`Delete monitor ${monitorNames[0]}?`);
+  const [warningBody, setWarningBody] = useState<React.ReactNode>('This action cannot be undone.');
+  const [allowDelete, setAllowDelete] = useState(true);
 
-  if (monitors.length === 1 && monitors[0].associatedCompositeMonitorCnt > 0) {
-    if (monitors[0].associated_workflows) {
-      setAssociatedWorkflows(monitors[0].associated_workflows);
+  useEffect(() => {
+    if (monitors.length === 1 && monitors[0].associatedCompositeMonitorCnt > 0) {
+      if (monitors[0].associated_workflows) {
+        setAssociatedWorkflows(monitors[0].associated_workflows);
+      }
+      else {
+        httpClient?.get(`../api/alerting/monitors/${monitors[0].id}`)
+        .then((res: any) => {
+          setAssociatedWorkflows(res.resp.associated_workflows);
+        })
+        .catch((err :any) => {
+          console.error('err', err);
+        });
+      }
+
+      setWarningHeading(`Unable to delete ${monitorNames[0]}`);
+      setWarningBody(
+        <>
+          {`The monitor ${monitorNames[0]} is currently being used as a delegate monitor for composite monitors. Unlink from the following composite monitors before deleting this monitor:`}
+          { associatedWorkflows ?
+                <ul>
+                  {associatedWorkflows.map(({ id, name }) => <li><EuiLink target='_blank' href={`${PLUGIN_NAME}#/monitors/${id}?type=workflow`}>{name}</EuiLink></li>)}
+                </ul>
+              : null
+          }
+        </>
+      );
+      setAllowDelete(false);
     }
-    else {
-      httpClient?.get(`../api/alerting/monitors/${monitors[0].id}`)
-      .then((res: any) => {
-        setAssociatedWorkflows(res.resp.associated_workflows);
-      })
-      .catch((err :any) => {
-        console.error('err', err);
-      });
+    else if (monitorNames.length > 1) {
+      setWarningHeading(`Delete ${monitorNames.length} monitors?`);
+      setWarningBody(
+        <>
+          {`The following monitors will be permanently deleted. ${warningBody}`}
+          <ul>
+            {monitorNames.map((name, idx) => <li key={idx}>{name}</li>)}
+          </ul>
+        </>
+      );
     }
-
-    warningHeading = `Unable to delete ${monitorNames[0]}`;
-    warningBody = (
-      <>
-        {`The monitor ${monitorNames[0]} is currently being used as a delegate monitor for composite monitors. Unlink from the following composite monitors before deleting this monitor:`}
-        { associatedWorkflows ?
-              <ul>
-                {associatedWorkflows.map(({ id, name }) => <li><EuiLink target='_blank' href={`${PLUGIN_NAME}#/monitors/${id}?type=workflow`}>{name}</EuiLink></li>)}
-              </ul>
-            : null
-        }
-      </>
-    )
-    allowDelete = false;
-  }
-  else if (monitorNames.length > 1) {
-    warningHeading = `Delete ${monitorNames.length} monitors?`;
-    warningBody = (
-      <>
-        {`The following monitors will be permanently deleted. ${warningBody}`}
-        <ul>
-          {monitorNames.map((name, idx) => <li key={idx}>{name}</li>)}
-        </ul>
-      </>
-    )
-  }
+  }, [associatedWorkflows]);
 
   return (
     <EuiOverlayMask>

diff --git a/public/pages/CreateMonitor/components/AssociateMonitors/AssociateMonitors.js b/public/pages/CreateMonitor/components/AssociateMonitors/AssociateMonitors.js
@@ -4,10 +4,11 @@
  */
 
 import React, { Fragment, useState, useEffect } from 'react';
-import { EuiLink, EuiSpacer, EuiText } from '@elastic/eui';
+import { EuiLink, EuiSpacer, EuiText, EuiTitle } from '@elastic/eui';
 import MonitorsList from './components/MonitorsList';
 import MonitorsEditor from './components/MonitorsEditor';
 import { monitorTypesForComposition } from '../../../../utils/constants';
+import { titleTemplate } from '../../../../utils/helpers';
 
 export const getMonitors = async (httpClient) => {
   const response = await httpClient.get('../api/alerting/monitors', {
@@ -49,9 +50,7 @@ const AssociateMonitors = ({ isDarkMode, values, httpClient, errors }) => {
 
   return (
     <Fragment>
-      <EuiText size={'m'} style={{ paddingBottom: '0px', marginBottom: '0px' }}>
-        <h4>Delegate monitors</h4>
-      </EuiText>
+      {titleTemplate('Delegate monitors')}
       <EuiText color={'subdued'} size={'xs'}>
         Delegate two or more monitors to run as part of this workflow. The order in which you select
         the monitors determines their order in the workflow. The monitor types per query, per

diff --git a/...s/CreateMonitor/components/AssociateMonitors/__snapshots__/AssociateMonitors.test.js.snap b/...s/CreateMonitor/components/AssociateMonitors/__snapshots__/AssociateMonitors.test.js.snap
@@ -2,14 +2,11 @@
 
 exports[`AssociateMonitors renders 1`] = `
 Array [
-  <div
-    class="euiText euiText--medium"
-    style="padding-bottom:0px;margin-bottom:0px"
+  <h4
+    class="euiTitle euiTitle--xsmall"
   >
-    <h4>
-      Delegate monitors
-    </h4>
-  </div>,
+    Delegate monitors
+  </h4>,
   <div
     class="euiText euiText--extraSmall"
   >

diff --git a/public/pages/CreateMonitor/components/AssociateMonitors/components/MonitorsList.js b/public/pages/CreateMonitor/components/AssociateMonitors/components/MonitorsList.js
@@ -269,7 +269,7 @@ const MonitorsList = ({ values, httpClient }) => {
                       bottom: '24px',
                     }}
                   >
-                    <EuiToolTip title={'Remove monitor'}>
+                    <EuiToolTip content={'Remove monitor'}>
                       <EuiButtonIcon
                         aria-label={'Delete selection'}
                         iconType={'trash'}

diff --git a/public/pages/CreateMonitor/components/MonitorType/MonitorType.js b/public/pages/CreateMonitor/components/MonitorType/MonitorType.js
@@ -78,7 +78,7 @@ const documentLevelDescription = // TODO DRAFT: confirm wording
 const compositeLevelDescription = (
   <EuiText color={'subdued'} size={'xs'} style={{ paddingBottom: '10px', paddingTop: '0px' }}>
     Composite monitors chain the outputs of different monitor types and focus trigger conditions to
-    reduce alert noise.
+    reduce alert noise and generate finer results.
   </EuiText>
 );
 

diff --git a/public/pages/CreateMonitor/components/MonitorType/__snapshots__/MonitorType.test.js.snap b/public/pages/CreateMonitor/components/MonitorType/__snapshots__/MonitorType.test.js.snap
@@ -276,7 +276,7 @@ Array [
               <div
                 class="euiTextColor euiTextColor--subdued"
               >
-                Composite monitors chain the outputs of different monitor types and focus trigger conditions to reduce alert noise.
+                Composite monitors chain the outputs of different monitor types and focus trigger conditions to reduce alert noise and generate finer results.
               </div>
             </div>
           </div>

diff --git a/...pages/CreateMonitor/containers/WorkflowDetails/__snapshots__/WorkflowDetails.test.js.snap b/...pages/CreateMonitor/containers/WorkflowDetails/__snapshots__/WorkflowDetails.test.js.snap
@@ -249,14 +249,11 @@ exports[`WorkflowDetails renders 1`] = `
     <div
       class="euiSpacer euiSpacer--xl"
     />
-    <div
-      class="euiText euiText--medium"
-      style="padding-bottom:0px;margin-bottom:0px"
+    <h4
+      class="euiTitle euiTitle--xsmall"
     >
-      <h4>
-        Delegate monitors
-      </h4>
-    </div>
+      Delegate monitors
+    </h4>
     <div
       class="euiText euiText--extraSmall"
     >

diff --git a/...pages/CreateTrigger/containers/DefineCompositeLevelTrigger/DefineCompositeLevelTrigger.js b/...pages/CreateTrigger/containers/DefineCompositeLevelTrigger/DefineCompositeLevelTrigger.js
@@ -6,25 +6,14 @@
 import React, { Component } from 'react';
 import PropTypes from 'prop-types';
 import _ from 'lodash';
-import { EuiSpacer, EuiText, EuiTitle, EuiAccordion, EuiButton } from '@elastic/eui';
+import { EuiSpacer, EuiTitle, EuiAccordion, EuiButton } from '@elastic/eui';
 import { FormikFieldText, FormikSelect } from '../../../../components/FormControls';
 import { hasError, isInvalid, required } from '../../../../utils/validate';
 import { DEFAULT_TRIGGER_NAME, SEVERITY_OPTIONS } from '../../utils/constants';
 import CompositeTriggerCondition from '../../components/CompositeTriggerCondition/CompositeTriggerCondition';
 import TriggerNotifications from './TriggerNotifications';
 import { FORMIK_COMPOSITE_INITIAL_TRIGGER_VALUES } from '../CreateTrigger/utils/constants';
-
-export const titleTemplate = (title, subTitle) => (
-  <EuiText>
-    <h5>{title}</h5>
-    {subTitle && (
-      <EuiText color={'subdued'} size={'xs'}>
-        {subTitle}
-      </EuiText>
-    )}
-    <EuiSpacer size={'s'} />
-  </EuiText>
-);
+import { titleTemplate } from '../../../../utils/helpers';
 
 const defaultRowProps = {
   label: titleTemplate('Trigger name'),

diff --git a/...ic/pages/CreateTrigger/containers/DefineCompositeLevelTrigger/NotificationConfigDialog.js b/...ic/pages/CreateTrigger/containers/DefineCompositeLevelTrigger/NotificationConfigDialog.js
@@ -13,14 +13,13 @@ import {
   EuiModalHeader,
   EuiModalHeaderTitle,
 } from '@elastic/eui';
-import { titleTemplate } from './DefineCompositeLevelTrigger';
 import Message from '../../components/Action/actions';
 import { FORMIK_INITIAL_ACTION_VALUES } from '../../utils/constants';
 import { getTriggerContext } from '../../utils/helper';
 import { formikToMonitor } from '../../../CreateMonitor/containers/CreateMonitor/utils/formikToMonitor';
 import _ from 'lodash';
 import { formikToTrigger } from '../CreateTrigger/utils/formikToTrigger';
-import { backendErrorNotification } from '../../../../utils/helpers';
+import { backendErrorNotification, titleTemplate } from '../../../../utils/helpers';
 import { checkForError } from '../ConfigureActions/ConfigureActions';
 import { TRIGGER_TYPE } from '../CreateTrigger/utils/constants';
 

diff --git a/public/pages/CreateTrigger/containers/DefineCompositeLevelTrigger/TriggerNotifications.js b/public/pages/CreateTrigger/containers/DefineCompositeLevelTrigger/TriggerNotifications.js
@@ -11,11 +11,12 @@ import {
   EuiAccordion,
   EuiHorizontalRule,
   EuiButtonIcon,
+  EuiToolTip,
 } from '@elastic/eui';
 import TriggerNotificationsContent from './TriggerNotificationsContent';
-import { titleTemplate } from './DefineCompositeLevelTrigger';
 import { MAX_CHANNELS_RESULT_SIZE, OS_NOTIFICATION_PLUGIN } from '../../../../utils/constants';
 import { CHANNEL_TYPES } from '../../utils/constants';
+import { titleTemplate } from '../../../../utils/helpers';
 
 const TriggerNotifications = ({
   httpClient,
@@ -115,13 +116,15 @@ const TriggerNotifications = ({
                 buttonContent={<EuiText>{`Notification ${actionIndex + 1}`}</EuiText>}
                 paddingSize={'s'}
                 extraAction={
-                  <EuiButtonIcon
-                    color={'danger'}
-                    aria-label={'Delete notification'}
-                    iconType={'trash'}
-                    onClick={() => onRemoveNotification(actionIndex)}
-                    size={'s'}
-                  />
+                  <EuiToolTip content={'Remove notification'}>
+                    <EuiButtonIcon
+                      color={'danger'}
+                      aria-label={'Delete notification'}
+                      iconType={'trash'}
+                      onClick={() => onRemoveNotification(actionIndex)}
+                      size={'s'}
+                    />
+                  </EuiToolTip>
                 }
               >
                 <TriggerNotificationsContent

diff --git a/...ainers/DefineCompositeLevelTrigger/__snapshots__/DefineCompositeLevelTrigger.test.js.snap b/...ainers/DefineCompositeLevelTrigger/__snapshots__/DefineCompositeLevelTrigger.test.js.snap
@@ -82,16 +82,11 @@ exports[`DefineCompositeLevelTrigger renders 1`] = `
                 class="euiFormLabel euiFormRow__label"
                 for="triggerDefinitions[undefined].name-form-row"
               >
-                <div
-                  class="euiText euiText--medium"
+                <h4
+                  class="euiTitle euiTitle--xsmall"
                 >
-                  <h5>
-                    Trigger name
-                  </h5>
-                  <div
-                    class="euiSpacer euiSpacer--s"
-                  />
-                </div>
+                  Trigger name
+                </h4>
               </label>
             </div>
             <div
@@ -129,24 +124,21 @@ exports[`DefineCompositeLevelTrigger renders 1`] = `
                 class="euiFormLabel euiFormRow__label"
                 for="triggerConditionFieldsContainer_undefined-form-row"
               >
+                <h4
+                  class="euiTitle euiTitle--xsmall"
+                >
+                  Trigger condition
+                </h4>
                 <div
-                  class="euiText euiText--medium"
+                  class="euiText euiText--extraSmall"
                 >
-                  <h5>
-                    Trigger condition
-                  </h5>
                   <div
-                    class="euiText euiText--extraSmall"
+                    class="euiTextColor euiTextColor--subdued"
                   >
-                    <div
-                      class="euiTextColor euiTextColor--subdued"
-                    >
+                    <p>
                       When selected monitors meet the specified conditions for alert generation, the composite monitor triggers its own alert.
-                    </div>
+                    </p>
                   </div>
-                  <div
-                    class="euiSpacer euiSpacer--s"
-                  />
                 </div>
               </label>
             </div>
@@ -213,16 +205,11 @@ exports[`DefineCompositeLevelTrigger renders 1`] = `
           <div
             class="euiSpacer euiSpacer--l"
           />
-          <div
-            class="euiText euiText--medium"
+          <h4
+            class="euiTitle euiTitle--xsmall"
           >
-            <h5>
-              Alert severity
-            </h5>
-            <div
-              class="euiSpacer euiSpacer--s"
-            />
-          </div>
+            Alert severity
+          </h4>
           <div
             class="euiFormRow"
             id="triggerDefinitions[undefined].severity-form-row-row"
@@ -297,16 +284,11 @@ exports[`DefineCompositeLevelTrigger renders 1`] = `
           <div
             class="euiSpacer euiSpacer--xl"
           />
-          <div
-            class="euiText euiText--medium"
+          <h4
+            class="euiTitle euiTitle--xsmall"
           >
-            <h5>
-              Notifications
-            </h5>
-            <div
-              class="euiSpacer euiSpacer--s"
-            />
-          </div>
+            Notifications
+          </h4>
           <div
             class="euiSpacer euiSpacer--m"
           />

diff --git a/...er/containers/DefineCompositeLevelTrigger/__snapshots__/TriggerNotifications.test.js.snap b/...er/containers/DefineCompositeLevelTrigger/__snapshots__/TriggerNotifications.test.js.snap
@@ -2,16 +2,11 @@
 
 exports[`TriggerNotifications renders 1`] = `
 Array [
-  <div
-    class="euiText euiText--medium"
+  <h4
+    class="euiTitle euiTitle--xsmall"
   >
-    <h5>
-      Notifications
-    </h5>
-    <div
-      class="euiSpacer euiSpacer--s"
-    />
-  </div>,
+    Notifications
+  </h4>,
   <div
     class="euiSpacer euiSpacer--m"
   />,
-Original file line number
+Diff line change
@@ Expand Up / @@ -276,7 +276,7 @@ Array [ @@
                   <div
                     class="euiTextColor euiTextColor--subdued"
                   >
-                    Composite monitors chain the outputs of different monitor types and focus trigger conditions to reduce alert noise.
+                    Composite monitors chain the outputs of different monitor types and focus trigger conditions to reduce alert noise and generate finer results.
                   </div>
                 </div>
               </div>
@@ Expand Down @@