From 977184b878f4ba0c5ef19eb6c2ba43bcbefeceb8 Mon Sep 17 00:00:00 2001 From: Sonia Aguilar <33540275+soniaAguilarPeiron@users.noreply.github.com> Date: Fri, 22 Nov 2024 12:07:45 +0100 Subject: [PATCH] Alerting: Simplify notification step (#96430) * Move evaluation outside folder section, and move labels in instead * rename file * update translations * refactor * rename file and component * refactor * fix test * refactor * rename files and components * update translations * fix style * update translations * Add feature toggle for simplified mode in notifications step * WIP * Use useAppNotification for toasts * update label when group can not be selected yet * update translations * WIP * update some texts and add comment * update translations * remove duplicated code * fix typo * update translations * update styles and remove label * update simplified_notifications_section name according BE changes * remove FolderWithoutGroup * remove commented code * prettier * remove SIMPLIFIED_NOTIFICATION_STEP_KEY and use MANUAL_ROUTING_KEY instead * styles cleanup * Update docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md Co-authored-by: Gilles De Mey * merge main and prettier doc --------- Co-authored-by: Gilles De Mey --- .betterer.results | 3 +- .../feature-toggles/index.md | 1 + .../src/types/featureToggles.gen.ts | 1 + pkg/services/featuremgmt/registry.go | 7 +++ pkg/services/featuremgmt/toggles_gen.csv | 1 + pkg/services/featuremgmt/toggles_gen.go | 4 ++ pkg/services/featuremgmt/toggles_gen.json | 16 ++++++ .../rule-editor/NotificationsStep.tsx | 52 ++++++++++++++++-- .../NotificationPreview.tsx | 55 +++---------------- .../QueryAndExpressionsStep.tsx | 4 +- .../determineAdvancedMode.test.ts | 7 +-- .../alerting/unified/types/rule-form.ts | 1 + .../alerting/unified/utils/rule-form.ts | 7 ++- public/app/types/unified-alerting-dto.ts | 1 + 14 files changed, 98 insertions(+), 62 deletions(-) diff --git a/.betterer.results b/.betterer.results index b017487e851..2d0143c98db 100644 --- a/.betterer.results +++ b/.betterer.results @@ -1378,8 +1378,7 @@ exports[`better eslint`] = { [0, 0, 0, "No untranslated strings. Wrap text with ", "8"], [0, 0, 0, "No untranslated strings. Wrap text with ", "9"], [0, 0, 0, "No untranslated strings. Wrap text with ", "10"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "11"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "12"] + [0, 0, 0, "No untranslated strings. Wrap text with ", "11"] ], "public/app/features/alerting/unified/components/rule-editor/PreviewRule.tsx:5381": [ [0, 0, 0, "No untranslated strings. Wrap text with ", "0"], diff --git a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md index 4fe79154ad9..a9199b6ea11 100644 --- a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md +++ b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md @@ -226,6 +226,7 @@ Experimental features might be changed or removed without prior notice. | `enableSCIM` | Enables SCIM support for user and group management | | `crashDetection` | Enables browser crash detection reporting to Faro. | | `jaegerBackendMigration` | Enables querying the Jaeger data source without the proxy | +| `alertingNotificationsStepMode` | Enables simplified step mode in the notifications section | ## Development feature toggles diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index 1fcbf6f7144..663f65888fc 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -241,4 +241,5 @@ export interface FeatureToggles { jaegerBackendMigration?: boolean; reportingUseRawTimeRange?: boolean; alertingUIOptimizeReducer?: boolean; + alertingNotificationsStepMode?: boolean; } diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index 78e87ef142c..2260ec8c0eb 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -1666,6 +1666,13 @@ var ( Owner: grafanaAlertingSquad, Expression: "true", // enabled by default }, + { + Name: "alertingNotificationsStepMode", + Description: "Enables simplified step mode in the notifications section", + Stage: FeatureStageExperimental, + Owner: grafanaAlertingSquad, + FrontendOnly: true, + }, } ) diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index 6d50ebc98f1..eb3759fc0ac 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -222,3 +222,4 @@ crashDetection,experimental,@grafana/observability-traces-and-profiling,false,fa jaegerBackendMigration,experimental,@grafana/oss-big-tent,false,false,false reportingUseRawTimeRange,preview,@grafana/sharing-squad,false,false,false alertingUIOptimizeReducer,GA,@grafana/alerting-squad,false,false,true +alertingNotificationsStepMode,experimental,@grafana/alerting-squad,false,false,true diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index 14e5e4d42e2..7e012332b13 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -898,4 +898,8 @@ const ( // FlagAlertingUIOptimizeReducer // Enables removing the reducer from the alerting UI when creating a new alert rule and using instant query FlagAlertingUIOptimizeReducer = "alertingUIOptimizeReducer" + + // FlagAlertingNotificationsStepMode + // Enables simplified step mode in the notifications section + FlagAlertingNotificationsStepMode = "alertingNotificationsStepMode" ) diff --git a/pkg/services/featuremgmt/toggles_gen.json b/pkg/services/featuremgmt/toggles_gen.json index 44b5c3bbb2f..0f692b83d5a 100644 --- a/pkg/services/featuremgmt/toggles_gen.json +++ b/pkg/services/featuremgmt/toggles_gen.json @@ -241,6 +241,22 @@ "hideFromAdminPage": true } }, + { + "metadata": { + "name": "alertingNotificationsStepMode", + "resourceVersion": "1732272457848", + "creationTimestamp": "2024-11-06T09:35:49Z", + "annotations": { + "grafana.app/updatedTimestamp": "2024-11-22 10:47:37.848635 +0000 UTC" + } + }, + "spec": { + "description": "Enables simplified step mode in the notifications section", + "stage": "experimental", + "codeowner": "@grafana/alerting-squad", + "frontend": true + } + }, { "metadata": { "name": "alertingPrometheusRulesPrimary", diff --git a/public/app/features/alerting/unified/components/rule-editor/NotificationsStep.tsx b/public/app/features/alerting/unified/components/rule-editor/NotificationsStep.tsx index 5e63b3afed5..5bdcb964f13 100644 --- a/public/app/features/alerting/unified/components/rule-editor/NotificationsStep.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/NotificationsStep.tsx @@ -41,12 +41,13 @@ export const NotificationsStep = ({ alertUid }: NotificationsStepProps) => { const { watch, getValues, setValue } = useFormContext(); const styles = useStyles2(getStyles); - const [type] = watch(['type', 'labels', 'queries', 'condition', 'folder', 'name', 'manualRouting']); + const [type, manualRouting] = watch(['type', 'manualRouting']); const [showLabelsEditor, setShowLabelsEditor] = useState(false); const dataSourceName = watch('dataSourceName') ?? GRAFANA_RULES_SOURCE_NAME; const isGrafanaManaged = isGrafanaManagedRuleByType(type); const simplifiedRoutingToggleEnabled = config.featureToggles.alertingSimplifiedRouting ?? false; + const simplifiedModeInNotificationsStepEnabled = config.featureToggles.alertingNotificationsStepMode ?? false; const shouldRenderpreview = type === RuleFormType.grafana; const hasInternalAlertmanagerEnabled = useHasInternalAlertmanagerEnabled(); @@ -66,6 +67,16 @@ export const NotificationsStep = ({ alertUid }: NotificationsStepProps) => { const step = !isGrafanaManaged ? 4 : 5; + const switchMode = + isGrafanaManaged && simplifiedModeInNotificationsStepEnabled + ? { + isAdvancedMode: !manualRouting, + setAdvancedMode: (isAdvanced: boolean) => { + setValue('editorSettings.simplifiedNotificationEditor', !isAdvanced); + setValue('manualRouting', !isAdvanced); + }, + } + : undefined; const title = isRecordingRuleByType(type) ? 'Add labels' : isGrafanaManaged @@ -91,6 +102,7 @@ export const NotificationsStep = ({ alertUid }: NotificationsStepProps) => { )} } + switchMode={switchMode} fullWidth > {!isGrafanaManaged && ( @@ -106,14 +118,16 @@ export const NotificationsStep = ({ alertUid }: NotificationsStepProps) => { )} {shouldAllowSimplifiedRouting && (
- Notifications - - Select who should receive a notification when an alert rule fires. - + Recipient
)} {shouldAllowSimplifiedRouting ? ( // when simplified routing is enabled and is grafana rule - + simplifiedModeInNotificationsStepEnabled ? ( // simplified mode is enabled + + ) : ( + // simplified mode is disabled + + ) ) : // when simplified routing is not enabled, render the notification preview as we did before shouldRenderpreview ? ( @@ -167,6 +181,32 @@ function ManualAndAutomaticRouting({ alertUid }: { alertUid?: string }) { ); } +/** + * Preconditions: + * - simplified routing is enabled + * - simple mode for notifications step is enabled + * - the alert rule is a grafana rule + * + * This component will render the switch between the select contact point routing and the notification policy routing. + * It also renders the section body of the NotificationsStep, depending on the routing option selected. + * If select contact point routing is selected, it will render the SimplifiedRouting component. + * If notification policy routing is selected, it will render the AutomaticRouting component. + * + */ +function ManualAndAutomaticRoutingSimplified({ alertUid }: { alertUid?: string }) { + const { watch } = useFormContext(); + + const [manualRouting] = watch(['manualRouting']); + + return ( + + + + {manualRouting ? : } + + ); +} + interface AutomaticRootingProps { alertUid?: string; } diff --git a/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationPreview.tsx b/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationPreview.tsx index 5f42608231a..2763a7147cc 100644 --- a/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationPreview.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationPreview.tsx @@ -1,11 +1,8 @@ -import { css } from '@emotion/css'; import { compact } from 'lodash'; import { lazy, Suspense } from 'react'; -import { GrafanaTheme2 } from '@grafana/data'; -import { Button, LoadingPlaceholder, Text, useStyles2 } from '@grafana/ui'; +import { Button, LoadingPlaceholder, Stack, Text } from '@grafana/ui'; import { alertRuleApi } from 'app/features/alerting/unified/api/alertRuleApi'; -import { Stack } from 'app/plugins/datasource/parca/QueryEditor/Stack'; import { AlertQuery } from 'app/types/unified-alerting-dto'; import { Folder, KBObjectArray } from '../../../types/rule-form'; @@ -32,7 +29,6 @@ export const NotificationPreview = ({ alertName, alertUid, }: NotificationPreviewProps) => { - const styles = useStyles2(getStyles); const disabled = !condition || !folder; const previewEndpoint = alertRuleApi.endpoints.preview; @@ -66,8 +62,8 @@ export const NotificationPreview = ({ return ( -
-
+ + Alert instance routing preview {isLoading && previewUninitialized && ( @@ -85,13 +81,11 @@ export const NotificationPreview = ({ notification policy below to view more details. )} -
-
- -
-
+ + + {!isLoading && !previewUninitialized && potentialInstances.length > 0 && ( }> {alertManagerDataSources.map((alertManagerSource) => ( @@ -107,36 +101,3 @@ export const NotificationPreview = ({ ); }; - -const getStyles = (theme: GrafanaTheme2) => ({ - collapsableSection: css({ - width: 'auto', - border: 0, - }), - previewHeader: css({ - margin: 0, - }), - routePreviewHeaderRow: css({ - display: 'flex', - flexDirection: 'row', - justifyContent: 'space-between', - alignItems: 'flex-start', - marginTop: theme.spacing(1), - }), - collapseLabel: css({ - flex: 1, - }), - button: css({ - justifyContent: 'flex-end', - }), - tagsInDetails: css({ - display: 'flex', - justifyContent: 'flex-start', - flexWrap: 'wrap', - }), - policyPathItemMatchers: css({ - display: 'flex', - flexDirection: 'row', - gap: theme.spacing(1), - }), -}); diff --git a/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/QueryAndExpressionsStep.tsx b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/QueryAndExpressionsStep.tsx index 5bf853f9c64..7d0eaeb5b97 100644 --- a/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/QueryAndExpressionsStep.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/QueryAndExpressionsStep.tsx @@ -484,7 +484,7 @@ export const QueryAndExpressionsStep = ({ editingExistingRule, onDataChange }: P return; } } - setValue('editorSettings', { simplifiedQueryEditor: !isAdvanced }); + setValue('editorSettings.simplifiedQueryEditor', !isAdvanced); }, } : undefined; @@ -688,7 +688,7 @@ export const QueryAndExpressionsStep = ({ editingExistingRule, onDataChange }: P confirmText="Deactivate" icon="exclamation-triangle" onConfirm={() => { - setValue('editorSettings', { simplifiedQueryEditor: true }); + setValue('editorSettings.simplifiedNotificationEditor', true); setShowResetModal(false); dispatch(resetToSimpleCondition()); }} diff --git a/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/determineAdvancedMode.test.ts b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/determineAdvancedMode.test.ts index ad598680709..e82f1497399 100644 --- a/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/determineAdvancedMode.test.ts +++ b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/determineAdvancedMode.test.ts @@ -9,7 +9,7 @@ const expressionQueries = [reduceExpression, thresholdExpression]; describe('determineAdvancedMode', () => { it('should return true if simplifiedQueryEditor is false', () => { - const editorSettings = { simplifiedQueryEditor: false }; + const editorSettings = { simplifiedQueryEditor: false, simplifiedNotificationEditor: true }; const isGrafanaAlertingType = true; const isNewFromQueryParams = false; @@ -25,7 +25,7 @@ describe('determineAdvancedMode', () => { }); it('should return true if isGrafanaAlertingType is false', () => { - const editorSettings = { simplifiedQueryEditor: true }; + const editorSettings = { simplifiedQueryEditor: true, simplifiedNotificationEditor: true }; const isGrafanaAlertingType = false; const isNewFromQueryParams = false; @@ -40,8 +40,8 @@ describe('determineAdvancedMode', () => { expect(result).toBe(true); }); + const editorSettings = { simplifiedQueryEditor: true, simplifiedNotificationEditor: true }; it('should return true if isNewFromQueryParams is true and queries are not transformable', () => { - const editorSettings = { simplifiedQueryEditor: true }; const isGrafanaAlertingType = true; const isNewFromQueryParams = true; @@ -61,7 +61,6 @@ describe('determineAdvancedMode', () => { }); it('should return false if all conditions are false', () => { - const editorSettings = { simplifiedQueryEditor: true }; const isGrafanaAlertingType = true; const isNewFromQueryParams = false; diff --git a/public/app/features/alerting/unified/types/rule-form.ts b/public/app/features/alerting/unified/types/rule-form.ts index 8314961ba5d..07547e9c509 100644 --- a/public/app/features/alerting/unified/types/rule-form.ts +++ b/public/app/features/alerting/unified/types/rule-form.ts @@ -25,6 +25,7 @@ export interface AlertManagerManualRouting { export interface SimplifiedEditor { simplifiedQueryEditor: boolean; + simplifiedNotificationEditor: boolean; } export type KVObject = { key: string; value: string }; diff --git a/public/app/features/alerting/unified/utils/rule-form.ts b/public/app/features/alerting/unified/utils/rule-form.ts index f3be44d152b..5a9deaa0106 100644 --- a/public/app/features/alerting/unified/utils/rule-form.ts +++ b/public/app/features/alerting/unified/utils/rule-form.ts @@ -136,10 +136,12 @@ function getDefaultEditorSettings() { if (!editorSettingsEnabled) { return undefined; } - //then, check in local storage if the user has saved last rule with simplified query editor + //then, check in local storage if the user has saved last rule with sections simplified const queryEditorSettings = localStorage.getItem(SIMPLIFIED_QUERY_EDITOR_KEY); + const notificationStepSettings = localStorage.getItem(MANUAL_ROUTING_KEY); return { simplifiedQueryEditor: queryEditorSettings !== 'false', + simplifiedNotificationEditor: notificationStepSettings !== 'false', }; } @@ -229,6 +231,7 @@ export function getNotificationSettingsForDTO( function getEditorSettingsForDTO(simplifiedEditor: SimplifiedEditor) { return { simplified_query_and_expressions_section: simplifiedEditor.simplifiedQueryEditor, + simplified_notifications_section: simplifiedEditor.simplifiedNotificationEditor, }; } export function formValuesToRulerGrafanaRuleDTO(values: RuleFormValues): PostableRuleGrafanaRuleDTO { @@ -348,11 +351,13 @@ function getEditorSettingsFromDTO(ga: GrafanaRuleDefinition) { if (ga.metadata?.editor_settings) { return { simplifiedQueryEditor: ga.metadata.editor_settings.simplified_query_and_expressions_section, + simplifiedNotificationEditor: ga.metadata.editor_settings.simplified_notifications_section, }; } return { simplifiedQueryEditor: false, + simplifiedNotificationEditor: Boolean(ga.notification_settings), // in case this rule was created before the new field was added, we'll default to current routing settings }; } diff --git a/public/app/types/unified-alerting-dto.ts b/public/app/types/unified-alerting-dto.ts index 3edbb586d21..4b7082e16e4 100644 --- a/public/app/types/unified-alerting-dto.ts +++ b/public/app/types/unified-alerting-dto.ts @@ -224,6 +224,7 @@ export interface GrafanaNotificationSettings { export interface GrafanaEditorSettings { simplified_query_and_expressions_section: boolean; + simplified_notifications_section: boolean; } export interface PostableGrafanaRuleDefinition { uid?: string;