Returned id for alert notifications which were created without uid

pkg/services/alerting/extractor_test.go

@@ -8,6 +8,7 @@ import (
 	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/components/simplejson"
 	m "github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/sqlstore"
 	. "github.com/smartystreets/goconvey/convey"
 )
 
@@ -197,74 +198,86 @@ func TestAlertRuleExtraction(t *testing.T) {
 			})
 		})
 
-		Convey("Parse and validate dashboard containing influxdb alert", func() {
-			json, err := ioutil.ReadFile("./testdata/influxdb-alert.json")
+		Convey("Alert notifications are in DB", func() {
+			sqlstore.InitTestDB(t)
+			err := sqlstore.CreateOrg(&m.CreateOrgCommand{Name: "Main Org."})
 			So(err, ShouldBeNil)
-
-			dashJson, err := simplejson.NewJson(json)
+			firstNotification := m.CreateAlertNotificationCommand{Uid: "notifier1", OrgId: 1, Name: "1"}
+			err = sqlstore.CreateAlertNotificationCommand(&firstNotification)
+			So(err, ShouldBeNil)
+			secondNotification := m.CreateAlertNotificationCommand{Uid: "notifier2", OrgId: 1, Name: "2"}
+			err = sqlstore.CreateAlertNotificationCommand(&secondNotification)
 			So(err, ShouldBeNil)
-			dash := m.NewDashboardFromJson(dashJson)
-			extractor := NewDashAlertExtractor(dash, 1, nil)
 
-			alerts, err := extractor.GetAlerts()
+			Convey("Parse and validate dashboard containing influxdb alert", func() {
+				json, err := ioutil.ReadFile("./testdata/influxdb-alert.json")
+				So(err, ShouldBeNil)
 
-			Convey("Get rules without error", func() {
+				dashJson, err := simplejson.NewJson(json)
 				So(err, ShouldBeNil)
-			})
+				dash := m.NewDashboardFromJson(dashJson)
+				extractor := NewDashAlertExtractor(dash, 1, nil)
 
-			Convey("should be able to read interval", func() {
-				So(len(alerts), ShouldEqual, 1)
+				alerts, err := extractor.GetAlerts()
 
-				for _, alert := range alerts {
-					So(alert.DashboardId, ShouldEqual, 4)
+				Convey("Get rules without error", func() {
+					So(err, ShouldBeNil)
+				})
 
-					conditions := alert.Settings.Get("conditions").MustArray()
-					cond := simplejson.NewFromAny(conditions[0])
+				Convey("should be able to read interval", func() {
+					So(len(alerts), ShouldEqual, 1)
 
-					So(cond.Get("query").Get("model").Get("interval").MustString(), ShouldEqual, ">10s")
-				}
+					for _, alert := range alerts {
+						So(alert.DashboardId, ShouldEqual, 4)
+
+						conditions := alert.Settings.Get("conditions").MustArray()
+						cond := simplejson.NewFromAny(conditions[0])
+
+						So(cond.Get("query").Get("model").Get("interval").MustString(), ShouldEqual, ">10s")
+					}
+				})
 			})
-		})
 
-		Convey("Should be able to extract collapsed panels", func() {
-			json, err := ioutil.ReadFile("./testdata/collapsed-panels.json")
-			So(err, ShouldBeNil)
+			Convey("Should be able to extract collapsed panels", func() {
+				json, err := ioutil.ReadFile("./testdata/collapsed-panels.json")
+				So(err, ShouldBeNil)
 
-			dashJson, err := simplejson.NewJson(json)
-			So(err, ShouldBeNil)
+				dashJson, err := simplejson.NewJson(json)
+				So(err, ShouldBeNil)
 
-			dash := m.NewDashboardFromJson(dashJson)
-			extractor := NewDashAlertExtractor(dash, 1, nil)
+				dash := m.NewDashboardFromJson(dashJson)
+				extractor := NewDashAlertExtractor(dash, 1, nil)
 
-			alerts, err := extractor.GetAlerts()
+				alerts, err := extractor.GetAlerts()
 
-			Convey("Get rules without error", func() {
-				So(err, ShouldBeNil)
-			})
+				Convey("Get rules without error", func() {
+					So(err, ShouldBeNil)
+				})
 
-			Convey("should be able to extract collapsed alerts", func() {
-				So(len(alerts), ShouldEqual, 4)
+				Convey("should be able to extract collapsed alerts", func() {
+					So(len(alerts), ShouldEqual, 4)
+				})
 			})
-		})
 
-		Convey("Parse and validate dashboard without id and containing an alert", func() {
-			json, err := ioutil.ReadFile("./testdata/dash-without-id.json")
-			So(err, ShouldBeNil)
+			Convey("Parse and validate dashboard without id and containing an alert", func() {
+				json, err := ioutil.ReadFile("./testdata/dash-without-id.json")
+				So(err, ShouldBeNil)
 
-			dashJSON, err := simplejson.NewJson(json)
-			So(err, ShouldBeNil)
-			dash := m.NewDashboardFromJson(dashJSON)
-			extractor := NewDashAlertExtractor(dash, 1, nil)
+				dashJSON, err := simplejson.NewJson(json)
+				So(err, ShouldBeNil)
+				dash := m.NewDashboardFromJson(dashJSON)
+				extractor := NewDashAlertExtractor(dash, 1, nil)
 
-			err = extractor.ValidateAlerts()
+				err = extractor.ValidateAlerts()
 
-			Convey("Should validate without error", func() {
-				So(err, ShouldBeNil)
-			})
+				Convey("Should validate without error", func() {
+					So(err, ShouldBeNil)
+				})
 
-			Convey("Should fail on save", func() {
-				_, err := extractor.GetAlerts()
-				So(err.Error(), ShouldEqual, "Alert validation error: Panel id is not correct, alertName=Influxdb, panelId=1")
+				Convey("Should fail on save", func() {
+					_, err := extractor.GetAlerts()
+					So(err.Error(), ShouldEqual, "Alert validation error: Panel id is not correct, alertName=Influxdb, panelId=1")
+				})
 			})
 		})
 	})

pkg/services/alerting/rule.go

@@ -9,6 +9,8 @@ import (
 
 	"github.com/grafana/grafana/pkg/components/simplejson"
 	m "github.com/grafana/grafana/pkg/models"
+
+	"github.com/grafana/grafana/pkg/bus"
 )
 
 var (
@@ -126,12 +128,33 @@ func NewRuleFromDBAlert(ruleDef *m.Alert) (*Rule, error) {
 
 	for _, v := range ruleDef.Settings.Get("notifications").MustArray() {
 		jsonModel := simplejson.NewFromAny(v)
-		uid, err := jsonModel.Get("uid").String()
-		if err != nil {
-			return nil, ValidationError{Reason: "Invalid notification schema", DashboardId: model.DashboardId, Alertid: model.Id, PanelId: model.PanelId}
+		if id, err := jsonModel.Get("id").Int64(); err == nil {
+			cmd := m.GetAlertNotificationsQuery{
+				Id:    id,
+				OrgId: ruleDef.OrgId,
+			}
+
+			if err = bus.Dispatch(&cmd); err != nil {
+				return nil, err
+			}
+
+			if cmd.Result == nil {
+				errString := fmt.Sprintf("Alert notification id %d doesn't exist", id)
+				return nil, ValidationError{Reason: errString, DashboardId: model.DashboardId, Alertid: model.Id, PanelId: model.PanelId}
+			}
+
+			if cmd.Result.Uid == "" {
+				errString := fmt.Sprintf("Alert notification id %d has empty uid", id)
+				return nil, ValidationError{Reason: errString, DashboardId: model.DashboardId, Alertid: model.Id, PanelId: model.PanelId}
+			}
+			model.Notifications = append(model.Notifications, cmd.Result.Uid)
+		} else {
+			if uid, err := jsonModel.Get("uid").String(); err != nil {
+				return nil, ValidationError{Reason: "Neither id nor uid is specified", DashboardId: model.DashboardId, Alertid: model.Id, PanelId: model.PanelId}
+			} else {
+				model.Notifications = append(model.Notifications, uid)
+			}
 		}
-		model.Notifications = append(model.Notifications, uid)
-
 	}
 
 	for index, condition := range ruleDef.Settings.Get("conditions").MustArray() {

pkg/services/alerting/rule_test.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/grafana/grafana/pkg/components/simplejson"
 	m "github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/sqlstore"
 	. "github.com/smartystreets/goconvey/convey"
 )
 
@@ -45,6 +46,7 @@ func TestAlertRuleFrequencyParsing(t *testing.T) {
 }
 
 func TestAlertRuleModel(t *testing.T) {
+	sqlstore.InitTestDB(t)
 	Convey("Testing alert rule", t, func() {
 
 		RegisterCondition("test", func(model *simplejson.Json, index int) (Condition, error) {
@@ -57,26 +59,73 @@ func TestAlertRuleModel(t *testing.T) {
 		})
 
 		Convey("can construct alert rule model", func() {
+			err := sqlstore.CreateOrg(&m.CreateOrgCommand{Name: "Main Org."})
+			So(err, ShouldBeNil)
+			firstNotification := m.CreateAlertNotificationCommand{Uid: "notifier1", OrgId: 1, Name: "1"}
+			err = sqlstore.CreateAlertNotificationCommand(&firstNotification)
+			So(err, ShouldBeNil)
+			secondNotification := m.CreateAlertNotificationCommand{Uid: "notifier2", OrgId: 1, Name: "2"}
+			err = sqlstore.CreateAlertNotificationCommand(&secondNotification)
+			So(err, ShouldBeNil)
+
+			Convey("with notification id and uid", func() {
+				json := `
+				{
+					"name": "name2",
+					"description": "desc2",
+					"handler": 0,
+					"noDataMode": "critical",
+					"enabled": true,
+					"frequency": "60s",
+			"conditions": [
+			{
+				"type": "test",
+				"prop": 123
+						}
+			],
+			"notifications": [
+						{"id": 1},
+						{"uid": "notifier2"}
+					]
+				}
+				`
+
+				alertJSON, jsonErr := simplejson.NewJson([]byte(json))
+				So(jsonErr, ShouldBeNil)
+
+				alert := &m.Alert{
+					Id:          1,
+					OrgId:       1,
+					DashboardId: 1,
+					PanelId:     1,
+
+					Settings: alertJSON,
+				}
+
+				alertRule, err := NewRuleFromDBAlert(alert)
+				So(err, ShouldBeNil)
+
+				So(len(alertRule.Conditions), ShouldEqual, 1)
+
+				Convey("Can read notifications", func() {
+					So(len(alertRule.Notifications), ShouldEqual, 2)
+					So(alertRule.Notifications, ShouldContain, "notifier1")
+					So(alertRule.Notifications, ShouldContain, "notifier2")
+				})
+			})
+		})
+
+		Convey("can construct alert rule model with invalid frequency", func() {
 			json := `
 			{
 				"name": "name2",
 				"description": "desc2",
-				"handler": 0,
 				"noDataMode": "critical",
 				"enabled": true,
-				"frequency": "60s",
-        "conditions": [
-          {
-            "type": "test",
-            "prop": 123
-					}
-        ],
-        "notifications": [
-					{"uid": "1134"},
-					{"uid": "22"}
-				]
-			}
-			`
+				"frequency": "0s",
+        		"conditions": [ { "type": "test", "prop": 123 } ],
+        		"notifications": []
+			}`
 
 			alertJSON, jsonErr := simplejson.NewJson([]byte(json))
 			So(jsonErr, ShouldBeNil)
@@ -86,31 +135,35 @@ func TestAlertRuleModel(t *testing.T) {
 				OrgId:       1,
 				DashboardId: 1,
 				PanelId:     1,
+				Frequency:   0,
 
 				Settings: alertJSON,
 			}
 
 			alertRule, err := NewRuleFromDBAlert(alert)
 			So(err, ShouldBeNil)
-
-			So(len(alertRule.Conditions), ShouldEqual, 1)
-
-			Convey("Can read notifications", func() {
-				So(len(alertRule.Notifications), ShouldEqual, 2)
-			})
+			So(alertRule.Frequency, ShouldEqual, 60)
 		})
 
-		Convey("can construct alert rule model with invalid frequency", func() {
+		Convey("raise error in case of missing notification id and uid", func() {
 			json := `
 			{
 				"name": "name2",
 				"description": "desc2",
 				"noDataMode": "critical",
 				"enabled": true,
-				"frequency": "0s",
-        		"conditions": [ { "type": "test", "prop": 123 } ],
-        		"notifications": []
-			}`
+				"frequency": "60s",
+        "conditions": [
+          {
+            "type": "test",
+            "prop": 123
+					}
+        ],
+        "notifications": [
+					{"not_id_uid": "1134"}
+				]
+			}
+			`
 
 			alertJSON, jsonErr := simplejson.NewJson([]byte(json))
 			So(jsonErr, ShouldBeNil)
@@ -125,9 +178,9 @@ func TestAlertRuleModel(t *testing.T) {
 				Settings: alertJSON,
 			}
 
-			alertRule, err := NewRuleFromDBAlert(alert)
-			So(err, ShouldBeNil)
-			So(alertRule.Frequency, ShouldEqual, 60)
+			_, err := NewRuleFromDBAlert(alert)
+			So(err, ShouldNotBeNil)
+			So(err.Error(), ShouldEqual, "Alert validation error: Neither id nor uid is specified AlertId: 1 PanelId: 1 DashboardId: 1")
 		})
 
 	})

pkg/services/alerting/testdata/dash-without-id.json

@@ -45,6 +45,9 @@
               "notifications": [
                 {
                   "uid": "notifier1"
+                },
+                {
+                  "id": 2
                 }
               ]
             },

pkg/services/alerting/testdata/influxdb-alert.json

@@ -45,7 +45,10 @@
               "noDataState": "no_data",
               "notifications": [
                 {
-                  "uid": "notifier1"
+                  "id": 1
+                },
+                {
+                  "uid": "notifier2"
                 }
               ]
             },

pkg/services/sqlstore/alert_notification.go

@@ -130,6 +130,7 @@ func getAlertNotificationInternal(query *m.GetAlertNotificationsQuery, sess *DBS
 
 	sql.WriteString(`SELECT
 										alert_notification.id,
+										alert_notification.uid,
 										alert_notification.org_id,
 										alert_notification.name,
 										alert_notification.type,