feat(alerting): move alert deletion to alert code

pkg/services/sqlstore/alert_rule_changes_test.go

@@ -0,0 +1,75 @@
+package sqlstore
+
+import (
+	"testing"
+
+	m "github.com/grafana/grafana/pkg/models"
+	. "github.com/smartystreets/goconvey/convey"
+)
+
+var (
+	FakeOrgId int64 = 2
+)
+
+func TestAlertRuleChangesDataAccess(t *testing.T) {
+
+	Convey("Testing Alert rule changes data access", t, func() {
+		InitTestDB(t)
+
+		testDash := insertTestDashboard("dashboard with alerts", 2, "alert")
+		var err error
+
+		Convey("When dashboard is removed", func() {
+			items := []m.AlertRule{
+				{
+					PanelId:     1,
+					DashboardId: testDash.Id,
+					Query:       "Query",
+					QueryRefId:  "A",
+					WarnLevel:   "> 30",
+					CritLevel:   "> 50",
+					Interval:    "10",
+					Title:       "Alerting title",
+					Description: "Alerting description",
+					QueryRange:  "5m",
+					Aggregator:  "avg",
+					OrgId:       FakeOrgId,
+				},
+			}
+
+			cmd := m.SaveAlertsCommand{
+				Alerts:      &items,
+				DashboardId: testDash.Id,
+				OrgId:       FakeOrgId,
+				UserId:      2,
+			}
+
+			SaveAlerts(&cmd)
+
+			alertChanges, er := GetAlertRuleChanges(FakeOrgId)
+			So(er, ShouldBeNil)
+			So(len(alertChanges), ShouldEqual, 1)
+
+			err = DeleteDashboard(&m.DeleteDashboardCommand{
+				OrgId: FakeOrgId,
+				Slug:  testDash.Slug,
+			})
+
+			So(err, ShouldBeNil)
+
+			Convey("Alerts should be removed", func() {
+				alerts, err2 := GetAlertsByDashboardId(testDash.Id)
+
+				So(testDash.Id, ShouldEqual, 1)
+				So(err2, ShouldBeNil)
+				So(len(alerts), ShouldEqual, 0)
+			})
+
+			Convey("should add one more alert_rule_change", func() {
+				alertChanges, er := GetAlertRuleChanges(FakeOrgId)
+				So(er, ShouldBeNil)
+				So(len(alertChanges), ShouldEqual, 2)
+			})
+		})
+	})
+}

pkg/services/sqlstore/alerting.go

@@ -12,21 +12,37 @@ func init() {
 	bus.AddHandler("sql", SaveAlerts)
 }
 
