From d10fdc0f02818cff78bbeb9268fe06dce51578dc Mon Sep 17 00:00:00 2001 From: Yuri Tseretyan Date: Thu, 27 Mar 2025 13:31:08 -0400 Subject: [PATCH] Alerting: ListDeletedRules to return rules sorted by date (#102945) --------- Signed-off-by: Yuri Tseretyan --- pkg/services/ngalert/store/alert_rule.go | 2 +- pkg/services/ngalert/store/alert_rule_test.go | 35 +++++++++++++------ 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/pkg/services/ngalert/store/alert_rule.go b/pkg/services/ngalert/store/alert_rule.go index c747e4461e5..433b417c545 100644 --- a/pkg/services/ngalert/store/alert_rule.go +++ b/pkg/services/ngalert/store/alert_rule.go @@ -242,7 +242,7 @@ func (st DBstore) ListDeletedRules(ctx context.Context, orgID int64) ([]*ngmodel alertRules := make([]*ngmodels.AlertRule, 0) err := st.SQLStore.WithDbSession(ctx, func(sess *db.Session) error { // take only the latest versions of each rule by GUID - rows, err := sess.Table(alertRuleVersion{}).Where("rule_org_id = ? AND rule_uid = ''", orgID).Rows(alertRuleVersion{}) + rows, err := sess.Table(alertRuleVersion{}).Where("rule_org_id = ? AND rule_uid = ''", orgID).Desc("created", "id").Rows(alertRuleVersion{}) if err != nil { return err } diff --git a/pkg/services/ngalert/store/alert_rule_test.go b/pkg/services/ngalert/store/alert_rule_test.go index 11614bb6841..efe5772934d 100644 --- a/pkg/services/ngalert/store/alert_rule_test.go +++ b/pkg/services/ngalert/store/alert_rule_test.go @@ -1834,24 +1834,26 @@ func TestIntegration_ListDeletedRules(t *testing.T) { gen := models.RuleGen gen = gen.With(gen.WithIntervalMatching(store.Cfg.BaseInterval), gen.WithOrgID(orgID)) - result, err := store.InsertAlertRules(context.Background(), &models.AlertingUserUID, []models.AlertRule{gen.Generate()}) + result, err := store.InsertAlertRules(context.Background(), &models.AlertingUserUID, []models.AlertRule{gen.Generate(), gen.Generate()}) require.NoError(t, err) - rule, err := store.GetAlertRuleByUID(context.Background(), &models.GetAlertRuleByUIDQuery{UID: result[0].UID}) + rule1, err := store.GetAlertRuleByUID(context.Background(), &models.GetAlertRuleByUIDQuery{UID: result[0].UID}) + require.NoError(t, err) + rule2, err := store.GetAlertRuleByUID(context.Background(), &models.GetAlertRuleByUIDQuery{UID: result[1].UID}) require.NoError(t, err) clk.Add(1 * time.Hour) - rule2 := models.CopyRule(rule, gen.WithTitle(util.GenerateShortUID())) + rule1v2 := models.CopyRule(rule1, gen.WithTitle(util.GenerateShortUID())) err = store.UpdateAlertRules(context.Background(), &models.AlertingUserUID, []models.UpdateRule{ { - Existing: rule, - New: *rule2, + Existing: rule1, + New: *rule1v2, }, }) require.NoError(t, err) - rule2, err = store.GetAlertRuleByUID(context.Background(), &models.GetAlertRuleByUIDQuery{UID: result[0].UID}) + rule1v2, err = store.GetAlertRuleByUID(context.Background(), &models.GetAlertRuleByUIDQuery{UID: result[0].UID}) require.NoError(t, err) - versions, err := store.GetAlertRuleVersions(context.Background(), orgID, rule.GUID) + versions, err := store.GetAlertRuleVersions(context.Background(), orgID, rule1.GUID) require.NoError(t, err) require.Len(t, versions, 2) @@ -1861,16 +1863,29 @@ func TestIntegration_ListDeletedRules(t *testing.T) { require.Empty(t, list) }) + // delete the second rule clk.Add(1 * time.Hour) - err = store.DeleteAlertRulesByUID(context.Background(), orgID, util.Pointer(models.UserUID("test")), false, rule.UID) + err = store.DeleteAlertRulesByUID(context.Background(), orgID, util.Pointer(models.UserUID("test")), false, rule2.UID) require.NoError(t, err) + // and the first rule hour later + clk.Add(1 * time.Hour) + err = store.DeleteAlertRulesByUID(context.Background(), orgID, util.Pointer(models.UserUID("test")), false, rule1.UID) + require.NoError(t, err) + + t.Run("should return deleted rules sorted by date desc", func(t *testing.T) { + list, err := store.ListDeletedRules(context.Background(), orgID) + require.NoError(t, err) + require.Len(t, list, 2) + require.Equal(t, rule1.GUID, list[0].GUID) + require.Equal(t, rule2.GUID, list[1].GUID) + }) + t.Run("should return the last deleted rule", func(t *testing.T) { list, err := store.ListDeletedRules(context.Background(), orgID) require.NoError(t, err) - require.Len(t, list, 1) assert.Empty(t, list[0].UID) - assert.Empty(t, rule2.Diff(list[0], "ID", "UID", "DashboardUID", "PanelID", "Updated", "UpdatedBy")) // ignore updated because it's not + assert.Empty(t, rule1v2.Diff(list[0], "ID", "UID", "DashboardUID", "PanelID", "Updated", "UpdatedBy")) // ignore updated because it's not assert.Equal(t, list[0].Updated.UTC(), clk.Now().UTC()) assert.EqualValues(t, list[0].UpdatedBy, util.Pointer(models.UserUID("test"))) })