From 0a1c401ce98e21ca613ff3e6ee6812d90558a71d Mon Sep 17 00:00:00 2001 From: christineweng Date: Tue, 8 Aug 2023 17:17:26 -0500 Subject: [PATCH] fix pr comments --- .../src/components/preview_section.tsx | 2 +- ...t_details_preview_panel_rule_preview.cy.ts | 3 - ...lert_details_preview_panel_rule_preview.ts | 2 - .../pages/rule_details/index.tsx | 31 ++--- .../rules/description_step/index.tsx | 18 ++- .../components/rules/rule_info/index.test.tsx | 70 +++++++++++ .../components/rules/rule_info/index.tsx | 79 ++++++++++++ .../rules/rule_info/translations.ts | 15 +++ .../rules/step_about_rule/index.tsx | 2 +- .../rules/step_define_rule/index.tsx | 2 +- .../rules/step_schedule_rule/index.tsx | 2 +- .../preview/components/rule_preview.test.tsx | 21 +--- .../preview/components/rule_preview.tsx | 94 ++++++-------- .../components/rule_preview_title.test.tsx | 35 ------ .../preview/components/rule_preview_title.tsx | 117 ++++-------------- .../flyout/preview/components/test_ids.ts | 1 - .../flyout/preview/components/translations.ts | 5 - .../preview/hooks/use_rule_switch.test.tsx | 101 --------------- .../flyout/preview/hooks/use_rule_switch.ts | 77 ------------ 19 files changed, 247 insertions(+), 430 deletions(-) create mode 100644 x-pack/plugins/security_solution/public/detections/components/rules/rule_info/index.test.tsx create mode 100644 x-pack/plugins/security_solution/public/detections/components/rules/rule_info/index.tsx create mode 100644 x-pack/plugins/security_solution/public/detections/components/rules/rule_info/translations.ts delete mode 100644 x-pack/plugins/security_solution/public/flyout/preview/hooks/use_rule_switch.test.tsx delete mode 100644 x-pack/plugins/security_solution/public/flyout/preview/hooks/use_rule_switch.ts diff --git a/packages/kbn-expandable-flyout/src/components/preview_section.tsx b/packages/kbn-expandable-flyout/src/components/preview_section.tsx index 60e86ef4195bc..1bb3f84d1b5f5 100644 --- a/packages/kbn-expandable-flyout/src/components/preview_section.tsx +++ b/packages/kbn-expandable-flyout/src/components/preview_section.tsx @@ -137,7 +137,7 @@ export const PreviewSection: React.FC = ({ css={css` margin: ${euiTheme.size.xs}; height: 99%; - box-shadow: 0px 0px 5px 5px #999999; + box-shadow: 0px 0px 5px 5px ${euiTheme.colors.darkShade}; `} className="eui-yScroll" data-test-subj={PREVIEW_SECTION} diff --git a/x-pack/plugins/security_solution/cypress/e2e/investigations/alerts/expandable_flyout/alert_details_preview_panel_rule_preview.cy.ts b/x-pack/plugins/security_solution/cypress/e2e/investigations/alerts/expandable_flyout/alert_details_preview_panel_rule_preview.cy.ts index 80177f5e89397..116162983f0b2 100644 --- a/x-pack/plugins/security_solution/cypress/e2e/investigations/alerts/expandable_flyout/alert_details_preview_panel_rule_preview.cy.ts +++ b/x-pack/plugins/security_solution/cypress/e2e/investigations/alerts/expandable_flyout/alert_details_preview_panel_rule_preview.cy.ts @@ -12,7 +12,6 @@ import { DOCUMENT_DETAILS_FLYOUT_RULE_PREVIEW_TITLE, DOCUMENT_DETAILS_FLYOUT_CREATED_BY, DOCUMENT_DETAILS_FLYOUT_UPDATED_BY, - DOCUMENT_DETAILS_FLYOUT_RULE_SWITCH, DOCUMENT_DETAILS_FLYOUT_RULE_PREVIEW_BODY, DOCUMENT_DETAILS_FLYOUT_RULE_PREVIEW_ABOUT_SECTION_HEADER, DOCUMENT_DETAILS_FLYOUT_RULE_PREVIEW_ABOUT_SECTION_CONTENT, @@ -64,8 +63,6 @@ describe( cy.get(DOCUMENT_DETAILS_FLYOUT_RULE_PREVIEW_TITLE).should('be.visible'); cy.get(DOCUMENT_DETAILS_FLYOUT_CREATED_BY).should('be.visible'); cy.get(DOCUMENT_DETAILS_FLYOUT_UPDATED_BY).should('be.visible'); - cy.get(DOCUMENT_DETAILS_FLYOUT_RULE_SWITCH).should('not.be.disabled'); - cy.get(DOCUMENT_DETAILS_FLYOUT_RULE_SWITCH).should('be.enabled'); cy.log('about'); diff --git a/x-pack/plugins/security_solution/cypress/screens/expandable_flyout/alert_details_preview_panel_rule_preview.ts b/x-pack/plugins/security_solution/cypress/screens/expandable_flyout/alert_details_preview_panel_rule_preview.ts index 23a0c2e91ca18..5ccb58e1ef969 100644 --- a/x-pack/plugins/security_solution/cypress/screens/expandable_flyout/alert_details_preview_panel_rule_preview.ts +++ b/x-pack/plugins/security_solution/cypress/screens/expandable_flyout/alert_details_preview_panel_rule_preview.ts @@ -38,8 +38,6 @@ export const DOCUMENT_DETAILS_FLYOUT_UPDATED_BY = getDataTestSubjectSelector( RULE_PREVIEW_RULE_UPDATED_BY_TEST_ID ); -export const DOCUMENT_DETAILS_FLYOUT_RULE_SWITCH = getDataTestSubjectSelector('ruleSwitch'); - export const DOCUMENT_DETAILS_FLYOUT_RULE_PREVIEW_BODY = getDataTestSubjectSelector(RULE_PREVIEW_BODY_TEST_ID); diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_details_ui/pages/rule_details/index.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_details_ui/pages/rule_details/index.tsx index bebd373b3c2ab..eb0a3c24db15c 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_details_ui/pages/rule_details/index.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_details_ui/pages/rule_details/index.tsx @@ -22,7 +22,6 @@ import type { Filter } from '@kbn/es-query'; import { i18n as i18nTranslate } from '@kbn/i18n'; import { Routes, Route } from '@kbn/shared-ux-router'; -import { FormattedMessage } from '@kbn/i18n-react'; import { noop, omit } from 'lodash/fp'; import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { useParams } from 'react-router-dom'; @@ -53,7 +52,6 @@ import { import { useKibana } from '../../../../common/lib/kibana'; import type { UpdateDateRange } from '../../../../common/components/charts/common'; import { FiltersGlobal } from '../../../../common/components/filters_global'; -import { FormattedDate } from '../../../../common/components/formatted_date'; import { getDetectionEngineUrl, getRuleDetailsTabUrl, @@ -81,6 +79,7 @@ import { getStepsData, redirectToDetections, } from '../../../../detections/pages/detection_engine/rules/helpers'; +import { CreatedBy, UpdatedBy } from '../../../../detections/components/rules/rule_info'; import { useGlobalTime } from '../../../../common/containers/use_global_time'; import { inputsSelectors } from '../../../../common/store/inputs'; import { setAbsoluteRangeDatePicker } from '../../../../common/store/inputs/actions'; @@ -468,32 +467,16 @@ const RuleDetailsPageComponent: React.FC = ({ () => rule ? ( [ - - ), - }} + createdBy={rule?.created_by} + createdAt={rule?.created_at} />, rule?.updated_by != null ? ( - - ), - }} + updatedBy={rule?.updated_by} + updatedAt={rule?.updated_at} /> ) : ( '' diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/description_step/index.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/description_step/index.tsx index 686a7058d5aa6..d4f7c026d369e 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/description_step/index.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/description_step/index.tsx @@ -9,7 +9,7 @@ import { EuiDescriptionList, EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; import { isEmpty, chunk, get, pick, isNumber } from 'lodash/fp'; import React, { memo, useState } from 'react'; import styled from 'styled-components'; - +import { css } from '@emotion/css'; import type { ThreatMapping, Threats, Type } from '@kbn/securitysolution-io-ts-alerting-types'; import type { DataViewBase, Filter } from '@kbn/es-query'; import { FilterStateStore } from '@kbn/es-query'; @@ -68,13 +68,11 @@ const DescriptionListContainer = styled(EuiDescriptionList)` } `; -export const DescriptionListPanelContainer = styled(EuiDescriptionList)` - ${({ theme }) => ` - dt { - font-size: ${theme.eui.euiFontSizeXS} !important; - } - text-overflow: ellipsis; - `} +const panelViewStyle = css` + dt { + font-size: 90% !important; + } + text-overflow: ellipsis; `; interface StepRuleDescriptionProps { @@ -82,7 +80,7 @@ interface StepRuleDescriptionProps { data: unknown; indexPatterns?: DataViewBase; schema: FormSchema; - isInPanelView?: boolean; + isInPanelView?: boolean; // Option to show description list in smaller font } export const StepRuleDescriptionComponent = ({ @@ -141,7 +139,7 @@ export const StepRuleDescriptionComponent = ({ return ( - + ); diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/rule_info/index.test.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/rule_info/index.test.tsx new file mode 100644 index 0000000000000..6b070f2d4285a --- /dev/null +++ b/x-pack/plugins/security_solution/public/detections/components/rules/rule_info/index.test.tsx @@ -0,0 +1,70 @@ +/* + * 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 { CreatedBy, UpdatedBy } from '.'; +import { render } from '@testing-library/react'; +import { TestProviders } from '../../../../common/mock'; + +describe('Rule related info', () => { + describe('', () => { + it('should render created correctly when by and date are passed', () => { + const { getByTestId } = render( + + + + ); + expect(getByTestId('createdBy')).toHaveTextContent( + 'Created by: test on Jan 1, 2023 @ 22:01:00.000' + ); + }); + + it('should render created unknown when created by is not available', () => { + const { getByTestId } = render( + + + + ); + expect(getByTestId('createdBy')).toHaveTextContent( + 'Created by: Unknown on Jan 1, 2023 @ 22:01:00.000' + ); + }); + }); + describe('', () => { + it('should render updated by correctly when by and date are passed', () => { + const { getByTestId } = render( + + + + ); + expect(getByTestId('updatedBy')).toHaveTextContent( + 'Updated by: test on Jan 1, 2023 @ 22:01:00.000' + ); + }); + + it('should render updated by correctly when updated by is not available', () => { + const { getByTestId } = render( + + + + ); + expect(getByTestId('updatedBy')).toHaveTextContent( + 'Updated by: Unknown on Jan 1, 2023 @ 22:01:00.000' + ); + }); + }); +}); diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/rule_info/index.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/rule_info/index.tsx new file mode 100644 index 0000000000000..02d8b631a7c94 --- /dev/null +++ b/x-pack/plugins/security_solution/public/detections/components/rules/rule_info/index.tsx @@ -0,0 +1,79 @@ +/* + * 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 { FormattedMessage } from '@kbn/i18n-react'; +import { UNKNOWN_TEXT } from './translations'; +import { FormattedDate } from '../../../../common/components/formatted_date'; + +interface CreatedByProps { + id: string; + createdBy?: string; + createdAt?: string; + ['data-test-subj']?: string; +} + +/** + * Created by and created at text that are shown on rule details + */ +export const CreatedBy: React.FC = ({ + id, + createdBy, + createdAt, + 'data-test-subj': dataTestSubj, +}) => { + return ( +
+ + ), + }} + /> +
+ ); +}; + +CreatedBy.displayName = 'CreatedBy'; + +interface UpdatedByProps { + id: string; + updatedBy?: string; + updatedAt?: string; + ['data-test-subj']?: string; +} + +/** + * Updated by and updated at text that are shown on rule details + */ +export const UpdatedBy: React.FC = ({ + id, + updatedBy, + updatedAt, + 'data-test-subj': dataTestSubj, +}) => { + return ( +
+ + ), + }} + /> +
+ ); +}; + +UpdatedBy.displayName = 'UpdatedBy'; diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/rule_info/translations.ts b/x-pack/plugins/security_solution/public/detections/components/rules/rule_info/translations.ts new file mode 100644 index 0000000000000..5a17540b0c90d --- /dev/null +++ b/x-pack/plugins/security_solution/public/detections/components/rules/rule_info/translations.ts @@ -0,0 +1,15 @@ +/* + * 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 { i18n } from '@kbn/i18n'; + +export const UNKNOWN_TEXT = i18n.translate( + 'xpack.securitySolution.detectionEngine.ruleInfo.UnknownText', + { + defaultMessage: 'Unknown', + } +); diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/step_about_rule/index.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/step_about_rule/index.tsx index 62b101aa4e76d..7052c29ce7881 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/step_about_rule/index.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/step_about_rule/index.tsx @@ -57,7 +57,7 @@ interface StepAboutRuleReadOnlyProps { addPadding: boolean; descriptionColumns: 'multi' | 'single' | 'singleSplit'; defaultValues: AboutStepRule; - isInPanelView?: boolean; + isInPanelView?: boolean; // Option to show description list in smaller font } const ThreeQuartersContainer = styled.div` diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/step_define_rule/index.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/step_define_rule/index.tsx index 585e9dcbd6719..779fe5f35a820 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/step_define_rule/index.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/step_define_rule/index.tsx @@ -116,7 +116,7 @@ interface StepDefineRuleReadOnlyProps { descriptionColumns: 'multi' | 'single' | 'singleSplit'; defaultValues: DefineStepRule; indexPattern: DataViewBase; - isInPanelView?: boolean; + isInPanelView?: boolean; // Option to show description list in smaller font } export const MyLabelButton = styled(EuiButtonEmpty)` diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/step_schedule_rule/index.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/step_schedule_rule/index.tsx index 00693dfe2c598..30699d60912cb 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/step_schedule_rule/index.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/step_schedule_rule/index.tsx @@ -27,7 +27,7 @@ interface StepScheduleRuleReadOnlyProps { addPadding: boolean; descriptionColumns: 'multi' | 'single' | 'singleSplit'; defaultValues: ScheduleStepRule; - isInPanelView?: boolean; + isInPanelView?: boolean; // Option to show description list in smaller font } const StepScheduleRuleComponent: FC = ({ diff --git a/x-pack/plugins/security_solution/public/flyout/preview/components/rule_preview.test.tsx b/x-pack/plugins/security_solution/public/flyout/preview/components/rule_preview.test.tsx index 409d51ed64596..fc9b8616d5fd7 100644 --- a/x-pack/plugins/security_solution/public/flyout/preview/components/rule_preview.test.tsx +++ b/x-pack/plugins/security_solution/public/flyout/preview/components/rule_preview.test.tsx @@ -15,11 +15,8 @@ import { ExpandableFlyoutContext } from '@kbn/expandable-flyout/src/context'; import { ThemeProvider } from 'styled-components'; import { getMockTheme } from '../../../common/lib/kibana/kibana_react.mock'; import { TestProviders } from '../../../common/mock'; -import { useAppToasts } from '../../../common/hooks/use_app_toasts'; -import { useAppToastsMock } from '../../../common/hooks/use_app_toasts.mock'; import { useRuleWithFallback } from '../../../detection_engine/rule_management/logic/use_rule_with_fallback'; import { getStepsData } from '../../../detections/pages/detection_engine/rules/helpers'; -import { useRuleSwitch } from '../hooks/use_rule_switch'; import { mockAboutStepRule, mockDefineStepRule, @@ -46,16 +43,6 @@ jest.mock('../../../detection_engine/rule_management/logic/use_rule_with_fallbac const mockGetStepsData = getStepsData as jest.Mock; jest.mock('../../../detections/pages/detection_engine/rules/helpers'); -const mockUseRuleSwitch = useRuleSwitch as jest.Mock; -jest.mock('../hooks/use_rule_switch'); - -jest.mock('../../../detection_engine/rule_management/logic/use_start_ml_jobs', () => ({ - useStartMlJobs: jest.fn().mockReturnValue({ startMlJobs: jest.fn() }), -})); - -jest.mock('../../../common/hooks/use_app_toasts'); -const useAppToastsValueMock = useAppToastsMock.create(); - const mockTheme = getMockTheme({ eui: { euiColorMediumShade: '#ece' } }); const contextValue = { @@ -65,13 +52,7 @@ const contextValue = { describe('', () => { beforeEach(() => { - (useAppToasts as jest.Mock).mockReturnValue(useAppToastsValueMock); - mockUseRuleSwitch.mockReturnValue({ - tooltipText: '', - userInfoLoading: false, - isButtonDisabled: false, - isRuleEnabled: true, - }); + // (useAppToasts as jest.Mock).mockReturnValue(useAppToastsValueMock); }); afterEach(() => { diff --git a/x-pack/plugins/security_solution/public/flyout/preview/components/rule_preview.tsx b/x-pack/plugins/security_solution/public/flyout/preview/components/rule_preview.tsx index 1559555eaa38e..0a78f6a29141f 100644 --- a/x-pack/plugins/security_solution/public/flyout/preview/components/rule_preview.tsx +++ b/x-pack/plugins/security_solution/public/flyout/preview/components/rule_preview.tsx @@ -12,7 +12,6 @@ import { ExpandableSection } from '../../right/components/expandable_section'; import { useRuleWithFallback } from '../../../detection_engine/rule_management/logic/use_rule_with_fallback'; import { getStepsData } from '../../../detections/pages/detection_engine/rules/helpers'; import { RulePreviewTitle } from './rule_preview_title'; -import { useRuleSwitch } from '../hooks/use_rule_switch'; import { StepAboutRuleReadOnly } from '../../../detections/components/rules/step_about_rule'; import { StepDefineRuleReadOnly } from '../../../detections/components/rules/step_define_rule'; import { StepScheduleRuleReadOnly } from '../../../detections/components/rules/step_schedule_rule'; @@ -33,11 +32,7 @@ import * as i18n from './translations'; export const RulePreview: React.FC = memo(() => { const { ruleId, indexPattern } = usePreviewPanelContext(); const [rule, setRule] = useState(null); - const { - rule: maybeRule, - loading: ruleLoading, - isExistingRule, - } = useRuleWithFallback(ruleId ?? ''); + const { rule: maybeRule, loading: ruleLoading } = useRuleWithFallback(ruleId ?? ''); // persist rule until refresh is complete useEffect(() => { @@ -46,11 +41,6 @@ export const RulePreview: React.FC = memo(() => { } }, [maybeRule]); - const { userInfoLoading, tooltipText, isButtonDisabled, isRuleEnabled } = useRuleSwitch({ - rule, - isExistingRule, - }); - const { aboutRuleData, defineRuleData, scheduleRuleData, ruleActionsData } = rule != null ? getStepsData({ rule, detailsView: true }) @@ -61,19 +51,13 @@ export const RulePreview: React.FC = memo(() => { ruleActionsData: null, }; - const hasNotificationActions = ruleActionsData != null && ruleActionsData.actions.length > 0; - const hasResponseActions = - ruleActionsData != null && (ruleActionsData.responseActions || []).length > 0; - const hasActions = hasNotificationActions || hasResponseActions; + const hasNotificationActions = Boolean(ruleActionsData?.actions?.length); + const hasResponseActions = Boolean(ruleActionsData?.responseActions?.length); + const hasActions = ruleActionsData != null && (hasNotificationActions || hasResponseActions); return rule ? ( - + { )} - - {defineRuleData && ( - - )} - - - - {scheduleRuleData && ( - - )} - - + {defineRuleData && ( + <> + + + + + + )} + {scheduleRuleData && ( + <> + + + + + + )} {hasActions && ( { )} - ) : ruleLoading || userInfoLoading ? ( + ) : ruleLoading ? ( ) : null; }); diff --git a/x-pack/plugins/security_solution/public/flyout/preview/components/rule_preview_title.test.tsx b/x-pack/plugins/security_solution/public/flyout/preview/components/rule_preview_title.test.tsx index ec74268b7a73c..589d0d4e3b456 100644 --- a/x-pack/plugins/security_solution/public/flyout/preview/components/rule_preview_title.test.tsx +++ b/x-pack/plugins/security_solution/public/flyout/preview/components/rule_preview_title.test.tsx @@ -14,19 +14,12 @@ import { TestProviders } from '../../../common/mock'; import type { Rule } from '../../../detection_engine/rule_management/logic'; import { RULE_PREVIEW_TITLE_TEST_ID, - RULE_PREVIEW_RULE_SWITCH_TEST_ID, RULE_PREVIEW_RULE_CREATED_BY_TEST_ID, RULE_PREVIEW_RULE_UPDATED_BY_TEST_ID, } from './test_ids'; -jest.mock('../../../detection_engine/rule_management/logic/use_start_ml_jobs', () => ({ - useStartMlJobs: jest.fn().mockReturnValue({ startMlJobs: jest.fn() }), -})); - const defaultProps = { rule: { id: 'id' } as Rule, - isButtonDisabled: false, - isRuleEnabled: true, }; describe('', () => { @@ -41,33 +34,5 @@ describe('', () => { expect(getByTestId(RULE_PREVIEW_TITLE_TEST_ID)).toBeInTheDocument(); expect(getByTestId(RULE_PREVIEW_RULE_CREATED_BY_TEST_ID)).toBeInTheDocument(); expect(getByTestId(RULE_PREVIEW_RULE_UPDATED_BY_TEST_ID)).toBeInTheDocument(); - expect(getByTestId(RULE_PREVIEW_RULE_SWITCH_TEST_ID)).toBeInTheDocument(); - }); - - it('should render updated by and created correctly', () => { - const { getByTestId } = render( - - - - - - ); - expect(getByTestId(RULE_PREVIEW_RULE_CREATED_BY_TEST_ID)).toHaveTextContent( - 'Created by: test on Jan 1, 2023 @ 22:01:00.000' - ); - expect(getByTestId(RULE_PREVIEW_RULE_UPDATED_BY_TEST_ID)).toHaveTextContent( - 'Updated by: elastic on Feb 1, 2023 @ 22:01:00.000' - ); }); }); diff --git a/x-pack/plugins/security_solution/public/flyout/preview/components/rule_preview_title.tsx b/x-pack/plugins/security_solution/public/flyout/preview/components/rule_preview_title.tsx index 51b4f5281faa9..6f1e671487035 100644 --- a/x-pack/plugins/security_solution/public/flyout/preview/components/rule_preview_title.tsx +++ b/x-pack/plugins/security_solution/public/flyout/preview/components/rule_preview_title.tsx @@ -4,115 +4,27 @@ * 2.0; you may not use this file except in compliance with the Elastic License * 2.0. */ -import React, { useCallback, useMemo } from 'react'; -import { EuiTitle, EuiText, EuiSpacer, EuiFlexGroup, EuiFlexItem, EuiToolTip } from '@elastic/eui'; -import { FormattedMessage } from '@kbn/i18n-react'; +import React from 'react'; +import { EuiTitle, EuiText, EuiSpacer, EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; import type { Rule } from '../../../detection_engine/rule_management/logic'; -import { FormattedDate } from '../../../common/components/formatted_date'; -import { RuleSwitch } from '../../../detections/components/rules/rule_switch'; -import { useStartMlJobs } from '../../../detection_engine/rule_management/logic/use_start_ml_jobs'; +import { CreatedBy, UpdatedBy } from '../../../detections/components/rules/rule_info'; import { RULE_PREVIEW_TITLE_TEST_ID, RULE_PREVIEW_RULE_CREATED_BY_TEST_ID, RULE_PREVIEW_RULE_UPDATED_BY_TEST_ID, } from './test_ids'; -import * as i18n from './translations'; interface RulePreviewTitleProps { /** * Rule object that represents relevant information about a rule */ rule: Rule; - /** - * Tooltip text that explains why a user does not have permission to rule - */ - tooltipText?: string; - /** - * Boolean that indivates whether the rule switch button is isabled - */ - isButtonDisabled: boolean; - /** - * Boolean that indivates whether the rule switch shoud be enabled - */ - isRuleEnabled: boolean; } /** * Title component that shows basic information of a rule. This is displayed above rule preview body in rule preview panel */ -export const RulePreviewTitle: React.FC = ({ - rule, - tooltipText, - isButtonDisabled, - isRuleEnabled, -}) => { - const { startMlJobs } = useStartMlJobs(); - const startMlJobsIfNeeded = useCallback( - () => startMlJobs(rule?.machine_learning_job_id), - [rule, startMlJobs] - ); - - const createdBy = useMemo( - () => ( - - - ), - }} - /> - - ), - [rule] - ); - - const updatedBy = useMemo( - () => ( - - - ), - }} - /> - - ), - [rule] - ); - - const enableRule = useMemo( - () => ( - - - - - - {i18n.ENABLE_RULE_TEXT} - - - ), - [rule, tooltipText, isButtonDisabled, isRuleEnabled, startMlJobsIfNeeded] - ); - +export const RulePreviewTitle: React.FC = ({ rule }) => { return (
@@ -120,9 +32,24 @@ export const RulePreviewTitle: React.FC = ({ - {createdBy} - {updatedBy} - {enableRule} + + + + + + + + + +
); diff --git a/x-pack/plugins/security_solution/public/flyout/preview/components/test_ids.ts b/x-pack/plugins/security_solution/public/flyout/preview/components/test_ids.ts index 1119b285b03df..32a26d3f87db9 100644 --- a/x-pack/plugins/security_solution/public/flyout/preview/components/test_ids.ts +++ b/x-pack/plugins/security_solution/public/flyout/preview/components/test_ids.ts @@ -10,7 +10,6 @@ import { CONTENT_TEST_ID, HEADER_TEST_ID } from '../../right/components/expandab /* Rule preview */ export const RULE_PREVIEW_TITLE_TEST_ID = 'securitySolutionDocumentDetailsFlyoutRulePreviewTitle'; -export const RULE_PREVIEW_RULE_SWITCH_TEST_ID = 'ruleSwitch'; export const RULE_PREVIEW_RULE_CREATED_BY_TEST_ID = 'securitySolutionDocumentDetailsFlyoutRulePreviewCreatedByText'; export const RULE_PREVIEW_RULE_UPDATED_BY_TEST_ID = diff --git a/x-pack/plugins/security_solution/public/flyout/preview/components/translations.ts b/x-pack/plugins/security_solution/public/flyout/preview/components/translations.ts index 717ccb04f76f4..8112b796c1d39 100644 --- a/x-pack/plugins/security_solution/public/flyout/preview/components/translations.ts +++ b/x-pack/plugins/security_solution/public/flyout/preview/components/translations.ts @@ -36,8 +36,3 @@ export const ENABLE_RULE_TEXT = i18n.translate( 'xpack.securitySolution.flyout.documentDetails.rulePreviewEnableRuleText', { defaultMessage: 'Enable' } ); - -export const UNKNOWN_TEXT = i18n.translate( - 'xpack.securitySolution.flyout.documentDetails.rulePreviewUnknownText', - { defaultMessage: 'Unknown' } -); diff --git a/x-pack/plugins/security_solution/public/flyout/preview/hooks/use_rule_switch.test.tsx b/x-pack/plugins/security_solution/public/flyout/preview/hooks/use_rule_switch.test.tsx deleted file mode 100644 index 8c3be18dc2e47..0000000000000 --- a/x-pack/plugins/security_solution/public/flyout/preview/hooks/use_rule_switch.test.tsx +++ /dev/null @@ -1,101 +0,0 @@ -/* - * 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 type { RenderHookResult } from '@testing-library/react-hooks'; -import { renderHook } from '@testing-library/react-hooks'; -import type { Rule } from '../../../detection_engine/rule_management/logic'; -import type { UseRuleSwitchParams, UseRuleSwitchResult } from './use_rule_switch'; -import { useRuleSwitch } from './use_rule_switch'; -import { useHasMlPermissions } from '../../../detection_engine/rule_management_ui/components/rules_table/use_has_ml_permissions'; -import { useUserData } from '../../../detections/components/user_info'; -import { canEditRuleWithActions, hasUserCRUDPermission } from '../../../common/utils/privileges'; - -const mockUseUserData = useUserData as jest.Mock; -jest.mock('../../../detections/components/user_info'); - -const mockCanEditRuleWithActions = canEditRuleWithActions as jest.Mock; -const mockHasUserCRUDPermission = hasUserCRUDPermission as jest.Mock; -jest.mock('../../../common/utils/privileges'); - -const mockUseHasMlPermissions = useHasMlPermissions as jest.Mock; -jest.mock( - '../../../detection_engine/rule_management_ui/components/rules_table/use_has_ml_permissions' -); - -jest.mock( - '../../../detection_engine/rule_management_ui/components/rules_table/use_has_actions_privileges' -); - -const defaultProps = { - rule: {} as Rule, - isExistingRule: true, -}; - -const mlProps = { - rule: { type: 'machine_learning' } as unknown as Rule, - isExistingRule: true, -}; - -describe('useRuleSwitch', () => { - let hookResult: RenderHookResult; - - it('should return correct userInfoLoading', () => { - mockUseUserData.mockReturnValue([{ loading: true }]); - hookResult = renderHook(() => useRuleSwitch(defaultProps)); - expect(hookResult.result.current.userInfoLoading).toEqual(true); - - mockUseUserData.mockReturnValue([{ loading: false }]); - hookResult = renderHook(() => useRuleSwitch(defaultProps)); - expect(hookResult.result.current.userInfoLoading).toEqual(false); - }); - - describe('should return correct button state if rule is ml rule', () => { - beforeEach(() => { - mockCanEditRuleWithActions.mockReturnValue(true); - mockUseUserData.mockReturnValue([{ loading: false, canUserCRUD: true }]); - mockHasUserCRUDPermission.mockReturnValue(true); - }); - - it('button should not be disabled with ml permission', () => { - mockUseHasMlPermissions.mockReturnValue(true); - hookResult = renderHook(() => useRuleSwitch(mlProps)); - expect(hookResult.result.current.isButtonDisabled).toEqual(false); - }); - - it('button should be disabled without ml permission', () => { - mockUseHasMlPermissions.mockReturnValue(false); - hookResult = renderHook(() => useRuleSwitch(mlProps)); - expect(hookResult.result.current.isButtonDisabled).toEqual(true); - }); - }); - - describe('should return correct button state if rule is not ml rule', () => { - beforeEach(() => { - mockUseUserData.mockReturnValue([{ loading: false, canUserCRUD: true }]); - mockCanEditRuleWithActions.mockReturnValue(true); - mockHasUserCRUDPermission.mockReturnValue(true); - mockUseHasMlPermissions.mockReturnValue(false); - }); - - it('should disable button if rule is not an existing rule', () => { - hookResult = renderHook(() => useRuleSwitch({ ...defaultProps, isExistingRule: false })); - expect(hookResult.result.current.isButtonDisabled).toEqual(true); - }); - - it('should disable button if user cannot edut rule with actions', () => { - mockCanEditRuleWithActions.mockReturnValue(false); - hookResult = renderHook(() => useRuleSwitch(defaultProps)); - expect(hookResult.result.current.isButtonDisabled).toEqual(true); - }); - - it('should disable button if user does not have CRUD permission', () => { - mockHasUserCRUDPermission.mockReturnValue(false); - hookResult = renderHook(() => useRuleSwitch(defaultProps)); - expect(hookResult.result.current.isButtonDisabled).toEqual(true); - }); - }); -}); diff --git a/x-pack/plugins/security_solution/public/flyout/preview/hooks/use_rule_switch.ts b/x-pack/plugins/security_solution/public/flyout/preview/hooks/use_rule_switch.ts deleted file mode 100644 index 84fc3edb788b3..0000000000000 --- a/x-pack/plugins/security_solution/public/flyout/preview/hooks/use_rule_switch.ts +++ /dev/null @@ -1,77 +0,0 @@ -/* - * 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 { useHasActionsPrivileges } from '../../../detection_engine/rule_management_ui/components/rules_table/use_has_actions_privileges'; -import { useHasMlPermissions } from '../../../detection_engine/rule_management_ui/components/rules_table/use_has_ml_permissions'; -import { isMlRule } from '../../../../common/machine_learning/helpers'; -import { useUserData } from '../../../detections/components/user_info'; -import { - canEditRuleWithActions, - explainLackOfPermission, - hasUserCRUDPermission, -} from '../../../common/utils/privileges'; -import type { Rule } from '../../../detection_engine/rule_management/logic'; -export interface UseRuleSwitchParams { - /** - * Rule object that represents relevant information about a rule - */ - rule: Rule | null; - /** - * Boolean that indicates whether the rule currently exists - */ - isExistingRule: boolean; -} - -export interface UseRuleSwitchResult { - /** - * Boolean of wheather useUserData is loading - */ - userInfoLoading: boolean; - /** - * Tooltip text that explains why a user does not have permission to rule - */ - tooltipText?: string; - /** - * Boolean that indivates whether the rule switch button is isabled - */ - isButtonDisabled: boolean; - /** - * Boolean that indivates whether the rule switch shoud be enabled - */ - isRuleEnabled: boolean; -} - -/** - * This hook is used to retrieved data for rule switch - * @param rule data - * @param isExistingrule is this rule an existing rule - */ -export const useRuleSwitch = ({ - rule, - isExistingRule, -}: UseRuleSwitchParams): UseRuleSwitchResult => { - const hasMlPermissions = useHasMlPermissions(); - const hasActionsPrivileges = useHasActionsPrivileges(); - const [{ loading: userInfoLoading, canUserCRUD }] = useUserData(); - - const tooltipText = explainLackOfPermission( - rule, - hasMlPermissions, - hasActionsPrivileges, - canUserCRUD - ); - - return { - userInfoLoading, - tooltipText, - isButtonDisabled: - !isExistingRule || - !canEditRuleWithActions(rule, hasActionsPrivileges) || - !hasUserCRUDPermission(canUserCRUD) || - (isMlRule(rule?.type) && !hasMlPermissions), - isRuleEnabled: isExistingRule && (rule?.enabled ?? false), - }; -};