From d29563ff8e126325be103ee49df680d1926e6847 Mon Sep 17 00:00:00 2001 From: bergquist Date: Tue, 27 Mar 2018 09:19:14 +0200 Subject: [PATCH] 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 | 6 ++++- pkg/services/alerting/notifiers/base_test.go | 24 +++++++++++++++++++ .../alerting/notification_edit_ctrl.ts | 3 ++- 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/pkg/services/alerting/notifiers/base.go b/pkg/services/alerting/notifiers/base.go index 7a3cc71c4db..51676efdfd5 100644 --- a/pkg/services/alerting/notifiers/base.go +++ b/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 { - uploadImage := model.Get("uploadImage").MustBool(false) + uploadImage := true + value, exist := model.CheckGet("uploadImage") + if exist { + uploadImage = value.MustBool() + } return NotifierBase{ Id: id, diff --git a/pkg/services/alerting/notifiers/base_test.go b/pkg/services/alerting/notifiers/base_test.go index 4225e203a3d..b7142d144cc 100644 --- a/pkg/services/alerting/notifiers/base_test.go +++ b/pkg/services/alerting/notifiers/base_test.go @@ -4,6 +4,7 @@ import ( "context" "testing" + "github.com/grafana/grafana/pkg/components/simplejson" m "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/alerting" . "github.com/smartystreets/goconvey/convey" @@ -11,6 +12,29 @@ import ( func TestBaseNotifier(t *testing.T) { 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("pending -> ok", func() { context := alerting.NewEvalContext(context.TODO(), &alerting.Rule{ diff --git a/public/app/features/alerting/notification_edit_ctrl.ts b/public/app/features/alerting/notification_edit_ctrl.ts index bca6f6e8137..18b1c4d1d55 100644 --- a/public/app/features/alerting/notification_edit_ctrl.ts +++ b/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 => { this.navModel.breadcrumbs.push({ text: result.name }); this.navModel.node = { text: result.name }; + result.settings = _.defaults(result.settings, this.defaults.settings); return result; }); }) @@ -89,7 +90,7 @@ export class AlertNotificationEditCtrl { } typeChanged() { - this.model.settings = {}; + this.model.settings = _.defaults({}, this.defaults.settings); this.notifierTemplateId = this.getNotifierTemplateId(this.model.type); }