fixed due to comments

elastic · Dec 10, 2020 · 291a830 · 291a830
1 parent 2b6082e
commit 291a830
Show file tree

Hide file tree

Showing 5 changed files with 212 additions and 30 deletions.
diff --git a/x-pack/plugins/triggers_actions_ui/public/application/lib/alert_type_compare.test.ts b/x-pack/plugins/triggers_actions_ui/public/application/lib/alert_type_compare.test.ts
@@ -0,0 +1,182 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import { AlertTypeModel } from '../../types';
+import { alertTypeGroupCompare, alertTypeCompare } from './alert_type_compare';
+import { IsEnabledResult, IsDisabledResult } from './check_alert_type_enabled';
+
+test('should sort groups by containing enabled alert types first and then by name', async () => {
+  const alertTypes: Array<
+    [
+      string,
+      Array<{
+        id: string;
+        name: string;
+        checkEnabledResult: IsEnabledResult | IsDisabledResult;
+        alertTypeItem: AlertTypeModel;
+      }>
+    ]
+  > = [
+    [
+      'abc',
+      [
+        {
+          id: '1',
+          name: 'test2',
+          checkEnabledResult: { isEnabled: false, message: 'gold license' },
+          alertTypeItem: {
+            id: 'my-alert-type',
+            iconClass: 'test',
+            name: 'test-alert',
+            description: 'Alert when testing',
+            documentationUrl: 'https://localhost.local/docs',
+            validate: () => {
+              return { errors: {} };
+            },
+            alertParamsExpression: () => null,
+            requiresAppContext: false,
+          },
+        },
+      ],
+    ],
+    [
+      'bcd',
+      [
+        {
+          id: '2',
+          name: 'abc',
+          checkEnabledResult: { isEnabled: false, message: 'platinum license' },
+          alertTypeItem: {
+            id: 'my-alert-type',
+            iconClass: 'test',
+            name: 'test-alert',
+            description: 'Alert when testing',
+            documentationUrl: 'https://localhost.local/docs',
+            validate: () => {
+              return { errors: {} };
+            },
+            alertParamsExpression: () => null,
+            requiresAppContext: false,
+          },
+        },
+        {
+          id: '3',
+          name: 'cdf',
+          checkEnabledResult: { isEnabled: true },
+          alertTypeItem: {
+            id: 'disabled-alert-type',
+            iconClass: 'test',
+            name: 'test-alert',
+            description: 'Alert when testing',
+            documentationUrl: 'https://localhost.local/docs',
+            validate: () => {
+              return { errors: {} };
+            },
+            alertParamsExpression: () => null,
+            requiresAppContext: false,
+          },
+        },
+      ],
+    ],
+    [
+      'cde',
+      [
+        {
+          id: '4',
+          name: 'cde',
+          checkEnabledResult: { isEnabled: true },
+          alertTypeItem: {
+            id: 'my-alert-type',
+            iconClass: 'test',
+            name: 'test-alert',
+            description: 'Alert when testing',
+            documentationUrl: 'https://localhost.local/docs',
+            validate: () => {
+              return { errors: {} };
+            },
+            alertParamsExpression: () => null,
+            requiresAppContext: false,
+          },
+        },
+      ],
+    ],
+  ];
+
+  const groups = new Map<string, string>();
+  groups.set('abc', 'ABC');
+  groups.set('bcd', 'BCD');
+  groups.set('cde', 'CDE');
+
+  const result = [...alertTypes].sort((right, left) => alertTypeGroupCompare(right, left, groups));
+  expect(result[0]).toEqual(alertTypes[1]);
+  expect(result[1]).toEqual(alertTypes[2]);
+  expect(result[2]).toEqual(alertTypes[0]);
+});
+
+test('should sort alert types by enabled first and then by name', async () => {
+  const alertTypes: Array<{
+    id: string;
+    name: string;
+    checkEnabledResult: IsEnabledResult | IsDisabledResult;
+    alertTypeItem: AlertTypeModel;
+  }> = [
+    {
+      id: '1',
+      name: 'bcd',
+      checkEnabledResult: { isEnabled: false, message: 'gold license' },
+      alertTypeItem: {
+        id: 'my-alert-type',
+        iconClass: 'test',
+        name: 'test-alert',
+        description: 'Alert when testing',
+        documentationUrl: 'https://localhost.local/docs',
+        validate: () => {
+          return { errors: {} };
+        },
+        alertParamsExpression: () => null,
+        requiresAppContext: false,
+      },
+    },
+    {
+      id: '2',
+      name: 'abc',
+      checkEnabledResult: { isEnabled: false, message: 'platinum license' },
+      alertTypeItem: {
+        id: 'my-alert-type',
+        iconClass: 'test',
+        name: 'test-alert',
+        description: 'Alert when testing',
+        documentationUrl: 'https://localhost.local/docs',
+        validate: () => {
+          return { errors: {} };
+        },
+        alertParamsExpression: () => null,
+        requiresAppContext: false,
+      },
+    },
+    {
+      id: '3',
+      name: 'cdf',
+      checkEnabledResult: { isEnabled: true },
+      alertTypeItem: {
+        id: 'disabled-alert-type',
+        iconClass: 'test',
+        name: 'test-alert',
+        description: 'Alert when testing',
+        documentationUrl: 'https://localhost.local/docs',
+        validate: () => {
+          return { errors: {} };
+        },
+        alertParamsExpression: () => null,
+        requiresAppContext: false,
+      },
+    },
+  ];
+  const result = [...alertTypes].sort(alertTypeCompare);
+  expect(result[0]).toEqual(alertTypes[2]);
+  expect(result[1]).toEqual(alertTypes[1]);
+  expect(result[2]).toEqual(alertTypes[0]);
+});
diff --git a/x-pack/plugins/triggers_actions_ui/public/application/lib/alert_type_compare.ts b/x-pack/plugins/triggers_actions_ui/public/application/lib/alert_type_compare.ts
@@ -7,8 +7,8 @@
 import { AlertTypeModel } from '../../types';
 import { IsEnabledResult, IsDisabledResult } from './check_alert_type_enabled';
 
