Skip to content

Commit

Permalink
Rename alert status OK to Recovered and fix some UX issues around dis…
Browse files Browse the repository at this point in the history
…abling a rule while being in an error state (#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]>
  • Loading branch information
mikecote and kibanamachine authored May 13, 2021
1 parent f94bbc9 commit bb7057c
Showing 8 changed files with 96 additions and 5 deletions.
Binary file modified docs/user/alerting/images/rule-details-alerts-inactive.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
5 changes: 5 additions & 0 deletions x-pack/plugins/alerting/server/alerts_client/alerts_client.ts
Original file line number Diff line number Diff line change
@@ -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 });
10 changes: 10 additions & 0 deletions x-pack/plugins/alerting/server/alerts_client/tests/enable.test.ts
Original file line number Diff line number Diff line change
@@ -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',
Original file line number Diff line number Diff line change
@@ -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', () => {
Original file line number Diff line number Diff line change
@@ -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"
Original file line number Diff line number Diff line change
@@ -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,
Original file line number Diff line number Diff line change
@@ -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 {
Original file line number Diff line number Diff line change
@@ -655,7 +655,7 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => {
).to.eql([
{
instance: 'eu/east',
status: 'OK',
status: 'Recovered',
start: '',
duration: '',
},

0 comments on commit bb7057c

Please sign in to comment.