From ffe091135cdd259e3386c87421418b8686b9e715 Mon Sep 17 00:00:00 2001 From: Jiawei Wu Date: Wed, 25 Sep 2024 21:21:53 -0700 Subject: [PATCH 1/5] update rule specific flapping tooltip UI --- .../sections/rule_form/rule_add.test.tsx | 10 + .../sections/rule_form/rule_edit.test.tsx | 10 + .../sections/rule_form/rule_form.test.tsx | 15 ++ .../rule_form_advanced_options.test.tsx | 12 +- .../rule_form/rule_form_advanced_options.tsx | 194 ++++++++++++++++-- .../public/common/constants/index.ts | 2 +- 6 files changed, 221 insertions(+), 22 deletions(-) diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_add.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_add.test.tsx index 7a9bdc88b10b4..af8bda5704b0f 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_add.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_add.test.tsx @@ -67,6 +67,13 @@ jest.mock('../../lib/action_connector_api', () => ({ loadAllActions: jest.fn(), })); +jest.mock('../../lib/rule_api/get_flapping_settings', () => ({ + getFlappingSettings: jest.fn().mockResolvedValue({ + lookBackWindow: 20, + statusChangeThreshold: 20, + }), +})); + const actionTypeRegistry = actionTypeRegistryMock.create(); const ruleTypeRegistry = ruleTypeRegistryMock.create(); @@ -149,6 +156,9 @@ describe('rule_add', () => { // eslint-disable-next-line react-hooks/rules-of-hooks useKibanaMock().services.application.capabilities = { ...capabilities, + rulesSettings: { + writeFlappingSettingsUI: true, + }, rules: { show: true, save: true, diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_edit.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_edit.test.tsx index 0109ba0acec19..331b10505a5d7 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_edit.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_edit.test.tsx @@ -63,6 +63,13 @@ jest.mock('@kbn/alerts-ui-shared/src/common/apis/fetch_ui_health_status', () => fetchUiHealthStatus: jest.fn(() => ({ isRulesAvailable: true })), })); +jest.mock('../../lib/rule_api/get_flapping_settings', () => ({ + getFlappingSettings: jest.fn().mockResolvedValue({ + lookBackWindow: 20, + statusChangeThreshold: 20, + }), +})); + describe('rule_edit', () => { let wrapper: ReactWrapper; let mockedCoreSetup: ReturnType; @@ -80,6 +87,9 @@ describe('rule_edit', () => { // eslint-disable-next-line react-hooks/rules-of-hooks useKibanaMock().services.application.capabilities = { ...capabilities, + rulesSettings: { + writeFlappingSettingsUI: true, + }, rules: { show: true, save: true, diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form.test.tsx index 7eafae8518df0..38ee1c73ac40b 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form.test.tsx @@ -71,6 +71,12 @@ jest.mock('../../lib/capabilities', () => ({ hasShowActionsCapability: jest.fn(() => true), hasExecuteActionsCapability: jest.fn(() => true), })); +jest.mock('../../lib/rule_api/get_flapping_settings', () => ({ + getFlappingSettings: jest.fn().mockResolvedValue({ + lookBackWindow: 20, + statusChangeThreshold: 20, + }), +})); describe('rule_form', () => { const ruleType = { @@ -203,6 +209,9 @@ describe('rule_form', () => { // eslint-disable-next-line react-hooks/rules-of-hooks useKibanaMock().services.application.capabilities = { ...capabilities, + rulesSettings: { + writeFlappingSettingsUI: true, + }, rules: { show: true, save: true, @@ -358,6 +367,9 @@ describe('rule_form', () => { // eslint-disable-next-line react-hooks/rules-of-hooks useKibanaMock().services.application.capabilities = { ...capabilities, + rulesSettings: { + writeFlappingSettingsUI: true, + }, rules: { show: true, save: true, @@ -1071,6 +1083,9 @@ describe('rule_form', () => { // eslint-disable-next-line react-hooks/rules-of-hooks useKibanaMock().services.application.capabilities = { ...capabilities, + rulesSettings: { + writeFlappingSettingsUI: true, + }, rules: { show: true, save: true, diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form_advanced_options.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form_advanced_options.test.tsx index 38d46c74d53f1..f6534f7451405 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form_advanced_options.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form_advanced_options.test.tsx @@ -13,6 +13,7 @@ import { __IntlProvider as IntlProvider } from '@kbn/i18n-react'; import { RuleFormAdvancedOptions } from './rule_form_advanced_options'; import { useKibana } from '../../../common/lib/kibana'; import userEvent from '@testing-library/user-event'; +import { ApplicationStart } from '@kbn/core-application-browser'; jest.mock('../../../common/lib/kibana'); @@ -38,6 +39,11 @@ describe('ruleFormAdvancedOptions', () => { enabled: true, }); useKibanaMock().services.http = http; + useKibanaMock().services.application.capabilities = { + rulesSettings: { + writeFlappingSettingsUI: true, + }, + } as unknown as ApplicationStart['capabilities']; }); afterEach(() => { @@ -80,7 +86,7 @@ describe('ruleFormAdvancedOptions', () => { expect(await screen.findByText('ON')).toBeInTheDocument(); expect(screen.getByTestId('ruleFormAdvancedOptionsOverrideSwitch')).not.toBeChecked(); - expect(screen.queryByText('Override')).not.toBeInTheDocument(); + expect(screen.queryByText('Custom')).not.toBeInTheDocument(); expect(screen.getByTestId('ruleSettingsFlappingMessage')).toHaveTextContent( 'An alert is flapping if it changes status at least 3 times in the last 10 rule runs.' ); @@ -111,7 +117,7 @@ describe('ruleFormAdvancedOptions', () => { ); expect(await screen.findByTestId('ruleFormAdvancedOptionsOverrideSwitch')).toBeChecked(); - expect(screen.getByText('Override')).toBeInTheDocument(); + expect(screen.getByText('Custom')).toBeInTheDocument(); expect(screen.getByTestId('lookBackWindowRangeInput')).toHaveValue('6'); expect(screen.getByTestId('statusChangeThresholdRangeInput')).toHaveValue('4'); expect(screen.getByTestId('ruleSettingsFlappingMessage')).toHaveTextContent( @@ -148,7 +154,7 @@ describe('ruleFormAdvancedOptions', () => { ); expect(await screen.findByText('OFF')).toBeInTheDocument(); - expect(screen.queryByText('Override')).not.toBeInTheDocument(); + expect(screen.queryByText('Custom')).not.toBeInTheDocument(); expect(screen.queryByTestId('ruleFormAdvancedOptionsOverrideSwitch')).not.toBeInTheDocument(); expect(screen.queryByTestId('ruleSettingsFlappingMessage')).not.toBeInTheDocument(); }); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form_advanced_options.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form_advanced_options.tsx index c9ec2adc6d770..5dd147d10c322 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form_advanced_options.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form_advanced_options.tsx @@ -5,7 +5,7 @@ * 2.0. */ -import React, { useCallback, useMemo, useRef } from 'react'; +import React, { useCallback, useMemo, useRef, useState } from 'react'; import { i18n } from '@kbn/i18n'; import { EuiBadge, @@ -24,11 +24,16 @@ import { EuiSplitPanel, EuiLoadingSpinner, EuiLink, + EuiButtonIcon, + EuiPopover, + EuiPopoverTitle, } from '@elastic/eui'; import { RuleSettingsFlappingInputs } from '@kbn/alerts-ui-shared/src/rule_settings/rule_settings_flapping_inputs'; import { RuleSettingsFlappingMessage } from '@kbn/alerts-ui-shared/src/rule_settings/rule_settings_flapping_message'; import { RuleSpecificFlappingProperties } from '@kbn/alerting-plugin/common'; +import { FormattedMessage } from '@kbn/i18n-react'; import { useGetFlappingSettings } from '../../hooks/use_get_flapping_settings'; +import { useKibana } from '../../../common/lib/kibana'; const alertDelayFormRowLabel = i18n.translate( 'xpack.triggersActionsUI.sections.ruleForm.alertDelayLabel', @@ -80,7 +85,7 @@ const flappingOffLabel = i18n.translate( const flappingOverrideLabel = i18n.translate( 'xpack.triggersActionsUI.ruleFormAdvancedOptions.overrideLabel', { - defaultMessage: 'Override', + defaultMessage: 'Custom', } ); @@ -105,11 +110,38 @@ const flappingFormRowLabel = i18n.translate( } ); -const flappingIconTipDescription = i18n.translate( - 'xpack.triggersActionsUI.ruleFormAdvancedOptions.flappingIconTipDescription', +const flappingOffContentRules = i18n.translate( + 'xpack.triggersActionsUI.ruleFormAdvancedOptions.flappingOffContentRules', { - defaultMessage: - 'Detect alerts that switch quickly between active and recovered states and reduce unwanted noise for these flapping alerts.', + defaultMessage: 'Rules', + } +); + +const flappingOffContentSettings = i18n.translate( + 'xpack.triggersActionsUI.ruleFormAdvancedOptions.flappingOffContentSettings', + { + defaultMessage: 'Settings', + } +); + +const flappingTitlePopoverFlappingDetection = i18n.translate( + 'xpack.triggersActionsUI.ruleFormAdvancedOptions.flappingTitlePopoverFlappingDetection', + { + defaultMessage: 'flapping detection', + } +); + +const flappingTitlePopoverAlertStatus = i18n.translate( + 'xpack.triggersActionsUI.ruleFormAdvancedOptions.flappingTitlePopoverAlertStatus', + { + defaultMessage: 'alert status change threshold', + } +); + +const flappingTitlePopoverLookBack = i18n.translate( + 'xpack.triggersActionsUI.ruleFormAdvancedOptions.flappingTitlePopoverLookBack', + { + defaultMessage: 'rule run look back window', } ); @@ -139,6 +171,17 @@ export const RuleFormAdvancedOptions = (props: RuleFormAdvancedOptionsProps) => onFlappingChange, } = props; + const { + application: { + capabilities: { + rulesSettings: { writeFlappingSettingsUI }, + }, + }, + } = useKibana().services; + + const [isFlappingOffPopoverOpen, setIsFlappingOffPopoverOpen] = useState(false); + const [isFlappingTitlePopoverOpen, setIsFlappingTitlePopoverOpen] = useState(false); + const cachedFlappingSettings = useRef(); const isDesktop = useIsWithinMinBreakpoint('xl'); @@ -209,6 +252,121 @@ export const RuleFormAdvancedOptions = (props: RuleFormAdvancedOptionsProps) => }); }, [spaceFlappingSettings, flappingSettings, onFlappingChange]); + const flappingTitleTooltip = useMemo(() => { + return ( + setIsFlappingTitlePopoverOpen(false)} + panelStyle={{ + width: 500, + }} + button={ + setIsFlappingTitlePopoverOpen(true)} + /> + } + > + Alert flapping detection + + {flappingTitlePopoverFlappingDetection}, + }} + /> + + + + {flappingTitlePopoverAlertStatus}, + }} + /> + + + + {flappingTitlePopoverLookBack}, + }} + /> + + + + {flappingOffContentRules}, + settings: {flappingOffContentSettings}, + }} + /> + + + ); + }, [isFlappingTitlePopoverOpen]); + + const flappingOffTooltip = useMemo(() => { + if (!spaceFlappingSettings) { + return null; + } + const { enabled } = spaceFlappingSettings; + if (enabled) { + return null; + } + + if (writeFlappingSettingsUI) { + return ( + setIsFlappingOffPopoverOpen(false)} + panelStyle={{ + width: 250, + }} + button={ + setIsFlappingOffPopoverOpen(true)} + /> + } + > + + {flappingOffContentRules}, + settings: {flappingOffContentSettings}, + }} + /> + + + ); + } + // TODO: Add the external doc link here! + return ( + + {flappingExternalLinkLabel} + + ); + }, [writeFlappingSettingsUI, isFlappingOffPopoverOpen, spaceFlappingSettings]); + const flappingFormHeader = useMemo(() => { if (!spaceFlappingSettings) { return null; @@ -243,12 +401,7 @@ export const RuleFormAdvancedOptions = (props: RuleFormAdvancedOptionsProps) => onChange={onFlappingToggle} /> )} - {!enabled && ( - // TODO: Add the help link here - - {flappingExternalLinkLabel} - - )} + {flappingOffTooltip} {flappingSettings && ( @@ -259,7 +412,14 @@ export const RuleFormAdvancedOptions = (props: RuleFormAdvancedOptionsProps) => )} ); - }, [isDesktop, euiTheme, spaceFlappingSettings, flappingSettings, onFlappingToggle]); + }, [ + isDesktop, + euiTheme, + spaceFlappingSettings, + flappingSettings, + flappingOffTooltip, + onFlappingToggle, + ]); const flappingFormBody = useMemo(() => { if (!flappingSettings) { @@ -332,11 +492,9 @@ export const RuleFormAdvancedOptions = (props: RuleFormAdvancedOptionsProps) => - {flappingFormRowLabel} - - - + + {flappingFormRowLabel} + {flappingTitleTooltip} } data-test-subj="alertFlappingFormRow" diff --git a/x-pack/plugins/triggers_actions_ui/public/common/constants/index.ts b/x-pack/plugins/triggers_actions_ui/public/common/constants/index.ts index a2b54a0562f66..27fa3e1effa3b 100644 --- a/x-pack/plugins/triggers_actions_ui/public/common/constants/index.ts +++ b/x-pack/plugins/triggers_actions_ui/public/common/constants/index.ts @@ -27,7 +27,7 @@ export { } from '@kbn/alerts-ui-shared/src/common/constants/i18n_weekdays'; // Feature flag for frontend rule specific flapping in rule flyout -export const IS_RULE_SPECIFIC_FLAPPING_ENABLED = false; +export const IS_RULE_SPECIFIC_FLAPPING_ENABLED = true; export const builtInComparators: { [key: string]: Comparator } = { [COMPARATORS.GREATER_THAN]: { From f2ea1e22063eac6964374b9cfa25e8652e2fb7a0 Mon Sep 17 00:00:00 2001 From: Jiawei Wu Date: Wed, 25 Sep 2024 21:23:07 -0700 Subject: [PATCH 2/5] Turn off flag --- .../triggers_actions_ui/public/common/constants/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/triggers_actions_ui/public/common/constants/index.ts b/x-pack/plugins/triggers_actions_ui/public/common/constants/index.ts index 27fa3e1effa3b..a2b54a0562f66 100644 --- a/x-pack/plugins/triggers_actions_ui/public/common/constants/index.ts +++ b/x-pack/plugins/triggers_actions_ui/public/common/constants/index.ts @@ -27,7 +27,7 @@ export { } from '@kbn/alerts-ui-shared/src/common/constants/i18n_weekdays'; // Feature flag for frontend rule specific flapping in rule flyout -export const IS_RULE_SPECIFIC_FLAPPING_ENABLED = true; +export const IS_RULE_SPECIFIC_FLAPPING_ENABLED = false; export const builtInComparators: { [key: string]: Comparator } = { [COMPARATORS.GREATER_THAN]: { From 373c9d177ebf4ac7ac08eb527ca885effd217441 Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Thu, 26 Sep 2024 04:38:20 +0000 Subject: [PATCH 3/5] [CI] Auto-commit changed files from 'node scripts/lint_ts_projects --fix' --- x-pack/plugins/triggers_actions_ui/tsconfig.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/triggers_actions_ui/tsconfig.json b/x-pack/plugins/triggers_actions_ui/tsconfig.json index 02456cc04af6b..ff488d41ccbbb 100644 --- a/x-pack/plugins/triggers_actions_ui/tsconfig.json +++ b/x-pack/plugins/triggers_actions_ui/tsconfig.json @@ -70,7 +70,8 @@ "@kbn/alerting-types", "@kbn/visualization-utils", "@kbn/core-ui-settings-browser", - "@kbn/observability-alerting-rule-utils" + "@kbn/observability-alerting-rule-utils", + "@kbn/core-application-browser" ], "exclude": ["target/**/*"] } From 6f120d5cfab3dda3bbd5f428b641998527d6c4c5 Mon Sep 17 00:00:00 2001 From: Jiawei Wu Date: Tue, 1 Oct 2024 00:41:52 -0600 Subject: [PATCH 4/5] Fix bug with popover not closing --- .../rule_form/rule_form_advanced_options.tsx | 163 +++++++++--------- 1 file changed, 83 insertions(+), 80 deletions(-) diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form_advanced_options.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form_advanced_options.tsx index 5dd147d10c322..560ba3e578cf7 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form_advanced_options.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form_advanced_options.tsx @@ -27,6 +27,7 @@ import { EuiButtonIcon, EuiPopover, EuiPopoverTitle, + EuiOutsideClickDetector, } from '@elastic/eui'; import { RuleSettingsFlappingInputs } from '@kbn/alerts-ui-shared/src/rule_settings/rule_settings_flapping_inputs'; import { RuleSettingsFlappingMessage } from '@kbn/alerts-ui-shared/src/rule_settings/rule_settings_flapping_message'; @@ -254,101 +255,58 @@ export const RuleFormAdvancedOptions = (props: RuleFormAdvancedOptionsProps) => const flappingTitleTooltip = useMemo(() => { return ( - setIsFlappingTitlePopoverOpen(false)} - panelStyle={{ - width: 500, - }} - button={ - setIsFlappingTitlePopoverOpen(true)} - /> - } - > - Alert flapping detection - - {flappingTitlePopoverFlappingDetection}, - }} - /> - - - - {flappingTitlePopoverAlertStatus}, - }} - /> - - - - {flappingTitlePopoverLookBack}, - }} - /> - - - - {flappingOffContentRules}, - settings: {flappingOffContentSettings}, - }} - /> - - - ); - }, [isFlappingTitlePopoverOpen]); - - const flappingOffTooltip = useMemo(() => { - if (!spaceFlappingSettings) { - return null; - } - const { enabled } = spaceFlappingSettings; - if (enabled) { - return null; - } - - if (writeFlappingSettingsUI) { - return ( + setIsFlappingTitlePopoverOpen(false)}> setIsFlappingOffPopoverOpen(false)} panelStyle={{ - width: 250, + width: 500, }} button={ setIsFlappingOffPopoverOpen(true)} + aria-label="Flapping title info" + onClick={() => setIsFlappingTitlePopoverOpen(!isFlappingTitlePopoverOpen)} /> } > + Alert flapping detection + + {flappingTitlePopoverFlappingDetection}, + }} + /> + + + + {flappingTitlePopoverAlertStatus}, + }} + /> + + {flappingTitlePopoverLookBack}, + }} + /> + + + + {flappingOffContentRules}, @@ -357,6 +315,51 @@ export const RuleFormAdvancedOptions = (props: RuleFormAdvancedOptionsProps) => /> + + ); + }, [isFlappingTitlePopoverOpen]); + + const flappingOffTooltip = useMemo(() => { + if (!spaceFlappingSettings) { + return null; + } + const { enabled } = spaceFlappingSettings; + if (enabled) { + return null; + } + + if (writeFlappingSettingsUI) { + return ( + setIsFlappingOffPopoverOpen(false)}> + setIsFlappingOffPopoverOpen(!isFlappingOffPopoverOpen)} + /> + } + > + + {flappingOffContentRules}, + settings: {flappingOffContentSettings}, + }} + /> + + + ); } // TODO: Add the external doc link here! From 7c33ea7c657fe78bd939f7d87c5c91707b7818c3 Mon Sep 17 00:00:00 2001 From: Jiawei Wu Date: Wed, 2 Oct 2024 20:02:34 -0600 Subject: [PATCH 5/5] Fix unit test --- .../sections/rule_form/rule_form_advanced_options.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form_advanced_options.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form_advanced_options.tsx index 560ba3e578cf7..e51f24b53c2a4 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form_advanced_options.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form_advanced_options.tsx @@ -174,12 +174,12 @@ export const RuleFormAdvancedOptions = (props: RuleFormAdvancedOptionsProps) => const { application: { - capabilities: { - rulesSettings: { writeFlappingSettingsUI }, - }, + capabilities: { rulesSettings }, }, } = useKibana().services; + const { writeFlappingSettingsUI = false } = rulesSettings || {}; + const [isFlappingOffPopoverOpen, setIsFlappingOffPopoverOpen] = useState(false); const [isFlappingTitlePopoverOpen, setIsFlappingTitlePopoverOpen] = useState(false);