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 <konradlalik@gmail.com>
This commit is contained in:
co-authored by
Konrad Lalik
parent
9e9233051e
commit
41ed2aeb23
@@ -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();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
+24
-2
@@ -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<RulerGrafanaRuleDTO<GrafanaRuleDefinition>>();
|
||||
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<GrafanaRuleDefinition>) => {
|
||||
setShowConfirmModal(true);
|
||||
setRuleToRestore(ruleToRestore);
|
||||
@@ -52,6 +56,15 @@ export function VersionHistoryTable({
|
||||
|
||||
const unknown = t('alerting.alertVersionHistory.unknown', 'Unknown');
|
||||
|
||||
const notesColumn: Column<RulerGrafanaRuleDTO<GrafanaRuleDefinition>> = {
|
||||
id: 'notes',
|
||||
header: t('core.versionHistory.table.notes', 'Notes'),
|
||||
cell: ({ row }) => {
|
||||
const message = row.original.grafana_alert.message;
|
||||
return message || null;
|
||||
},
|
||||
};
|
||||
|
||||
const columns: Array<Column<RulerGrafanaRuleDTO<GrafanaRuleDefinition>>> = [
|
||||
{
|
||||
disableGrow: true,
|
||||
@@ -91,9 +104,12 @@ export function VersionHistoryTable({
|
||||
if (!value) {
|
||||
return unknown;
|
||||
}
|
||||
return dateTimeFormat(value) + ' (' + dateTimeFormatTimeAgo(value) + ')';
|
||||
return (
|
||||
<span className={styles.nowrap}>{dateTimeFormat(value) + ' (' + dateTimeFormatTimeAgo(value) + ')'}</span>
|
||||
);
|
||||
},
|
||||
},
|
||||
...(hasAnyNotes ? [notesColumn] : []),
|
||||
{
|
||||
id: 'diff',
|
||||
disableGrow: true,
|
||||
@@ -179,3 +195,9 @@ export function VersionHistoryTable({
|
||||
</>
|
||||
);
|
||||
}
|
||||
|
||||
const getStyles = () => ({
|
||||
nowrap: css({
|
||||
whiteSpace: 'nowrap',
|
||||
}),
|
||||
});
|
||||
|
||||
@@ -154,6 +154,7 @@ export const rulerRuleVersionHistoryHandler = () => {
|
||||
uid: 'service',
|
||||
name: '',
|
||||
};
|
||||
draft.grafana_alert.message = 'Updated by provisioning service';
|
||||
}),
|
||||
produce(grafanaRulerRule, (draft: RulerGrafanaRuleDTO<GrafanaRuleDefinition>) => {
|
||||
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<GrafanaRuleDefinition>) => {
|
||||
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<GrafanaRuleDefinition>) => {
|
||||
draft.grafana_alert.version = 1;
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -4416,6 +4416,7 @@
|
||||
},
|
||||
"no-properties-changed": "No relevant properties changed",
|
||||
"table": {
|
||||
"notes": "Notes",
|
||||
"updated": "Date",
|
||||
"updatedBy": "Updated By",
|
||||
"version": "Version"
|
||||
|
||||
Reference in New Issue
Block a user