Skip to content

Commit

Permalink
[Security Solution][Alert details] - bring back last alert status cha…
Browse files Browse the repository at this point in the history
…nge to flyout (elastic#205224)

## Summary

Over a year ago, [this
PR](elastic#171589) added some
information to the alert details flyout, to show when an alert's status
(`closed`, `open` or `aknowledged`) had been modified last and by which
user.
Shortly after, [this follow up
PR](elastic#172888) removed the UI from
the alert details flyout, as the information wasn't extremely important
and was taking some valuable vertical space, pushing down below the
`Highlighted fields` section, that users were finding very important.

A few months later, we added the ability to persist which of the top
sections (`About`, `Investigation`, `Visualizations`, `Insights` and
`Response`) were collapsed or expanded. That way the user wouldn't have
to always collapse or expand sections they would often don't need.

This PR brings back the alert's last status changes to the `About`
section, as the vertical space is no longer a big issues, because users
can now collapse the entire `About` section.

#### If data is not present, the last change UI is not shown
![Screenshot 2024-12-27 at 3 46
14 PM](https://github.com/user-attachments/assets/24e033d7-fb15-496a-97be-ecf78996d243)

#### If the correct data is shown:
![Screenshot 2024-12-27 at 3 50
12 PM](https://github.com/user-attachments/assets/a13f54d8-1804-4baf-a12b-5203beb4f92d)

### How to test

- have a few alerts in the alerts table
- open the alert details flyout for one alert and change the status
(button in the header)
- verify that the last status change section is shown in the `About`
section

### Checklist

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
PhilippeOberti authored Jan 8, 2025

Verified

This commit was signed with the committer’s verified signature.
edubart Eduardo Bart
1 parent d4a3c96 commit a4b1975
Showing 7 changed files with 187 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -16,6 +16,7 @@ import {
REASON_TITLE_TEST_ID,
MITRE_ATTACK_TITLE_TEST_ID,
EVENT_RENDERER_TEST_ID,
WORKFLOW_STATUS_TITLE_TEST_ID,
} from './test_ids';
import { TestProviders } from '../../../../common/mock';
import { AboutSection } from './about_section';
@@ -106,6 +107,7 @@ describe('<AboutSection />', () => {
expect(queryByTestId(ALERT_DESCRIPTION_TITLE_TEST_ID)).not.toBeInTheDocument();
expect(queryByTestId(REASON_TITLE_TEST_ID)).not.toBeInTheDocument();
expect(queryByTestId(MITRE_ATTACK_TITLE_TEST_ID)).not.toBeInTheDocument();
expect(queryByTestId(WORKFLOW_STATUS_TITLE_TEST_ID)).not.toBeInTheDocument();

expect(getByTestId(EVENT_KIND_DESCRIPTION_TEST_ID)).toBeInTheDocument();

@@ -135,6 +137,7 @@ describe('<AboutSection />', () => {
expect(queryByTestId(ALERT_DESCRIPTION_TITLE_TEST_ID)).not.toBeInTheDocument();
expect(queryByTestId(REASON_TITLE_TEST_ID)).not.toBeInTheDocument();
expect(queryByTestId(MITRE_ATTACK_TITLE_TEST_ID)).not.toBeInTheDocument();
expect(queryByTestId(WORKFLOW_STATUS_TITLE_TEST_ID)).not.toBeInTheDocument();

expect(queryByTestId(EVENT_KIND_DESCRIPTION_TEST_ID)).not.toBeInTheDocument();

Original file line number Diff line number Diff line change
@@ -20,6 +20,7 @@ import { isEcsAllowedValue } from '../utils/event_utils';
import { EventCategoryDescription } from './event_category_description';
import { EventKindDescription } from './event_kind_description';
import { EventRenderer } from './event_renderer';
import { AlertStatus } from './alert_status';

const KEY = 'about';

@@ -42,6 +43,7 @@ export const AboutSection = memo(() => {
<AlertDescription />
<Reason />
<MitreAttack />
<AlertStatus />
</>
) : (
<>
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import React from 'react';
import { act, render } from '@testing-library/react';
import { AlertStatus } from './alert_status';
import { mockContextValue } from '../../shared/mocks/mock_context';
import { DocumentDetailsContext } from '../../shared/context';
import { WORKFLOW_STATUS_DETAILS_TEST_ID, WORKFLOW_STATUS_TITLE_TEST_ID } from './test_ids';
import { TestProviders } from '../../../../common/mock';
import { useBulkGetUserProfiles } from '../../../../common/components/user_profiles/use_bulk_get_user_profiles';

jest.mock('../../../../common/components/user_profiles/use_bulk_get_user_profiles');

const renderAlertStatus = (contextValue: DocumentDetailsContext) =>
render(
<TestProviders>
<DocumentDetailsContext.Provider value={contextValue}>
<AlertStatus />
</DocumentDetailsContext.Provider>
</TestProviders>
);

const mockUserProfiles = [
{ uid: 'user-id-1', enabled: true, user: { username: 'user1', full_name: 'User 1' }, data: {} },
];

describe('<AlertStatus />', () => {
beforeEach(() => {
jest.clearAllMocks();
});

it('should render alert status history information', async () => {
(useBulkGetUserProfiles as jest.Mock).mockReturnValue({
isLoading: false,
data: mockUserProfiles,
});
const contextValue = {
...mockContextValue,
getFieldsData: jest.fn().mockImplementation((field: string) => {
if (field === 'kibana.alert.workflow_user') return ['user-id-1'];
if (field === 'kibana.alert.workflow_status_updated_at')
return ['2023-11-01T22:33:26.893Z'];
}),
};

const { getByTestId } = renderAlertStatus(contextValue);

await act(async () => {
expect(getByTestId(WORKFLOW_STATUS_TITLE_TEST_ID)).toBeInTheDocument();
expect(getByTestId(WORKFLOW_STATUS_DETAILS_TEST_ID)).toBeInTheDocument();
});
});

it('should render empty component if missing workflow_user value', async () => {
const { container } = renderAlertStatus(mockContextValue);

await act(async () => {
expect(container).toBeEmptyDOMElement();
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { EuiFlexGroup, EuiFlexItem, EuiSpacer, EuiTitle } from '@elastic/eui';
import { getUserDisplayName } from '@kbn/user-profile-components';
import { FormattedMessage } from '@kbn/i18n-react';
import React, { memo, useMemo } from 'react';
import { WORKFLOW_STATUS_DETAILS_TEST_ID, WORKFLOW_STATUS_TITLE_TEST_ID } from './test_ids';
import { useBulkGetUserProfiles } from '../../../../common/components/user_profiles/use_bulk_get_user_profiles';
import { PreferenceFormattedDate } from '../../../../common/components/formatted_date';
import { useDocumentDetailsContext } from '../../shared/context';
import { getField } from '../../shared/utils';

/**
* Displays info about who last updated the alert's workflow status and when.
*/
export const AlertStatus = memo(() => {
const { getFieldsData } = useDocumentDetailsContext();
const statusUpdatedBy = getFieldsData('kibana.alert.workflow_user');
const statusUpdatedAt = getField(getFieldsData('kibana.alert.workflow_status_updated_at'));

const result = useBulkGetUserProfiles({ uids: new Set(statusUpdatedBy) });
const user = result.data?.[0]?.user;

const lastStatusChange = useMemo(
() => (
<>
{user && statusUpdatedAt && (
<FormattedMessage
id="xpack.securitySolution.flyout.right.about.status.statusHistoryDetails"
defaultMessage="Alert status updated by {user} on {date}"
values={{
user: getUserDisplayName(user),
date: <PreferenceFormattedDate value={new Date(statusUpdatedAt)} />,
}}
/>
)}
</>
),
[statusUpdatedAt, user]
);

if (!statusUpdatedBy || !statusUpdatedAt || result.isLoading || user == null) {
return null;
}

return (
<EuiFlexGroup direction="column" gutterSize="s">
<EuiSpacer size="xs" />
<EuiFlexItem data-test-subj={WORKFLOW_STATUS_TITLE_TEST_ID}>
<EuiTitle size="xxs">
<h5>
<FormattedMessage
id="xpack.securitySolution.flyout.right.about.status.statusHistoryTitle"
defaultMessage="Last alert status change"
/>
</h5>
</EuiTitle>
</EuiFlexItem>
<EuiFlexItem data-test-subj={WORKFLOW_STATUS_DETAILS_TEST_ID}>{lastStatusChange}</EuiFlexItem>
</EuiFlexGroup>
);
});

AlertStatus.displayName = 'AlertStatus';
Original file line number Diff line number Diff line change
@@ -75,6 +75,10 @@ export const MITRE_ATTACK_DETAILS_TEST_ID = `${MITRE_ATTACK_TEST_ID}Details` as

export const EVENT_RENDERER_TEST_ID = `${PREFIX}EventRenderer` as const;

export const WORKFLOW_STATUS_TEST_ID = `${PREFIX}WorkflowStatus` as const;
export const WORKFLOW_STATUS_TITLE_TEST_ID = `${WORKFLOW_STATUS_TEST_ID}Title` as const;
export const WORKFLOW_STATUS_DETAILS_TEST_ID = `${WORKFLOW_STATUS_TEST_ID}Details` as const;

/* Investigation section */

export const INVESTIGATION_SECTION_TEST_ID = `${PREFIX}InvestigationSection` as const;
Original file line number Diff line number Diff line change
@@ -65,6 +65,15 @@ import { ALERTS_URL } from '../../../../urls/navigation';
import { waitForAlertsToPopulate } from '../../../../tasks/create_new_rule';
import { TOASTER } from '../../../../screens/alerts_detection_rules';
import { ELASTICSEARCH_USERNAME, IS_SERVERLESS } from '../../../../env_var_names_constants';
import {
goToAcknowledgedAlerts,
goToClosedAlerts,
toggleKPICharts,
} from '../../../../tasks/alerts';
import {
DOCUMENT_DETAILS_FLYOUT_OVERVIEW_TAB_WORKFLOW_STATUS_DETAILS,
DOCUMENT_DETAILS_FLYOUT_OVERVIEW_TAB_WORKFLOW_STATUS_TITLE,
} from '../../../../screens/expandable_flyout/alert_details_right_panel_overview_tab';

// We need to use the 'soc_manager' role in order to have the 'Respond' action displayed in serverless
const isServerless = Cypress.env(IS_SERVERLESS);
@@ -171,6 +180,21 @@ describe('Alert details expandable flyout right panel', { tags: ['@ess', '@serve

cy.get(TOASTER).should('have.text', 'Successfully marked 1 alert as acknowledged.');
cy.get(EMPTY_ALERT_TABLE).should('exist');

// collapsing the KPI section prevents the test from being flaky, as when the KPI is expanded, the view
// scrolls to the bottom of the page (for some unknown reason) and the test can't select the page filter...
toggleKPICharts();
goToAcknowledgedAlerts();
expandAlertAtIndexExpandableFlyout();

cy.get(DOCUMENT_DETAILS_FLYOUT_OVERVIEW_TAB_WORKFLOW_STATUS_TITLE).should(
'have.text',
'Last alert status change'
);
cy.get(DOCUMENT_DETAILS_FLYOUT_OVERVIEW_TAB_WORKFLOW_STATUS_DETAILS).should(
'contain.text',
'Alert status updated'
);
});

it('should mark as closed', () => {
@@ -181,6 +205,21 @@ describe('Alert details expandable flyout right panel', { tags: ['@ess', '@serve

cy.get(TOASTER).should('have.text', 'Successfully closed 1 alert.');
cy.get(EMPTY_ALERT_TABLE).should('exist');

// collapsing the KPI section prevents the test from being flaky, as when the KPI is expanded, the view
// scrolls to the bottom of the page (for some unknown reason) and the test can't select the page filter...
toggleKPICharts();
goToClosedAlerts();
expandAlertAtIndexExpandableFlyout();

cy.get(DOCUMENT_DETAILS_FLYOUT_OVERVIEW_TAB_WORKFLOW_STATUS_TITLE).should(
'have.text',
'Last alert status change'
);
cy.get(DOCUMENT_DETAILS_FLYOUT_OVERVIEW_TAB_WORKFLOW_STATUS_DETAILS).should(
'contain.text',
'Alert status updated'
);
});

// these actions are now grouped together as we're not really testing their functionality but just the existence of the option in the dropdown
Original file line number Diff line number Diff line change
@@ -36,6 +36,10 @@ export const DOCUMENT_DETAILS_FLYOUT_OVERVIEW_TAB_MITRE_ATTACK_TITLE = getDataTe
export const DOCUMENT_DETAILS_FLYOUT_OVERVIEW_TAB_MITRE_ATTACK_DETAILS = getDataTestSubjectSelector(
'securitySolutionFlyoutMitreAttackDetails'
);
export const DOCUMENT_DETAILS_FLYOUT_OVERVIEW_TAB_WORKFLOW_STATUS_TITLE =
getDataTestSubjectSelector('securitySolutionFlyoutWorkflowStatusTitle');
export const DOCUMENT_DETAILS_FLYOUT_OVERVIEW_TAB_WORKFLOW_STATUS_DETAILS =
getDataTestSubjectSelector('securitySolutionFlyoutWorkflowStatusDetails');

/* Investigation section */

0 comments on commit a4b1975

Please sign in to comment.