From c4dcf5a4ee1cde40201a50f7f49c6cefc9b89549 Mon Sep 17 00:00:00 2001 From: Dave Waters Date: Tue, 16 Oct 2018 17:33:38 -0400 Subject: [PATCH] fix gofmt, add test, correct noted concerns with default value --- pkg/api/dtos/alerting.go | 52 +++++++++---------- pkg/models/alert_notifications.go | 50 +++++++++--------- pkg/services/alerting/notifiers/base.go | 33 ++++++------ pkg/services/alerting/notifiers/base_test.go | 5 ++ pkg/services/sqlstore/alert_notification.go | 18 +++---- .../sqlstore/alert_notification_test.go | 22 ++++---- pkg/services/sqlstore/migrations/alert_mig.go | 2 +- 7 files changed, 93 insertions(+), 89 deletions(-) diff --git a/pkg/api/dtos/alerting.go b/pkg/api/dtos/alerting.go index 2f762910d63..33f10c805ab 100644 --- a/pkg/api/dtos/alerting.go +++ b/pkg/api/dtos/alerting.go @@ -49,30 +49,30 @@ func formatShort(interval time.Duration) string { func NewAlertNotification(notification *models.AlertNotification) *AlertNotification { return &AlertNotification{ - Id: notification.Id, - Name: notification.Name, - Type: notification.Type, - IsDefault: notification.IsDefault, - Created: notification.Created, - Updated: notification.Updated, - Frequency: formatShort(notification.Frequency), - SendReminder: notification.SendReminder, - DisableResolvedMessage: notification.DisableResolvedMessage, - Settings: notification.Settings, + Id: notification.Id, + Name: notification.Name, + Type: notification.Type, + IsDefault: notification.IsDefault, + Created: notification.Created, + Updated: notification.Updated, + Frequency: formatShort(notification.Frequency), + SendReminder: notification.SendReminder, + DisableResolvedMessage: notification.DisableResolvedMessage, + Settings: notification.Settings, } } type AlertNotification struct { - Id int64 `json:"id"` - Name string `json:"name"` - Type string `json:"type"` - IsDefault bool `json:"isDefault"` - SendReminder bool `json:"sendReminder"` - DisableResolvedMessage bool `json:"disableResolvedMessage"` - Frequency string `json:"frequency"` - Created time.Time `json:"created"` - Updated time.Time `json:"updated"` - Settings *simplejson.Json `json:"settings"` + Id int64 `json:"id"` + Name string `json:"name"` + Type string `json:"type"` + IsDefault bool `json:"isDefault"` + SendReminder bool `json:"sendReminder"` + DisableResolvedMessage bool `json:"disableResolvedMessage"` + Frequency string `json:"frequency"` + Created time.Time `json:"created"` + Updated time.Time `json:"updated"` + Settings *simplejson.Json `json:"settings"` } type AlertTestCommand struct { @@ -102,12 +102,12 @@ type EvalMatch struct { } type NotificationTestCommand struct { - Name string `json:"name"` - Type string `json:"type"` - SendReminder bool `json:"sendReminder"` - DisableResolvedMessage bool `json:"disableResolvedMessage"` - Frequency string `json:"frequency"` - Settings *simplejson.Json `json:"settings"` + Name string `json:"name"` + Type string `json:"type"` + SendReminder bool `json:"sendReminder"` + DisableResolvedMessage bool `json:"disableResolvedMessage"` + Frequency string `json:"frequency"` + Settings *simplejson.Json `json:"settings"` } type PauseAlertCommand struct { diff --git a/pkg/models/alert_notifications.go b/pkg/models/alert_notifications.go index 14577fd29e6..f9327a4d4bc 100644 --- a/pkg/models/alert_notifications.go +++ b/pkg/models/alert_notifications.go @@ -23,41 +23,41 @@ var ( ) type AlertNotification struct { - Id int64 `json:"id"` - OrgId int64 `json:"-"` - Name string `json:"name"` - Type string `json:"type"` - SendReminder bool `json:"sendReminder"` - DisableResolvedMessage bool `json:"disableResolvedMessage"` - Frequency time.Duration `json:"frequency"` - IsDefault bool `json:"isDefault"` - Settings *simplejson.Json `json:"settings"` - Created time.Time `json:"created"` - Updated time.Time `json:"updated"` + Id int64 `json:"id"` + OrgId int64 `json:"-"` + Name string `json:"name"` + Type string `json:"type"` + SendReminder bool `json:"sendReminder"` + DisableResolvedMessage bool `json:"disableResolvedMessage"` + Frequency time.Duration `json:"frequency"` + IsDefault bool `json:"isDefault"` + Settings *simplejson.Json `json:"settings"` + Created time.Time `json:"created"` + Updated time.Time `json:"updated"` } type CreateAlertNotificationCommand struct { - Name string `json:"name" binding:"Required"` - Type string `json:"type" binding:"Required"` - SendReminder bool `json:"sendReminder"` + Name string `json:"name" binding:"Required"` + Type string `json:"type" binding:"Required"` + SendReminder bool `json:"sendReminder"` DisableResolvedMessage bool `json:"disableResolvedMessage"` - Frequency string `json:"frequency"` - IsDefault bool `json:"isDefault"` - Settings *simplejson.Json `json:"settings"` + Frequency string `json:"frequency"` + IsDefault bool `json:"isDefault"` + Settings *simplejson.Json `json:"settings"` OrgId int64 `json:"-"` Result *AlertNotification } type UpdateAlertNotificationCommand struct { - Id int64 `json:"id" binding:"Required"` - Name string `json:"name" binding:"Required"` - Type string `json:"type" binding:"Required"` - SendReminder bool `json:"sendReminder"` - DisableResolvedMessage bool `json:"disableResolvedMessage"` - Frequency string `json:"frequency"` - IsDefault bool `json:"isDefault"` - Settings *simplejson.Json `json:"settings" binding:"Required"` + Id int64 `json:"id" binding:"Required"` + Name string `json:"name" binding:"Required"` + Type string `json:"type" binding:"Required"` + SendReminder bool `json:"sendReminder"` + DisableResolvedMessage bool `json:"disableResolvedMessage"` + Frequency string `json:"frequency"` + IsDefault bool `json:"isDefault"` + Settings *simplejson.Json `json:"settings" binding:"Required"` OrgId int64 `json:"-"` Result *AlertNotification diff --git a/pkg/services/alerting/notifiers/base.go b/pkg/services/alerting/notifiers/base.go index 320148c49c4..1b14059b1e6 100644 --- a/pkg/services/alerting/notifiers/base.go +++ b/pkg/services/alerting/notifiers/base.go @@ -6,7 +6,6 @@ import ( "github.com/grafana/grafana/pkg/log" "github.com/grafana/grafana/pkg/models" - "github.com/grafana/grafana/pkg/services/alerting" ) @@ -15,14 +14,14 @@ const ( ) type NotifierBase struct { - Name string - Type string - Id int64 - IsDeault bool - UploadImage bool - SendReminder bool - DisableResolvedMessage bool - Frequency time.Duration + Name string + Type string + Id int64 + IsDeault bool + UploadImage bool + SendReminder bool + DisableResolvedMessage bool + Frequency time.Duration log log.Logger } @@ -35,15 +34,15 @@ func NewNotifierBase(model *models.AlertNotification) NotifierBase { } return NotifierBase{ - Id: model.Id, - Name: model.Name, - IsDeault: model.IsDefault, - Type: model.Type, - UploadImage: uploadImage, - SendReminder: model.SendReminder, + Id: model.Id, + Name: model.Name, + IsDeault: model.IsDefault, + Type: model.Type, + UploadImage: uploadImage, + SendReminder: model.SendReminder, DisableResolvedMessage: model.DisableResolvedMessage, - Frequency: model.Frequency, - log: log.New("alerting.notifier." + model.Name), + Frequency: model.Frequency, + log: log.New("alerting.notifier." + model.Name), } } diff --git a/pkg/services/alerting/notifiers/base_test.go b/pkg/services/alerting/notifiers/base_test.go index 5e46d3ad72e..281d0bfb238 100644 --- a/pkg/services/alerting/notifiers/base_test.go +++ b/pkg/services/alerting/notifiers/base_test.go @@ -179,5 +179,10 @@ func TestBaseNotifier(t *testing.T) { base := NewNotifierBase(model) So(base.UploadImage, ShouldBeTrue) }) + + Convey("default value should be false for backwards compatibility", func() { + base := NewNotifierBase(model) + So(base.DisableResolvedMessage, ShouldBeFalse) + }) }) } diff --git a/pkg/services/sqlstore/alert_notification.go b/pkg/services/sqlstore/alert_notification.go index e95d4579342..d82d5814869 100644 --- a/pkg/services/sqlstore/alert_notification.go +++ b/pkg/services/sqlstore/alert_notification.go @@ -168,16 +168,16 @@ func CreateAlertNotificationCommand(cmd *m.CreateAlertNotificationCommand) error } alertNotification := &m.AlertNotification{ - OrgId: cmd.OrgId, - Name: cmd.Name, - Type: cmd.Type, - Settings: cmd.Settings, - SendReminder: cmd.SendReminder, + OrgId: cmd.OrgId, + Name: cmd.Name, + Type: cmd.Type, + Settings: cmd.Settings, + SendReminder: cmd.SendReminder, DisableResolvedMessage: cmd.DisableResolvedMessage, - Frequency: frequency, - Created: time.Now(), - Updated: time.Now(), - IsDefault: cmd.IsDefault, + Frequency: frequency, + Created: time.Now(), + Updated: time.Now(), + IsDefault: cmd.IsDefault, } if _, err = sess.MustCols("send_reminder").Insert(alertNotification); err != nil { diff --git a/pkg/services/sqlstore/alert_notification_test.go b/pkg/services/sqlstore/alert_notification_test.go index d804988a55a..116cff7021d 100644 --- a/pkg/services/sqlstore/alert_notification_test.go +++ b/pkg/services/sqlstore/alert_notification_test.go @@ -227,14 +227,14 @@ func TestAlertNotificationSQLAccess(t *testing.T) { Convey("Can update alert notification", func() { newCmd := &models.UpdateAlertNotificationCommand{ - Name: "NewName", - Type: "webhook", - OrgId: cmd.Result.OrgId, - SendReminder: true, + Name: "NewName", + Type: "webhook", + OrgId: cmd.Result.OrgId, + SendReminder: true, DisableResolvedMessage: true, - Frequency: "60s", - Settings: simplejson.New(), - Id: cmd.Result.Id, + Frequency: "60s", + Settings: simplejson.New(), + Id: cmd.Result.Id, } err := UpdateAlertNotification(newCmd) So(err, ShouldBeNil) @@ -259,10 +259,10 @@ func TestAlertNotificationSQLAccess(t *testing.T) { }) Convey("Can search using an array of ids", func() { - cmd1 := models.CreateAlertNotificationCommand{Name: "nagios", Type: "webhook", OrgId: 1, SendReminder: true, DisableResolvedMessage: false, Frequency: "10s", Settings: simplejson.New()} - cmd2 := models.CreateAlertNotificationCommand{Name: "slack", Type: "webhook", OrgId: 1, SendReminder: true, DisableResolvedMessage: false, Frequency: "10s", Settings: simplejson.New()} - cmd3 := models.CreateAlertNotificationCommand{Name: "ops2", Type: "email", OrgId: 1, SendReminder: true, DisableResolvedMessage: false, Frequency: "10s", Settings: simplejson.New()} - cmd4 := models.CreateAlertNotificationCommand{IsDefault: true, Name: "default", Type: "email", OrgId: 1, SendReminder: true, DisableResolvedMessage: false, Frequency: "10s", Settings: simplejson.New()} + cmd1 := m.CreateAlertNotificationCommand{Name: "nagios", Type: "webhook", OrgId: 1, SendReminder: true, Frequency: "10s", Settings: simplejson.New()} + cmd2 := m.CreateAlertNotificationCommand{Name: "slack", Type: "webhook", OrgId: 1, SendReminder: true, Frequency: "10s", Settings: simplejson.New()} + cmd3 := m.CreateAlertNotificationCommand{Name: "ops2", Type: "email", OrgId: 1, SendReminder: true, Frequency: "10s", Settings: simplejson.New()} + cmd4 := m.CreateAlertNotificationCommand{IsDefault: true, Name: "default", Type: "email", OrgId: 1, SendReminder: true, Frequency: "10s", Settings: simplejson.New()} otherOrg := models.CreateAlertNotificationCommand{Name: "default", Type: "email", OrgId: 2, SendReminder: true, DisableResolvedMessage: false, Frequency: "10s", Settings: simplejson.New()} diff --git a/pkg/services/sqlstore/migrations/alert_mig.go b/pkg/services/sqlstore/migrations/alert_mig.go index bc2d9647e03..7add7ee3e86 100644 --- a/pkg/services/sqlstore/migrations/alert_mig.go +++ b/pkg/services/sqlstore/migrations/alert_mig.go @@ -72,7 +72,7 @@ func addAlertMigrations(mg *Migrator) { Name: "send_reminder", Type: DB_Bool, Nullable: true, Default: "0", })) mg.AddMigration("Add column disable_resolved_message", NewAddColumnMigration(alert_notification, &Column{ - Name: "disable_resolved_message", Type: DB_Bool, Nullable: false, Default: "1", + Name: "disable_resolved_message", Type: DB_Bool, Nullable: false, Default: "0", })) mg.AddMigration("add index alert_notification org_id & name", NewAddIndexMigration(alert_notification, alert_notification.Indices[0]))