From a0ae83c421951a762fa3d00d0e57d2cda8d144a9 Mon Sep 17 00:00:00 2001 From: christineweng <18648970+christineweng@users.noreply.github.com> Date: Thu, 20 Feb 2025 11:05:54 -0600 Subject: [PATCH] [Security Solution] Fix flyout history flickering (#211662) ## Summary This PR fixed a flickering issue in flyout history **Before** `Event details` is shown and then replaced by actual alert title https://github.com/user-attachments/assets/edb1e6eb-c290-4cdc-a5f9-3f270a26a58b **After** Show a loading skeleton text while fetching rule name ![image](https://github.com/user-attachments/assets/eb20892d-7bbc-4687-bf11-a9cbc65288d3) ### Checklist - [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 - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) (cherry picked from commit e2730f70db3bf04efac6a6f227962f88fcc63fce) --- .../components/flyout_history_row.test.tsx | 30 +++++++++++++++++++ .../shared/components/flyout_history_row.tsx | 22 ++++++++++++-- .../flyout/shared/components/test_ids.ts | 1 + 3 files changed, 50 insertions(+), 3 deletions(-) diff --git a/x-pack/solutions/security/plugins/security_solution/public/flyout/shared/components/flyout_history_row.test.tsx b/x-pack/solutions/security/plugins/security_solution/public/flyout/shared/components/flyout_history_row.test.tsx index 471cbae65427b..5ba5301e56d38 100644 --- a/x-pack/solutions/security/plugins/security_solution/public/flyout/shared/components/flyout_history_row.test.tsx +++ b/x-pack/solutions/security/plugins/security_solution/public/flyout/shared/components/flyout_history_row.test.tsx @@ -22,6 +22,7 @@ import { } from '@kbn/expandable-flyout'; import { useRuleDetails } from '../../rule_details/hooks/use_rule_details'; import { useBasicDataFromDetailsData } from '../../document_details/shared/hooks/use_basic_data_from_details_data'; +import { useEventDetails } from '../../document_details/shared/hooks/use_event_details'; import { DocumentDetailsRightPanelKey } from '../../document_details/shared/constants/panel_keys'; import { RulePanelKey } from '../../rule_details/right'; import { NetworkPanelKey } from '../../network_details'; @@ -32,6 +33,7 @@ import { NETWORK_HISTORY_ROW_TEST_ID, RULE_HISTORY_ROW_TEST_ID, USER_HISTORY_ROW_TEST_ID, + HISTORY_ROW_LOADING_TEST_ID, } from './test_ids'; import { HostPanelKey, UserPanelKey } from '../../entity_details/shared/constants'; @@ -45,6 +47,7 @@ jest.mock('@kbn/expandable-flyout', () => ({ jest.mock('../../../detection_engine/rule_management/logic/use_rule_with_fallback'); jest.mock('../../document_details/shared/hooks/use_basic_data_from_details_data'); jest.mock('../../rule_details/hooks/use_rule_details'); +jest.mock('../../document_details/shared/hooks/use_event_details'); const flyoutContextValue = { openFlyout: jest.fn(), @@ -112,6 +115,12 @@ describe('FlyoutHistoryRow', () => { jest.mocked(useRuleDetails).mockReturnValue({ ...mockedRuleResponse, rule: { name: 'rule name' } as RuleResponse, + loading: false, + }); + (useEventDetails as jest.Mock).mockReturnValue({ + dataFormattedForFieldBrowser: {}, + getFieldsData: jest.fn(), + loading: false, }); (useBasicDataFromDetailsData as jest.Mock).mockReturnValue({ isAlert: false }); }); @@ -182,6 +191,11 @@ describe('FlyoutHistoryRow', () => { describe('DocumentDetailsHistoryRow', () => { beforeEach(() => { jest.mocked(useExpandableFlyoutApi).mockReturnValue(flyoutContextValue); + (useEventDetails as jest.Mock).mockReturnValue({ + dataFormattedForFieldBrowser: {}, + getFieldsData: jest.fn(), + loading: false, + }); }); it('should render alert title when isAlert is true and rule name is defined', () => { @@ -291,6 +305,22 @@ describe('GenericHistoryRow', () => { fireEvent.click(getByTestId(`${0}-${GENERIC_HISTORY_ROW_TEST_ID}`)); }); + it('should render empty context menu item when isLoading is true', () => { + const { getByTestId } = render( + + + + ); + expect(getByTestId(HISTORY_ROW_LOADING_TEST_ID)).toBeInTheDocument(); + }); + it('should open the flyout when clicked', () => { const { getByTestId } = render( diff --git a/x-pack/solutions/security/plugins/security_solution/public/flyout/shared/components/flyout_history_row.tsx b/x-pack/solutions/security/plugins/security_solution/public/flyout/shared/components/flyout_history_row.tsx index ed36c14326555..92ac1b6821781 100644 --- a/x-pack/solutions/security/plugins/security_solution/public/flyout/shared/components/flyout_history_row.tsx +++ b/x-pack/solutions/security/plugins/security_solution/public/flyout/shared/components/flyout_history_row.tsx @@ -14,6 +14,7 @@ import { EuiFlexItem, type EuiIconProps, useEuiTheme, + EuiSkeletonText, } from '@elastic/eui'; import type { FlyoutPanelHistory } from '@kbn/expandable-flyout'; import { useExpandableFlyoutApi } from '@kbn/expandable-flyout'; @@ -32,6 +33,7 @@ import { NETWORK_HISTORY_ROW_TEST_ID, RULE_HISTORY_ROW_TEST_ID, USER_HISTORY_ROW_TEST_ID, + HISTORY_ROW_LOADING_TEST_ID, } from './test_ids'; import { HostPanelKey, UserPanelKey } from '../../entity_details/shared/constants'; @@ -99,7 +101,7 @@ export const FlyoutHistoryRow: FC = memo(({ item, index } * Row item for a document details */ export const DocumentDetailsHistoryRow: FC = memo(({ item, index }) => { - const { dataFormattedForFieldBrowser, getFieldsData } = useEventDetails({ + const { dataFormattedForFieldBrowser, getFieldsData, loading } = useEventDetails({ eventId: String(item?.panel?.params?.id), indexName: String(item?.panel?.params?.indexName), }); @@ -122,6 +124,7 @@ export const DocumentDetailsHistoryRow: FC = memo(({ item title={title} icon={isAlert ? 'warning' : 'analyzeEvent'} name={isAlert ? 'Alert' : 'Event'} + isLoading={loading} dataTestSubj={DOCUMENT_DETAILS_HISTORY_ROW_TEST_ID} /> ); @@ -171,7 +174,7 @@ const RowTitle: FC = memo(({ type, value }) => { */ export const RuleHistoryRow: FC = memo(({ item, index }) => { const ruleId = String(item?.panel?.params?.ruleId); - const { rule } = useRuleDetails({ ruleId }); + const { rule, loading } = useRuleDetails({ ruleId }); return ( = memo(({ item, index }) title={rule?.name ?? ''} icon={'indexSettings'} name={'Rule'} + isLoading={loading} dataTestSubj={RULE_HISTORY_ROW_TEST_ID} /> ); @@ -202,6 +206,10 @@ interface GenericHistoryRowProps extends FlyoutHistoryRowProps { * Name to display */ name: string; + /** + * Whether the row is loading + */ + isLoading?: boolean; /** * Data test subject */ @@ -212,13 +220,21 @@ interface GenericHistoryRowProps extends FlyoutHistoryRowProps { * Row item for a generic history row where the title is accessible in flyout params */ export const GenericHistoryRow: FC = memo( - ({ item, index, title, icon, name, dataTestSubj }) => { + ({ item, index, title, icon, name, isLoading, dataTestSubj }) => { const { euiTheme } = useEuiTheme(); const { openFlyout } = useExpandableFlyoutApi(); const onClick = useCallback(() => { openFlyout({ right: item.panel }); }, [openFlyout, item.panel]); + if (isLoading) { + return ( + + + + ); + } + return ( `${dataTestSubj /* History */ export const FLYOUT_HISTORY_TEST_ID = `${PREFIX}History` as const; +export const HISTORY_ROW_LOADING_TEST_ID = `${FLYOUT_HISTORY_TEST_ID}RowLoading` as const; export const FLYOUT_HISTORY_BUTTON_TEST_ID = `${FLYOUT_HISTORY_TEST_ID}Button` as const; export const FLYOUT_HISTORY_CONTEXT_PANEL_TEST_ID = `${FLYOUT_HISTORY_TEST_ID}ContextPanel` as const;