feat(alerting): make some settings properties required

pkg/services/alerting/notifier.go

@@ -1,6 +1,8 @@
 package alerting
 
 import (
+	"fmt"
+
 	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/components/simplejson"
 	"github.com/grafana/grafana/pkg/log"
@@ -108,27 +110,44 @@ func (n *NotifierImpl) getNotifiers(orgId int64, notificationGroups []int64) []*
 }
 
 func NewNotificationFromDBModel(model *m.AlertNotification) (*Notification, error) {
+	notifier, err := createNotifier(model.Type, model.Settings)
+
+	if err != nil {
+		return nil, err
+	}
+
 	return &Notification{
 		Name:         model.Name,
 		Type:         model.Type,
-		Notifierr:    createNotifier(model.Type, model.Settings),
+		Notifierr:    notifier,
 		SendCritical: !model.Settings.Get("ignoreCrit").MustBool(),
 		SendWarning:  !model.Settings.Get("ignoreWarn").MustBool(),
 	}, nil
 }
 
-var createNotifier = func(notificationType string, settings *simplejson.Json) NotificationDispatcher {
+var createNotifier = func(notificationType string, settings *simplejson.Json) (NotificationDispatcher, error) {
 	if notificationType == "email" {
-		return &EmailNotifier{
-			To:  settings.Get("to").MustString(),
-			log: log.New("alerting.notification.email"),
+		to := settings.Get("to").MustString()
+
+		if to == "" {
+			return nil, fmt.Errorf("Could not find to propertie in settings")
 		}
+
+		return &EmailNotifier{
+			To:  to,
+			log: log.New("alerting.notification.email"),
+		}, nil
+	}
+
+	url := settings.Get("url").MustString()
+	if url == "" {
+		return nil, fmt.Errorf("Could not find url propertie in settings")
 	}
 
 	return &WebhookNotifier{
-		Url:      settings.Get("url").MustString(),
+		Url:      url,
 		User:     settings.Get("user").MustString(),
 		Password: settings.Get("password").MustString(),
 		log:      log.New("alerting.notification.webhook"),
-	}
+	}, nil
 }

pkg/services/alerting/notifier_test.go

@@ -13,54 +13,87 @@ import (
 func TestAlertNotificationExtraction(t *testing.T) {
 
 	Convey("Parsing alert notification from settings", t, func() {
-		Convey("Parsing email notification from settings", func() {
-			json := `
-            {
-                "to": "ops@grafana.org"
-            }`
+		Convey("Parsing email", func() {
+			Convey("empty settings should return error", func() {
+				json := `{ }`
 
-			settingsJSON, _ := simplejson.NewJson([]byte(json))
-			model := &m.AlertNotification{
-				Name:     "ops",
-				Type:     "email",
-				Settings: settingsJSON,
-			}
+				settingsJSON, _ := simplejson.NewJson([]byte(json))
+				model := &m.AlertNotification{
+					Name:     "ops",
+					Type:     "email",
+					Settings: settingsJSON,
+				}
 
-			not, err := NewNotificationFromDBModel(model)
+				_, err := NewNotificationFromDBModel(model)
+				So(err, ShouldNotBeNil)
+			})
 
-			So(err, ShouldBeNil)
-			So(not.Name, ShouldEqual, "ops")
-			So(not.Type, ShouldEqual, "email")
-			So(reflect.TypeOf(not.Notifierr).Elem().String(), ShouldEqual, "alerting.EmailNotifier")
+			Convey("from settings", func() {
+				json := `
+				{
+					"to": "ops@grafana.org"
+				}`
 
-			email := not.Notifierr.(*EmailNotifier)
-			So(email.To, ShouldEqual, "ops@grafana.org")
+				settingsJSON, _ := simplejson.NewJson([]byte(json))
+				model := &m.AlertNotification{
+					Name:     "ops",
+					Type:     "email",
+					Settings: settingsJSON,
+				}
+
+				not, err := NewNotificationFromDBModel(model)
+
+				So(err, ShouldBeNil)
+				So(not.Name, ShouldEqual, "ops")
+				So(not.Type, ShouldEqual, "email")
+				So(reflect.TypeOf(not.Notifierr).Elem().String(), ShouldEqual, "alerting.EmailNotifier")
+
+				email := not.Notifierr.(*EmailNotifier)
+				So(email.To, ShouldEqual, "ops@grafana.org")
+			})
 		})
 
-		Convey("Parsing webhook notification from settings", func() {
-			json := `
-            {
-                "url": "http://localhost:3000",
-                "username": "username",
-                "password": "password"
-            }`
+		Convey("Parsing webhook", func() {
+			Convey("empty settings should return error", func() {
+				json := `{ }`
 
-			settingsJSON, _ := simplejson.NewJson([]byte(json))
-			model := &m.AlertNotification{
-				Name:     "slack",
-				Type:     "webhook",
-				Settings: settingsJSON,
-			}
+				settingsJSON, _ := simplejson.NewJson([]byte(json))
+				model := &m.AlertNotification{
+					Name:     "ops",
+					Type:     "webhook",
+					Settings: settingsJSON,
+				}
 
-			not, err := NewNotificationFromDBModel(model)
+				_, err := NewNotificationFromDBModel(model)
+				So(err, ShouldNotBeNil)
+			})
 
-			So(err, ShouldBeNil)
-			So(not.Name, ShouldEqual, "slack")
-			So(not.Type, ShouldEqual, "webhook")
-			So(reflect.TypeOf(not.Notifierr).Elem().String(), ShouldEqual, "alerting.WebhookNotifier")
+			Convey("from settings", func() {
+				json := `
+				{
+					"url": "http://localhost:3000",
+					"username": "username",
+					"password": "password"
+				}`
 
-			webhook := not.Notifierr.(*WebhookNotifier)
-			So(webhook.Url, ShouldEqual, "http://localhost:3000")
+				settingsJSON, _ := simplejson.NewJson([]byte(json))
+				model := &m.AlertNotification{
+					Name:     "slack",
+					Type:     "webhook",
+					Settings: settingsJSON,
+				}
+
+				not, err := NewNotificationFromDBModel(model)
+
+				So(err, ShouldBeNil)
+				So(not.Name, ShouldEqual, "slack")
+				So(not.Type, ShouldEqual, "webhook")
+				So(reflect.TypeOf(not.Notifierr).Elem().String(), ShouldEqual, "alerting.WebhookNotifier")
+
+				webhook := not.Notifierr.(*WebhookNotifier)
+				So(webhook.Url, ShouldEqual, "http://localhost:3000")
+			})
 		})
+
 	})
 }