Rename alert status OK to Recovered and fix some UX issues around dis…

…abling a rule while being in an error state (elastic#98135) * Fix UX when alert is disabled and in an error state * Reset executionStatus to pending after enabling an alert * Renames alert instance status OK to Recovered * Fix end to end test * Update doc screenshot * Fix confusing test name * Remove flakiness in integration test Co-authored-by: Kibana Machine <[email protected]>
kibanamachine · May 13, 2021 · 412cf59 · 412cf59
1 parent facd1a4
commit 412cf59
Show file tree

Hide file tree

Showing 8 changed files with 96 additions and 5 deletions.
diff --git a/docs/user/alerting/images/rule-details-alerts-inactive.png b/docs/user/alerting/images/rule-details-alerts-inactive.png
diff --git a/x-pack/plugins/alerting/server/alerts_client/alerts_client.ts b/x-pack/plugins/alerting/server/alerts_client/alerts_client.ts
@@ -981,6 +981,11 @@ export class AlertsClient {
         ...this.apiKeyAsAlertAttributes(createdAPIKey, username),
         updatedBy: username,
         updatedAt: new Date().toISOString(),
+        executionStatus: {
+          status: 'pending',
+          lastExecutionDate: new Date().toISOString(),
+          error: null,
+        },
       });
       try {
         await this.unsecuredSavedObjectsClient.update('alert', id, updateAttributes, { version });

diff --git a/x-pack/plugins/alerting/server/alerts_client/tests/enable.test.ts b/x-pack/plugins/alerting/server/alerts_client/tests/enable.test.ts
@@ -248,6 +248,11 @@ describe('enable()', () => {
             },
           },
         ],
+        executionStatus: {
+          status: 'pending',
+          lastExecutionDate: '2019-02-12T21:01:22.479Z',
+          error: null,
+        },
       },
       {
         version: '123',
@@ -352,6 +357,11 @@ describe('enable()', () => {
             },
           },
         ],
