From 324503ee8b09039b9fc213c760c670d36a15f0c6 Mon Sep 17 00:00:00 2001 From: Alexander Akhmetov Date: Thu, 14 Nov 2024 12:55:54 +0100 Subject: [PATCH] Alerting: Add simplified_notifications_section field to the alert rule metadata (#95988) --- pkg/services/ngalert/api/api_ruler_test.go | 2 + .../ngalert/api/api_ruler_validation.go | 1 + .../ngalert/api/api_ruler_validation_test.go | 30 ++++++++++++ pkg/services/ngalert/api/compat.go | 1 + pkg/services/ngalert/api/compat_test.go | 17 +++++++ .../api/tooling/definitions/cortex-ruler.go | 1 + pkg/services/ngalert/models/alert_rule.go | 1 + pkg/services/ngalert/models/testing.go | 10 +++- .../ngalert/schedule/registry_test.go | 1 + .../api/alerting/api_alertmanager_test.go | 21 +++++--- pkg/tests/api/alerting/api_ruler_test.go | 49 +++++++++++++++++-- .../alerting/test-data/rulegroup-1-get.json | 6 ++- .../alerting/test-data/rulegroup-2-get.json | 3 +- .../alerting/test-data/rulegroup-3-get.json | 6 ++- 14 files changed, 132 insertions(+), 17 deletions(-) diff --git a/pkg/services/ngalert/api/api_ruler_test.go b/pkg/services/ngalert/api/api_ruler_test.go index 7994712bbb7..6a346c29102 100644 --- a/pkg/services/ngalert/api/api_ruler_test.go +++ b/pkg/services/ngalert/api/api_ruler_test.go @@ -337,6 +337,7 @@ func TestRouteGetRuleByUID(t *testing.T) { createdRules := gen.With( gen.WithUniqueGroupIndex(), gen.WithUniqueID(), gen.WithEditorSettingsSimplifiedQueryAndExpressionsSection(true), + gen.WithEditorSettingsSimplifiedNotificationsSection(true), ).GenerateManyRef(3) require.Len(t, createdRules, 3) ruleStore.PutRule(context.Background(), createdRules...) @@ -356,6 +357,7 @@ func TestRouteGetRuleByUID(t *testing.T) { require.Equal(t, expectedRule.RuleGroup, result.GrafanaManagedAlert.RuleGroup) require.Equal(t, expectedRule.Title, result.GrafanaManagedAlert.Title) require.True(t, result.GrafanaManagedAlert.Metadata.EditorSettings.SimplifiedQueryAndExpressionsSection) + require.True(t, result.GrafanaManagedAlert.Metadata.EditorSettings.SimplifiedNotificationsSection) }) t.Run("error when fetching rule with non-existent UID", func(t *testing.T) { diff --git a/pkg/services/ngalert/api/api_ruler_validation.go b/pkg/services/ngalert/api/api_ruler_validation.go index ae5bd72b840..1ed855378fd 100644 --- a/pkg/services/ngalert/api/api_ruler_validation.go +++ b/pkg/services/ngalert/api/api_ruler_validation.go @@ -147,6 +147,7 @@ func validateAlertingRuleFields(in *apimodels.PostableExtendedRuleNode, newRule if in.GrafanaManagedAlert.Metadata != nil { newRule.Metadata.EditorSettings = ngmodels.EditorSettings{ SimplifiedQueryAndExpressionsSection: in.GrafanaManagedAlert.Metadata.EditorSettings.SimplifiedQueryAndExpressionsSection, + SimplifiedNotificationsSection: in.GrafanaManagedAlert.Metadata.EditorSettings.SimplifiedNotificationsSection, } } diff --git a/pkg/services/ngalert/api/api_ruler_validation_test.go b/pkg/services/ngalert/api/api_ruler_validation_test.go index 4d70960da5c..33574690840 100644 --- a/pkg/services/ngalert/api/api_ruler_validation_test.go +++ b/pkg/services/ngalert/api/api_ruler_validation_test.go @@ -1046,6 +1046,36 @@ func TestValidateRuleNodeNotificationSettings(t *testing.T) { } } +func TestValidateRuleNodeEditorSettings(t *testing.T) { + cfg := config(t) + limits := makeLimits(cfg) + + editorSettings := models.EditorSettings{ + SimplifiedQueryAndExpressionsSection: true, + SimplifiedNotificationsSection: true, + } + + testCases := []struct { + name string + editorSettings models.EditorSettings + }{ + { + name: "valid editor settings", + editorSettings: editorSettings, + }, + } + + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + r := validRule() + r.GrafanaManagedAlert.Metadata = AlertRuleMetadataFromModelMetadata(models.AlertRuleMetadata{EditorSettings: tt.editorSettings}) + newRule, err := validateRuleNode(&r, util.GenerateShortUID(), cfg.BaseInterval*time.Duration(rand.Int63n(10)+1), rand.Int63(), randFolder().UID, limits) + require.NoError(t, err) + require.Equal(t, tt.editorSettings, newRule.Metadata.EditorSettings) + }) + } +} + func TestValidateRuleNodeReservedLabels(t *testing.T) { cfg := config(t) limits := makeLimits(cfg) diff --git a/pkg/services/ngalert/api/compat.go b/pkg/services/ngalert/api/compat.go index 30266fc504b..ba78f5f613d 100644 --- a/pkg/services/ngalert/api/compat.go +++ b/pkg/services/ngalert/api/compat.go @@ -425,6 +425,7 @@ func MuteTimingIntervalToMuteTimeIntervalHclExport(m definitions.MuteTimeInterva func AlertRuleEditorSettingsFromModelEditorSettings(es models.EditorSettings) *definitions.AlertRuleEditorSettings { return &definitions.AlertRuleEditorSettings{ SimplifiedQueryAndExpressionsSection: es.SimplifiedQueryAndExpressionsSection, + SimplifiedNotificationsSection: es.SimplifiedNotificationsSection, } } diff --git a/pkg/services/ngalert/api/compat_test.go b/pkg/services/ngalert/api/compat_test.go index bd06d64d393..e2dd108c815 100644 --- a/pkg/services/ngalert/api/compat_test.go +++ b/pkg/services/ngalert/api/compat_test.go @@ -6,6 +6,7 @@ import ( "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" + "github.com/grafana/grafana/pkg/services/ngalert/models" ) func TestToModel(t *testing.T) { @@ -67,3 +68,19 @@ func TestToModel(t *testing.T) { require.Nil(t, rule.NotificationSettings) }) } + +func TestAlertRuleMetadataFromModelMetadata(t *testing.T) { + t.Run("should convert model metadata to api metadata", func(t *testing.T) { + modelMetadata := models.AlertRuleMetadata{ + EditorSettings: models.EditorSettings{ + SimplifiedQueryAndExpressionsSection: true, + SimplifiedNotificationsSection: true, + }, + } + + apiMetadata := AlertRuleMetadataFromModelMetadata(modelMetadata) + + require.True(t, apiMetadata.EditorSettings.SimplifiedQueryAndExpressionsSection) + require.True(t, apiMetadata.EditorSettings.SimplifiedNotificationsSection) + }) +} diff --git a/pkg/services/ngalert/api/tooling/definitions/cortex-ruler.go b/pkg/services/ngalert/api/tooling/definitions/cortex-ruler.go index bca1323a41b..a6383efa0b6 100644 --- a/pkg/services/ngalert/api/tooling/definitions/cortex-ruler.go +++ b/pkg/services/ngalert/api/tooling/definitions/cortex-ruler.go @@ -460,6 +460,7 @@ type AlertRuleMetadata struct { // swagger:model type AlertRuleEditorSettings struct { SimplifiedQueryAndExpressionsSection bool `json:"simplified_query_and_expressions_section" yaml:"simplified_query_and_expressions_section"` + SimplifiedNotificationsSection bool `json:"simplified_notifications_section" yaml:"simplified_notifications_section"` } // swagger:model diff --git a/pkg/services/ngalert/models/alert_rule.go b/pkg/services/ngalert/models/alert_rule.go index 32ca13a33d4..c299a9f6aa1 100644 --- a/pkg/services/ngalert/models/alert_rule.go +++ b/pkg/services/ngalert/models/alert_rule.go @@ -278,6 +278,7 @@ type AlertRuleMetadata struct { type EditorSettings struct { SimplifiedQueryAndExpressionsSection bool `json:"simplified_query_and_expressions_section"` + SimplifiedNotificationsSection bool `json:"simplified_notifications_section"` } // Namespaced describes a class of resources that are stored in a specific namespace. diff --git a/pkg/services/ngalert/models/testing.go b/pkg/services/ngalert/models/testing.go index b8b2b5ceed2..53279b09cbb 100644 --- a/pkg/services/ngalert/models/testing.go +++ b/pkg/services/ngalert/models/testing.go @@ -197,9 +197,15 @@ func (a *AlertRuleMutators) WithUniqueID() AlertRuleMutator { } } -func (a *AlertRuleMutators) WithEditorSettingsSimplifiedQueryAndExpressionsSection(mode bool) AlertRuleMutator { +func (a *AlertRuleMutators) WithEditorSettingsSimplifiedQueryAndExpressionsSection(enabled bool) AlertRuleMutator { return func(rule *AlertRule) { - rule.Metadata.EditorSettings.SimplifiedQueryAndExpressionsSection = mode + rule.Metadata.EditorSettings.SimplifiedQueryAndExpressionsSection = enabled + } +} + +func (a *AlertRuleMutators) WithEditorSettingsSimplifiedNotificationsSection(enabled bool) AlertRuleMutator { + return func(rule *AlertRule) { + rule.Metadata.EditorSettings.SimplifiedNotificationsSection = enabled } } diff --git a/pkg/services/ngalert/schedule/registry_test.go b/pkg/services/ngalert/schedule/registry_test.go index 8c770dfb640..a28ec6265aa 100644 --- a/pkg/services/ngalert/schedule/registry_test.go +++ b/pkg/services/ngalert/schedule/registry_test.go @@ -207,6 +207,7 @@ func TestRuleWithFolderFingerprint(t *testing.T) { Metadata: models.AlertRuleMetadata{ EditorSettings: models.EditorSettings{ SimplifiedQueryAndExpressionsSection: false, + SimplifiedNotificationsSection: false, }, }, } diff --git a/pkg/tests/api/alerting/api_alertmanager_test.go b/pkg/tests/api/alerting/api_alertmanager_test.go index e5d7b5fb01f..67f6bb696eb 100644 --- a/pkg/tests/api/alerting/api_alertmanager_test.go +++ b/pkg/tests/api/alerting/api_alertmanager_test.go @@ -793,7 +793,8 @@ func TestIntegrationDeleteFolderWithRules(t *testing.T) { "exec_err_state": "Alerting", "metadata": { "editor_settings": { - "simplified_query_and_expressions_section": false + "simplified_query_and_expressions_section": false, + "simplified_notifications_section": false } } } @@ -1276,7 +1277,8 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { "exec_err_state":"Alerting", "metadata": { "editor_settings": { - "simplified_query_and_expressions_section": false + "simplified_query_and_expressions_section": false, + "simplified_notifications_section": false } } } @@ -1317,7 +1319,8 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { "exec_err_state":"Alerting", "metadata": { "editor_settings": { - "simplified_query_and_expressions_section": false + "simplified_query_and_expressions_section": false, + "simplified_notifications_section": false } } } @@ -1630,7 +1633,8 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { "exec_err_state":"Alerting", "metadata": { "editor_settings": { - "simplified_query_and_expressions_section": false + "simplified_query_and_expressions_section": false, + "simplified_notifications_section": false } } } @@ -1744,7 +1748,8 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { "exec_err_state":"Alerting", "metadata": { "editor_settings": { - "simplified_query_and_expressions_section": false + "simplified_query_and_expressions_section": false, + "simplified_notifications_section": false } } } @@ -1837,7 +1842,8 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { "exec_err_state":"Alerting", "metadata": { "editor_settings": { - "simplified_query_and_expressions_section": false + "simplified_query_and_expressions_section": false, + "simplified_notifications_section": false } } } @@ -2369,7 +2375,8 @@ func TestIntegrationQuota(t *testing.T) { "exec_err_state":"Alerting", "metadata": { "editor_settings": { - "simplified_query_and_expressions_section": false + "simplified_query_and_expressions_section": false, + "simplified_notifications_section": false } } } diff --git a/pkg/tests/api/alerting/api_ruler_test.go b/pkg/tests/api/alerting/api_ruler_test.go index f96d6facd43..b54f70dec15 100644 --- a/pkg/tests/api/alerting/api_ruler_test.go +++ b/pkg/tests/api/alerting/api_ruler_test.go @@ -919,6 +919,46 @@ func TestIntegrationAlertRuleEditorSettings(t *testing.T) { require.False(t, updatedRuleGroup.Rules[0].GrafanaManagedAlert.Metadata.EditorSettings.SimplifiedQueryAndExpressionsSection) }) + t.Run("set simplified notifications editor in editor settings", func(t *testing.T) { + metadata := &apimodels.AlertRuleMetadata{ + EditorSettings: apimodels.AlertRuleEditorSettings{ + SimplifiedQueryAndExpressionsSection: false, + }, + } + createdRuleGroup := createAlertInGrafana(metadata) + + rulesWithUID := convertGettableRuleGroupToPostable(createdRuleGroup) + rulesWithUID.Rules[0].GrafanaManagedAlert.Metadata.EditorSettings.SimplifiedNotificationsSection = true + + _, status, _ := apiClient.PostRulesGroupWithStatus(t, folderName, &rulesWithUID) + assert.Equal(t, http.StatusAccepted, status) + + updatedRuleGroup := apiClient.GetRulesGroup(t, folderName, groupName).GettableRuleGroupConfig + require.Len(t, updatedRuleGroup.Rules, 1) + require.True(t, updatedRuleGroup.Rules[0].GrafanaManagedAlert.Metadata.EditorSettings.SimplifiedNotificationsSection) + }) + + t.Run("disable simplified notifications editor in editor settings", func(t *testing.T) { + metadata := &apimodels.AlertRuleMetadata{ + EditorSettings: apimodels.AlertRuleEditorSettings{ + SimplifiedNotificationsSection: true, + }, + } + createdRuleGroup := createAlertInGrafana(metadata) + + rulesWithUID := convertGettableRuleGroupToPostable(createdRuleGroup) + + // disabling the editor + rulesWithUID.Rules[0].GrafanaManagedAlert.Metadata.EditorSettings.SimplifiedNotificationsSection = false + + _, status, _ := apiClient.PostRulesGroupWithStatus(t, folderName, &rulesWithUID) + assert.Equal(t, http.StatusAccepted, status) + + updatedRuleGroup := apiClient.GetRulesGroup(t, folderName, groupName).GettableRuleGroupConfig + require.Len(t, updatedRuleGroup.Rules, 1) + require.False(t, updatedRuleGroup.Rules[0].GrafanaManagedAlert.Metadata.EditorSettings.SimplifiedNotificationsSection) + }) + t.Run("post alert without metadata", func(t *testing.T) { createAlertInGrafana(nil) @@ -1154,7 +1194,8 @@ func TestIntegrationRulerRulesFilterByDashboard(t *testing.T) { "exec_err_state": "Alerting", "metadata": { "editor_settings": { - "simplified_query_and_expressions_section": false + "simplified_query_and_expressions_section": false, + "simplified_notifications_section": false } } } @@ -1192,7 +1233,8 @@ func TestIntegrationRulerRulesFilterByDashboard(t *testing.T) { "exec_err_state": "Alerting", "metadata": { "editor_settings": { - "simplified_query_and_expressions_section": false + "simplified_query_and_expressions_section": false, + "simplified_notifications_section": false } } } @@ -1242,7 +1284,8 @@ func TestIntegrationRulerRulesFilterByDashboard(t *testing.T) { "exec_err_state": "Alerting", "metadata": { "editor_settings": { - "simplified_query_and_expressions_section": false + "simplified_query_and_expressions_section": false, + "simplified_notifications_section": false } } } diff --git a/pkg/tests/api/alerting/test-data/rulegroup-1-get.json b/pkg/tests/api/alerting/test-data/rulegroup-1-get.json index ab1231f3bd8..cb2b3df8345 100644 --- a/pkg/tests/api/alerting/test-data/rulegroup-1-get.json +++ b/pkg/tests/api/alerting/test-data/rulegroup-1-get.json @@ -44,7 +44,8 @@ "is_paused": false, "metadata": { "editor_settings": { - "simplified_query_and_expressions_section": false + "simplified_query_and_expressions_section": false, + "simplified_notifications_section": false } } } @@ -91,7 +92,8 @@ "is_paused": false, "metadata": { "editor_settings": { - "simplified_query_and_expressions_section": false + "simplified_query_and_expressions_section": false, + "simplified_notifications_section": false } } } diff --git a/pkg/tests/api/alerting/test-data/rulegroup-2-get.json b/pkg/tests/api/alerting/test-data/rulegroup-2-get.json index 397066bf202..572454ad1e7 100644 --- a/pkg/tests/api/alerting/test-data/rulegroup-2-get.json +++ b/pkg/tests/api/alerting/test-data/rulegroup-2-get.json @@ -44,7 +44,8 @@ "is_paused": false, "metadata": { "editor_settings": { - "simplified_query_and_expressions_section": false + "simplified_query_and_expressions_section": false, + "simplified_notifications_section": false } } } diff --git a/pkg/tests/api/alerting/test-data/rulegroup-3-get.json b/pkg/tests/api/alerting/test-data/rulegroup-3-get.json index 2956c506930..5e454967b5d 100644 --- a/pkg/tests/api/alerting/test-data/rulegroup-3-get.json +++ b/pkg/tests/api/alerting/test-data/rulegroup-3-get.json @@ -44,7 +44,8 @@ "is_paused": false, "metadata": { "editor_settings": { - "simplified_query_and_expressions_section": false + "simplified_query_and_expressions_section": false, + "simplified_notifications_section": false } } } @@ -91,7 +92,8 @@ "is_paused": false, "metadata": { "editor_settings": { - "simplified_query_and_expressions_section": false + "simplified_query_and_expressions_section": false, + "simplified_notifications_section": false } } }