From 41ed2aeb232056fe329fa43b02103916afcfc88e Mon Sep 17 00:00:00 2001 From: Alexander Akhmetov Date: Wed, 7 Jan 2026 15:06:41 +0100 Subject: [PATCH] Alerting: Display change message next to the rule version when exists (#115664) * Alerting: Display change message next to the rule version when exists * Alerting: Update version history tests for message field Updates test mocks and assertions to include message fields in version history data. Adds three message examples to the mock handler and updates test expectations to verify the Notes column displays correctly when messages are present or absent. --------- Co-authored-by: Konrad Lalik --- .../rule-viewer/RuleViewer.test.tsx | 67 ++++++++++++++++--- .../version-history/VersionHistoryTable.tsx | 26 ++++++- .../mocks/server/handlers/grafanaRuler.ts | 3 + public/app/types/unified-alerting-dto.ts | 1 + public/locales/en-US/grafana.json | 1 + 5 files changed, 87 insertions(+), 11 deletions(-) diff --git a/public/app/features/alerting/unified/components/rule-viewer/RuleViewer.test.tsx b/public/app/features/alerting/unified/components/rule-viewer/RuleViewer.test.tsx index 0d8636c98bc..fcd9edebb2e 100644 --- a/public/app/features/alerting/unified/components/rule-viewer/RuleViewer.test.tsx +++ b/public/app/features/alerting/unified/components/rule-viewer/RuleViewer.test.tsx @@ -3,7 +3,8 @@ import { render, screen, userEvent, waitFor } from 'test/test-utils'; import { byLabelText, byRole, byText } from 'testing-library-selector'; import { setPluginLinksHook } from '@grafana/runtime'; -import { setupMswServer } from 'app/features/alerting/unified/mockApi'; +import server from '@grafana/test-utils/server'; +import { mockAlertRuleApi, setupMswServer } from 'app/features/alerting/unified/mockApi'; import { AlertManagerDataSourceJsonData } from 'app/plugins/datasource/alertmanager/types'; import { AccessControlAction } from 'app/types/accessControl'; import { CombinedRule, RuleIdentifier } from 'app/types/unified-alerting'; @@ -22,6 +23,7 @@ import { mockPluginLinkExtension, mockPromAlertingRule, mockRulerGrafanaRecordingRule, + mockRulerGrafanaRule, } from '../../mocks'; import { grafanaRulerRule } from '../../mocks/grafanaRulerApi'; import { grantPermissionsHelper } from '../../test/test-utils'; @@ -130,6 +132,8 @@ const dataSources = { }; describe('RuleViewer', () => { + const api = mockAlertRuleApi(server); + beforeEach(() => { setupDataSources(...Object.values(dataSources)); }); @@ -249,19 +253,22 @@ describe('RuleViewer', () => { expect(screen.getAllByRole('row')).toHaveLength(7); expect(screen.getAllByRole('row')[1]).toHaveTextContent(/6Provisioning2025-01-18 04:35:17/i); - expect(screen.getAllByRole('row')[1]).toHaveTextContent('+3-3Latest'); + expect(screen.getAllByRole('row')[1]).toHaveTextContent('Updated by provisioning service'); + expect(screen.getAllByRole('row')[1]).toHaveTextContent('+4-3Latest'); expect(screen.getAllByRole('row')[2]).toHaveTextContent(/5Alerting2025-01-17 04:35:17/i); - expect(screen.getAllByRole('row')[2]).toHaveTextContent('+5-5'); + expect(screen.getAllByRole('row')[2]).toHaveTextContent('+5-6'); expect(screen.getAllByRole('row')[3]).toHaveTextContent(/4different user2025-01-16 04:35:17/i); - expect(screen.getAllByRole('row')[3]).toHaveTextContent('+5-5'); + expect(screen.getAllByRole('row')[3]).toHaveTextContent('Changed alert title and thresholds'); + expect(screen.getAllByRole('row')[3]).toHaveTextContent('+6-5'); expect(screen.getAllByRole('row')[4]).toHaveTextContent(/3user12025-01-15 04:35:17/i); - expect(screen.getAllByRole('row')[4]).toHaveTextContent('+5-9'); + expect(screen.getAllByRole('row')[4]).toHaveTextContent('+5-10'); expect(screen.getAllByRole('row')[5]).toHaveTextContent(/2User ID foo2025-01-14 04:35:17/i); - expect(screen.getAllByRole('row')[5]).toHaveTextContent('+11-7'); + expect(screen.getAllByRole('row')[5]).toHaveTextContent('Updated evaluation interval and routing'); + expect(screen.getAllByRole('row')[5]).toHaveTextContent('+12-7'); expect(screen.getAllByRole('row')[6]).toHaveTextContent(/1Unknown 2025-01-13 04:35:17/i); @@ -275,9 +282,10 @@ describe('RuleViewer', () => { await renderRuleViewer(mockRule, mockRuleIdentifier, ActiveTab.VersionHistory); expect(await screen.findByRole('button', { name: /Compare versions/i })).toBeDisabled(); - expect(screen.getByRole('cell', { name: /provisioning/i })).toBeInTheDocument(); - expect(screen.getByRole('cell', { name: /alerting/i })).toBeInTheDocument(); - expect(screen.getByRole('cell', { name: /Unknown/i })).toBeInTheDocument(); + // Check for special updated_by values - use getAllByRole since some text appears in multiple columns + expect(screen.getAllByRole('cell', { name: /provisioning/i }).length).toBeGreaterThan(0); + expect(screen.getByRole('cell', { name: /^alerting$/i })).toBeInTheDocument(); + expect(screen.getByRole('cell', { name: /^Unknown$/i })).toBeInTheDocument(); expect(screen.getByRole('cell', { name: /user id foo/i })).toBeInTheDocument(); }); @@ -321,6 +329,47 @@ describe('RuleViewer', () => { await renderRuleViewer(rule, ruleIdentifier); expect(screen.queryByText('Labels')).not.toBeInTheDocument(); }); + + it('shows Notes column when versions have messages', async () => { + await renderRuleViewer(mockRule, mockRuleIdentifier, ActiveTab.VersionHistory); + + expect(await screen.findByRole('columnheader', { name: /Notes/i })).toBeInTheDocument(); + expect(screen.getAllByRole('row')).toHaveLength(7); // 1 header + 6 data rows + expect(screen.getByRole('cell', { name: /Updated by provisioning service/i })).toBeInTheDocument(); + expect(screen.getByRole('cell', { name: /Changed alert title and thresholds/i })).toBeInTheDocument(); + expect(screen.getByRole('cell', { name: /Updated evaluation interval and routing/i })).toBeInTheDocument(); + }); + + it('does not show Notes column when no versions have messages', async () => { + const versionsWithoutMessages = [ + mockRulerGrafanaRule( + {}, + { + uid: grafanaRulerRule.grafana_alert.uid, + version: 2, + updated: '2025-01-14T09:35:17.000Z', + updated_by: { uid: 'foo', name: '' }, + } + ), + mockRulerGrafanaRule( + {}, + { + uid: grafanaRulerRule.grafana_alert.uid, + version: 1, + updated: '2025-01-13T09:35:17.000Z', + updated_by: null, + } + ), + ]; + api.getAlertRuleVersionHistory(grafanaRulerRule.grafana_alert.uid, versionsWithoutMessages); + + await renderRuleViewer(mockRule, mockRuleIdentifier, ActiveTab.VersionHistory); + + await screen.findByRole('button', { name: /Compare versions/i }); + + expect(screen.getAllByRole('row')).toHaveLength(3); // 1 header + 2 data rows + expect(screen.queryByRole('columnheader', { name: /Notes/i })).not.toBeInTheDocument(); + }); }); }); diff --git a/public/app/features/alerting/unified/components/rule-viewer/tabs/version-history/VersionHistoryTable.tsx b/public/app/features/alerting/unified/components/rule-viewer/tabs/version-history/VersionHistoryTable.tsx index 74dabe18449..ac6c75b93ef 100644 --- a/public/app/features/alerting/unified/components/rule-viewer/tabs/version-history/VersionHistoryTable.tsx +++ b/public/app/features/alerting/unified/components/rule-viewer/tabs/version-history/VersionHistoryTable.tsx @@ -1,8 +1,9 @@ +import { css } from '@emotion/css'; import { useMemo, useState } from 'react'; import { dateTimeFormat, dateTimeFormatTimeAgo } from '@grafana/data'; import { Trans, t } from '@grafana/i18n'; -import { Badge, Button, Checkbox, Column, InteractiveTable, Stack, Text } from '@grafana/ui'; +import { Badge, Button, Checkbox, Column, InteractiveTable, Stack, Text, useStyles2 } from '@grafana/ui'; import { GRAFANA_RULES_SOURCE_NAME } from 'app/features/alerting/unified/utils/datasource'; import { computeVersionDiff } from 'app/features/alerting/unified/utils/diff'; import { RuleIdentifier } from 'app/types/unified-alerting'; @@ -33,6 +34,7 @@ export function VersionHistoryTable({ onRestoreError, canRestore, }: VersionHistoryTableProps) { + const styles = useStyles2(getStyles); const [showConfirmModal, setShowConfirmModal] = useState(false); const [ruleToRestore, setRuleToRestore] = useState>(); const ruleToRestoreUid = ruleToRestore?.grafana_alert?.uid ?? ''; @@ -41,6 +43,8 @@ export function VersionHistoryTable({ [ruleToRestoreUid] ); + const hasAnyNotes = useMemo(() => ruleVersions.some((v) => v.grafana_alert.message), [ruleVersions]); + const showConfirmation = (ruleToRestore: RulerGrafanaRuleDTO) => { setShowConfirmModal(true); setRuleToRestore(ruleToRestore); @@ -52,6 +56,15 @@ export function VersionHistoryTable({ const unknown = t('alerting.alertVersionHistory.unknown', 'Unknown'); + const notesColumn: Column> = { + id: 'notes', + header: t('core.versionHistory.table.notes', 'Notes'), + cell: ({ row }) => { + const message = row.original.grafana_alert.message; + return message || null; + }, + }; + const columns: Array>> = [ { disableGrow: true, @@ -91,9 +104,12 @@ export function VersionHistoryTable({ if (!value) { return unknown; } - return dateTimeFormat(value) + ' (' + dateTimeFormatTimeAgo(value) + ')'; + return ( + {dateTimeFormat(value) + ' (' + dateTimeFormatTimeAgo(value) + ')'} + ); }, }, + ...(hasAnyNotes ? [notesColumn] : []), { id: 'diff', disableGrow: true, @@ -179,3 +195,9 @@ export function VersionHistoryTable({ ); } + +const getStyles = () => ({ + nowrap: css({ + whiteSpace: 'nowrap', + }), +}); diff --git a/public/app/features/alerting/unified/mocks/server/handlers/grafanaRuler.ts b/public/app/features/alerting/unified/mocks/server/handlers/grafanaRuler.ts index b0f58408306..85905d480b3 100644 --- a/public/app/features/alerting/unified/mocks/server/handlers/grafanaRuler.ts +++ b/public/app/features/alerting/unified/mocks/server/handlers/grafanaRuler.ts @@ -154,6 +154,7 @@ export const rulerRuleVersionHistoryHandler = () => { uid: 'service', name: '', }; + draft.grafana_alert.message = 'Updated by provisioning service'; }), produce(grafanaRulerRule, (draft: RulerGrafanaRuleDTO) => { draft.grafana_alert.version = 5; @@ -171,6 +172,7 @@ export const rulerRuleVersionHistoryHandler = () => { uid: 'different', name: 'different user', }; + draft.grafana_alert.message = 'Changed alert title and thresholds'; }), produce(grafanaRulerRule, (draft: RulerGrafanaRuleDTO) => { draft.grafana_alert.version = 3; @@ -193,6 +195,7 @@ export const rulerRuleVersionHistoryHandler = () => { uid: 'foo', name: '', }; + draft.grafana_alert.message = 'Updated evaluation interval and routing'; }), produce(grafanaRulerRule, (draft: RulerGrafanaRuleDTO) => { draft.grafana_alert.version = 1; diff --git a/public/app/types/unified-alerting-dto.ts b/public/app/types/unified-alerting-dto.ts index c6440d0bac1..8bc02c3e7bd 100644 --- a/public/app/types/unified-alerting-dto.ts +++ b/public/app/types/unified-alerting-dto.ts @@ -293,6 +293,7 @@ export interface GrafanaRuleDefinition extends PostableGrafanaRuleDefinition { updated?: string; updated_by?: UpdatedBy | null; version?: number; + message?: string; } // types for Grafana-managed recording and alerting rules diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index 6aacf5f3939..baffdefbadf 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -4416,6 +4416,7 @@ }, "no-properties-changed": "No relevant properties changed", "table": { + "notes": "Notes", "updated": "Date", "updatedBy": "Updated By", "version": "Version"