From 321a886b8b9455d29bc0e24705d54d67249bf101 Mon Sep 17 00:00:00 2001 From: Konrad Lalik Date: Tue, 18 Mar 2025 12:27:53 +0100 Subject: [PATCH] Alerting: Add details and edit pages for groups (#100884) * Add basic details page for groups * Remove unused imports * Add basic edit page for groups * Add functional group details page * Improve form, add namespaces for DS groups * Add support for multiple actions in useProduceNewRuleGroup * Attach real actions to form submit * Add tests for the group details page * Add basic tests for the group edit page * Add tests for Mimir update * Add rule group consistency check * Extract draggable rules table to a separate file * Add prom consistency waiting after group saving * Add duration measure for Prometheus reconciliation time * Remove a blinking error when redirecting to a new group * Improve group details page. Use ruler or prom api depending on the ds capabilities * Add group delete action for DMA * Fix GroupDetailsPage tests * Update tests * Add and improve Edit page tests * Add Group export for GMA groups * Fix RulesGroup tests, add translations * Disable editing plugin provided groups * Fix alertingApi options, fix tests * Fix lint errors, update translations * use name for grafana managed recording rules * add namespace to nav * Remove group modals from the list page * add cancel button to edit form * add test for cancel butotn * fix recording rule badge for Grafana managed rules * Add doc comments, improve code * Move url changes to be the last action in form submit * Add returnTo URL handling for alert rule group navigation * Create dedicated Title component showing breadcrumb navigation between folder and group name. Add label distinction between folders and namespaces based on the rule source (Grafana vs external). * Address PR feedback, minor refactorings * Update rule group links to include return path and refactor rule type checks - Modified `RulesGroup` and `GroupDetailsPage` components to include `includeReturnTo` in edit page links. - Refactored rule type checks in `DraggableRulesTable` and `GroupDetailsPage` to use `rulerRuleType` for better clarity and maintainability. - Updated documentation in `useUpdateRuleGroup` to clarify functionality for updating or moving rule groups. * Refactor RulesGroup component and tests for improved link handling and permissions checks - Added `includeReturnTo` parameter to rule group detail links in `RulesGroup` for better navigation. - Updated test cases to verify rendering of edit and view buttons based on user permissions. - Simplified test setup by removing unnecessary Redux provider wrapping in tests. * Refactor: Update routing and test assertions in GroupDetails and GroupEdit pages - Modified route paths in GroupDetailsPage and GroupEditPage tests to use `dataSourceUid` instead of `sourceId`. - Updated test assertions to reflect changes in folder title and link structure in GroupDetailsPage. - Simplified Title component by removing folder-related props and logic, focusing solely on the group name. * Refactor: Simplify Title rendering in GroupDetailsPage - Updated the renderTitle function in GroupDetailsPage to remove the folder prop from the Title component, focusing solely on the group name. * Update GroupDetailsPage to prevent editing of provisioned groups * Fix imports * Improve styles * Fix navigation when served from subpath * Improve group removal handling in Prom consistency check. Fix Delete group button --------- Co-authored-by: Gilles De Mey --- .betterer.results | 25 +- public/app/features/alerting/routes.tsx | 22 ++ .../alerting/unified/api/alertingApi.ts | 4 +- .../unified/api/featureDiscoveryApi.ts | 2 +- .../rule-editor/GrafanaEvaluationBehavior.tsx | 12 +- .../components/rule-viewer/RuleViewer.tsx | 8 +- .../components/rules/EditRuleGroupModal.tsx | 7 +- .../components/rules/RulesGroup.test.tsx | 143 +++---- .../unified/components/rules/RulesGroup.tsx | 198 +++------- .../group-details/GroupDetailsPage.test.tsx | 284 ++++++++++++++ .../group-details/GroupDetailsPage.tsx | 334 ++++++++++++++++ .../group-details/GroupEditPage.test.tsx | 345 ++++++++++++++++ .../unified/group-details/GroupEditPage.tsx | 369 ++++++++++++++++++ .../alerting/unified/group-details/Title.tsx | 16 + .../components/DraggableRulesTable.tsx | 175 +++++++++ .../hooks/ruleGroup/useDeleteRuleFromGroup.ts | 2 +- .../hooks/ruleGroup/usePauseAlertRule.ts | 2 +- .../hooks/ruleGroup/useProduceNewRuleGroup.ts | 17 +- .../hooks/ruleGroup/useUpdateRuleGroup.ts | 138 ++++++- .../ruleGroup/useUpsertRuleFromRuleGroup.ts | 6 +- .../hooks/usePrometheusConsistencyCheck.ts | 182 ++++++++- .../alerting/unified/hooks/useReturnTo.ts | 10 +- .../unified/mocks/server/configure.ts | 75 +++- .../RuleEditorCloudRules.test.tsx.snap | 11 - .../RuleEditorRecordingRule.test.tsx.snap | 11 - .../alerting/unified/utils/navigation.ts | 45 +++ .../features/alerting/unified/utils/rules.ts | 25 +- .../features/alerting/unified/utils/time.ts | 7 + .../features/alerting/unified/utils/url.ts | 15 +- public/locales/en-US/grafana.json | 52 +++ 30 files changed, 2215 insertions(+), 327 deletions(-) create mode 100644 public/app/features/alerting/unified/group-details/GroupDetailsPage.test.tsx create mode 100644 public/app/features/alerting/unified/group-details/GroupDetailsPage.tsx create mode 100644 public/app/features/alerting/unified/group-details/GroupEditPage.test.tsx create mode 100644 public/app/features/alerting/unified/group-details/GroupEditPage.tsx create mode 100644 public/app/features/alerting/unified/group-details/Title.tsx create mode 100644 public/app/features/alerting/unified/group-details/components/DraggableRulesTable.tsx create mode 100644 public/app/features/alerting/unified/utils/navigation.ts diff --git a/.betterer.results b/.betterer.results index c5c5835206e..1f388c86a06 100644 --- a/.betterer.results +++ b/.betterer.results @@ -2316,28 +2316,9 @@ exports[`better eslint`] = { "public/app/features/alerting/unified/components/rules/RulesGroup.tsx:5381": [ [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"], [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "1"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "2"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "3"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "4"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "5"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "6"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "7"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "8"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "9"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "10"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "11"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "12"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "13"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "14"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "15"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "16"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "17"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "18"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "19"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "20"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "21"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "22"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "23"] + [0, 0, 0, "No untranslated strings. Wrap text with ", "2"], + [0, 0, 0, "No untranslated strings. Wrap text with ", "3"], + [0, 0, 0, "No untranslated strings. Wrap text with ", "4"] ], "public/app/features/alerting/unified/components/rules/central-state-history/utils.ts:5381": [ [0, 0, 0, "\'@grafana/data/src/field/fieldComparers\' import is restricted from being used by a pattern. Import from the public export instead.", "0"] diff --git a/public/app/features/alerting/routes.tsx b/public/app/features/alerting/routes.tsx index 490d264c5fa..e0cb793b655 100644 --- a/public/app/features/alerting/routes.tsx +++ b/public/app/features/alerting/routes.tsx @@ -264,6 +264,28 @@ export function getAlertingRoutes(cfg = config): RouteDescriptor[] { import(/* webpackChunkName: "AlertingRedirectToRule"*/ 'app/features/alerting/unified/RedirectToRuleViewer') ), }, + { + path: '/alerting/:dataSourceUid/namespaces/:namespaceId/groups/:groupName/view', + pageClass: 'page-alerting', + roles: evaluateAccess([AccessControlAction.AlertingRuleRead, AccessControlAction.AlertingRuleExternalRead]), + component: importAlertingComponent( + () => + import( + /* webpackChunkName: "AlertingGroupDetails" */ 'app/features/alerting/unified/group-details/GroupDetailsPage' + ) + ), + }, + { + path: '/alerting/:dataSourceUid/namespaces/:namespaceId/groups/:groupName/edit', + pageClass: 'page-alerting', + roles: evaluateAccess([AccessControlAction.AlertingRuleRead, AccessControlAction.AlertingRuleExternalRead]), + component: importAlertingComponent( + () => + import( + /* webpackChunkName: "AlertingGroupEdit" */ 'app/features/alerting/unified/group-details/GroupEditPage' + ) + ), + }, { path: '/alerting/admin', roles: () => ['Admin'], diff --git a/public/app/features/alerting/unified/api/alertingApi.ts b/public/app/features/alerting/unified/api/alertingApi.ts index becd84d297c..ae45cc7678d 100644 --- a/public/app/features/alerting/unified/api/alertingApi.ts +++ b/public/app/features/alerting/unified/api/alertingApi.ts @@ -1,5 +1,5 @@ import { BaseQueryFn, createApi, defaultSerializeQueryArgs } from '@reduxjs/toolkit/query/react'; -import { omit } from 'lodash'; +import { isBoolean, omit } from 'lodash'; import { lastValueFrom } from 'rxjs'; import { AppEvents } from '@grafana/data'; @@ -64,6 +64,8 @@ export const backendSrvBaseQuery = const modifiedRequestOptions: BackendSrvRequest = { ...requestOptions, ...(body && { data: body }), + ...(isBoolean(showSuccessAlert) && { showSuccessAlert }), + ...(isBoolean(showErrorAlert) && { showErrorAlert }), ...(successMessage && { showSuccessAlert: false }), ...((errorMessage || hideErrorMessage) && { showErrorAlert: false }), }; diff --git a/public/app/features/alerting/unified/api/featureDiscoveryApi.ts b/public/app/features/alerting/unified/api/featureDiscoveryApi.ts index b6d8c812163..e5f90c2baf6 100644 --- a/public/app/features/alerting/unified/api/featureDiscoveryApi.ts +++ b/public/app/features/alerting/unified/api/featureDiscoveryApi.ts @@ -16,7 +16,7 @@ export const GRAFANA_RULER_CONFIG: RulerDataSourceConfig = { apiVersion: 'legacy', }; -interface RulesSourceFeatures { +export interface RulesSourceFeatures { name: string; uid: string; application: RulesSourceApplication; diff --git a/public/app/features/alerting/unified/components/rule-editor/GrafanaEvaluationBehavior.tsx b/public/app/features/alerting/unified/components/rule-editor/GrafanaEvaluationBehavior.tsx index c8623783a84..ea84662c638 100644 --- a/public/app/features/alerting/unified/components/rule-editor/GrafanaEvaluationBehavior.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/GrafanaEvaluationBehavior.tsx @@ -21,7 +21,7 @@ import { useStyles2, } from '@grafana/ui'; import { Trans, t } from 'app/core/internationalization'; -import { RulerRuleGroupDTO, RulerRulesConfigDTO } from 'app/types/unified-alerting-dto'; +import { RulerRulesConfigDTO } from 'app/types/unified-alerting-dto'; import { alertRuleApi } from '../../api/alertRuleApi'; import { GRAFANA_RULER_CONFIG } from '../../api/featureDiscoveryApi'; @@ -31,7 +31,7 @@ import { isGrafanaAlertingRuleByType, isGrafanaManagedRuleByType, isGrafanaRecordingRuleByType, - rulerRuleType, + isProvisionedRuleGroup, } from '../../utils/rules'; import { parsePrometheusDuration } from '../../utils/time'; import { CollapseToggle } from '../CollapseToggle'; @@ -67,7 +67,7 @@ const namespaceToGroupOptions = (rulerNamespace: RulerRulesConfigDTO, enableProv return folderGroups .map>((group) => { - const isProvisioned = isProvisionedGroup(group); + const isProvisioned = isProvisionedRuleGroup(group); return { label: group.name, value: group.name, @@ -81,12 +81,6 @@ const namespaceToGroupOptions = (rulerNamespace: RulerRulesConfigDTO, enableProv .sort(sortByLabel); }; -const isProvisionedGroup = (group: RulerRuleGroupDTO) => { - return group.rules.some( - (rule) => rulerRuleType.grafana.rule(rule) && Boolean(rule.grafana_alert.provenance) === true - ); -}; - const sortByLabel = (a: SelectableValue, b: SelectableValue) => { return a.label?.localeCompare(b.label ?? '') || 0; }; diff --git a/public/app/features/alerting/unified/components/rule-viewer/RuleViewer.tsx b/public/app/features/alerting/unified/components/rule-viewer/RuleViewer.tsx index 119d15a21f1..82e46f5c574 100644 --- a/public/app/features/alerting/unified/components/rule-viewer/RuleViewer.tsx +++ b/public/app/features/alerting/unified/components/rule-viewer/RuleViewer.tsx @@ -20,6 +20,7 @@ import { useReturnTo } from '../../hooks/useReturnTo'; import { PluginOriginBadge } from '../../plugins/PluginOriginBadge'; import { Annotation } from '../../utils/constants'; import { makeDashboardLink, makePanelLink, stringifyErrorLike } from '../../utils/misc'; +import { createListFilterLink } from '../../utils/navigation'; import { RulePluginOrigin, getRulePluginOrigin, @@ -28,7 +29,6 @@ import { prometheusRuleType, rulerRuleType, } from '../../utils/rules'; -import { createRelativeUrl } from '../../utils/url'; import { AlertLabels } from '../AlertLabels'; import { AlertingPageWrapper } from '../AlertingPageWrapper'; import { ProvisionedResource, ProvisioningAlert } from '../Provisioning'; @@ -232,12 +232,6 @@ const createMetadata = (rule: CombinedRule): PageInfoItem[] => { return metadata; }; -// TODO move somewhere else -export const createListFilterLink = (values: Array<[string, string]>) => { - const params = new URLSearchParams([['search', values.map(([key, value]) => `${key}:"${value}"`).join(' ')]]); - return createRelativeUrl(`/alerting/list`, params); -}; - interface TitleProps { name: string; paused?: boolean; diff --git a/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx b/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx index 0d748604cf4..ab1a5640ad8 100644 --- a/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx +++ b/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx @@ -172,10 +172,11 @@ export const evaluateEveryValidationOptions = (rules: Rul const { forDuration } = getAlertInfo(rule, evaluateEvery); return forDuration ? safeParsePrometheusDuration(forDuration) : null; }); - const largestPendingPeriod = Math.min( - ...rulePendingPeriods.filter((period): period is number => period !== null) + // 0 is a special case which disables the pending period at all + const smallestPendingPeriod = Math.min( + ...rulePendingPeriods.filter((period): period is number => period !== null && period !== 0) ); - return `Evaluation interval should be smaller or equal to "pending period" values for existing rules in this rule group. Choose a value smaller than or equal to "${formatPrometheusDuration(largestPendingPeriod)}".`; + return `Evaluation interval should be smaller or equal to "pending period" values for existing rules in this rule group. Choose a value smaller than or equal to "${formatPrometheusDuration(smallestPendingPeriod)}".`; } } catch (error) { return error instanceof Error ? error.message : 'Failed to parse duration'; diff --git a/public/app/features/alerting/unified/components/rules/RulesGroup.test.tsx b/public/app/features/alerting/unified/components/rules/RulesGroup.test.tsx index 9baab7600cd..67e5486f9f8 100644 --- a/public/app/features/alerting/unified/components/rules/RulesGroup.test.tsx +++ b/public/app/features/alerting/unified/components/rules/RulesGroup.test.tsx @@ -1,19 +1,15 @@ -import { render, screen, waitFor } from '@testing-library/react'; -import userEvent from '@testing-library/user-event'; -import { Provider } from 'react-redux'; -import { Props } from 'react-virtualized-auto-sizer'; -import { byRole, byTestId, byText } from 'testing-library-selector'; +import { render, screen } from 'test/test-utils'; +import { byRole } from 'testing-library-selector'; import { contextSrv } from 'app/core/services/context_srv'; -import { configureStore } from 'app/store/configureStore'; import { AccessControlAction } from 'app/types'; import { CombinedRuleGroup, CombinedRuleNamespace, RulerDataSourceConfig } from 'app/types/unified-alerting'; import * as analytics from '../../Analytics'; import { GRAFANA_RULER_CONFIG } from '../../api/featureDiscoveryApi'; import { useHasRuler } from '../../hooks/useHasRuler'; -import { mockExportApi, mockFolderApi, setupMswServer } from '../../mockApi'; -import { grantUserPermissions, mockCombinedRule, mockDataSource, mockFolder, mockGrafanaRulerRule } from '../../mocks'; +import { mockFolderApi, setupMswServer } from '../../mockApi'; +import { grantUserPermissions, mockCombinedRule, mockFolder, mockGrafanaRulerRule } from '../../mocks'; import { mimirDataSource } from '../../mocks/server/configure'; import { RulesGroup } from './RulesGroup'; @@ -22,20 +18,6 @@ jest.mock('../../hooks/useHasRuler'); jest.spyOn(analytics, 'logInfo'); -jest.mock('react-virtualized-auto-sizer', () => { - return ({ children }: Props) => - children({ - height: 600, - scaledHeight: 600, - scaledWidth: 1, - width: 1, - }); -}); -jest.mock('@grafana/ui', () => ({ - ...jest.requireActual('@grafana/ui'), - CodeEditor: ({ value }: { value: string }) =>