From 1b8d269daa9874b5c59088ea66a5c38cffbd4ab7 Mon Sep 17 00:00:00 2001 From: Jacek Kolezynski Date: Wed, 27 Nov 2024 13:13:58 +0100 Subject: [PATCH] [Security Solution] Disable Install All button when installation is in progress (#201731) **Resolves: #180660** ## Summary In this PR I am fixing a problem of the `Install All` button being re-enabled when entering the page again. I am fixing only the UI problem here, only for the scenario described in the ticket: single user interacting with single instance of Kibana. During investigation I discovered and discussed with the team other scenarios which may lead to race condition when installing rules, but the team agreed to work on them separately. The fix uses `useIsMutating` hook to check that the mutation is pending. Such information is added to the `AddPrebuiltRulesTableContext` and then used to disable the `Install All` button in the component. Recording: https://github.com/user-attachments/assets/777dd6a5-2441-4101-8368-04cdcede1409 ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [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 - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/7483 https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/7484 (cherry picked from commit 9ca719c2926c766a2a8623bab5814996a3e43833) --- .../add_prebuilt_rules_header_buttons.tsx | 3 +- .../add_prebuilt_rules_install_button.tsx | 6 +- .../add_prebuilt_rules_table.test.tsx | 55 ++++++++++++------- .../add_prebuilt_rules_table_context.tsx | 29 +++++++--- .../use_add_prebuilt_rules_table_columns.tsx | 26 +++++++-- 5 files changed, 82 insertions(+), 37 deletions(-) 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 b4ff6ab29a3ff..05078d8492e31 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 @@ -28,6 +28,7 @@ export const AddPrebuiltRulesHeaderButtons = () => { loadingRules, isRefetching, isUpgradingSecurityPackages, + isInstallingAllRules, hasRulesToInstall, }, actions: { installAllRules, installSelectedRules }, @@ -38,7 +39,7 @@ export const AddPrebuiltRulesHeaderButtons = () => { const numberOfSelectedRules = selectedRules.length ?? 0; const shouldDisplayInstallSelectedRulesButton = numberOfSelectedRules > 0; - const isRuleInstalling = loadingRules.length > 0; + const isRuleInstalling = loadingRules.length > 0 || isInstallingAllRules; const isRequestInProgress = isRuleInstalling || isRefetching || isUpgradingSecurityPackages; const [isOverflowPopoverOpen, setOverflowPopover] = useBoolean(false); 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 ea83efae768fa..fc1656ea8cdea 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 @@ -19,7 +19,7 @@ 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 { type AddPrebuiltRulesTableActions } from './add_prebuilt_rules_table_context'; import * as i18n from './translations'; export interface PrebuiltRulesInstallButtonProps { @@ -28,6 +28,7 @@ export interface PrebuiltRulesInstallButtonProps { installOneRule: AddPrebuiltRulesTableActions['installOneRule']; loadingRules: RuleSignatureId[]; isDisabled: boolean; + isInstallingAllRules: boolean; } export const PrebuiltRulesInstallButton = ({ @@ -36,8 +37,9 @@ export const PrebuiltRulesInstallButton = ({ installOneRule, loadingRules, isDisabled, + isInstallingAllRules, }: PrebuiltRulesInstallButtonProps) => { - const isRuleInstalling = loadingRules.includes(ruleId); + const isRuleInstalling = loadingRules.includes(ruleId) || isInstallingAllRules; const isInstallButtonDisabled = isRuleInstalling || isDisabled; const [isPopoverOpen, setPopover] = useBoolean(false); 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.test.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/add_prebuilt_rules_table/add_prebuilt_rules_table.test.tsx index 4be2547598b5d..c8ee77040b184 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/add_prebuilt_rules_table/add_prebuilt_rules_table.test.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/add_prebuilt_rules_table/add_prebuilt_rules_table.test.tsx @@ -14,6 +14,7 @@ import { useUserData } from '../../../../../detections/components/user_info'; import { usePrebuiltRulesInstallReview } from '../../../../rule_management/logic/prebuilt_rules/use_prebuilt_rules_install_review'; import { useFetchPrebuiltRulesStatusQuery } from '../../../../rule_management/api/hooks/prebuilt_rules/use_fetch_prebuilt_rules_status_query'; import { useIsUpgradingSecurityPackages } from '../../../../rule_management/logic/use_upgrade_security_packages'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; // Mock components not needed in this test suite jest.mock('../../../../rule_management/components/rule_details/rule_details_flyout', () => ({ @@ -109,10 +110,12 @@ describe('AddPrebuiltRulesTable', () => { ]); render( - - - - + + + + + + ); const installAllButton = screen.getByTestId('installAllRulesButton'); @@ -132,10 +135,12 @@ describe('AddPrebuiltRulesTable', () => { (useIsUpgradingSecurityPackages as jest.Mock).mockReturnValueOnce(true); render( - - - - + + + + + + ); const installAllButton = screen.getByTestId('installAllRulesButton'); @@ -153,10 +158,12 @@ describe('AddPrebuiltRulesTable', () => { ]); render( - - - - + + + + + + ); const installAllButton = screen.getByTestId('installAllRulesButton'); @@ -198,9 +205,11 @@ describe('AddPrebuiltRulesTable', () => { }); const { findByText } = render( - - - + + + + + ); expect(await findByText('All Elastic rules have been installed')).toBeInTheDocument(); @@ -245,9 +254,11 @@ describe('AddPrebuiltRulesTable', () => { }); render( - - - + + + + + ); const installRuleButton = screen.queryByTestId(`installSinglePrebuiltRuleButton-${id}`); @@ -293,9 +304,11 @@ describe('AddPrebuiltRulesTable', () => { }); render( - - - + + + + + ); const installRuleButton = screen.queryByTestId(`installSinglePrebuiltRuleButton-${id}`); 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 14e539ec40ae1..dca4809001971 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 @@ -5,25 +5,27 @@ * 2.0. */ +import { EuiButton, EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; +import { useIsMutating } from '@tanstack/react-query'; import type { Dispatch, SetStateAction } from 'react'; import React, { createContext, useCallback, useContext, useMemo, useState } from 'react'; -import { EuiButton, EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; -import { useUserData } from '../../../../../detections/components/user_info'; -import { useFetchPrebuiltRulesStatusQuery } from '../../../../rule_management/api/hooks/prebuilt_rules/use_fetch_prebuilt_rules_status_query'; -import { useIsUpgradingSecurityPackages } from '../../../../rule_management/logic/use_upgrade_security_packages'; import type { RuleSignatureId } from '../../../../../../common/api/detection_engine'; +import type { RuleResponse } from '../../../../../../common/api/detection_engine/model/rule_schema'; import { invariant } from '../../../../../../common/utils/invariant'; +import { useUserData } from '../../../../../detections/components/user_info'; +import { useFetchPrebuiltRulesStatusQuery } from '../../../../rule_management/api/hooks/prebuilt_rules/use_fetch_prebuilt_rules_status_query'; +import { PERFORM_ALL_RULES_INSTALLATION_KEY } from '../../../../rule_management/api/hooks/prebuilt_rules/use_perform_all_rules_install_mutation'; import { usePerformInstallAllRules, usePerformInstallSpecificRules, } from '../../../../rule_management/logic/prebuilt_rules/use_perform_rule_install'; import { usePrebuiltRulesInstallReview } from '../../../../rule_management/logic/prebuilt_rules/use_prebuilt_rules_install_review'; -import type { AddPrebuiltRulesTableFilterOptions } from './use_filter_prebuilt_rules_to_install'; -import { useFilterPrebuiltRulesToInstall } from './use_filter_prebuilt_rules_to_install'; +import { useIsUpgradingSecurityPackages } from '../../../../rule_management/logic/use_upgrade_security_packages'; import { useRulePreviewFlyout } from '../use_rule_preview_flyout'; -import type { RuleResponse } from '../../../../../../common/api/detection_engine/model/rule_schema'; -import * as i18n from './translations'; import { isUpgradeReviewRequestEnabled } from './add_prebuilt_rules_utils'; +import * as i18n from './translations'; +import type { AddPrebuiltRulesTableFilterOptions } from './use_filter_prebuilt_rules_to_install'; +import { useFilterPrebuiltRulesToInstall } from './use_filter_prebuilt_rules_to_install'; export interface AddPrebuiltRulesTableState { /** @@ -59,6 +61,11 @@ export interface AddPrebuiltRulesTableState { * package in background */ isUpgradingSecurityPackages: boolean; + + /** + * Is true when performing Install All Rules mutation + */ + isInstallingAllRules: boolean; /** * List of rule IDs that are currently being upgraded */ @@ -112,6 +119,10 @@ export const AddPrebuiltRulesTableContextProvider = ({ const { data: prebuiltRulesStatus } = useFetchPrebuiltRulesStatusQuery(); const isUpgradingSecurityPackages = useIsUpgradingSecurityPackages(); + const isInstallingAllRules = + useIsMutating({ + mutationKey: PERFORM_ALL_RULES_INSTALLATION_KEY, + }) > 0; const { data: { rules, stats: { tags } } = { @@ -269,6 +280,7 @@ export const AddPrebuiltRulesTableContextProvider = ({ loadingRules, isRefetching, isUpgradingSecurityPackages, + isInstallingAllRules, selectedRules, lastUpdated: dataUpdatedAt, }, @@ -284,6 +296,7 @@ export const AddPrebuiltRulesTableContextProvider = ({ loadingRules, isRefetching, isUpgradingSecurityPackages, + isInstallingAllRules, selectedRules, dataUpdatedAt, actions, diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/add_prebuilt_rules_table/use_add_prebuilt_rules_table_columns.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/add_prebuilt_rules_table/use_add_prebuilt_rules_table_columns.tsx index eaf3af79ee360..a0a317e72b4c2 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/add_prebuilt_rules_table/use_add_prebuilt_rules_table_columns.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/add_prebuilt_rules_table/use_add_prebuilt_rules_table_columns.tsx @@ -110,7 +110,8 @@ const INTEGRATIONS_COLUMN: TableColumn = { const createInstallButtonColumn = ( installOneRule: AddPrebuiltRulesTableActions['installOneRule'], loadingRules: RuleSignatureId[], - isDisabled: boolean + isDisabled: boolean, + isInstallingAllRules: boolean ): TableColumn => ({ field: 'rule_id', name: , @@ -121,6 +122,7 @@ const createInstallButtonColumn = ( installOneRule={installOneRule} loadingRules={loadingRules} isDisabled={isDisabled} + isInstallingAllRules={isInstallingAllRules} /> ), width: '10%', @@ -132,11 +134,11 @@ export const useAddPrebuiltRulesTableColumns = (): TableColumn[] => { const hasCRUDPermissions = hasUserCRUDPermission(canUserCRUD); const [showRelatedIntegrations] = useUiSetting$(SHOW_RELATED_INTEGRATIONS_SETTING); const { - state: { loadingRules, isRefetching, isUpgradingSecurityPackages }, + state: { loadingRules, isRefetching, isUpgradingSecurityPackages, isInstallingAllRules }, actions: { installOneRule }, } = useAddPrebuiltRulesTableContext(); - const isDisabled = isRefetching || isUpgradingSecurityPackages; + const isDisabled = isRefetching || isUpgradingSecurityPackages || isInstallingAllRules; return useMemo( () => [ @@ -164,9 +166,23 @@ export const useAddPrebuiltRulesTableColumns = (): TableColumn[] => { width: '12%', }, ...(hasCRUDPermissions - ? [createInstallButtonColumn(installOneRule, loadingRules, isDisabled)] + ? [ + createInstallButtonColumn( + installOneRule, + loadingRules, + isDisabled, + isInstallingAllRules + ), + ] : []), ], - [hasCRUDPermissions, installOneRule, loadingRules, isDisabled, showRelatedIntegrations] + [ + hasCRUDPermissions, + installOneRule, + loadingRules, + isDisabled, + showRelatedIntegrations, + isInstallingAllRules, + ] ); };