alerting: bad default state for notifiers

when we made image upload optional we didnt
show the default value properly in the UI. Which
caused confusing. This commit apply the default
values to existing notifiers in the edit pages
and reverts back to using uploadimage=true as the
default value.

pkg/services/alerting/notifiers/base.go

@@ -15,7 +15,11 @@ type NotifierBase struct {
 }
 }
 
 
 func NewNotifierBase(id int64, isDefault bool, name, notifierType string, model *simplejson.Json) NotifierBase {
 func NewNotifierBase(id int64, isDefault bool, name, notifierType string, model *simplejson.Json) NotifierBase {
-	uploadImage := model.Get("uploadImage").MustBool(false)
+	uploadImage := true
+	value, exist := model.CheckGet("uploadImage")
+	if exist {
+		uploadImage = value.MustBool()
+	}
 
 
 	return NotifierBase{
 	return NotifierBase{
 		Id:          id,
 		Id:          id,

pkg/services/alerting/notifiers/base_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"context"
 	"testing"
 	"testing"
 
 
+	"github.com/grafana/grafana/pkg/components/simplejson"
 	m "github.com/grafana/grafana/pkg/models"
 	m "github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/alerting"
 	"github.com/grafana/grafana/pkg/services/alerting"
 	. "github.com/smartystreets/goconvey/convey"
 	. "github.com/smartystreets/goconvey/convey"
@@ -11,6 +12,29 @@ import (
 
 
 func TestBaseNotifier(t *testing.T) {
 func TestBaseNotifier(t *testing.T) {
 	Convey("Base notifier tests", t, func() {
 	Convey("Base notifier tests", t, func() {
+		Convey("default constructor for notifiers", func() {
+			bJson := simplejson.New()
+
+			Convey("can parse false value", func() {
+				bJson.Set("uploadImage", false)
+
+				base := NewNotifierBase(1, false, "name", "email", bJson)
+				So(base.UploadImage, ShouldBeFalse)
+			})
+
+			Convey("can parse true value", func() {
+				bJson.Set("uploadImage", true)
+
+				base := NewNotifierBase(1, false, "name", "email", bJson)
+				So(base.UploadImage, ShouldBeTrue)
+			})
+
+			Convey("default value should be true for backwards compatibility", func() {
+				base := NewNotifierBase(1, false, "name", "email", bJson)
+				So(base.UploadImage, ShouldBeTrue)
+			})
+		})
+
 		Convey("should notify", func() {
 		Convey("should notify", func() {
 			Convey("pending -> ok", func() {
 			Convey("pending -> ok", func() {
 				context := alerting.NewEvalContext(context.TODO(), &alerting.Rule{
 				context := alerting.NewEvalContext(context.TODO(), &alerting.Rule{

public/app/features/alerting/notification_edit_ctrl.ts

@@ -43,6 +43,7 @@ export class AlertNotificationEditCtrl {
         return this.backendSrv.get(`/api/alert-notifications/${this.$routeParams.id}`).then(result => {
         return this.backendSrv.get(`/api/alert-notifications/${this.$routeParams.id}`).then(result => {
           this.navModel.breadcrumbs.push({ text: result.name });
           this.navModel.breadcrumbs.push({ text: result.name });
           this.navModel.node = { text: result.name };
           this.navModel.node = { text: result.name };
+          result.settings = _.defaults(result.settings, this.defaults.settings);
           return result;
           return result;
         });
         });
       })
       })
@@ -89,7 +90,7 @@ export class AlertNotificationEditCtrl {
   }
   }
 
 
   typeChanged() {
   typeChanged() {
-    this.model.settings = {};
+    this.model.settings = _.defaults({}, this.defaults.settings);
     this.notifierTemplateId = this.getNotifierTemplateId(this.model.type);
     this.notifierTemplateId = this.getNotifierTemplateId(this.model.type);
   }
   }