Skip to content

Commit

Permalink
[Fleet] Move delete policy action from settings -> menus (#165921)
Browse files Browse the repository at this point in the history
## Summary

Closes #165888

Move "delete policy" action from a button on the policy settings screen
to an action button on the agent policy list and in the "Actions" menu
on the policy details page. This aligns the delete action with other
Elastic/Kibana products, and with how we treat deletion for other
resource types.

<img width="592" alt="image"
src="https://github.com/elastic/kibana/assets/6766512/0e893d34-fb80-4b40-933f-5c1145ab92e1">

<img width="665" alt="image"
src="https://github.com/elastic/kibana/assets/6766512/fdcf76c3-2e29-4912-99de-3738c2cbbddb">

If an agent policy contains a managed package policy, the delete option
will be disabled with a tooltip, e.g:

<img width="514" alt="image"
src="https://github.com/elastic/kibana/assets/6766512/d59226ab-a603-4a7f-80fd-9004382361f0">

I'd like to see if I can write a basic test case for the disabled logic
at least before this PR lands.

### Checklist

Delete any items that are not applicable to this PR.

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [ ] [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
- [x] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
  • Loading branch information
kpollich authored Sep 8, 2023
1 parent c6f72c7 commit 5773a1b
Show file tree
Hide file tree
Showing 10 changed files with 133 additions and 80 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
/*
* 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 type { AgentPolicy, PackagePolicy } from '../../../../../../common/types';

import { createFleetTestRendererMock } from '../../../../../mock';

import { AgentPolicyActionMenu } from './actions_menu';

describe('AgentPolicyActionMenu', () => {
const baseAgentPolicy: AgentPolicy = {
id: 'test',
is_managed: false,
is_protected: false,
name: 'test-agent-policy',
namespace: 'default',
package_policies: [] as PackagePolicy[],
revision: 1,
status: 'active',
updated_at: new Date().toISOString(),
updated_by: 'test',
};

describe('delete action', () => {
it('is enabled when a managed package policy is not present', () => {
const testRenderer = createFleetTestRendererMock();
const agentPolicyWithStandardPackagePolicy: AgentPolicy = {
...baseAgentPolicy,
package_policies: [
{
id: 'test-package-policy',
is_managed: false,
created_at: new Date().toISOString(),
created_by: 'test',
enabled: true,
inputs: [],
name: 'test-package-policy',
namespace: 'default',
policy_id: 'test',
revision: 1,
updated_at: new Date().toISOString(),
updated_by: 'test',
},
],
};

const result = testRenderer.render(
<AgentPolicyActionMenu agentPolicy={agentPolicyWithStandardPackagePolicy} />
);

const agentActionsButton = result.getByTestId('agentActionsBtn');
agentActionsButton.click();

const deleteButton = result.getByTestId('agentPolicyActionMenuDeleteButton');
expect(deleteButton).not.toHaveAttribute('disabled');
});

it('is disabled when a managed package policy is present', () => {
const testRenderer = createFleetTestRendererMock();
const agentPolicyWithManagedPackagePolicy: AgentPolicy = {
...baseAgentPolicy,
package_policies: [
{
id: 'test-package-policy',
is_managed: true,
created_at: new Date().toISOString(),
created_by: 'test',
enabled: true,
inputs: [],
name: 'test-package-policy',
namespace: 'default',
policy_id: 'test',
revision: 1,
updated_at: new Date().toISOString(),
updated_by: 'test',
},
],
};

const result = testRenderer.render(
<AgentPolicyActionMenu agentPolicy={agentPolicyWithManagedPackagePolicy} />
);

const agentActionsButton = result.getByTestId('agentActionsBtn');
agentActionsButton.click();

const deleteButton = result.getByTestId('agentPolicyActionMenuDeleteButton');
expect(deleteButton).toHaveAttribute('disabled');
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,11 @@ import {
} from '../../../components';
import { FLEET_SERVER_PACKAGE } from '../../../constants';

import { ExperimentalFeaturesService } from '../../../../../services/experimental_features';
import { policyHasFleetServer, ExperimentalFeaturesService } from '../../../services';

import { AgentPolicyYamlFlyout } from './agent_policy_yaml_flyout';
import { AgentPolicyCopyProvider } from './agent_policy_copy_provider';
import { AgentPolicyDeleteProvider } from './agent_policy_delete_provider';

export const AgentPolicyActionMenu = memo<{
agentPolicy: AgentPolicy;
Expand Down Expand Up @@ -55,6 +56,10 @@ export const AgentPolicyActionMenu = memo<{
[agentPolicy]
);

const hasManagedPackagePolicy =
'package_policies' in agentPolicy &&
agentPolicy?.package_policies?.some((packagePolicy) => packagePolicy.is_managed);

const [isContextMenuOpen, setIsContextMenuOpen] = useState(false);

const onContextMenuChange = useCallback(
Expand Down Expand Up @@ -129,6 +134,35 @@ export const AgentPolicyActionMenu = memo<{
defaultMessage="Duplicate policy"
/>
</EuiContextMenuItem>,
<AgentPolicyDeleteProvider
hasFleetServer={policyHasFleetServer(agentPolicy as AgentPolicy)}
key="deletePolicy"
>
{(deleteAgentPolicyPrompt) => (
<EuiContextMenuItem
data-test-subj="agentPolicyActionMenuDeleteButton"
disabled={hasManagedPackagePolicy}
toolTipContent={
hasManagedPackagePolicy ? (
<FormattedMessage
id="xpack.fleet.policyForm.deletePolicyActionText.disabled"
defaultMessage="Agent policy with managed package policies cannot be deleted."
data-test-subj="agentPolicyActionMenuDeleteButtonDisabledTooltip"
/>
) : undefined
}
icon="trash"
onClick={() => {
deleteAgentPolicyPrompt(agentPolicy.id);
}}
>
<FormattedMessage
id="xpack.fleet.agentPolicyActionMenu.deletePolicyActionText"
defaultMessage="Delete policy"
/>
</EuiContextMenuItem>
)}
</AgentPolicyDeleteProvider>,
];

if (agentTamperProtectionEnabled && !agentPolicy?.is_managed) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import {
EuiComboBox,
EuiIconTip,
EuiCheckboxGroup,
EuiButton,
EuiLink,
EuiFieldNumber,
EuiFieldText,
Expand Down Expand Up @@ -41,10 +40,9 @@ import { useStartServices, useConfig, useGetAgentPolicies, useLicense } from '..
import { AgentPolicyPackageBadge } from '../../../../components';
import { UninstallCommandFlyout } from '../../../../../../components';

import { AgentPolicyDeleteProvider } from '../agent_policy_delete_provider';
import type { ValidationResults } from '../agent_policy_validation';

import { ExperimentalFeaturesService, policyHasFleetServer } from '../../../../services';
import { ExperimentalFeaturesService } from '../../../../services';

import { policyHasEndpointSecurity as hasElasticDefend } from '../../../../../../../common/services';

Expand All @@ -60,15 +58,13 @@ interface Props {
updateAgentPolicy: (u: Partial<NewAgentPolicy | AgentPolicy>) => void;
validation: ValidationResults;
isEditing?: boolean;
onDelete?: () => void;
}

export const AgentPolicyAdvancedOptionsContent: React.FunctionComponent<Props> = ({
agentPolicy,
updateAgentPolicy,
validation,
isEditing = false,
onDelete = () => {},
}) => {
const { docLinks } = useStartServices();
const config = useConfig();
Expand Down Expand Up @@ -101,10 +97,6 @@ export const AgentPolicyAdvancedOptionsContent: React.FunctionComponent<Props> =
// agent monitoring checkbox group can appear multiple times in the DOM, ids have to be unique to work correctly
const monitoringCheckboxIdSuffix = Date.now();

const hasManagedPackagePolicy =
'package_policies' in agentPolicy &&
agentPolicy?.package_policies?.some((packagePolicy) => packagePolicy.is_managed);

const { agentTamperProtectionEnabled } = ExperimentalFeaturesService.get();
const licenseService = useLicense();
const [isUninstallCommandFlyoutOpen, setIsUninstallCommandFlyoutOpen] = useState(false);
Expand Down Expand Up @@ -725,57 +717,7 @@ export const AgentPolicyAdvancedOptionsContent: React.FunctionComponent<Props> =
/>
</EuiFormRow>
</EuiDescribedFormGroup>
{isEditing && 'id' in agentPolicy && !agentPolicy.is_managed ? (
<EuiDescribedFormGroup
title={
<h4>
<FormattedMessage
id="xpack.fleet.policyForm.deletePolicyGroupTitle"
defaultMessage="Delete policy"
/>
</h4>
}
description={
<>
<FormattedMessage
id="xpack.fleet.policyForm.deletePolicyGroupDescription"
defaultMessage="Existing data will not be deleted."
/>
<EuiSpacer size="s" />
<AgentPolicyDeleteProvider
hasFleetServer={policyHasFleetServer(agentPolicy as AgentPolicy)}
>
{(deleteAgentPolicyPrompt) => {
return (
<EuiToolTip
content={
hasManagedPackagePolicy ? (
<FormattedMessage
id="xpack.fleet.policyForm.deletePolicyActionText.disabled"
defaultMessage="Agent policy with managed package policies cannot be deleted."
/>
) : undefined
}
>
<EuiButton
data-test-subj="agentPolicyForm.downloadSource.deleteBtn"
color="danger"
onClick={() => deleteAgentPolicyPrompt(agentPolicy.id!, onDelete)}
isDisabled={hasManagedPackagePolicy}
>
<FormattedMessage
id="xpack.fleet.policyForm.deletePolicyActionText"
defaultMessage="Delete policy"
/>
</EuiButton>
</EuiToolTip>
);
}}
</AgentPolicyDeleteProvider>
</>
}
/>
) : null}
<EuiSpacer size="l" />
</>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,6 @@ export const AgentPolicyCreateInlineForm: React.FunctionComponent<Props> = ({
updateAgentPolicy={updateNewAgentPolicy}
validation={validation}
isEditing={false}
onDelete={() => {}}
/>
</StyledEuiAccordion>
</>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ interface Props {
updateSysMonitoring: (newValue: boolean) => void;
validation: ValidationResults;
isEditing?: boolean;
onDelete?: () => void;
}

export const AgentPolicyForm: React.FunctionComponent<Props> = ({
Expand All @@ -46,7 +45,6 @@ export const AgentPolicyForm: React.FunctionComponent<Props> = ({
updateSysMonitoring,
validation,
isEditing = false,
onDelete = () => {},
}) => {
const generalSettingsWrapper = (children: JSX.Element[]) => (
<EuiDescribedFormGroup
Expand Down Expand Up @@ -112,7 +110,6 @@ export const AgentPolicyForm: React.FunctionComponent<Props> = ({
updateAgentPolicy={updateAgentPolicy}
validation={validation}
isEditing={isEditing}
onDelete={onDelete}
/>
</StyledEuiAccordion>
</>
Expand All @@ -122,7 +119,6 @@ export const AgentPolicyForm: React.FunctionComponent<Props> = ({
updateAgentPolicy={updateAgentPolicy}
validation={validation}
isEditing={isEditing}
onDelete={onDelete}
/>
)}
</EuiForm>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ export const AgentPolicyIntegrationForm: React.FunctionComponent<Props> = ({
updateAgentPolicy={updateAgentPolicy}
validation={validation}
isEditing={isEditing}
onDelete={onDelete}
/>
</StyledEuiAccordion>
</>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
*/

import React, { memo, useMemo, useState } from 'react';
import { useHistory } from 'react-router-dom';
import styled from 'styled-components';
import { pick } from 'lodash';
import {
Expand All @@ -22,7 +21,6 @@ import { FormattedMessage } from '@kbn/i18n-react';

import type { AgentPolicy } from '../../../../../types';
import {
useLink,
useStartServices,
useAuthz,
sendUpdateAgentPolicy,
Expand Down Expand Up @@ -69,8 +67,6 @@ export const SettingsView = memo<{ agentPolicy: AgentPolicy }>(
const {
agents: { enabled: isFleetEnabled },
} = useConfig();
const history = useHistory();
const { getPath } = useLink();
const hasFleetAllPrivileges = useAuthz().fleet.all;
const refreshAgentPolicy = useAgentPolicyRefresh();
const [agentPolicy, setAgentPolicy] = useState<AgentPolicy>({
Expand Down Expand Up @@ -173,9 +169,6 @@ export const SettingsView = memo<{ agentPolicy: AgentPolicy }>(
updateSysMonitoring={(newValue) => setWithSysMonitoring(newValue)}
validation={validation}
isEditing={true}
onDelete={() => {
history.push(getPath('policies_list'));
}}
/>

{hasChanges ? (
Expand Down
2 changes: 0 additions & 2 deletions x-pack/plugins/translations/translations/fr-FR.json
Original file line number Diff line number Diff line change
Expand Up @@ -16564,8 +16564,6 @@
"xpack.fleet.policyDetailsPackagePolicies.createFirstTitle": "Ajouter votre première intégration",
"xpack.fleet.policyForm.deletePolicyActionText": "Supprimer la stratégie",
"xpack.fleet.policyForm.deletePolicyActionText.disabled": "La politique d'agent avec les politiques de package géré ne peut pas être supprimée.",
"xpack.fleet.policyForm.deletePolicyGroupDescription": "Les données existantes ne sont pas supprimées.",
"xpack.fleet.policyForm.deletePolicyGroupTitle": "Supprimer la stratégie",
"xpack.fleet.policyForm.generalSettingsGroupDescription": "Attribuez un nom et ajoutez une description à votre stratégie d'agent.",
"xpack.fleet.policyForm.generalSettingsGroupTitle": "Paramètres généraux",
"xpack.fleet.renameAgentTags.errorNotificationTitle": "La balise n’a pas pu être renommée",
Expand Down
2 changes: 0 additions & 2 deletions x-pack/plugins/translations/translations/ja-JP.json
Original file line number Diff line number Diff line change
Expand Up @@ -16578,8 +16578,6 @@
"xpack.fleet.policyDetailsPackagePolicies.createFirstTitle": "最初の統合を追加",
"xpack.fleet.policyForm.deletePolicyActionText": "ポリシーを削除",
"xpack.fleet.policyForm.deletePolicyActionText.disabled": "管理されたパッケージポリシーのエージェントポリシーは削除できません。",
"xpack.fleet.policyForm.deletePolicyGroupDescription": "既存のデータは削除されません。",
"xpack.fleet.policyForm.deletePolicyGroupTitle": "ポリシーを削除",
"xpack.fleet.policyForm.generalSettingsGroupDescription": "エージェントポリシーの名前と説明を選択してください。",
"xpack.fleet.policyForm.generalSettingsGroupTitle": "一般設定",
"xpack.fleet.renameAgentTags.errorNotificationTitle": "タグ名の変更が失敗しました",
Expand Down
2 changes: 0 additions & 2 deletions x-pack/plugins/translations/translations/zh-CN.json
Original file line number Diff line number Diff line change
Expand Up @@ -16578,8 +16578,6 @@
"xpack.fleet.policyDetailsPackagePolicies.createFirstTitle": "添加您的首个集成",
"xpack.fleet.policyForm.deletePolicyActionText": "删除策略",
"xpack.fleet.policyForm.deletePolicyActionText.disabled": "无法删除包含托管软件包策略的代理策略。",
"xpack.fleet.policyForm.deletePolicyGroupDescription": "现有数据将不会删除。",
"xpack.fleet.policyForm.deletePolicyGroupTitle": "删除策略",
"xpack.fleet.policyForm.generalSettingsGroupDescription": "为您的代理策略选择名称和描述。",
"xpack.fleet.policyForm.generalSettingsGroupTitle": "常规设置",
"xpack.fleet.renameAgentTags.errorNotificationTitle": "标签重命名失败",
Expand Down

0 comments on commit 5773a1b

Please sign in to comment.