From 77e43ce39990421c5be86913d5dd8ed825f86596 Mon Sep 17 00:00:00 2001 From: christineweng <18648970+christineweng@users.noreply.github.com> Date: Wed, 6 Nov 2024 16:57:55 -0600 Subject: [PATCH] [Security Solution] Update rule link in cases activities (#198836) ## Summary As a follow up to https://github.com/elastic/kibana/pull/191764, this PR updates rule names in `Activity` tab in cases to open a rule flyout instead of going to rule details page. **Security solution changes**: replace rule page navigation with a `openFlyout` call **Cases plugin changes**: update the rules link component to accept either `href` or `onClick`. ![image](https://github.com/user-attachments/assets/c5dca885-61b8-4481-adfa-f9e615a01265) ### 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 --- .../cases/public/components/links/index.tsx | 2 +- .../user_actions/comment/alert_event.test.tsx | 63 ++++++++++++++++--- .../user_actions/comment/alert_event.tsx | 10 ++- .../public/cases/pages/index.tsx | 33 ++++------ 4 files changed, 75 insertions(+), 33 deletions(-) diff --git a/x-pack/plugins/cases/public/components/links/index.tsx b/x-pack/plugins/cases/public/components/links/index.tsx index f1e8ca5cdb4af..9b610db63ed10 100644 --- a/x-pack/plugins/cases/public/components/links/index.tsx +++ b/x-pack/plugins/cases/public/components/links/index.tsx @@ -12,7 +12,7 @@ import { useCaseViewNavigation, useConfigureCasesNavigation } from '../../common import * as i18n from './translations'; export interface CasesNavigation { - href: K extends 'configurable' ? (arg: T) => string : string; + href?: K extends 'configurable' ? (arg: T) => string : string; onClick: K extends 'configurable' ? (arg: T, arg2: React.MouseEvent | MouseEvent) => Promise | void : (arg: T) => Promise | void; diff --git a/x-pack/plugins/cases/public/components/user_actions/comment/alert_event.test.tsx b/x-pack/plugins/cases/public/components/user_actions/comment/alert_event.test.tsx index 60d5759de6e21..d060f1d9d71ac 100644 --- a/x-pack/plugins/cases/public/components/user_actions/comment/alert_event.test.tsx +++ b/x-pack/plugins/cases/public/components/user_actions/comment/alert_event.test.tsx @@ -47,10 +47,38 @@ describe('Alert events', () => { expect(wrapper.text()).toBe('added an alert from Awesome rule'); }); - it('does NOT render the link when the rule id is null', async () => { + it('renders the link when onClick is provided but href is not valid', async () => { const wrapper = mount( - + + + ); + + expect( + wrapper.find(`[data-test-subj="alert-rule-link-action-id-1"]`).first().exists() + ).toBeTruthy(); + }); + + it('renders the link when href is valid but onClick is not available', async () => { + const wrapper = mount( + + + + ); + + expect( + wrapper.find(`[data-test-subj="alert-rule-link-action-id-1"]`).first().exists() + ).toBeTruthy(); + }); + + it('does NOT render the link when the href and onclick are invalid but it shows the rule name', async () => { + const wrapper = mount( + + ); @@ -61,10 +89,10 @@ describe('Alert events', () => { expect(wrapper.text()).toBe('added an alert from Awesome rule'); }); - it('does NOT render the link when the href is invalid but it shows the rule name', async () => { + it('does NOT render the link when the rule id is null', async () => { const wrapper = mount( - + ); @@ -131,9 +159,28 @@ describe('Alert events', () => { expect(result.getByTestId('alert-rule-link-action-id-1')).toHaveTextContent('Awesome rule'); }); - it('does NOT render the link when the rule id is null', async () => { + it('renders the link when onClick is provided but href is not valid', async () => { const result = appMock.render( - + + ); + expect(result.getByTestId('alert-rule-link-action-id-1')).toHaveTextContent('Awesome rule'); + }); + + it('renders the link when href is valid but onClick is not available', async () => { + const result = appMock.render( + + ); + expect(result.getByTestId('alert-rule-link-action-id-1')).toHaveTextContent('Awesome rule'); + }); + + it('does NOT render the link when the href and onclick are invalid but it shows the rule name', async () => { + const result = appMock.render( + ); expect(result.getByTestId('multiple-alerts-user-action-action-id-1')).toHaveTextContent( @@ -142,9 +189,9 @@ describe('Alert events', () => { expect(result.queryByTestId('alert-rule-link-action-id-1')).toBeFalsy(); }); - it('does NOT render the link when the href is invalid but it shows the rule name', async () => { + it('does NOT render the link when the rule id is null', async () => { const result = appMock.render( - + ); expect(result.getByTestId('multiple-alerts-user-action-action-id-1')).toHaveTextContent( diff --git a/x-pack/plugins/cases/public/components/user_actions/comment/alert_event.tsx b/x-pack/plugins/cases/public/components/user_actions/comment/alert_event.tsx index 42e5e6b9d4427..a8ffbb987a021 100644 --- a/x-pack/plugins/cases/public/components/user_actions/comment/alert_event.tsx +++ b/x-pack/plugins/cases/public/components/user_actions/comment/alert_event.tsx @@ -5,7 +5,7 @@ * 2.0. */ -import React, { memo, useCallback } from 'react'; +import React, { memo, useCallback, useMemo } from 'react'; import { isEmpty } from 'lodash'; import { EuiLoadingSpinner } from '@elastic/eui'; @@ -38,12 +38,18 @@ const RuleLink: React.FC = memo( const ruleDetailsHref = getRuleDetailsHref?.(ruleId); const finalRuleName = ruleName ?? i18n.UNKNOWN_RULE; + const isValidLink = useMemo(() => { + if (!onRuleDetailsClick && !ruleDetailsHref) { + return false; + } + return !isEmpty(ruleId); + }, [onRuleDetailsClick, ruleDetailsHref, ruleId]); if (loadingAlertData) { return ; } - if (!isEmpty(ruleId) && ruleDetailsHref != null) { + if (isValidLink) { return ( { const { getAppUrl, navigateTo } = useNavigation(); const userCasesPermissions = cases.helpers.canUseCases([APP_ID]); const dispatch = useDispatch(); - const { formatUrl: detectionsFormatUrl, search: detectionsUrlSearch } = useFormatUrl( - SecurityPageName.rules - ); const { openFlyout } = useExpandableFlyoutApi(); - const getDetectionsRuleDetailsHref = useCallback( - (ruleId: string | null | undefined) => - detectionsFormatUrl(getRuleDetailsUrl(ruleId ?? '', detectionsUrlSearch)), - [detectionsFormatUrl, detectionsUrlSearch] - ); - const interactionsUpsellingMessage = useUpsellingMessage('investigation_guide_interactions'); const showAlertDetails = useCallback( @@ -71,6 +60,15 @@ const CaseContainerComponent: React.FC = () => { [openFlyout, telemetry] ); + const onRuleDetailsClick = useCallback( + (ruleId: string | null | undefined) => { + if (ruleId) { + openFlyout({ right: { id: RulePanelKey, params: { ruleId } } }); + } + }, + [openFlyout] + ); + const { onLoad: onAlertsTableLoaded } = useFetchNotes(); const endpointDetailsHref = (endpointId: string) => @@ -138,16 +136,7 @@ const CaseContainerComponent: React.FC = () => { }, }, ruleDetailsNavigation: { - href: getDetectionsRuleDetailsHref, - onClick: async (ruleId: string | null | undefined, e) => { - if (e) { - e.preventDefault(); - } - return navigateTo({ - deepLinkId: SecurityPageName.rules, - path: getRuleDetailsUrl(ruleId ?? ''), - }); - }, + onClick: onRuleDetailsClick, }, showAlertDetails, timelineIntegration: {