From 5d92f3eee5eb3660c400320c87f40450c3855fdd Mon Sep 17 00:00:00 2001 From: Gilles De Mey Date: Wed, 9 Jul 2025 15:44:44 +0200 Subject: [PATCH] Alerting: Fix silence / delete on new list view (#107885) --- .../unified/components/MenuItemPauseRule.tsx | 15 +++--- .../components/rule-viewer/AlertRuleMenu.tsx | 37 +++++++++++--- .../components/rules/RuleActionsButtons.tsx | 13 +++-- .../silences/SilenceGrafanaRuleDrawer.tsx | 49 ++++++++----------- .../rule-list/GrafanaGroupLoader.test.tsx | 13 +++-- .../components/RuleActionsButtons.V2.tsx | 21 ++++++-- .../features/alerting/unified/utils/rules.ts | 19 +++++++ 7 files changed, 113 insertions(+), 54 deletions(-) diff --git a/public/app/features/alerting/unified/components/MenuItemPauseRule.tsx b/public/app/features/alerting/unified/components/MenuItemPauseRule.tsx index 0424566f79d..1610330aad0 100644 --- a/public/app/features/alerting/unified/components/MenuItemPauseRule.tsx +++ b/public/app/features/alerting/unified/components/MenuItemPauseRule.tsx @@ -1,15 +1,14 @@ import { Menu } from '@grafana/ui'; import { useAppNotification } from 'app/core/copy/appNotification'; import { GrafanaRuleGroupIdentifier } from 'app/types/unified-alerting'; -import { RulerGrafanaRuleDTO } from 'app/types/unified-alerting-dto'; import { usePauseRuleInGroup } from '../hooks/ruleGroup/usePauseAlertRule'; import { isLoading } from '../hooks/useAsync'; import { stringifyErrorLike } from '../utils/misc'; -import { isPausedRule } from '../utils/rules'; interface Props { - rule: RulerGrafanaRuleDTO; + uid: string; + isPaused: boolean; groupIdentifier: GrafanaRuleGroupIdentifier; /** * Method invoked after the request to change the paused state has completed @@ -21,20 +20,18 @@ interface Props { * Menu item to display correct text for pausing/resuming an alert, * and triggering API call to do so */ -const MenuItemPauseRule = ({ rule, groupIdentifier, onPauseChange }: Props) => { +const MenuItemPauseRule = ({ uid, isPaused, groupIdentifier, onPauseChange }: Props) => { const notifyApp = useAppNotification(); const [pauseRule, updateState] = usePauseRuleInGroup(); - const [icon, title] = isPausedRule(rule) - ? ['play' as const, 'Resume evaluation'] - : ['pause' as const, 'Pause evaluation']; + const [icon, title] = isPaused ? ['play' as const, 'Resume evaluation'] : ['pause' as const, 'Pause evaluation']; /** * Triggers API call to update the current rule to the new `is_paused` state */ const setRulePause = async (newIsPaused: boolean) => { try { - await pauseRule.execute(groupIdentifier, rule.grafana_alert.uid, newIsPaused); + await pauseRule.execute(groupIdentifier, uid, newIsPaused); } catch (error) { notifyApp.error(`Failed to ${newIsPaused ? 'pause' : 'resume'} the rule: ${stringifyErrorLike(error)}`); return; @@ -49,7 +46,7 @@ const MenuItemPauseRule = ({ rule, groupIdentifier, onPauseChange }: Props) => { icon={icon} disabled={isLoading(updateState)} onClick={() => { - setRulePause(!rule.grafana_alert.is_paused); + setRulePause(!isPaused); }} /> ); diff --git a/public/app/features/alerting/unified/components/rule-viewer/AlertRuleMenu.tsx b/public/app/features/alerting/unified/components/rule-viewer/AlertRuleMenu.tsx index 77e751238cf..0ad498ebc65 100644 --- a/public/app/features/alerting/unified/components/rule-viewer/AlertRuleMenu.tsx +++ b/public/app/features/alerting/unified/components/rule-viewer/AlertRuleMenu.tsx @@ -7,7 +7,7 @@ import appEvents from 'app/core/app_events'; import MenuItemPauseRule from 'app/features/alerting/unified/components/MenuItemPauseRule'; import MoreButton from 'app/features/alerting/unified/components/MoreButton'; import { useRulePluginLinkExtension } from 'app/features/alerting/unified/plugins/useRulePluginLinkExtensions'; -import { Rule, RuleGroupIdentifierV2, RuleIdentifier } from 'app/types/unified-alerting'; +import { EditableRuleIdentifier, Rule, RuleGroupIdentifierV2, RuleIdentifier } from 'app/types/unified-alerting'; import { PromAlertingRuleState, RulerRuleDTO } from 'app/types/unified-alerting-dto'; import { @@ -18,7 +18,13 @@ import { } from '../../hooks/useAbilities'; import { createShareLink, isLocalDevEnv, isOpenSourceEdition } from '../../utils/misc'; import * as ruleId from '../../utils/rule-id'; -import { prometheusRuleType, rulerRuleType } from '../../utils/rules'; +import { + getRuleUID, + isEditableRuleIdentifier, + isPausedRule, + prometheusRuleType, + rulerRuleType, +} from '../../utils/rules'; import { createRelativeUrl } from '../../utils/url'; import { DeclareIncidentMenuItem } from '../bridges/DeclareIncidentButton'; @@ -28,7 +34,7 @@ interface Props { identifier: RuleIdentifier; groupIdentifier: RuleGroupIdentifierV2; handleSilence: () => void; - handleDelete: (rule: RulerRuleDTO, groupIdentifier: RuleGroupIdentifierV2) => void; + handleDelete: (identifier: EditableRuleIdentifier, groupIdentifier: RuleGroupIdentifierV2) => void; handleDuplicateRule: (identifier: RuleIdentifier) => void; onPauseChange?: () => void; buttonSize?: ComponentSize; @@ -118,10 +124,22 @@ const AlertRuleMenu = ({ const showDivider = [canPause, canSilence, shouldShowDeclareIncidentButton, canDuplicate].some(Boolean) && [canExport].some(Boolean); + // grab the UID from either rulerRule or promRule + const ruleUid = getRuleUID(rulerRule ?? promRule); + + const isPaused = + (rulerRuleType.grafana.rule(rulerRule) && isPausedRule(rulerRule)) || + (prometheusRuleType.grafana.rule(promRule) && promRule.isPaused); + const menuItems = ( <> - {canPause && rulerRuleType.grafana.rule(rulerRule) && groupIdentifier.groupOrigin === 'grafana' && ( - + {canPause && ruleUid && groupIdentifier.groupOrigin === 'grafana' && ( + )} {canSilence && ( )} - {canDelete && rulerRule && ( + {canDelete && ( <> handleDelete(rulerRule, groupIdentifier)} + onClick={() => { + // if the identifier is not for a editable rule I wonder how you even got here. + if (isEditableRuleIdentifier(identifier)) { + handleDelete(identifier, groupIdentifier); + } + }} /> )} diff --git a/public/app/features/alerting/unified/components/rules/RuleActionsButtons.tsx b/public/app/features/alerting/unified/components/rules/RuleActionsButtons.tsx index fc1142d3656..c6bcd640657 100644 --- a/public/app/features/alerting/unified/components/rules/RuleActionsButtons.tsx +++ b/public/app/features/alerting/unified/components/rules/RuleActionsButtons.tsx @@ -1,3 +1,4 @@ +import { isString } from 'lodash'; import { useState } from 'react'; import { Trans, t } from '@grafana/i18n'; @@ -16,7 +17,7 @@ import { GRAFANA_RULES_SOURCE_NAME, getRulesSourceName } from '../../utils/datas import { groupIdentifier } from '../../utils/groupIdentifier'; import { createViewLink } from '../../utils/misc'; import * as ruleId from '../../utils/rule-id'; -import { rulerRuleType } from '../../utils/rules'; +import { getRuleUID, prometheusRuleType, rulerRuleType } from '../../utils/rules'; import { createRelativeUrl } from '../../utils/url'; import { RedirectToCloneRule } from './CloneRule'; @@ -104,6 +105,12 @@ export const RuleActionsButtons = ({ compact, showViewButton, rule, rulesSource return null; } + // determine if this rule can be silenced by checking for Grafana Alert rule type and extracting the UID + const ruleUid = getRuleUID(rule.rulerRule ?? rule.promRule); + const silenceableRule = + isString(ruleUid) && + (rulerRuleType.grafana.alertingRule(rule.rulerRule) || prometheusRuleType.grafana.alertingRule(rule.promRule)); + return ( {buttons} @@ -132,8 +139,8 @@ export const RuleActionsButtons = ({ compact, showViewButton, rule, rulesSource buttonSize={buttonSize} /> {deleteModal} - {rulerRuleType.grafana.alertingRule(rule.rulerRule) && showSilenceDrawer && ( - setShowSilenceDrawer(false)} /> + {silenceableRule && showSilenceDrawer && ( + setShowSilenceDrawer(false)} /> )} {redirectToClone?.identifier && ( void; }; @@ -17,32 +16,26 @@ type Props = { * For a given Grafana managed rule, renders a drawer containing silences editor and Alertmanager selection */ const SilenceGrafanaRuleDrawer = React.memo( - ({ rulerRule, onClose }: Props) => { - const { uid } = rulerRule.grafana_alert; - - return ( - - - - - - - - ); - }, - (prevProps, nextProps) => { - return prevProps.rulerRule.grafana_alert.uid === nextProps.rulerRule.grafana_alert.uid; - } + ({ ruleUid, onClose }: Props) => ( + + + + + + + + ), + (prevProps, nextProps) => prevProps.ruleUid === nextProps.ruleUid ); export default SilenceGrafanaRuleDrawer; diff --git a/public/app/features/alerting/unified/rule-list/GrafanaGroupLoader.test.tsx b/public/app/features/alerting/unified/rule-list/GrafanaGroupLoader.test.tsx index e4de86c39e8..3169996b88c 100644 --- a/public/app/features/alerting/unified/rule-list/GrafanaGroupLoader.test.tsx +++ b/public/app/features/alerting/unified/rule-list/GrafanaGroupLoader.test.tsx @@ -40,6 +40,7 @@ const ui = { copyLink: () => byRole('menuitem', { name: /copy link/i }), export: () => byRole('menuitem', { name: /export/i }), delete: () => byRole('menuitem', { name: /delete/i }), + pause: () => byRole('menuitem', { name: /pause/i }), }, }; @@ -188,7 +189,7 @@ describe('GrafanaGroupLoader', () => { const menu = byRole('menu').get(); expect(menu).toBeInTheDocument(); - // With proper permissions, all 4 menu actions should be available: + // With proper permissions, all 6 menu actions should be available: // 1. Silence notifications - available for alerting rules (AlertingSilenceCreate permission) expect(ui.menuItems.silence().get()).toBeInTheDocument(); @@ -202,9 +203,15 @@ describe('GrafanaGroupLoader', () => { // 4. Export - should be available for Grafana alerting rules (AlertingRuleRead permission) expect(ui.menuItems.export().get()).toBeInTheDocument(); - // Verify that the menu contains all 4 expected menu items + // 5. Delete - should be available for Grafana alerting rules (AlertingRuleDelete permission) + expect(ui.menuItems.delete().get()).toBeInTheDocument(); + + // 6. Pause - should be available for Grafana alerting rules (AlertingRuleUpdate permission) + expect(ui.menuItems.pause().get()).toBeInTheDocument(); + + // Verify that the menu contains all 6 expected menu items const menuItems = byRole('menuitem').getAll(); - expect(menuItems.length).toBe(4); + expect(menuItems.length).toBe(6); }); }); diff --git a/public/app/features/alerting/unified/rule-list/components/RuleActionsButtons.V2.tsx b/public/app/features/alerting/unified/rule-list/components/RuleActionsButtons.V2.tsx index 37e56db4058..4570b3dd8e9 100644 --- a/public/app/features/alerting/unified/rule-list/components/RuleActionsButtons.V2.tsx +++ b/public/app/features/alerting/unified/rule-list/components/RuleActionsButtons.V2.tsx @@ -1,3 +1,4 @@ +import { isString } from 'lodash'; import { useState } from 'react'; import { RequireAtLeastOne } from 'type-fest'; @@ -19,7 +20,13 @@ import { RulerRuleDTO } from 'app/types/unified-alerting-dto'; import { logWarning } from '../../Analytics'; import { AlertRuleAction, skipToken, useGrafanaPromRuleAbility, useRulerRuleAbility } from '../../hooks/useAbilities'; import * as ruleId from '../../utils/rule-id'; -import { isProvisionedPromRule, isProvisionedRule, prometheusRuleType, rulerRuleType } from '../../utils/rules'; +import { + getRuleUID, + isProvisionedPromRule, + isProvisionedRule, + prometheusRuleType, + rulerRuleType, +} from '../../utils/rules'; import { createRelativeUrl } from '../../utils/url'; type RuleProps = RequireAtLeastOne<{ @@ -69,6 +76,12 @@ export function RuleActionsButtons({ compact, rule, promRule, groupIdentifier }: return null; } + // determine if this rule can be silenced by checking for Grafana Alert rule type and extracting the UID + const ruleUid = getRuleUID(rule ?? promRule); + const silenceableRule = + isString(ruleUid) && + (rulerRuleType.grafana.alertingRule(rule) || prometheusRuleType.grafana.alertingRule(promRule)); + if (canEditRule) { const editURL = createRelativeUrl(`/alerting/${encodeURIComponent(ruleId.stringifyIdentifier(identifier))}/edit`); @@ -96,13 +109,13 @@ export function RuleActionsButtons({ compact, rule, promRule, groupIdentifier }: promRule={promRule} groupIdentifier={groupIdentifier} identifier={identifier} - handleDelete={() => showDeleteModal(identifier, groupIdentifier)} + handleDelete={(identifier, groupIdentifier) => showDeleteModal(identifier, groupIdentifier)} handleSilence={() => setShowSilenceDrawer(true)} handleDuplicateRule={() => setRedirectToClone({ identifier, isProvisioned })} /> {deleteModal} - {rulerRuleType.grafana.alertingRule(rule) && showSilenceDrawer && ( - setShowSilenceDrawer(false)} /> + {silenceableRule && showSilenceDrawer && ( + setShowSilenceDrawer(false)} /> )} {redirectToClone?.identifier && (