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"