From 97947bacf1b1005871e2d712a6e1a0edde2c0f20 Mon Sep 17 00:00:00 2001 From: Davis Plumlee Date: Fri, 13 Sep 2024 16:27:01 -0400 Subject: [PATCH] addresses comments --- ...perform_specific_rules_install_mutation.ts | 15 +- .../add_prebuilt_rules_header_buttons.tsx | 40 +++--- .../add_prebuilt_rules_install_button.tsx | 136 ++++++++++-------- .../add_prebuilt_rules_table_context.tsx | 12 +- .../add_prebuilt_rules_table/translations.ts | 4 +- 5 files changed, 114 insertions(+), 93 deletions(-) diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_management/api/hooks/prebuilt_rules/use_perform_specific_rules_install_mutation.ts b/x-pack/plugins/security_solution/public/detection_engine/rule_management/api/hooks/prebuilt_rules/use_perform_specific_rules_install_mutation.ts index 2767b22cb0b73..3b448219d6e01 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_management/api/hooks/prebuilt_rules/use_perform_specific_rules_install_mutation.ts +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_management/api/hooks/prebuilt_rules/use_perform_specific_rules_install_mutation.ts @@ -29,7 +29,7 @@ export const PERFORM_SPECIFIC_RULES_INSTALLATION_KEY = [ export interface UsePerformSpecificRulesInstallParams { rules: InstallSpecificRulesRequest['rules']; - enableOnInstall?: boolean; + enable?: boolean; } export const usePerformSpecificRulesInstallMutation = ( @@ -54,9 +54,8 @@ export const usePerformSpecificRulesInstallMutation = ( Error, UsePerformSpecificRulesInstallParams >( - (rulesToInstall: UsePerformSpecificRulesInstallParams) => { - return performInstallSpecificRules(rulesToInstall.rules); - }, + (rulesToInstall: UsePerformSpecificRulesInstallParams) => + performInstallSpecificRules(rulesToInstall.rules), { ...options, mutationKey: PERFORM_SPECIFIC_RULES_INSTALLATION_KEY, @@ -70,11 +69,11 @@ export const usePerformSpecificRulesInstallMutation = ( invalidateRuleStatus(); invalidateFetchCoverageOverviewQuery(); - const [response, , { enableOnInstall }] = args; + const [response, , { enable }] = args; - if (response && enableOnInstall) { - const idMap = response.results.created.map((rule) => rule.id); - const bulkAction: BulkAction = { type: 'enable', ids: idMap }; + if (response && enable) { + const ruleIdsToEnable = response.results.created.map((rule) => rule.id); + const bulkAction: BulkAction = { type: 'enable', ids: ruleIdsToEnable }; mutateAsync({ bulkAction }); } diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/add_prebuilt_rules_table/add_prebuilt_rules_header_buttons.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/add_prebuilt_rules_table/add_prebuilt_rules_header_buttons.tsx index 15a8c79a01552..b4ff6ab29a3ff 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/add_prebuilt_rules_table/add_prebuilt_rules_header_buttons.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/add_prebuilt_rules_table/add_prebuilt_rules_header_buttons.tsx @@ -15,7 +15,8 @@ import { EuiLoadingSpinner, EuiPopover, } from '@elastic/eui'; -import React, { useState } from 'react'; +import React, { useCallback, useMemo } from 'react'; +import { useBoolean } from 'react-use'; import { useUserData } from '../../../../../detections/components/user_info'; import { useAddPrebuiltRulesTableContext } from './add_prebuilt_rules_table_context'; import * as i18n from './translations'; @@ -40,26 +41,33 @@ export const AddPrebuiltRulesHeaderButtons = () => { const isRuleInstalling = loadingRules.length > 0; const isRequestInProgress = isRuleInstalling || isRefetching || isUpgradingSecurityPackages; - const [isOverflowPopoverOpen, setOverflowPopover] = useState(false); + const [isOverflowPopoverOpen, setOverflowPopover] = useBoolean(false); - const onButtonClick = () => { + const onOverflowButtonClick = () => { setOverflowPopover(!isOverflowPopoverOpen); }; - const closeOverflowPopover = () => { + const closeOverflowPopover = useCallback(() => { setOverflowPopover(false); - }; + }, [setOverflowPopover]); - const enableOnClick = () => { + const enableOnClick = useCallback(() => { installSelectedRules(true); closeOverflowPopover(); - }; + }, [closeOverflowPopover, installSelectedRules]); - const overflowItems = [ - - {i18n.INSTALL_AND_ENABLE_BUTTON_LABEL} - , - ]; + const installOnClick = useCallback(() => { + installSelectedRules(); + }, [installSelectedRules]); + + const overflowItems = useMemo( + () => [ + + {i18n.INSTALL_AND_ENABLE_BUTTON_LABEL} + , + ], + [enableOnClick] + ); return ( @@ -67,12 +75,12 @@ export const AddPrebuiltRulesHeaderButtons = () => { <> installSelectedRules()} + onClick={installOnClick} disabled={!canUserEditRules || isRequestInProgress} data-test-subj="installSelectedRulesButton" > {i18n.INSTALL_SELECTED_RULES(numberOfSelectedRules)} - {isRuleInstalling ? : undefined} + {isRuleInstalling && } @@ -83,7 +91,7 @@ export const AddPrebuiltRulesHeaderButtons = () => { size="m" iconType="boxesVertical" aria-label={i18n.INSTALL_RULES_OVERFLOW_BUTTON_ARIA_LABEL} - onClick={onButtonClick} + onClick={onOverflowButtonClick} disabled={!canUserEditRules || isRequestInProgress} /> } @@ -107,7 +115,7 @@ export const AddPrebuiltRulesHeaderButtons = () => { aria-label={i18n.INSTALL_ALL_ARIA_LABEL} > {i18n.INSTALL_ALL} - {isRuleInstalling ? : undefined} + {isRuleInstalling && } diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/add_prebuilt_rules_table/add_prebuilt_rules_install_button.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/add_prebuilt_rules_table/add_prebuilt_rules_install_button.tsx index f14499146d842..ea83efae768fa 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/add_prebuilt_rules_table/add_prebuilt_rules_install_button.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/add_prebuilt_rules_table/add_prebuilt_rules_install_button.tsx @@ -15,90 +15,104 @@ import { EuiLoadingSpinner, EuiPopover, } from '@elastic/eui'; -import React, { useState } from 'react'; +import React, { useCallback, useMemo } from 'react'; +import { useBoolean } from 'react-use'; import type { Rule } from '../../../../rule_management/logic'; import type { RuleSignatureId } from '../../../../../../common/api/detection_engine'; import type { AddPrebuiltRulesTableActions } from './add_prebuilt_rules_table_context'; import * as i18n from './translations'; +export interface PrebuiltRulesInstallButtonProps { + ruleId: RuleSignatureId; + record: Rule; + installOneRule: AddPrebuiltRulesTableActions['installOneRule']; + loadingRules: RuleSignatureId[]; + isDisabled: boolean; +} + export const PrebuiltRulesInstallButton = ({ ruleId, record, installOneRule, loadingRules, isDisabled, -}: { - ruleId: RuleSignatureId; - record: Rule; - installOneRule: AddPrebuiltRulesTableActions['installOneRule']; - loadingRules: RuleSignatureId[]; - isDisabled: boolean; -}) => { +}: PrebuiltRulesInstallButtonProps) => { const isRuleInstalling = loadingRules.includes(ruleId); const isInstallButtonDisabled = isRuleInstalling || isDisabled; - const [isPopoverOpen, setPopover] = useState(false); + const [isPopoverOpen, setPopover] = useBoolean(false); - const onOverflowButtonClick = () => { + const onOverflowButtonClick = useCallback(() => { setPopover(!isPopoverOpen); - }; + }, [isPopoverOpen, setPopover]); - const closeOverflowPopover = () => { + const closeOverflowPopover = useCallback(() => { setPopover(false); - }; + }, [setPopover]); - const enableOnClick = () => { + const enableOnClick = useCallback(() => { installOneRule(ruleId, true); closeOverflowPopover(); - }; + }, [closeOverflowPopover, installOneRule, ruleId]); + + const installOnClick = useCallback(() => { + installOneRule(ruleId); + }, [installOneRule, ruleId]); - const overflowItems = [ - - {i18n.INSTALL_AND_ENABLE_BUTTON_LABEL} - , - ]; + const overflowItems = useMemo( + () => [ + + {i18n.INSTALL_AND_ENABLE_BUTTON_LABEL} + , + ], + [enableOnClick] + ); + + const popoverButton = useMemo( + () => ( + + ), + [isInstallButtonDisabled, onOverflowButtonClick] + ); + if (isRuleInstalling) { + return ( + + ); + } return ( - <> - {isRuleInstalling ? ( - + + - ) : ( - - - installOneRule(ruleId)} - data-test-subj={`installSinglePrebuiltRuleButton-${ruleId}`} - aria-label={i18n.INSTALL_RULE_BUTTON_ARIA_LABEL(record.name)} - > - {i18n.INSTALL_BUTTON_LABEL} - - - - - } - isOpen={isPopoverOpen} - closePopover={closeOverflowPopover} - panelPaddingSize="s" - anchorPosition="downRight" - > - - - - - )} - + disabled={isInstallButtonDisabled} + onClick={installOnClick} + data-test-subj={`installSinglePrebuiltRuleButton-${ruleId}`} + aria-label={i18n.INSTALL_RULE_BUTTON_ARIA_LABEL(record.name)} + > + {i18n.INSTALL_BUTTON_LABEL} + + + + + + + + ); }; diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/add_prebuilt_rules_table/add_prebuilt_rules_table_context.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/add_prebuilt_rules_table/add_prebuilt_rules_table_context.tsx index c94e88ee1d667..6fdb473302911 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/add_prebuilt_rules_table/add_prebuilt_rules_table_context.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/add_prebuilt_rules_table/add_prebuilt_rules_table_context.tsx @@ -75,9 +75,9 @@ export interface AddPrebuiltRulesTableState { export interface AddPrebuiltRulesTableActions { reFetchRules: () => void; - installOneRule: (ruleId: RuleSignatureId, enableOnInstall?: boolean) => void; + installOneRule: (ruleId: RuleSignatureId, enable?: boolean) => void; installAllRules: () => void; - installSelectedRules: (enableOnInstall?: boolean) => void; + installSelectedRules: (enable?: boolean) => void; setFilterOptions: Dispatch>; selectRules: (rules: RuleResponse[]) => void; openRulePreview: (ruleId: RuleSignatureId) => void; @@ -140,7 +140,7 @@ export const AddPrebuiltRulesTableContextProvider = ({ const filteredRules = useFilterPrebuiltRulesToInstall({ filterOptions, rules }); const installOneRule = useCallback( - async (ruleId: RuleSignatureId, enableOnInstall?: boolean) => { + async (ruleId: RuleSignatureId, enable?: boolean) => { const rule = rules.find((r) => r.rule_id === ruleId); invariant(rule, `Rule with id ${ruleId} not found`); @@ -148,7 +148,7 @@ export const AddPrebuiltRulesTableContextProvider = ({ try { await installSpecificRulesRequest({ rules: [{ rule_id: ruleId, version: rule.version }], - enableOnInstall, + enable, }); } finally { setLoadingRules((prev) => prev.filter((id) => id !== ruleId)); @@ -158,14 +158,14 @@ export const AddPrebuiltRulesTableContextProvider = ({ ); const installSelectedRules = useCallback( - async (enableOnInstall?: boolean) => { + async (enable?: boolean) => { const rulesToUpgrade = selectedRules.map((rule) => ({ rule_id: rule.rule_id, version: rule.version, })); setLoadingRules((prev) => [...prev, ...rulesToUpgrade.map((r) => r.rule_id)]); try { - await installSpecificRulesRequest({ rules: rulesToUpgrade, enableOnInstall }); + await installSpecificRulesRequest({ rules: rulesToUpgrade, enable }); } finally { setLoadingRules((prev) => prev.filter((id) => !rulesToUpgrade.some((r) => r.rule_id === id)) diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/add_prebuilt_rules_table/translations.ts b/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/add_prebuilt_rules_table/translations.ts index 567351d78b8bc..c335f7624afd8 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/add_prebuilt_rules_table/translations.ts +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/add_prebuilt_rules_table/translations.ts @@ -46,14 +46,14 @@ export const INSTALL_BUTTON_LABEL = i18n.translate( ); export const INSTALL_WITHOUT_ENABLING_BUTTON_LABEL = i18n.translate( - 'xpack.securitySolution.detectionEngine.ruleDetails.installButtonLabel', + 'xpack.securitySolution.detectionEngine.ruleDetails.installWithoutEnablingButtonLabel', { defaultMessage: 'Install without enabling', } ); export const INSTALL_AND_ENABLE_BUTTON_LABEL = i18n.translate( - 'xpack.securitySolution.detectionEngine.ruleDetails.installButtonLabel', + 'xpack.securitySolution.detectionEngine.ruleDetails.installAndEnableButtonLabel', { defaultMessage: 'Install and enable', }