-export function alertTypeSolutionCompare(
-  a: [
+export function alertTypeGroupCompare(
+  left: [
     string,
     Array<{
       id: string;
@@ -17,7 +17,7 @@ export function alertTypeSolutionCompare(
       alertTypeItem: AlertTypeModel;
     }>
   ],
-  b: [
+  right: [
     string,
     Array<{
       id: string;
@@ -26,26 +26,31 @@ export function alertTypeSolutionCompare(
       alertTypeItem: AlertTypeModel;
     }>
   ],
-  solutions: Map<string, string> | undefined
+  groupNames: Map<string, string> | undefined
 ) {
-  // .sort(([a], [b]) =>
-  // solutions ? solutions.get(a)!.localeCompare(solutions.get(b)!) : a.localeCompare(b)
-  // )
-  const solutionA = a[0];
-  const solutionB = b[0];
-  const alertTypeItemsA = a[1].find((alertTypeItem) => alertTypeItem.checkEnabledResult.isEnabled);
-  const alertTypeItemsB = b[1].find((alertTypeItem) => alertTypeItem.checkEnabledResult.isEnabled);
+  const groupNameA = left[0];
+  const groupNameB = right[0];
+  const leftAlertTypesList = left[1];
+  const rightAlertTypesList = right[1];
 
-  if (!!alertTypeItemsA && !alertTypeItemsB) {
+  const hasEnabledAlertTypeInListLeft =
+    leftAlertTypesList.find((alertTypeItem) => alertTypeItem.checkEnabledResult.isEnabled) !==
+    undefined;
+
+  const hasEnabledAlertTypeInListRight =
+    rightAlertTypesList.find((alertTypeItem) => alertTypeItem.checkEnabledResult.isEnabled) !==
+    undefined;
+
+  if (hasEnabledAlertTypeInListLeft && !hasEnabledAlertTypeInListRight) {
     return -1;
   }
-  if (!alertTypeItemsA && !!alertTypeItemsB) {
+  if (!hasEnabledAlertTypeInListLeft && hasEnabledAlertTypeInListRight) {
     return 1;
   }
 
-  return solutions
-    ? solutions.get(solutionA)!.localeCompare(solutions.get(solutionB)!)
-    : solutionA.localeCompare(solutionB);
+  return groupNames
+    ? groupNames.get(groupNameA)!.localeCompare(groupNames.get(groupNameB)!)
+    : groupNameA.localeCompare(groupNameB);
 }
 
 export function alertTypeCompare(

diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_form.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_form.tsx
@@ -70,7 +70,7 @@ import { recoveredActionGroupMessage } from '../../constants';
 import { getDefaultsForActionParams } from '../../lib/get_defaults_for_action_params';
 import { IsEnabledResult, IsDisabledResult } from '../../lib/check_alert_type_enabled';
 import { checkAlertTypeEnabled } from '../../lib/check_alert_type_enabled';
-import { alertTypeCompare, alertTypeSolutionCompare } from '../../lib/alert_type_compare';
+import { alertTypeCompare, alertTypeGroupCompare } from '../../lib/alert_type_compare';
 import { VIEW_LICENSE_OPTIONS_LINK } from '../../../common/constants';
 
 const ENTER_KEY = 13;
@@ -394,7 +394,7 @@ export const AlertForm = ({
   );
 
   const alertTypeNodes = Object.entries(alertTypesByProducer)
-    .sort((a, b) => alertTypeSolutionCompare(a, b, solutions))
+    .sort((a, b) => alertTypeGroupCompare(a, b, solutions))
     .map(([solution, items], groupIndex) => (
       <Fragment key={`group${groupIndex}`}>
         <EuiFlexGroup

diff --git a/...s/triggers_actions_ui/public/application/sections/alerts_list/components/alerts_list.scss b/...s/triggers_actions_ui/public/application/sections/alerts_list/components/alerts_list.scss
@@ -4,8 +4,4 @@
   .actAlertsList__tableCellDisabled {
     color: $euiColorDarkShade;
   }
-
-  .euiLink + .euiIcon {
-    margin-left: $euiSizeXS;
-  }
 }
diff --git a/...ns/triggers_actions_ui/public/application/sections/alerts_list/components/alerts_list.tsx b/...ns/triggers_actions_ui/public/application/sections/alerts_list/components/alerts_list.tsx
@@ -26,7 +26,7 @@ import {
   EuiButtonEmpty,
   EuiHealth,
   EuiText,
-  EuiIconTip,
+  EuiToolTip,
 } from '@elastic/eui';
 import { useHistory } from 'react-router-dom';
 
@@ -274,14 +274,13 @@ export const AlertsList: React.FunctionComponent = () => {
         return checkEnabledResult.isEnabled ? (
           link
         ) : (
-          <Fragment>
+          <EuiToolTip
+            position="top"
+            data-test-subj={`${alert.id}-disabledTooltip`}
+            content={checkEnabledResult.message}
+          >
             {link}
-            <EuiIconTip
-              type="questionInCircle"
-              content={checkEnabledResult.message}
-              position="right"
-            />
-          </Fragment>
+          </EuiToolTip>
         );
       },
     },