-func SaveAlertChange(change string, alert m.AlertRule) error {
-	return inTransaction(func(sess *xorm.Session) error {
-		_, err := sess.Insert(&m.AlertRuleChange{
-			OrgId:   alert.OrgId,
-			Type:    change,
-			Created: time.Now(),
-			AlertId: alert.Id,
-		})
+func DeleteAlertDefinition(dashboardId int64, sess *xorm.Session) error {
+	alerts := make([]m.AlertRule, 0)
+	sess.Where("dashboard_id = ?", dashboardId).Find(&alerts)
 
+	for _, alert := range alerts {
+		_, err := sess.Exec("DELETE FROM alert_rule WHERE id = ? ", alert.Id)
 		if err != nil {
 			return err
 		}
 
-		return nil
+		if err := SaveAlertChange("DELETED", alert, sess); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+func SaveAlertChange(change string, alert m.AlertRule, sess *xorm.Session) error {
+	_, err := sess.Insert(&m.AlertRuleChange{
+		OrgId:   alert.OrgId,
+		Type:    change,
+		Created: time.Now(),
+		AlertId: alert.Id,
 	})
+
+	if err != nil {
+		return err
+	}
+
+	return nil
 }
 
 func alertIsDifferent(rule1, rule2 m.AlertRule) bool {
@@ -47,69 +63,81 @@ func alertIsDifferent(rule1, rule2 m.AlertRule) bool {
 
 func SaveAlerts(cmd *m.SaveAlertsCommand) error {
 	//this function should be refactored
+	return inTransaction(func(sess *xorm.Session) error {
+		fmt.Printf("Saving alerts for dashboard %v\n", cmd.DashboardId)
 
-	fmt.Printf("Saving alerts for dashboard %v\n", cmd.DashboardId)
+		alerts, err := GetAlertsByDashboardId2(cmd.DashboardId, sess)
+		if err != nil {
+			return err
+		}
 
-	alerts, err := GetAlertsByDashboardId(cmd.DashboardId)
-	if err != nil {
-		return err
-	}
+		for _, alert := range *cmd.Alerts {
+			update := false
+			var alertToUpdate m.AlertRule
 
-	for _, alert := range *cmd.Alerts {
-		update := false
-		var alertToUpdate m.AlertRule
+			for _, k := range alerts {
+				if alert.PanelId == k.PanelId {
+					update = true
+					alert.Id = k.Id
+					alertToUpdate = k
+				}
+			}
 
-		for _, k := range alerts {
-			if alert.PanelId == k.PanelId {
-				update = true
-				alert.Id = k.Id
-				alertToUpdate = k
+			if update {
+
+				if alertIsDifferent(alertToUpdate, alert) {
+					_, err = sess.Id(alert.Id).Update(&alert)
+					if err != nil {
+						return err
+					}
+
+					SaveAlertChange("UPDATED", alert, sess)
+				}
+
+			} else {
+				_, err = sess.Insert(&alert)
+				if err != nil {
+					return err
+				}
+				SaveAlertChange("CREATED", alert, sess)
 			}
 		}
 
-		if update {
+		for _, missingAlert := range alerts {
+			missing := true
 
-			if alertIsDifferent(alertToUpdate, alert) {
-				_, err = x.Id(alert.Id).Update(&alert)
+			for _, k := range *cmd.Alerts {
+				if missingAlert.PanelId == k.PanelId {
+					missing = false
+				}
+			}
+
+			if missing {
+				_, err = sess.Exec("DELETE FROM alert_rule WHERE id = ?", missingAlert.Id)
 				if err != nil {
 					return err
 				}
 
-				SaveAlertChange("UPDATED", alert)
+				err = SaveAlertChange("DELETED", missingAlert, sess)
+				if err != nil {
+					return err
+				}
 			}
-
-		} else {
-			_, err = x.Insert(&alert)
-			if err != nil {
-				return err
-			}
-			SaveAlertChange("CREATED", alert)
 		}
+
+		return nil
+	})
+}
+
+func GetAlertsByDashboardId2(dashboardId int64, sess *xorm.Session) ([]m.AlertRule, error) {
+	alerts := make([]m.AlertRule, 0)
+	err := sess.Where("dashboard_id = ?", dashboardId).Find(&alerts)
+
+	if err != nil {
+		return []m.AlertRule{}, err
 	}
 
-	for _, missingAlert := range alerts {
-		missing := true
-
-		for _, k := range *cmd.Alerts {
-			if missingAlert.PanelId == k.PanelId {
-				missing = false
-			}
-		}
-
-		if missing {
-			_, err = x.Exec("DELETE FROM alert_rule WHERE id = ?", missingAlert.Id)
-			if err != nil {
-				return err
-			}
-
-			err = SaveAlertChange("DELETED", missingAlert)
-			if err != nil {
-				return err
-			}
-		}
-	}
-
-	return nil
+	return alerts, nil
 }
 
 func GetAlertsByDashboardId(dashboardId int64) ([]m.AlertRule, error) {

pkg/services/sqlstore/dashboard.go

@@ -227,7 +227,6 @@ func DeleteDashboard(cmd *m.DeleteDashboardCommand) error {
 			"DELETE FROM dashboard_tag WHERE dashboard_id = ? ",
 			"DELETE FROM star WHERE dashboard_id = ? ",
 			"DELETE FROM dashboard WHERE id = ?",
-			"DELETE FROM alert_rule WHERE dashboard_id = ?",
 		}
 
 		for _, sql := range deletes {
@@ -237,6 +236,10 @@ func DeleteDashboard(cmd *m.DeleteDashboardCommand) error {
 			}
 		}
 
+		if err := DeleteAlertDefinition(dashboard.Id, sess.Session); err != nil {
+			return nil
+		}
+
 		return nil
 	})
 }