diff --git a/pkg/services/ngalert/models/alert_rule.go b/pkg/services/ngalert/models/alert_rule.go index fe9be233d03..bec7137c4b0 100644 --- a/pkg/services/ngalert/models/alert_rule.go +++ b/pkg/services/ngalert/models/alert_rule.go @@ -795,6 +795,13 @@ func PatchPartialAlertRule(existingRule *AlertRule, ruleToPatch *AlertRuleWithOp if !ruleToPatch.HasPause { ruleToPatch.IsPaused = existingRule.IsPaused } + + // Currently metadata contains only editor settings, so we can just copy it. + // If we add more fields to metadata, we might need to handle them separately, + // and/or merge or update their values. + if ruleToPatch.Metadata == (AlertRuleMetadata{}) { + ruleToPatch.Metadata = existingRule.Metadata + } } func ValidateRuleGroupInterval(intervalSeconds, baseIntervalSeconds int64) error { diff --git a/pkg/services/ngalert/models/alert_rule_test.go b/pkg/services/ngalert/models/alert_rule_test.go index 3cfbb07a392..652e0c23d41 100644 --- a/pkg/services/ngalert/models/alert_rule_test.go +++ b/pkg/services/ngalert/models/alert_rule_test.go @@ -253,10 +253,17 @@ func TestPatchPartialAlertRule(t *testing.T) { r.IsPaused = true }, }, + { + name: "No metadata", + mutator: func(r *AlertRuleWithOptionals) { + r.Metadata = AlertRuleMetadata{} + }, + }, } gen := RuleGen.With( - RuleMuts.WithFor(time.Duration(rand.Int63n(1000) + 1)), + RuleMuts.WithFor(time.Duration(rand.Int63n(1000)+1)), + RuleMuts.WithEditorSettingsSimplifiedQueryAndExpressionsSection(true), ) for _, testCase := range testCases { diff --git a/pkg/services/ngalert/provisioning/alert_rules.go b/pkg/services/ngalert/provisioning/alert_rules.go index 47a481b6efa..ee56a6e134b 100644 --- a/pkg/services/ngalert/provisioning/alert_rules.go +++ b/pkg/services/ngalert/provisioning/alert_rules.go @@ -598,6 +598,14 @@ func (service *AlertRuleService) UpdateAlertRule(ctx context.Context, user ident rule.Updated = time.Now() rule.ID = storedRule.ID rule.IntervalSeconds = storedRule.IntervalSeconds + + // Currently metadata contains only editor settings, so we can just copy it. + // If we add more fields to metadata, we might need to handle them separately, + // and/or merge or update their values. + if rule.Metadata == (models.AlertRuleMetadata{}) { + rule.Metadata = storedRule.Metadata + } + err = rule.SetDashboardAndPanelFromAnnotations() if err != nil { return models.AlertRule{}, err diff --git a/pkg/services/ngalert/provisioning/alert_rules_test.go b/pkg/services/ngalert/provisioning/alert_rules_test.go index 5728b976230..ed9bf7e3514 100644 --- a/pkg/services/ngalert/provisioning/alert_rules_test.go +++ b/pkg/services/ngalert/provisioning/alert_rules_test.go @@ -183,6 +183,74 @@ func TestAlertRuleService(t *testing.T) { require.Equal(t, int64(2), readGroup.Rules[0].Version) }) + t.Run("updating a group should not override its rules editor settings", func(t *testing.T) { + namespaceUID := "my-namespace" + groupTitle := "test-group-123" + + // create the rule group via the rule store, to persist the editor settings + rule := createTestRule(util.GenerateShortUID(), groupTitle, orgID, namespaceUID) + ruleMetadata := models.AlertRuleMetadata{ + EditorSettings: models.EditorSettings{ + SimplifiedQueryAndExpressionsSection: true, + }, + } + rule.Metadata = ruleMetadata + r, err := ruleService.ruleStore.InsertAlertRules(context.Background(), []models.AlertRule{rule}) + require.NoError(t, err) + require.Len(t, r, 1) + + // Set the UID for the rule to update it + rule.UID = r[0].UID + // clear the metadata to check that the existing metadata is not overridden + rule.Metadata = models.AlertRuleMetadata{} + + // Now update the rule group with the rule to update its metadata + group := models.AlertRuleGroup{ + Title: groupTitle, + Interval: 60, + FolderUID: namespaceUID, + Rules: []models.AlertRule{rule}, + } + + err = ruleService.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI) + require.NoError(t, err) + + readGroup, err := ruleService.GetRuleGroup(context.Background(), u, namespaceUID, groupTitle) + require.NoError(t, err) + require.NotEmpty(t, readGroup.Rules) + require.Len(t, readGroup.Rules, 1) + + // check that the metadata is still there + require.Equal(t, ruleMetadata, readGroup.Rules[0].Metadata) + }) + + t.Run("updating a rule should not override its editor settings", func(t *testing.T) { + rule := createTestRule(util.GenerateShortUID(), "my-group", orgID, "my-folder") + ruleMetadata := models.AlertRuleMetadata{ + EditorSettings: models.EditorSettings{ + SimplifiedQueryAndExpressionsSection: true, + }, + } + rule.Metadata = ruleMetadata + r, err := ruleService.ruleStore.InsertAlertRules(context.Background(), []models.AlertRule{rule}) + require.NoError(t, err) + require.Len(t, r, 1) + + // Set the UID for the rule to update it + rule.UID = r[0].UID + // clear the metadata to check that the existing metadata is not overridden + rule.Metadata = models.AlertRuleMetadata{} + + // Update the rule + _, err = ruleService.UpdateAlertRule(context.Background(), u, rule, models.ProvenanceAPI) + require.NoError(t, err) + + // Read the rule and check that the editor settings are preserved + readRule, _, err := ruleService.GetAlertRule(context.Background(), u, rule.UID) + require.NoError(t, err) + require.Equal(t, ruleMetadata, readRule.Metadata) + }) + t.Run("updating a group to temporarily overlap rule names should not throw unique constraint", func(t *testing.T) { var orgID int64 = 1 group := models.AlertRuleGroup{