Skip to content

Commit

Permalink
[Security Solution] Fix flyout history flickering (#211662)
Browse files Browse the repository at this point in the history
## 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)
  • Loading branch information
christineweng authored Feb 20, 2025
1 parent 258eef7 commit e2730f7
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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';

Expand All @@ -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(),
Expand Down Expand Up @@ -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 });
});
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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(
<TestProviders>
<GenericHistoryRow
item={rowItems.host}
name="Row name"
icon={'user'}
title="title"
index={0}
isLoading
/>
</TestProviders>
);
expect(getByTestId(HISTORY_ROW_LOADING_TEST_ID)).toBeInTheDocument();
});

it('should open the flyout when clicked', () => {
const { getByTestId } = render(
<TestProviders>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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';

Expand Down Expand Up @@ -99,7 +101,7 @@ export const FlyoutHistoryRow: FC<FlyoutHistoryRowProps> = memo(({ item, index }
* Row item for a document details
*/
export const DocumentDetailsHistoryRow: FC<FlyoutHistoryRowProps> = memo(({ item, index }) => {
const { dataFormattedForFieldBrowser, getFieldsData } = useEventDetails({
const { dataFormattedForFieldBrowser, getFieldsData, loading } = useEventDetails({
eventId: String(item?.panel?.params?.id),
indexName: String(item?.panel?.params?.indexName),
});
Expand All @@ -122,6 +124,7 @@ export const DocumentDetailsHistoryRow: FC<FlyoutHistoryRowProps> = memo(({ item
title={title}
icon={isAlert ? 'warning' : 'analyzeEvent'}
name={isAlert ? 'Alert' : 'Event'}
isLoading={loading}
dataTestSubj={DOCUMENT_DETAILS_HISTORY_ROW_TEST_ID}
/>
);
Expand Down Expand Up @@ -171,7 +174,7 @@ const RowTitle: FC<RowTitleProps> = memo(({ type, value }) => {
*/
export const RuleHistoryRow: FC<FlyoutHistoryRowProps> = memo(({ item, index }) => {
const ruleId = String(item?.panel?.params?.ruleId);
const { rule } = useRuleDetails({ ruleId });
const { rule, loading } = useRuleDetails({ ruleId });

return (
<GenericHistoryRow
Expand All @@ -180,6 +183,7 @@ export const RuleHistoryRow: FC<FlyoutHistoryRowProps> = memo(({ item, index })
title={rule?.name ?? ''}
icon={'indexSettings'}
name={'Rule'}
isLoading={loading}
dataTestSubj={RULE_HISTORY_ROW_TEST_ID}
/>
);
Expand All @@ -202,6 +206,10 @@ interface GenericHistoryRowProps extends FlyoutHistoryRowProps {
* Name to display
*/
name: string;
/**
* Whether the row is loading
*/
isLoading?: boolean;
/**
* Data test subject
*/
Expand All @@ -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<GenericHistoryRowProps> = 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 (
<EuiContextMenuItem key={index} data-test-subj={HISTORY_ROW_LOADING_TEST_ID}>
<EuiSkeletonText lines={1} isLoading size="s" />
</EuiContextMenuItem>
);
}

return (
<EuiContextMenuItem
key={index}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export const TITLE_LINK_ICON_TEST_ID = (dataTestSubj: string) => `${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;
Expand Down

0 comments on commit e2730f7

Please sign in to comment.