Skip to content

Commit

Permalink
[Security Solution] Disable Install All button when installation is i…
Browse files Browse the repository at this point in the history
…n progress (elastic#201731)

**Resolves: elastic#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
  • Loading branch information
jkelas authored Nov 27, 2024
1 parent 19a2ff8 commit 9ca719c
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export const AddPrebuiltRulesHeaderButtons = () => {
loadingRules,
isRefetching,
isUpgradingSecurityPackages,
isInstallingAllRules,
hasRulesToInstall,
},
actions: { installAllRules, installSelectedRules },
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import React, { useCallback, useMemo } from 'react';
import useBoolean from 'react-use/lib/useBoolean';
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 {
Expand All @@ -28,6 +28,7 @@ export interface PrebuiltRulesInstallButtonProps {
installOneRule: AddPrebuiltRulesTableActions['installOneRule'];
loadingRules: RuleSignatureId[];
isDisabled: boolean;
isInstallingAllRules: boolean;
}

export const PrebuiltRulesInstallButton = ({
Expand All @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => ({
Expand Down Expand Up @@ -109,10 +110,12 @@ describe('AddPrebuiltRulesTable', () => {
]);

render(
<AddPrebuiltRulesTableContextProvider>
<AddPrebuiltRulesHeaderButtons />
<AddPrebuiltRulesTable />
</AddPrebuiltRulesTableContextProvider>
<QueryClientProvider client={new QueryClient()}>
<AddPrebuiltRulesTableContextProvider>
<AddPrebuiltRulesHeaderButtons />
<AddPrebuiltRulesTable />
</AddPrebuiltRulesTableContextProvider>
</QueryClientProvider>
);

const installAllButton = screen.getByTestId('installAllRulesButton');
Expand All @@ -132,10 +135,12 @@ describe('AddPrebuiltRulesTable', () => {
(useIsUpgradingSecurityPackages as jest.Mock).mockReturnValueOnce(true);

render(
<AddPrebuiltRulesTableContextProvider>
<AddPrebuiltRulesHeaderButtons />
<AddPrebuiltRulesTable />
</AddPrebuiltRulesTableContextProvider>
<QueryClientProvider client={new QueryClient()}>
<AddPrebuiltRulesTableContextProvider>
<AddPrebuiltRulesHeaderButtons />
<AddPrebuiltRulesTable />
</AddPrebuiltRulesTableContextProvider>
</QueryClientProvider>
);

const installAllButton = screen.getByTestId('installAllRulesButton');
Expand All @@ -153,10 +158,12 @@ describe('AddPrebuiltRulesTable', () => {
]);

render(
<AddPrebuiltRulesTableContextProvider>
<AddPrebuiltRulesHeaderButtons />
<AddPrebuiltRulesTable />
</AddPrebuiltRulesTableContextProvider>
<QueryClientProvider client={new QueryClient()}>
<AddPrebuiltRulesTableContextProvider>
<AddPrebuiltRulesHeaderButtons />
<AddPrebuiltRulesTable />
</AddPrebuiltRulesTableContextProvider>
</QueryClientProvider>
);

const installAllButton = screen.getByTestId('installAllRulesButton');
Expand Down Expand Up @@ -198,9 +205,11 @@ describe('AddPrebuiltRulesTable', () => {
});

const { findByText } = render(
<AddPrebuiltRulesTableContextProvider>
<AddPrebuiltRulesTable />
</AddPrebuiltRulesTableContextProvider>
<QueryClientProvider client={new QueryClient()}>
<AddPrebuiltRulesTableContextProvider>
<AddPrebuiltRulesTable />
</AddPrebuiltRulesTableContextProvider>
</QueryClientProvider>
);

expect(await findByText('All Elastic rules have been installed')).toBeInTheDocument();
Expand Down Expand Up @@ -245,9 +254,11 @@ describe('AddPrebuiltRulesTable', () => {
});

render(
<AddPrebuiltRulesTableContextProvider>
<AddPrebuiltRulesTable />
</AddPrebuiltRulesTableContextProvider>
<QueryClientProvider client={new QueryClient()}>
<AddPrebuiltRulesTableContextProvider>
<AddPrebuiltRulesTable />
</AddPrebuiltRulesTableContextProvider>
</QueryClientProvider>
);

const installRuleButton = screen.queryByTestId(`installSinglePrebuiltRuleButton-${id}`);
Expand Down Expand Up @@ -293,9 +304,11 @@ describe('AddPrebuiltRulesTable', () => {
});

render(
<AddPrebuiltRulesTableContextProvider>
<AddPrebuiltRulesTable />
</AddPrebuiltRulesTableContextProvider>
<QueryClientProvider client={new QueryClient()}>
<AddPrebuiltRulesTableContextProvider>
<AddPrebuiltRulesTable />
</AddPrebuiltRulesTableContextProvider>
</QueryClientProvider>
);

const installRuleButton = screen.queryByTestId(`installSinglePrebuiltRuleButton-${id}`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
/**
Expand Down Expand Up @@ -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
*/
Expand Down Expand Up @@ -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 } } = {
Expand Down Expand Up @@ -269,6 +280,7 @@ export const AddPrebuiltRulesTableContextProvider = ({
loadingRules,
isRefetching,
isUpgradingSecurityPackages,
isInstallingAllRules,
selectedRules,
lastUpdated: dataUpdatedAt,
},
Expand All @@ -284,6 +296,7 @@ export const AddPrebuiltRulesTableContextProvider = ({
loadingRules,
isRefetching,
isUpgradingSecurityPackages,
isInstallingAllRules,
selectedRules,
dataUpdatedAt,
actions,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: <RulesTableEmptyColumnName name={i18n.INSTALL_RULE_BUTTON} />,
Expand All @@ -121,6 +122,7 @@ const createInstallButtonColumn = (
installOneRule={installOneRule}
loadingRules={loadingRules}
isDisabled={isDisabled}
isInstallingAllRules={isInstallingAllRules}
/>
),
width: '10%',
Expand All @@ -132,11 +134,11 @@ export const useAddPrebuiltRulesTableColumns = (): TableColumn[] => {
const hasCRUDPermissions = hasUserCRUDPermission(canUserCRUD);
const [showRelatedIntegrations] = useUiSetting$<boolean>(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(
() => [
Expand Down Expand Up @@ -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,
]
);
};

0 comments on commit 9ca719c

Please sign in to comment.