+        executionStatus: {
+          status: 'pending',
+          lastExecutionDate: '2019-02-12T21:01:22.479Z',
+          error: null,
+        },
       },
       {
         version: '123',

diff --git a/...rs_actions_ui/public/application/sections/alert_details/components/alert_details.test.tsx b/...rs_actions_ui/public/application/sections/alert_details/components/alert_details.test.tsx
@@ -8,6 +8,8 @@
 import * as React from 'react';
 import uuid from 'uuid';
 import { shallow } from 'enzyme';
+import { mountWithIntl, nextTick } from '@kbn/test/jest';
+import { act } from '@testing-library/react';
 import { AlertDetails } from './alert_details';
 import { Alert, ActionType, AlertTypeModel, AlertType } from '../../../../types';
 import { EuiTitle, EuiBadge, EuiFlexItem, EuiSwitch, EuiButtonEmpty, EuiText } from '@elastic/eui';
@@ -463,6 +465,74 @@ describe('disable button', () => {
     handler!({} as React.FormEvent);
     expect(enableAlert).toHaveBeenCalledTimes(1);
   });
+
+  it('should reset error banner dismissal after re-enabling the alert', async () => {
+    const alert = mockAlert({
+      enabled: true,
+      executionStatus: {
+        status: 'error',
+        lastExecutionDate: new Date('2020-08-20T19:23:38Z'),
+        error: {
+          reason: AlertExecutionStatusErrorReasons.Execute,
+          message: 'Fail',
+        },
+      },
+    });
+
+    const alertType: AlertType = {
+      id: '.noop',
+      name: 'No Op',
+      actionGroups: [{ id: 'default', name: 'Default' }],
+      recoveryActionGroup,
+      actionVariables: { context: [], state: [], params: [] },
+      defaultActionGroupId: 'default',
+      producer: ALERTS_FEATURE_ID,
+      authorizedConsumers,
+      minimumLicenseRequired: 'basic',
+      enabledInLicense: true,
+    };
+
+    const disableAlert = jest.fn();
+    const enableAlert = jest.fn();
+    const wrapper = mountWithIntl(
+      <AlertDetails
+        alert={alert}
+        alertType={alertType}
+        actionTypes={[]}
+        {...mockAlertApis}
+        disableAlert={disableAlert}
+        enableAlert={enableAlert}
+      />
+    );
+
+    await act(async () => {
+      await nextTick();
+      wrapper.update();
+    });
+
+    // Dismiss the error banner
+    await act(async () => {
+      wrapper.find('[data-test-subj="dismiss-execution-error"]').first().simulate('click');
+      await nextTick();
+    });
+
+    // Disable the alert
+    await act(async () => {
+      wrapper.find('[data-test-subj="disableSwitch"] .euiSwitch__button').first().simulate('click');
+      await nextTick();
+    });
+    expect(disableAlert).toHaveBeenCalled();
+
+    // Enable the alert
+    await act(async () => {
+      wrapper.find('[data-test-subj="disableSwitch"] .euiSwitch__button').first().simulate('click');
+      await nextTick();
+    });
+    expect(enableAlert).toHaveBeenCalled();
+
+    // Ensure error banner is back
+    expect(wrapper.find('[data-test-subj="dismiss-execution-error"]').length).toBeGreaterThan(0);
+  });
 });
 
 describe('mute button', () => {

diff --git a/...riggers_actions_ui/public/application/sections/alert_details/components/alert_details.tsx b/...riggers_actions_ui/public/application/sections/alert_details/components/alert_details.tsx
@@ -236,6 +236,8 @@ export const AlertDetails: React.FunctionComponent<AlertDetailsProps> = ({
                         if (isEnabled) {
                           setIsEnabled(false);
                           await disableAlert(alert);
+                          // Reset dismiss if previously clicked
+                          setDissmissAlertErrors(false);
                         } else {
                           setIsEnabled(true);
                           await enableAlert(alert);
@@ -277,7 +279,7 @@ export const AlertDetails: React.FunctionComponent<AlertDetailsProps> = ({
                 </EuiFlexGroup>
               </EuiFlexItem>
             </EuiFlexGroup>
-            {!dissmissAlertErrors && alert.executionStatus.status === 'error' ? (
+            {alert.enabled && !dissmissAlertErrors && alert.executionStatus.status === 'error' ? (
               <EuiFlexGroup>
                 <EuiFlexItem>
                   <EuiCallOut
@@ -293,7 +295,11 @@ export const AlertDetails: React.FunctionComponent<AlertDetailsProps> = ({
                     <EuiSpacer size="s" />
                     <EuiFlexGroup gutterSize="s" wrap={true}>
                       <EuiFlexItem grow={false}>
-                        <EuiButton color="danger" onClick={() => setDissmissAlertErrors(true)}>
+                        <EuiButton
+                          data-test-subj="dismiss-execution-error"
+                          color="danger"
+                          onClick={() => setDissmissAlertErrors(true)}
+                        >
                           <FormattedMessage
                             id="xpack.triggersActionsUI.sections.alertDetails.dismissButtonTitle"
                             defaultMessage="Dismiss"

diff --git a/..._actions_ui/public/application/sections/alert_details/components/alert_instances.test.tsx b/..._actions_ui/public/application/sections/alert_details/components/alert_instances.test.tsx
@@ -262,7 +262,7 @@ describe('alertInstanceToListItem', () => {
     };
     expect(alertInstanceToListItem(fakeNow.getTime(), alertType, 'id', instance)).toEqual({
       instance: 'id',
-      status: { label: 'OK', healthColor: 'subdued' },
+      status: { label: 'Recovered', healthColor: 'subdued' },
       start: undefined,
       duration: 0,
       sortPriority: 1,

diff --git a/...ggers_actions_ui/public/application/sections/alert_details/components/alert_instances.tsx b/...ggers_actions_ui/public/application/sections/alert_details/components/alert_instances.tsx
@@ -226,7 +226,7 @@ const ACTIVE_LABEL = i18n.translate(
 
 const INACTIVE_LABEL = i18n.translate(
   'xpack.triggersActionsUI.sections.alertDetails.alertInstancesList.status.inactive',
-  { defaultMessage: 'OK' }
+  { defaultMessage: 'Recovered' }
 );
 
 function getActionGroupName(alertType: AlertType, actionGroupId?: string): string | undefined {

diff --git a/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/details.ts b/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/details.ts
@@ -655,7 +655,7 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => {
         ).to.eql([
           {
             instance: 'eu/east',
-            status: 'OK',
+            status: 'Recovered',
             start: '',
             duration: '',
           },