From 8e1d0b9600a9046e0f278eb12c3b5ab4d12972a7 Mon Sep 17 00:00:00 2001 From: Amardeepsingh Siglani Date: Thu, 17 Aug 2023 10:19:50 -0700 Subject: [PATCH] updated composite monitors UX (#685) Signed-off-by: Amardeepsingh Siglani --- package.json | 3 +- .../DeleteModal/DeleteMonitorModal.tsx | 82 ++++++++++--------- .../AssociateMonitors/AssociateMonitors.js | 7 +- .../AssociateMonitors.test.js.snap | 11 +-- .../components/MonitorsList.js | 2 +- .../components/MonitorType/MonitorType.js | 2 +- .../__snapshots__/MonitorType.test.js.snap | 2 +- .../WorkflowDetails.test.js.snap | 11 +-- .../DefineCompositeLevelTrigger.js | 15 +--- .../NotificationConfigDialog.js | 3 +- .../TriggerNotifications.js | 19 +++-- .../DefineCompositeLevelTrigger.test.js.snap | 60 +++++--------- .../TriggerNotifications.test.js.snap | 13 +-- .../ChainedAlertDetails.tsx | 6 -- public/pages/Dashboard/utils/tableUtils.js | 13 +-- public/utils/helpers.js | 15 +++- server/services/MonitorService.js | 1 + 17 files changed, 118 insertions(+), 147 deletions(-) diff --git a/package.json b/package.json index dd1b40973..bd9f4d3d4 100644 --- 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 index b14f00ee1..6d969bc12 100644 --- 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); 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(`Delete monitor ${monitorNames[0]}?`); + const [warningBody, setWarningBody] = useState('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 ? +
    + {associatedWorkflows.map(({ id, name }) =>
  • {name}
    • )} +
+ : 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}`} +
    + {monitorNames.map((name, idx) =>
  • {name}
    • )} +
+ + ); } - - 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 ? -
    - {associatedWorkflows.map(({ id, name }) =>
  • {name}
    • )} -
- : null - } - - ) - allowDelete = false; - } - else if (monitorNames.length > 1) { - warningHeading = `Delete ${monitorNames.length} monitors?`; - warningBody = ( - <> - {`The following monitors will be permanently deleted. ${warningBody}`} -
    - {monitorNames.map((name, idx) =>
  • {name}
    • )} -
- - ) - } + }, [associatedWorkflows]); return ( diff --git a/public/pages/CreateMonitor/components/AssociateMonitors/AssociateMonitors.js b/public/pages/CreateMonitor/components/AssociateMonitors/AssociateMonitors.js index a08d50d55..06f7b3b78 100644 --- 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 ( - -

Delegate monitors

- + {titleTemplate('Delegate monitors')} 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/public/pages/CreateMonitor/components/AssociateMonitors/__snapshots__/AssociateMonitors.test.js.snap b/public/pages/CreateMonitor/components/AssociateMonitors/__snapshots__/AssociateMonitors.test.js.snap index b62d8bfdc..4abc447e1 100644 --- a/public/pages/CreateMonitor/components/AssociateMonitors/__snapshots__/AssociateMonitors.test.js.snap +++ b/public/pages/CreateMonitor/components/AssociateMonitors/__snapshots__/AssociateMonitors.test.js.snap @@ -2,14 +2,11 @@ exports[`AssociateMonitors renders 1`] = ` Array [ -
-

- Delegate monitors -

-
, + Delegate monitors + ,
diff --git a/public/pages/CreateMonitor/components/AssociateMonitors/components/MonitorsList.js b/public/pages/CreateMonitor/components/AssociateMonitors/components/MonitorsList.js index a6b776975..7664ffccc 100644 --- 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', }} > - + Composite monitors chain the outputs of different monitor types and focus trigger conditions to - reduce alert noise. + reduce alert noise and generate finer results. ); diff --git a/public/pages/CreateMonitor/components/MonitorType/__snapshots__/MonitorType.test.js.snap b/public/pages/CreateMonitor/components/MonitorType/__snapshots__/MonitorType.test.js.snap index a464e778f..f34d6e8e8 100644 --- 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 [
- 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.
diff --git a/public/pages/CreateMonitor/containers/WorkflowDetails/__snapshots__/WorkflowDetails.test.js.snap b/public/pages/CreateMonitor/containers/WorkflowDetails/__snapshots__/WorkflowDetails.test.js.snap index 967b66545..95085191f 100644 --- a/public/pages/CreateMonitor/containers/WorkflowDetails/__snapshots__/WorkflowDetails.test.js.snap +++ b/public/pages/CreateMonitor/containers/WorkflowDetails/__snapshots__/WorkflowDetails.test.js.snap @@ -249,14 +249,11 @@ exports[`WorkflowDetails renders 1`] = `
-
-

- Delegate monitors -

-
+ Delegate monitors +
diff --git a/public/pages/CreateTrigger/containers/DefineCompositeLevelTrigger/DefineCompositeLevelTrigger.js b/public/pages/CreateTrigger/containers/DefineCompositeLevelTrigger/DefineCompositeLevelTrigger.js index 1df6948a1..4cf833c81 100644 --- a/public/pages/CreateTrigger/containers/DefineCompositeLevelTrigger/DefineCompositeLevelTrigger.js +++ b/public/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) => ( - -
{title}
- {subTitle && ( - - {subTitle} - - )} - - -); +import { titleTemplate } from '../../../../utils/helpers'; const defaultRowProps = { label: titleTemplate('Trigger name'), diff --git a/public/pages/CreateTrigger/containers/DefineCompositeLevelTrigger/NotificationConfigDialog.js b/public/pages/CreateTrigger/containers/DefineCompositeLevelTrigger/NotificationConfigDialog.js index 8e365a86a..a27fe183c 100644 --- a/public/pages/CreateTrigger/containers/DefineCompositeLevelTrigger/NotificationConfigDialog.js +++ b/public/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 index 79457e5df..c270c9c0e 100644 --- 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={{`Notification ${actionIndex + 1}`}} paddingSize={'s'} extraAction={ - onRemoveNotification(actionIndex)} - size={'s'} - /> + + onRemoveNotification(actionIndex)} + size={'s'} + /> + } > -
-
- Trigger name -
-
-
+ Trigger name +
+

+ Trigger condition +

-
- Trigger condition -
-
+

When selected monitors meet the specified conditions for alert generation, the composite monitor triggers its own alert. -

+

-
@@ -213,16 +205,11 @@ exports[`DefineCompositeLevelTrigger renders 1`] = `
-
-
- Alert severity -
-
-
+ Alert severity +
-
-
- Notifications -
-
-
+ Notifications +
diff --git a/public/pages/CreateTrigger/containers/DefineCompositeLevelTrigger/__snapshots__/TriggerNotifications.test.js.snap b/public/pages/CreateTrigger/containers/DefineCompositeLevelTrigger/__snapshots__/TriggerNotifications.test.js.snap index 572f7a99d..c15871e64 100644 --- a/public/pages/CreateTrigger/containers/DefineCompositeLevelTrigger/__snapshots__/TriggerNotifications.test.js.snap +++ b/public/pages/CreateTrigger/containers/DefineCompositeLevelTrigger/__snapshots__/TriggerNotifications.test.js.snap @@ -2,16 +2,11 @@ exports[`TriggerNotifications renders 1`] = ` Array [ -
-
- Notifications -
-
-
, + Notifications + ,
, diff --git a/public/pages/Dashboard/components/ChainedAlertDetailsFlyout/ChainedAlertDetails.tsx b/public/pages/Dashboard/components/ChainedAlertDetailsFlyout/ChainedAlertDetails.tsx index b5fd8d2b2..070d21fa4 100644 --- a/public/pages/Dashboard/components/ChainedAlertDetailsFlyout/ChainedAlertDetails.tsx +++ b/public/pages/Dashboard/components/ChainedAlertDetailsFlyout/ChainedAlertDetails.tsx @@ -17,10 +17,6 @@ import { associatedAlertsTableColumns, renderTime } from '../../utils/tableUtils import _ from 'lodash'; export const ChainedAlertDetails = ({ alert, associatedAlerts }) => { - const [itemIdToExpandedRowMap, setItemIdToExpandedRowMap] = useState<{ - [key: string]: JSX.Element; - }>({}); - const overviewItems = [ { header: 'Trigger name', @@ -55,8 +51,6 @@ export const ChainedAlertDetails = ({ alert, associatedAlerts }) => { []} items={associatedAlerts} - itemId='id' - itemIdToExpandedRowMap={itemIdToExpandedRowMap} isExpandable={true} pagination={true} sorting={true} diff --git a/public/pages/Dashboard/utils/tableUtils.js b/public/pages/Dashboard/utils/tableUtils.js index 58c56a08b..8b54fb17e 100644 --- a/public/pages/Dashboard/utils/tableUtils.js +++ b/public/pages/Dashboard/utils/tableUtils.js @@ -227,19 +227,20 @@ export const associatedAlertsTableColumns = [ { field: 'severity', name: 'Severity', - sortable: false, + sortable: true, truncateText: false, width: '100px', }, { - field: 'monitor_name', name: 'Delegate monitor', - sortable: false, + sortable: true, truncateText: true, - render: (monitorName) => { + render: ({ monitor_id, monitor_name }) => { return ( - - {monitorName} + + + {monitor_name} + ); }, diff --git a/public/utils/helpers.js b/public/utils/helpers.js index 36e355510..357a83683 100644 --- a/public/utils/helpers.js +++ b/public/utils/helpers.js @@ -4,7 +4,7 @@ */ import React from 'react'; -import { EuiText } from '@elastic/eui'; +import { EuiText, EuiTitle } from '@elastic/eui'; import { htmlIdGenerator } from '@elastic/eui/lib/services'; import { displayAcknowledgedAlertsToast, @@ -123,3 +123,16 @@ export async function acknowledgeAlerts(httpClient, notifications, alerts) { return acknowledgePromises; } + +export const titleTemplate = (title, subTitle) => ( + <> + +

{title}

+ + {subTitle && ( + +

{subTitle}

+ + )} + +); diff --git a/server/services/MonitorService.js b/server/services/MonitorService.js index c38aac957..e45a7222e 100644 --- a/server/services/MonitorService.js +++ b/server/services/MonitorService.js @@ -155,6 +155,7 @@ export default class MonitorService { monitor = { ...monitor, associated_workflows, + associatedCompositeMonitorCnt: associated_workflows.length, }; } monitor = {