From ded7912ea3acc64a7c222afa32a248931bfd58f0 Mon Sep 17 00:00:00 2001 From: Gilles De Mey Date: Wed, 9 Jul 2025 15:41:20 +0200 Subject: [PATCH] Use Group Loader for the group detail view (#107780) --- .../group-details/GroupDetailsPage.test.tsx | 128 +++++++-- .../group-details/GroupDetailsPage.tsx | 250 +++++------------- .../alerting/unified/utils/navigation.ts | 26 ++ public/locales/en-US/grafana.json | 16 +- 4 files changed, 209 insertions(+), 211 deletions(-) diff --git a/public/app/features/alerting/unified/group-details/GroupDetailsPage.test.tsx b/public/app/features/alerting/unified/group-details/GroupDetailsPage.test.tsx index 988f3aaaeb1..6cd31efbe16 100644 --- a/public/app/features/alerting/unified/group-details/GroupDetailsPage.test.tsx +++ b/public/app/features/alerting/unified/group-details/GroupDetailsPage.test.tsx @@ -1,13 +1,16 @@ -import { HttpResponse } from 'msw'; +import { uniqueId } from 'lodash'; +import { HttpResponse, http } from 'msw'; import { Route, Routes } from 'react-router-dom-v5-compat'; import { Props } from 'react-virtualized-auto-sizer'; -import { render, screen, waitFor, within } from 'test/test-utils'; +import { render, screen, waitFor, waitForElementToBeRemoved, within } from 'test/test-utils'; import { byRole, byTestId } from 'testing-library-selector'; +import { setPluginLinksHook } from '@grafana/runtime'; import { AccessControlAction } from 'app/types/accessControl'; +import { GrafanaPromRuleGroupDTO, GrafanaPromRulesResponse } from 'app/types/unified-alerting-dto'; import { setupMswServer } from '../mockApi'; -import { grantUserPermissions, mockRulerGrafanaRule, mockRulerRuleGroup } from '../mocks'; +import { grantUserPermissions, mockGrafanaPromAlertingRule, mockRulerGrafanaRule, mockRulerRuleGroup } from '../mocks'; import { mimirDataSource, setFolderResponse, @@ -38,8 +41,8 @@ const ui = { header: byRole('heading', { level: 1 }), editLink: byRole('link', { name: 'Edit' }), exportButton: byRole('button', { name: 'Export' }), - tableRow: byTestId('row'), - rowsTable: byTestId('dynamic-table'), + ruleLoader: byTestId('alert-rule-list-item-loader'), + ruleItem: byRole('treeitem'), export: { dialog: byRole('dialog', { name: /Drawer title Export .* rules/ }), jsonTab: byRole('tab', { name: /JSON/ }), @@ -50,10 +53,16 @@ const ui = { }, }; -setupMswServer(); +const server = setupMswServer(); describe('GroupDetailsPage', () => { beforeEach(() => { + // mock this... + setPluginLinksHook(() => ({ + links: [], + isLoading: false, + })); + grantUserPermissions([ AccessControlAction.AlertingRuleRead, AccessControlAction.AlertingRuleUpdate, @@ -63,25 +72,71 @@ describe('GroupDetailsPage', () => { }); describe('Grafana managed rules', () => { - const rule1 = mockRulerGrafanaRule({ for: '10m' }, { title: 'High CPU Usage' }); - const rule2 = mockRulerGrafanaRule({ for: '5m' }, { title: 'Memory Pressure' }); - const provisionedRule = mockRulerGrafanaRule({ for: '10m' }, { title: 'Provisioned Rule', provenance: 'api' }); + const folder = { uid: 'test-folder-uid', canSave: true, title: 'test-folder-title' } as const; + + const rule1 = mockRulerGrafanaRule({ for: '10m' }, { title: 'High CPU Usage', uid: uniqueId() }); + const rule2 = mockRulerGrafanaRule({ for: '5m' }, { title: 'Memory Pressure', uid: uniqueId() }); + + const provisionedRule = mockRulerGrafanaRule( + { for: '10m' }, + { uid: uniqueId(), title: 'Provisioned Rule', provenance: 'api' } + ); const group = mockRulerRuleGroup({ name: 'test-group-cpu', interval: '3m', rules: [rule1, rule2], }); + const promGroup: GrafanaPromRuleGroupDTO = { + name: group.name, + rules: [ + mockGrafanaPromAlertingRule({ + uid: rule1.grafana_alert.uid, + name: rule1.grafana_alert.title, + }), + mockGrafanaPromAlertingRule({ + uid: rule2.grafana_alert.uid, + name: rule2.grafana_alert.title, + }), + ], + interval: 180, + folderUid: folder.uid, + file: folder.title, + }; const provisionedGroup = mockRulerRuleGroup({ name: 'provisioned-group-cpu', interval: '15m', rules: [provisionedRule], }); + const promProvisionedGroup: GrafanaPromRuleGroupDTO = { + name: provisionedGroup.name, + interval: 900, + rules: [ + mockGrafanaPromAlertingRule({ + uid: provisionedRule.grafana_alert.uid, + name: provisionedRule.grafana_alert.title, + provenance: provisionedRule.grafana_alert.provenance, + }), + ], + folderUid: folder.uid, + file: folder.title, + }; beforeEach(() => { setRulerRuleGroupHandler({ response: HttpResponse.json(group) }); - setFolderResponse({ uid: 'test-folder-uid', canSave: true, title: 'test-folder-title' }); + server.use( + http.get('/api/prometheus/grafana/api/v1/rules', () => + HttpResponse.json({ + status: 'success', + data: { + groups: [promGroup], + }, + }) + ) + ); + + setFolderResponse(folder); setGrafanaRuleGroupExportResolver(({ request }) => { const url = new URL(request.url); return HttpResponse.text( @@ -106,22 +161,24 @@ describe('GroupDetailsPage', () => { '/alerting/grafana/namespaces/test-folder-uid/groups/test-group-cpu/edit?returnTo=%2Falerting%2Fgrafana%2Fnamespaces%2Ftest-folder-uid%2Fgroups%2Ftest-group-cpu%2Fview' ); - const tableRows = await ui.tableRow.findAll(await ui.rowsTable.find()); - expect(tableRows).toHaveLength(2); + await waitForElementToBeRemoved(() => ui.ruleLoader.queryAll()); - expect(within(tableRows[0]).getByRole('link', { name: rule1.grafana_alert.title })).toHaveAttribute( + const alertRuleItems = await ui.ruleItem.findAll(); + expect(alertRuleItems).toHaveLength(2); + + // assert rule 1 + expect(within(alertRuleItems[0]).getByRole('link', { name: rule1.grafana_alert.title })).toHaveAttribute( 'href', `/alerting/grafana/${rule1.grafana_alert.uid}/view` ); - expect(tableRows[0]).toHaveTextContent(String(rule1.for)); - expect(tableRows[0]).toHaveTextContent('5'); + expect(alertRuleItems[0]).toHaveTextContent(rule1.grafana_alert.title); - expect(within(tableRows[1]).getByRole('link', { name: rule2.grafana_alert.title })).toHaveAttribute( + // assert rule 2 + expect(within(alertRuleItems[1]).getByRole('link', { name: rule2.grafana_alert.title })).toHaveAttribute( 'href', `/alerting/grafana/${rule2.grafana_alert.uid}/view` ); - expect(tableRows[1]).toHaveTextContent(String(rule2.for)); - expect(tableRows[1]).toHaveTextContent('3'); + expect(alertRuleItems[1]).toHaveTextContent(rule2.grafana_alert.title); }); it('should render error alert when API returns an error', async () => { @@ -164,10 +221,14 @@ describe('GroupDetailsPage', () => { // Act renderGroupDetailsPage('grafana', 'test-folder-uid', group.name); - const tableRows = await ui.tableRow.findAll(await ui.rowsTable.find()); + // wait for loaders to show and dissapear + await waitFor(() => expect(ui.ruleLoader.queryAll()).toHaveLength(3)); + await waitForElementToBeRemoved(() => ui.ruleLoader.queryAll()); + + const alertRuleItems = await ui.ruleItem.findAll(); // Assert - expect(tableRows).toHaveLength(2); + expect(alertRuleItems).toHaveLength(2); expect(ui.editLink.query()).not.toBeInTheDocument(); // Edit button should not be present }); @@ -177,24 +238,41 @@ describe('GroupDetailsPage', () => { // Act renderGroupDetailsPage('grafana', 'test-folder-uid', group.name); - const tableRows = await ui.tableRow.findAll(await ui.rowsTable.find()); + // wait for loaders to show and dissapear + await waitFor(() => expect(ui.ruleLoader.queryAll()).toHaveLength(3)); + await waitForElementToBeRemoved(() => ui.ruleLoader.queryAll()); + + const alertRuleItems = await ui.ruleItem.findAll(); // Assert - expect(tableRows).toHaveLength(2); + expect(alertRuleItems).toHaveLength(2); expect(ui.editLink.query()).not.toBeInTheDocument(); // Edit button should not be present }); it('should not allow editing if the group is provisioned', async () => { setRulerRuleGroupHandler({ response: HttpResponse.json(provisionedGroup) }); + server.use( + http.get('/api/prometheus/grafana/api/v1/rules', () => + HttpResponse.json({ + status: 'success', + data: { + groups: [promProvisionedGroup], + }, + }) + ) + ); // Act renderGroupDetailsPage('grafana', 'test-folder-uid', provisionedGroup.name); + // wait for loaders to show and dissapear + await waitFor(() => expect(ui.ruleLoader.queryAll()).toHaveLength(3)); + await waitForElementToBeRemoved(() => ui.ruleLoader.queryAll()); - const tableRows = await ui.tableRow.findAll(await ui.rowsTable.find()); + const alertRuleItems = await ui.ruleItem.findAll(); // Assert - expect(tableRows).toHaveLength(1); - expect(tableRows[0]).toHaveTextContent('Provisioned Rule'); + expect(alertRuleItems).toHaveLength(1); + expect(alertRuleItems[0]).toHaveTextContent('Provisioned Rule'); expect(ui.editLink.query()).not.toBeInTheDocument(); expect(ui.exportButton.query()).toBeInTheDocument(); }); diff --git a/public/app/features/alerting/unified/group-details/GroupDetailsPage.tsx b/public/app/features/alerting/unified/group-details/GroupDetailsPage.tsx index 0b354af9457..997b1bd44b7 100644 --- a/public/app/features/alerting/unified/group-details/GroupDetailsPage.tsx +++ b/public/app/features/alerting/unified/group-details/GroupDetailsPage.tsx @@ -1,35 +1,28 @@ import { skipToken } from '@reduxjs/toolkit/query'; -import { useMemo, useState } from 'react'; +import { useState } from 'react'; import { useParams } from 'react-router-dom-v5-compat'; import { Trans, t } from '@grafana/i18n'; -import { Alert, Badge, Button, LinkButton, Text, TextLink, withErrorBoundary } from '@grafana/ui'; +import { Alert, Button, Dropdown, Icon, LinkButton, Menu, TextLink, withErrorBoundary } from '@grafana/ui'; import { EntityNotFound } from 'app/core/components/PageNotFound/EntityNotFound'; import { FolderDTO } from 'app/types/folders'; -import { GrafanaRulesSourceSymbol, RuleGroup } from 'app/types/unified-alerting'; -import { PromRuleType, RulerRuleGroupDTO } from 'app/types/unified-alerting-dto'; +import { GrafanaRulesSourceSymbol } from 'app/types/unified-alerting'; +import { RulerRuleGroupDTO } from 'app/types/unified-alerting-dto'; import { alertRuleApi } from '../api/alertRuleApi'; import { RulesSourceFeatures, featureDiscoveryApi } from '../api/featureDiscoveryApi'; import { AlertingPageWrapper } from '../components/AlertingPageWrapper'; -import { DynamicTable, DynamicTableColumnProps } from '../components/DynamicTable'; import { GrafanaRuleGroupExporter } from '../components/export/GrafanaRuleGroupExporter'; import { useFolder } from '../hooks/useFolder'; import { DEFAULT_GROUP_EVALUATION_INTERVAL } from '../rule-editor/formDefaults'; -import { createViewLinkFromIdentifier } from '../rule-list/DataSourceRuleListItem'; +import { DataSourceGroupLoader } from '../rule-list/DataSourceGroupLoader'; +import { GrafanaGroupLoader } from '../rule-list/GrafanaGroupLoader'; import { useRulesAccess } from '../utils/accessControlHooks'; import { GRAFANA_RULES_SOURCE_NAME, getDataSourceByUid } from '../utils/datasource'; import { makeFolderLink, stringifyErrorLike } from '../utils/misc'; import { createListFilterLink, groups } from '../utils/navigation'; -import { fromRule, fromRulerRule } from '../utils/rule-id'; -import { - calcRuleEvalsToStartAlerting, - getRuleName, - isFederatedRuleGroup, - isProvisionedRuleGroup, - rulerRuleType, -} from '../utils/rules'; -import { formatPrometheusDuration, safeParsePrometheusDuration } from '../utils/time'; +import { isFederatedRuleGroup, isProvisionedRuleGroup } from '../utils/rules'; +import { formatPrometheusDuration } from '../utils/time'; import { Title } from './Title'; @@ -111,6 +104,7 @@ function GroupDetailsPage() { }, }} renderTitle={(title) => } + subTitle={t('alerting.titles.group-view.subtitle', 'Manage alert rules, recording rules and evaluation interval')} info={[ { label: namespaceLabel, value: namespaceValue }, { label: t('alerting.group-details.interval', 'Interval'), value: groupInterval }, @@ -150,13 +144,33 @@ function GroupDetailsPage() { <div>{stringifyErrorLike(ruleNamespacesError || ruleGroupError)}</div> </Alert> )} - {promGroup && ruleSourceName && ( - <GroupDetails group={promRuleGroupToRuleGroupDetails(ruleSourceName, namespaceName, promGroup)} /> - )} - {rulerGroup && ruleSourceName && ( - <GroupDetails group={rulerRuleGroupToRuleGroupDetails(ruleSourceName, namespaceName, rulerGroup)} /> - )} {!promGroup && !rulerGroup && <EntityNotFound entity={`${namespaceId}/${groupName}`} />} + + {ruleSourceName && ( + <ul role="tree"> + {isGrafanaRuleGroup ? ( + <GrafanaGroupLoader + groupIdentifier={{ groupName, groupOrigin: 'grafana', namespace: { uid: namespaceId } }} + namespaceName={namespaceName} + /> + ) : ( + <DataSourceGroupLoader + groupIdentifier={{ + groupName, + groupOrigin: 'datasource', + namespace: { + name: namespaceName, + }, + rulesSource: { + name: ruleSourceName, + uid: dataSourceUid, + ruleSourceType: 'datasource', + }, + }} + /> + )} + </ul> + )} </> </AlertingPageWrapper> ); @@ -195,13 +209,39 @@ function GroupActions({ dsFeatures, namespaceId, groupName, folder, rulerGroup } </Button> )} {canEdit && ( - <LinkButton - icon="pen" - href={groups.editPageLink(dsFeatures.uid, namespaceId, groupName, { includeReturnTo: true })} - variant="secondary" - > - <Trans i18nKey="alerting.group-details.edit">Edit</Trans> - </LinkButton> + <> + <LinkButton + icon="pen" + href={groups.editPageLink(dsFeatures.uid, namespaceId, groupName, { includeReturnTo: true })} + variant="secondary" + > + <Trans i18nKey="alerting.group-details.edit">Edit</Trans> + </LinkButton> + {/* Data source managed requires different URLs, a hassle to implement for now */} + {isGrafanaSource && ( + <Dropdown + overlay={ + <Menu> + <Menu.Item + icon="bell" + url={groups.newAlertRuleLink(folder?.title, folder?.uid, groupName)} + label={t('alerting.alert-rule.term', 'Alert rule')} + /> + <Menu.Item + icon="record-audio" + url={groups.newRecordingRuleLink(folder?.title, folder?.uid, groupName)} + label={t('alerting.recording-rule.term', 'Recording rule')} + /> + </Menu> + } + > + <Button variant="primary"> + {t('alerting.group-details.new', 'New')} + <Icon name="angle-down" /> + </Button> + </Dropdown> + )} + </> )} {folder && isExporting && ( <GrafanaRuleGroupExporter folderUid={folder.uid} groupName={groupName} onClose={() => setIsExporting(false)} /> @@ -210,158 +250,4 @@ function GroupActions({ dsFeatures, namespaceId, groupName, folder, rulerGroup } ); } -/** An common interface for both Prometheus and Ruler rule groups */ -interface RuleGroupDetails { - name: string; - interval: string; - rules: RuleDetails[]; -} - -interface AlertingRuleDetails { - name: string; - href?: string; - type: 'alerting'; - pendingPeriod: string; - evaluationsToFire: number; -} -interface RecordingRuleDetails { - name: string; - href?: string; - type: 'recording'; -} - -type RuleDetails = AlertingRuleDetails | RecordingRuleDetails; - -interface GroupDetailsProps { - group: RuleGroupDetails; -} - -function GroupDetails({ group }: GroupDetailsProps) { - return ( - <div> - <RulesTable rules={group.rules} /> - </div> - ); -} - -function RulesTable({ rules }: { rules: RuleDetails[] }) { - const rows = rules.map((rule: RuleDetails, index) => ({ - id: index, - data: rule, - })); - - const columns: Array<DynamicTableColumnProps<RuleDetails>> = useMemo(() => { - return [ - { - id: 'alertName', - label: t('alerting.group-details.rule-name', 'Rule name'), - renderCell: ({ data: { name, href } }) => { - if (href) { - return ( - <TextLink href={href} inline={false} color="primary"> - {name} - </TextLink> - ); - } - - return <Text truncate>{name}</Text>; - }, - size: 0.4, - }, - { - id: 'for', - label: t('alerting.group-details.pending-period', 'Pending period'), - renderCell: ({ data }) => { - switch (data.type) { - case 'alerting': - return <>{data.pendingPeriod}</>; - case 'recording': - return <Badge text={t('alerting.group-details.recording', 'Recording')} color="purple" />; - } - }, - size: 0.3, - }, - { - id: 'numberEvaluations', - label: t('alerting.group-details.evaluations-to-fire', 'Evaluation cycles to fire'), - renderCell: ({ data }) => { - switch (data.type) { - case 'alerting': - return <>{data.evaluationsToFire}</>; - case 'recording': - return null; - } - }, - size: 0.3, - }, - ]; - }, []); - - return <DynamicTable items={rows} cols={columns} />; -} - -function promRuleGroupToRuleGroupDetails( - ruleSourceName: string, - namespaceName: string, - group: RuleGroup -): RuleGroupDetails { - const groupIntervalMs = group.interval * 1000; - - return { - name: group.name, - interval: formatPrometheusDuration(group.interval * 1000), - rules: group.rules.map<RuleDetails>((rule) => { - const ruleIdentifier = fromRule(ruleSourceName, namespaceName, group.name, rule); - const href = ruleIdentifier ? createViewLinkFromIdentifier(ruleIdentifier) : undefined; - - switch (rule.type) { - case PromRuleType.Alerting: - return { - name: rule.name, - href, - type: 'alerting', - pendingPeriod: formatPrometheusDuration(rule.duration ? rule.duration * 1000 : 0), - evaluationsToFire: calcRuleEvalsToStartAlerting(rule.duration ? rule.duration * 1000 : 0, groupIntervalMs), - }; - case PromRuleType.Recording: - return { name: rule.name, href, type: 'recording' }; - } - }), - }; -} - -function rulerRuleGroupToRuleGroupDetails( - ruleSourceName: string, - namespaceName: string, - group: RulerRuleGroupDTO -): RuleGroupDetails { - const groupIntervalMs = safeParsePrometheusDuration(group.interval ?? DEFAULT_GROUP_EVALUATION_INTERVAL); - - return { - name: group.name, - interval: group.interval ?? DEFAULT_GROUP_EVALUATION_INTERVAL, - rules: group.rules.map<RuleDetails>((rule) => { - const name = getRuleName(rule); - - const ruleIdentifier = fromRulerRule(ruleSourceName, namespaceName, group.name, rule); - const href = createViewLinkFromIdentifier(ruleIdentifier); - - if (rulerRuleType.any.alertingRule(rule)) { - return { - name, - href, - type: 'alerting', - pendingPeriod: rule.for ?? '0s', - evaluationsToFire: calcRuleEvalsToStartAlerting( - rule.for ? safeParsePrometheusDuration(rule.for) : 0, - groupIntervalMs - ), - }; - } - - return { name, href, type: 'recording' }; - }), - }; -} - export default withErrorBoundary(GroupDetailsPage, { style: 'page' }); diff --git a/public/app/features/alerting/unified/utils/navigation.ts b/public/app/features/alerting/unified/utils/navigation.ts index 15081fb8b4e..259207f41b7 100644 --- a/public/app/features/alerting/unified/utils/navigation.ts +++ b/public/app/features/alerting/unified/utils/navigation.ts @@ -45,6 +45,32 @@ export const groups = { { skipSubPath: options?.skipSubPath } ); }, + newAlertRuleLink: (folderName?: string, folderUid?: string, groupName?: string) => { + const returnTo = createReturnTo(); + + const defaults = JSON.stringify({ + folder: { + title: folderName, + uid: folderUid, + }, + group: groupName, + }); + + return createRelativeUrl('/alerting/new', { defaults, returnTo }); + }, + newRecordingRuleLink: (folderName?: string, folderUid?: string, groupName?: string) => { + const returnTo = createReturnTo(); + + const defaults = JSON.stringify({ + folder: { + title: folderName, + uid: folderUid, + }, + group: groupName, + }); + + return createRelativeUrl('/alerting/new/grafana-recording', { defaults, returnTo }); + }, }; export const rulesNav = { diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index deaf63f3127..8cfdb522034 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -508,6 +508,9 @@ } } }, + "alert-rule": { + "term": "Alert rule" + }, "alert-rule-form": { "action-buttons": { "edit-yaml": "Edit YAML", @@ -1505,15 +1508,12 @@ "group-details": { "ds-features-error": "Error loading data source details", "edit": "Edit", - "evaluations-to-fire": "Evaluation cycles to fire", "export": "Export", "folder": "Folder", "group-loading-error": "Error loading the group", "interval": "Interval", "namespace": "Namespace", - "pending-period": "Pending period", - "recording": "Recording", - "rule-name": "Rule name" + "new": "New" }, "group-edit": { "ds-error": "Error loading data source details", @@ -2237,6 +2237,9 @@ "label-export-all": "Export all" } }, + "recording-rule": { + "term": "Recording rule" + }, "recording-rule-editor": { "error-no-query-editor": "Could not load query editor due to: {{errorMessage}}" }, @@ -2890,6 +2893,11 @@ "timestamp": { "time-ago": "({{time}} ago)" }, + "titles": { + "group-view": { + "subtitle": "Manage alert rules, recording rules and evaluation interval" + } + }, "to-gma": { "confirm-modal": { "body": "The target folder is not empty, some rules may be overwritten or removed. Are you sure you want to import these alert rules to Grafana-managed rules?",