From 6376154b16f0507657d143a4c70cc503d6140a14 Mon Sep 17 00:00:00 2001 From: Dave Waters Date: Mon, 15 Oct 2018 17:16:14 -0400 Subject: [PATCH 1/7] add channel option to disable the resolved alert (OK Message) that is sent when condition returns to normal. --- pkg/api/dtos/alerting.go | 49 ++++++++++--------- pkg/models/alert_notifications.go | 49 ++++++++++--------- pkg/services/alerting/interfaces.go | 1 + pkg/services/alerting/notifiers/base.go | 27 +++++++--- pkg/services/sqlstore/alert_notification.go | 22 +++++---- pkg/services/sqlstore/migrations/alert_mig.go | 3 ++ .../alerting/NotificationsEditCtrl.ts | 1 + .../alerting/partials/notification_edit.html | 7 +++ 8 files changed, 96 insertions(+), 63 deletions(-) diff --git a/pkg/api/dtos/alerting.go b/pkg/api/dtos/alerting.go index 697d0a35a08..2f762910d63 100644 --- a/pkg/api/dtos/alerting.go +++ b/pkg/api/dtos/alerting.go @@ -49,28 +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, - 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"` - 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 { @@ -100,11 +102,12 @@ type EvalMatch struct { } type NotificationTestCommand struct { - Name string `json:"name"` - Type string `json:"type"` - SendReminder bool `json:"sendReminder"` - 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 2128b469fa4..14577fd29e6 100644 --- a/pkg/models/alert_notifications.go +++ b/pkg/models/alert_notifications.go @@ -23,38 +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"` - 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"` - Frequency string `json:"frequency"` - IsDefault bool `json:"isDefault"` - Settings *simplejson.Json `json:"settings"` + 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"` 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"` - 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/interfaces.go b/pkg/services/alerting/interfaces.go index 96294f0624f..7cd0e6ffca8 100644 --- a/pkg/services/alerting/interfaces.go +++ b/pkg/services/alerting/interfaces.go @@ -27,6 +27,7 @@ type Notifier interface { GetNotifierId() int64 GetIsDefault() bool GetSendReminder() bool + GetDisableResolvedMessage() bool GetFrequency() time.Duration } diff --git a/pkg/services/alerting/notifiers/base.go b/pkg/services/alerting/notifiers/base.go index fbade2eccac..320148c49c4 100644 --- a/pkg/services/alerting/notifiers/base.go +++ b/pkg/services/alerting/notifiers/base.go @@ -21,6 +21,7 @@ type NotifierBase struct { IsDeault bool UploadImage bool SendReminder bool + DisableResolvedMessage bool Frequency time.Duration log log.Logger @@ -34,14 +35,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, - Frequency: model.Frequency, - log: log.New("alerting.notifier." + model.Name), + 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), } } @@ -83,6 +85,11 @@ func (n *NotifierBase) ShouldNotify(ctx context.Context, context *alerting.EvalC } } + // Do not notify when state is OK if DisableResolvedMessage is set to true + if context.Rule.State == models.AlertStateOK && n.DisableResolvedMessage { + return false + } + return true } @@ -106,6 +113,10 @@ func (n *NotifierBase) GetSendReminder() bool { return n.SendReminder } +func (n *NotifierBase) GetDisableResolvedMessage() bool { + return n.DisableResolvedMessage +} + func (n *NotifierBase) GetFrequency() time.Duration { return n.Frequency } diff --git a/pkg/services/sqlstore/alert_notification.go b/pkg/services/sqlstore/alert_notification.go index daaef945b96..e95d4579342 100644 --- a/pkg/services/sqlstore/alert_notification.go +++ b/pkg/services/sqlstore/alert_notification.go @@ -66,6 +66,7 @@ func GetAlertNotificationsToSend(query *m.GetAlertNotificationsToSendQuery) erro alert_notification.updated, alert_notification.settings, alert_notification.is_default, + alert_notification.disable_resolved_message, alert_notification.send_reminder, alert_notification.frequency FROM alert_notification @@ -106,6 +107,7 @@ func getAlertNotificationInternal(query *m.GetAlertNotificationsQuery, sess *DBS alert_notification.updated, alert_notification.settings, alert_notification.is_default, + alert_notification.disable_resolved_message, alert_notification.send_reminder, alert_notification.frequency FROM alert_notification @@ -166,15 +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, - Frequency: frequency, - Created: time.Now(), - Updated: time.Now(), - IsDefault: cmd.IsDefault, + 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, } if _, err = sess.MustCols("send_reminder").Insert(alertNotification); err != nil { @@ -210,6 +213,7 @@ func UpdateAlertNotification(cmd *m.UpdateAlertNotificationCommand) error { current.Type = cmd.Type current.IsDefault = cmd.IsDefault current.SendReminder = cmd.SendReminder + current.DisableResolvedMessage = cmd.DisableResolvedMessage if current.SendReminder { if cmd.Frequency == "" { diff --git a/pkg/services/sqlstore/migrations/alert_mig.go b/pkg/services/sqlstore/migrations/alert_mig.go index cadcccf6c95..bc2d9647e03 100644 --- a/pkg/services/sqlstore/migrations/alert_mig.go +++ b/pkg/services/sqlstore/migrations/alert_mig.go @@ -71,6 +71,9 @@ func addAlertMigrations(mg *Migrator) { mg.AddMigration("Add column send_reminder", NewAddColumnMigration(alert_notification, &Column{ 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", + })) mg.AddMigration("add index alert_notification org_id & name", NewAddIndexMigration(alert_notification, alert_notification.Indices[0])) diff --git a/public/app/features/alerting/NotificationsEditCtrl.ts b/public/app/features/alerting/NotificationsEditCtrl.ts index 315a9a619a1..92392559480 100644 --- a/public/app/features/alerting/NotificationsEditCtrl.ts +++ b/public/app/features/alerting/NotificationsEditCtrl.ts @@ -12,6 +12,7 @@ export class AlertNotificationEditCtrl { defaults: any = { type: 'email', sendReminder: false, + disableResolvedMessage: false, frequency: '15m', settings: { httpMethod: 'POST', diff --git a/public/app/features/alerting/partials/notification_edit.html b/public/app/features/alerting/partials/notification_edit.html index 7b198736b83..1c4003df91e 100644 --- a/public/app/features/alerting/partials/notification_edit.html +++ b/public/app/features/alerting/partials/notification_edit.html @@ -32,6 +32,13 @@ checked="ctrl.model.settings.uploadImage" tooltip="Captures an image and include it in the notification"> + + Date: Tue, 16 Oct 2018 07:12:32 -0400 Subject: [PATCH 2/7] rename UI Option, align with control, update tests --- .../sqlstore/alert_notification_test.go | 26 ++++++++++--------- .../alerting/partials/notification_edit.html | 4 +-- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/pkg/services/sqlstore/alert_notification_test.go b/pkg/services/sqlstore/alert_notification_test.go index ed682bae5c6..d804988a55a 100644 --- a/pkg/services/sqlstore/alert_notification_test.go +++ b/pkg/services/sqlstore/alert_notification_test.go @@ -227,18 +227,20 @@ func TestAlertNotificationSQLAccess(t *testing.T) { Convey("Can update alert notification", func() { newCmd := &models.UpdateAlertNotificationCommand{ - Name: "NewName", - Type: "webhook", - OrgId: cmd.Result.OrgId, - SendReminder: true, - Frequency: "60s", - Settings: simplejson.New(), - Id: cmd.Result.Id, + Name: "NewName", + Type: "webhook", + OrgId: cmd.Result.OrgId, + SendReminder: true, + DisableResolvedMessage: true, + Frequency: "60s", + Settings: simplejson.New(), + Id: cmd.Result.Id, } err := UpdateAlertNotification(newCmd) So(err, ShouldBeNil) So(newCmd.Result.Name, ShouldEqual, "NewName") So(newCmd.Result.Frequency, ShouldEqual, 60*time.Second) + So(newCmd.Result.DisableResolvedMessage, ShouldBeTrue) }) Convey("Can update alert notification to disable sending of reminders", func() { @@ -257,12 +259,12 @@ 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, Frequency: "10s", Settings: simplejson.New()} - cmd2 := models.CreateAlertNotificationCommand{Name: "slack", Type: "webhook", OrgId: 1, SendReminder: true, Frequency: "10s", Settings: simplejson.New()} - cmd3 := models.CreateAlertNotificationCommand{Name: "ops2", Type: "email", OrgId: 1, SendReminder: true, Frequency: "10s", Settings: simplejson.New()} - cmd4 := models.CreateAlertNotificationCommand{IsDefault: true, Name: "default", Type: "email", OrgId: 1, SendReminder: true, Frequency: "10s", Settings: simplejson.New()} + 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()} - otherOrg := models.CreateAlertNotificationCommand{Name: "default", Type: "email", OrgId: 2, 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()} So(CreateAlertNotificationCommand(&cmd1), ShouldBeNil) So(CreateAlertNotificationCommand(&cmd2), ShouldBeNil) diff --git a/public/app/features/alerting/partials/notification_edit.html b/public/app/features/alerting/partials/notification_edit.html index 1c4003df91e..1ecab3c3e3c 100644 --- a/public/app/features/alerting/partials/notification_edit.html +++ b/public/app/features/alerting/partials/notification_edit.html @@ -34,10 +34,10 @@ + tooltip="Disable the resolved message [OK] that is sent when alerting state returns to false"> Date: Tue, 16 Oct 2018 17:33:38 -0400 Subject: [PATCH 3/7] 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])) From 0eae7b077d1c63992cb5d8a85cc3fd6f49805e42 Mon Sep 17 00:00:00 2001 From: Dave Waters Date: Tue, 16 Oct 2018 17:38:59 -0400 Subject: [PATCH 4/7] fix gofmt, add test, correct noted concerns with default value --- .../sqlstore/alert_notification_test.go | 179 +++++------------- 1 file changed, 47 insertions(+), 132 deletions(-) diff --git a/pkg/services/sqlstore/alert_notification_test.go b/pkg/services/sqlstore/alert_notification_test.go index 116cff7021d..83fb42db9bb 100644 --- a/pkg/services/sqlstore/alert_notification_test.go +++ b/pkg/services/sqlstore/alert_notification_test.go @@ -6,7 +6,7 @@ import ( "time" "github.com/grafana/grafana/pkg/components/simplejson" - "github.com/grafana/grafana/pkg/models" + m "github.com/grafana/grafana/pkg/models" . "github.com/smartystreets/goconvey/convey" ) @@ -14,133 +14,50 @@ func TestAlertNotificationSQLAccess(t *testing.T) { Convey("Testing Alert notification sql access", t, func() { InitTestDB(t) - Convey("Alert notification state", func() { - var alertID int64 = 7 - var orgID int64 = 5 - var notifierID int64 = 10 - oldTimeNow := timeNow - now := time.Date(2018, 9, 30, 0, 0, 0, 0, time.UTC) - timeNow = func() time.Time { return now } + Convey("Alert notification journal", func() { + var alertId int64 = 5 + var orgId int64 = 5 + var notifierId int64 = 5 - Convey("Get no existing state should create a new state", func() { - query := &models.GetOrCreateNotificationStateQuery{AlertId: alertID, OrgId: orgID, NotifierId: notifierID} - err := GetOrCreateAlertNotificationState(context.Background(), query) - So(err, ShouldBeNil) - So(query.Result, ShouldNotBeNil) - So(query.Result.State, ShouldEqual, "unknown") - So(query.Result.Version, ShouldEqual, 0) - So(query.Result.UpdatedAt, ShouldEqual, now.Unix()) + Convey("Getting last journal should raise error if no one exists", func() { + query := &m.GetLatestNotificationQuery{AlertId: alertId, OrgId: orgId, NotifierId: notifierId} + err := GetLatestNotification(context.Background(), query) + So(err, ShouldEqual, m.ErrJournalingNotFound) - Convey("Get existing state should not create a new state", func() { - query2 := &models.GetOrCreateNotificationStateQuery{AlertId: alertID, OrgId: orgID, NotifierId: notifierID} - err := GetOrCreateAlertNotificationState(context.Background(), query2) + Convey("shoulbe be able to record two journaling events", func() { + createCmd := &m.RecordNotificationJournalCommand{AlertId: alertId, NotifierId: notifierId, OrgId: orgId, Success: true, SentAt: 1} + + err := RecordNotificationJournal(context.Background(), createCmd) So(err, ShouldBeNil) - So(query2.Result, ShouldNotBeNil) - So(query2.Result.Id, ShouldEqual, query.Result.Id) - So(query2.Result.UpdatedAt, ShouldEqual, now.Unix()) - }) - Convey("Update existing state to pending with correct version should update database", func() { - s := *query.Result + createCmd.SentAt += 1000 //increase epoch - cmd := models.SetAlertNotificationStateToPendingCommand{ - Id: s.Id, - Version: s.Version, - AlertRuleStateUpdatedVersion: s.AlertRuleStateUpdatedVersion, - } - - err := SetAlertNotificationStateToPendingCommand(context.Background(), &cmd) + err = RecordNotificationJournal(context.Background(), createCmd) So(err, ShouldBeNil) - So(cmd.ResultVersion, ShouldEqual, 1) - query2 := &models.GetOrCreateNotificationStateQuery{AlertId: alertID, OrgId: orgID, NotifierId: notifierID} - err = GetOrCreateAlertNotificationState(context.Background(), query2) - So(err, ShouldBeNil) - So(query2.Result.Version, ShouldEqual, 1) - So(query2.Result.State, ShouldEqual, models.AlertNotificationStatePending) - So(query2.Result.UpdatedAt, ShouldEqual, now.Unix()) - - Convey("Update existing state to completed should update database", func() { - s := *query.Result - setStateCmd := models.SetAlertNotificationStateToCompleteCommand{ - Id: s.Id, - Version: cmd.ResultVersion, - } - err := SetAlertNotificationStateToCompleteCommand(context.Background(), &setStateCmd) + Convey("get last journaling event", func() { + err := GetLatestNotification(context.Background(), query) So(err, ShouldBeNil) + So(query.Result.SentAt, ShouldEqual, 1001) - query3 := &models.GetOrCreateNotificationStateQuery{AlertId: alertID, OrgId: orgID, NotifierId: notifierID} - err = GetOrCreateAlertNotificationState(context.Background(), query3) - So(err, ShouldBeNil) - So(query3.Result.Version, ShouldEqual, 2) - So(query3.Result.State, ShouldEqual, models.AlertNotificationStateCompleted) - So(query3.Result.UpdatedAt, ShouldEqual, now.Unix()) - }) + Convey("be able to clear all journaling for an notifier", func() { + cmd := &m.CleanNotificationJournalCommand{AlertId: alertId, NotifierId: notifierId, OrgId: orgId} + err := CleanNotificationJournal(context.Background(), cmd) + So(err, ShouldBeNil) - Convey("Update existing state to completed should update database. regardless of version", func() { - s := *query.Result - unknownVersion := int64(1000) - cmd := models.SetAlertNotificationStateToCompleteCommand{ - Id: s.Id, - Version: unknownVersion, - } - err := SetAlertNotificationStateToCompleteCommand(context.Background(), &cmd) - So(err, ShouldBeNil) - - query3 := &models.GetOrCreateNotificationStateQuery{AlertId: alertID, OrgId: orgID, NotifierId: notifierID} - err = GetOrCreateAlertNotificationState(context.Background(), query3) - So(err, ShouldBeNil) - So(query3.Result.Version, ShouldEqual, unknownVersion+1) - So(query3.Result.State, ShouldEqual, models.AlertNotificationStateCompleted) - So(query3.Result.UpdatedAt, ShouldEqual, now.Unix()) + Convey("querying for last junaling should raise error", func() { + query := &m.GetLatestNotificationQuery{AlertId: alertId, OrgId: orgId, NotifierId: notifierId} + err := GetLatestNotification(context.Background(), query) + So(err, ShouldEqual, m.ErrJournalingNotFound) + }) + }) }) }) - - Convey("Update existing state to pending with incorrect version should return version mismatch error", func() { - s := *query.Result - s.Version = 1000 - cmd := models.SetAlertNotificationStateToPendingCommand{ - Id: s.NotifierId, - Version: s.Version, - AlertRuleStateUpdatedVersion: s.AlertRuleStateUpdatedVersion, - } - err := SetAlertNotificationStateToPendingCommand(context.Background(), &cmd) - So(err, ShouldEqual, models.ErrAlertNotificationStateVersionConflict) - }) - - Convey("Updating existing state to pending with incorrect version since alert rule state update version is higher", func() { - s := *query.Result - cmd := models.SetAlertNotificationStateToPendingCommand{ - Id: s.Id, - Version: s.Version, - AlertRuleStateUpdatedVersion: 1000, - } - err := SetAlertNotificationStateToPendingCommand(context.Background(), &cmd) - So(err, ShouldBeNil) - - So(cmd.ResultVersion, ShouldEqual, 1) - }) - - Convey("different version and same alert state change version should return error", func() { - s := *query.Result - s.Version = 1000 - cmd := models.SetAlertNotificationStateToPendingCommand{ - Id: s.Id, - Version: s.Version, - AlertRuleStateUpdatedVersion: s.AlertRuleStateUpdatedVersion, - } - err := SetAlertNotificationStateToPendingCommand(context.Background(), &cmd) - So(err, ShouldNotBeNil) - }) - }) - - Reset(func() { - timeNow = oldTimeNow }) }) Convey("Alert notifications should be empty", func() { - cmd := &models.GetAlertNotificationsQuery{ + cmd := &m.GetAlertNotificationsQuery{ OrgId: 2, Name: "email", } @@ -151,7 +68,7 @@ func TestAlertNotificationSQLAccess(t *testing.T) { }) Convey("Cannot save alert notifier with send reminder = true", func() { - cmd := &models.CreateAlertNotificationCommand{ + cmd := &m.CreateAlertNotificationCommand{ Name: "ops", Type: "email", OrgId: 1, @@ -161,7 +78,7 @@ func TestAlertNotificationSQLAccess(t *testing.T) { Convey("and missing frequency", func() { err := CreateAlertNotificationCommand(cmd) - So(err, ShouldEqual, models.ErrNotificationFrequencyNotFound) + So(err, ShouldEqual, m.ErrNotificationFrequencyNotFound) }) Convey("invalid frequency", func() { @@ -173,7 +90,7 @@ func TestAlertNotificationSQLAccess(t *testing.T) { }) Convey("Cannot update alert notifier with send reminder = false", func() { - cmd := &models.CreateAlertNotificationCommand{ + cmd := &m.CreateAlertNotificationCommand{ Name: "ops update", Type: "email", OrgId: 1, @@ -184,14 +101,14 @@ func TestAlertNotificationSQLAccess(t *testing.T) { err := CreateAlertNotificationCommand(cmd) So(err, ShouldBeNil) - updateCmd := &models.UpdateAlertNotificationCommand{ + updateCmd := &m.UpdateAlertNotificationCommand{ Id: cmd.Result.Id, SendReminder: true, } Convey("and missing frequency", func() { err := UpdateAlertNotification(updateCmd) - So(err, ShouldEqual, models.ErrNotificationFrequencyNotFound) + So(err, ShouldEqual, m.ErrNotificationFrequencyNotFound) }) Convey("invalid frequency", func() { @@ -204,7 +121,7 @@ func TestAlertNotificationSQLAccess(t *testing.T) { }) Convey("Can save Alert Notification", func() { - cmd := &models.CreateAlertNotificationCommand{ + cmd := &m.CreateAlertNotificationCommand{ Name: "ops", Type: "email", OrgId: 1, @@ -226,25 +143,23 @@ func TestAlertNotificationSQLAccess(t *testing.T) { }) Convey("Can update alert notification", func() { - newCmd := &models.UpdateAlertNotificationCommand{ - Name: "NewName", - Type: "webhook", - OrgId: cmd.Result.OrgId, - SendReminder: true, - DisableResolvedMessage: true, - Frequency: "60s", - Settings: simplejson.New(), - Id: cmd.Result.Id, + newCmd := &m.UpdateAlertNotificationCommand{ + Name: "NewName", + Type: "webhook", + OrgId: cmd.Result.OrgId, + SendReminder: true, + Frequency: "60s", + Settings: simplejson.New(), + Id: cmd.Result.Id, } err := UpdateAlertNotification(newCmd) So(err, ShouldBeNil) So(newCmd.Result.Name, ShouldEqual, "NewName") So(newCmd.Result.Frequency, ShouldEqual, 60*time.Second) - So(newCmd.Result.DisableResolvedMessage, ShouldBeTrue) }) Convey("Can update alert notification to disable sending of reminders", func() { - newCmd := &models.UpdateAlertNotificationCommand{ + newCmd := &m.UpdateAlertNotificationCommand{ Name: "NewName", Type: "webhook", OrgId: cmd.Result.OrgId, @@ -264,7 +179,7 @@ func TestAlertNotificationSQLAccess(t *testing.T) { 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()} + otherOrg := m.CreateAlertNotificationCommand{Name: "default", Type: "email", OrgId: 2, SendReminder: true, Frequency: "10s", Settings: simplejson.New()} So(CreateAlertNotificationCommand(&cmd1), ShouldBeNil) So(CreateAlertNotificationCommand(&cmd2), ShouldBeNil) @@ -273,7 +188,7 @@ func TestAlertNotificationSQLAccess(t *testing.T) { So(CreateAlertNotificationCommand(&otherOrg), ShouldBeNil) Convey("search", func() { - query := &models.GetAlertNotificationsToSendQuery{ + query := &m.GetAlertNotificationsToSendQuery{ Ids: []int64{cmd1.Result.Id, cmd2.Result.Id, 112341231}, OrgId: 1, } @@ -284,7 +199,7 @@ func TestAlertNotificationSQLAccess(t *testing.T) { }) Convey("all", func() { - query := &models.GetAllAlertNotificationsQuery{ + query := &m.GetAllAlertNotificationsQuery{ OrgId: 1, } From a3137e731d2520f0ef0aa96545d1ef7c778cc8ce Mon Sep 17 00:00:00 2001 From: Dave Waters Date: Tue, 16 Oct 2018 20:39:07 -0400 Subject: [PATCH 5/7] clean up tests --- .../sqlstore/alert_notification_test.go | 187 +++++++++++++----- 1 file changed, 136 insertions(+), 51 deletions(-) diff --git a/pkg/services/sqlstore/alert_notification_test.go b/pkg/services/sqlstore/alert_notification_test.go index 83fb42db9bb..1b922783bcb 100644 --- a/pkg/services/sqlstore/alert_notification_test.go +++ b/pkg/services/sqlstore/alert_notification_test.go @@ -6,7 +6,7 @@ import ( "time" "github.com/grafana/grafana/pkg/components/simplejson" - m "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/models" . "github.com/smartystreets/goconvey/convey" ) @@ -14,50 +14,133 @@ func TestAlertNotificationSQLAccess(t *testing.T) { Convey("Testing Alert notification sql access", t, func() { InitTestDB(t) - Convey("Alert notification journal", func() { - var alertId int64 = 5 - var orgId int64 = 5 - var notifierId int64 = 5 + Convey("Alert notification state", func() { + var alertID int64 = 7 + var orgID int64 = 5 + var notifierID int64 = 10 + oldTimeNow := timeNow + now := time.Date(2018, 9, 30, 0, 0, 0, 0, time.UTC) + timeNow = func() time.Time { return now } - Convey("Getting last journal should raise error if no one exists", func() { - query := &m.GetLatestNotificationQuery{AlertId: alertId, OrgId: orgId, NotifierId: notifierId} - err := GetLatestNotification(context.Background(), query) - So(err, ShouldEqual, m.ErrJournalingNotFound) + Convey("Get no existing state should create a new state", func() { + query := &models.GetOrCreateNotificationStateQuery{AlertId: alertID, OrgId: orgID, NotifierId: notifierID} + err := GetOrCreateAlertNotificationState(context.Background(), query) + So(err, ShouldBeNil) + So(query.Result, ShouldNotBeNil) + So(query.Result.State, ShouldEqual, "unknown") + So(query.Result.Version, ShouldEqual, 0) + So(query.Result.UpdatedAt, ShouldEqual, now.Unix()) - Convey("shoulbe be able to record two journaling events", func() { - createCmd := &m.RecordNotificationJournalCommand{AlertId: alertId, NotifierId: notifierId, OrgId: orgId, Success: true, SentAt: 1} - - err := RecordNotificationJournal(context.Background(), createCmd) + Convey("Get existing state should not create a new state", func() { + query2 := &models.GetOrCreateNotificationStateQuery{AlertId: alertID, OrgId: orgID, NotifierId: notifierID} + err := GetOrCreateAlertNotificationState(context.Background(), query2) So(err, ShouldBeNil) + So(query2.Result, ShouldNotBeNil) + So(query2.Result.Id, ShouldEqual, query.Result.Id) + So(query2.Result.UpdatedAt, ShouldEqual, now.Unix()) + }) - createCmd.SentAt += 1000 //increase epoch + Convey("Update existing state to pending with correct version should update database", func() { + s := *query.Result - err = RecordNotificationJournal(context.Background(), createCmd) + cmd := models.SetAlertNotificationStateToPendingCommand{ + Id: s.Id, + Version: s.Version, + AlertRuleStateUpdatedVersion: s.AlertRuleStateUpdatedVersion, + } + + err := SetAlertNotificationStateToPendingCommand(context.Background(), &cmd) So(err, ShouldBeNil) + So(cmd.ResultVersion, ShouldEqual, 1) - Convey("get last journaling event", func() { - err := GetLatestNotification(context.Background(), query) + query2 := &models.GetOrCreateNotificationStateQuery{AlertId: alertID, OrgId: orgID, NotifierId: notifierID} + err = GetOrCreateAlertNotificationState(context.Background(), query2) + So(err, ShouldBeNil) + So(query2.Result.Version, ShouldEqual, 1) + So(query2.Result.State, ShouldEqual, models.AlertNotificationStatePending) + So(query2.Result.UpdatedAt, ShouldEqual, now.Unix()) + + Convey("Update existing state to completed should update database", func() { + s := *query.Result + setStateCmd := models.SetAlertNotificationStateToCompleteCommand{ + Id: s.Id, + Version: cmd.ResultVersion, + } + err := SetAlertNotificationStateToCompleteCommand(context.Background(), &setStateCmd) So(err, ShouldBeNil) - So(query.Result.SentAt, ShouldEqual, 1001) - Convey("be able to clear all journaling for an notifier", func() { - cmd := &m.CleanNotificationJournalCommand{AlertId: alertId, NotifierId: notifierId, OrgId: orgId} - err := CleanNotificationJournal(context.Background(), cmd) - So(err, ShouldBeNil) + query3 := &models.GetOrCreateNotificationStateQuery{AlertId: alertID, OrgId: orgID, NotifierId: notifierID} + err = GetOrCreateAlertNotificationState(context.Background(), query3) + So(err, ShouldBeNil) + So(query3.Result.Version, ShouldEqual, 2) + So(query3.Result.State, ShouldEqual, models.AlertNotificationStateCompleted) + So(query3.Result.UpdatedAt, ShouldEqual, now.Unix()) + }) - Convey("querying for last junaling should raise error", func() { - query := &m.GetLatestNotificationQuery{AlertId: alertId, OrgId: orgId, NotifierId: notifierId} - err := GetLatestNotification(context.Background(), query) - So(err, ShouldEqual, m.ErrJournalingNotFound) - }) - }) + Convey("Update existing state to completed should update database. regardless of version", func() { + s := *query.Result + unknownVersion := int64(1000) + cmd := models.SetAlertNotificationStateToCompleteCommand{ + Id: s.Id, + Version: unknownVersion, + } + err := SetAlertNotificationStateToCompleteCommand(context.Background(), &cmd) + So(err, ShouldBeNil) + + query3 := &models.GetOrCreateNotificationStateQuery{AlertId: alertID, OrgId: orgID, NotifierId: notifierID} + err = GetOrCreateAlertNotificationState(context.Background(), query3) + So(err, ShouldBeNil) + So(query3.Result.Version, ShouldEqual, unknownVersion+1) + So(query3.Result.State, ShouldEqual, models.AlertNotificationStateCompleted) + So(query3.Result.UpdatedAt, ShouldEqual, now.Unix()) }) }) + + Convey("Update existing state to pending with incorrect version should return version mismatch error", func() { + s := *query.Result + s.Version = 1000 + cmd := models.SetAlertNotificationStateToPendingCommand{ + Id: s.NotifierId, + Version: s.Version, + AlertRuleStateUpdatedVersion: s.AlertRuleStateUpdatedVersion, + } + err := SetAlertNotificationStateToPendingCommand(context.Background(), &cmd) + So(err, ShouldEqual, models.ErrAlertNotificationStateVersionConflict) + }) + + Convey("Updating existing state to pending with incorrect version since alert rule state update version is higher", func() { + s := *query.Result + cmd := models.SetAlertNotificationStateToPendingCommand{ + Id: s.Id, + Version: s.Version, + AlertRuleStateUpdatedVersion: 1000, + } + err := SetAlertNotificationStateToPendingCommand(context.Background(), &cmd) + So(err, ShouldBeNil) + + So(cmd.ResultVersion, ShouldEqual, 1) + }) + + Convey("different version and same alert state change version should return error", func() { + s := *query.Result + s.Version = 1000 + cmd := models.SetAlertNotificationStateToPendingCommand{ + Id: s.Id, + Version: s.Version, + AlertRuleStateUpdatedVersion: s.AlertRuleStateUpdatedVersion, + } + err := SetAlertNotificationStateToPendingCommand(context.Background(), &cmd) + So(err, ShouldNotBeNil) + }) + }) + + Reset(func() { + timeNow = oldTimeNow }) }) Convey("Alert notifications should be empty", func() { - cmd := &m.GetAlertNotificationsQuery{ + cmd := &models.GetAlertNotificationsQuery{ OrgId: 2, Name: "email", } @@ -68,7 +151,7 @@ func TestAlertNotificationSQLAccess(t *testing.T) { }) Convey("Cannot save alert notifier with send reminder = true", func() { - cmd := &m.CreateAlertNotificationCommand{ + cmd := &models.CreateAlertNotificationCommand{ Name: "ops", Type: "email", OrgId: 1, @@ -78,7 +161,7 @@ func TestAlertNotificationSQLAccess(t *testing.T) { Convey("and missing frequency", func() { err := CreateAlertNotificationCommand(cmd) - So(err, ShouldEqual, m.ErrNotificationFrequencyNotFound) + So(err, ShouldEqual, models.ErrNotificationFrequencyNotFound) }) Convey("invalid frequency", func() { @@ -90,7 +173,7 @@ func TestAlertNotificationSQLAccess(t *testing.T) { }) Convey("Cannot update alert notifier with send reminder = false", func() { - cmd := &m.CreateAlertNotificationCommand{ + cmd := &models.CreateAlertNotificationCommand{ Name: "ops update", Type: "email", OrgId: 1, @@ -101,14 +184,14 @@ func TestAlertNotificationSQLAccess(t *testing.T) { err := CreateAlertNotificationCommand(cmd) So(err, ShouldBeNil) - updateCmd := &m.UpdateAlertNotificationCommand{ + updateCmd := &models.UpdateAlertNotificationCommand{ Id: cmd.Result.Id, SendReminder: true, } Convey("and missing frequency", func() { err := UpdateAlertNotification(updateCmd) - So(err, ShouldEqual, m.ErrNotificationFrequencyNotFound) + So(err, ShouldEqual, models.ErrNotificationFrequencyNotFound) }) Convey("invalid frequency", func() { @@ -121,7 +204,7 @@ func TestAlertNotificationSQLAccess(t *testing.T) { }) Convey("Can save Alert Notification", func() { - cmd := &m.CreateAlertNotificationCommand{ + cmd := &models.CreateAlertNotificationCommand{ Name: "ops", Type: "email", OrgId: 1, @@ -143,23 +226,25 @@ func TestAlertNotificationSQLAccess(t *testing.T) { }) Convey("Can update alert notification", func() { - newCmd := &m.UpdateAlertNotificationCommand{ - Name: "NewName", - Type: "webhook", - OrgId: cmd.Result.OrgId, - SendReminder: true, - Frequency: "60s", - Settings: simplejson.New(), - Id: cmd.Result.Id, + newCmd := &models.UpdateAlertNotificationCommand{ + Name: "NewName", + Type: "webhook", + OrgId: cmd.Result.OrgId, + SendReminder: true, + DisableResolvedMessage: true, + Frequency: "60s", + Settings: simplejson.New(), + Id: cmd.Result.Id, } err := UpdateAlertNotification(newCmd) So(err, ShouldBeNil) So(newCmd.Result.Name, ShouldEqual, "NewName") So(newCmd.Result.Frequency, ShouldEqual, 60*time.Second) + So(newCmd.Result.DisableResolvedMessage, ShouldBeTrue) }) Convey("Can update alert notification to disable sending of reminders", func() { - newCmd := &m.UpdateAlertNotificationCommand{ + newCmd := &models.UpdateAlertNotificationCommand{ Name: "NewName", Type: "webhook", OrgId: cmd.Result.OrgId, @@ -174,12 +259,12 @@ func TestAlertNotificationSQLAccess(t *testing.T) { }) Convey("Can search using an array of ids", func() { - 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()} + cmd1 := models.CreateAlertNotificationCommand{Name: "nagios", Type: "webhook", OrgId: 1, SendReminder: true, Frequency: "10s", Settings: simplejson.New()} + cmd2 := models.CreateAlertNotificationCommand{Name: "slack", Type: "webhook", OrgId: 1, SendReminder: true, Frequency: "10s", Settings: simplejson.New()} + cmd3 := models.CreateAlertNotificationCommand{Name: "ops2", Type: "email", OrgId: 1, SendReminder: true, Frequency: "10s", Settings: simplejson.New()} + cmd4 := models.CreateAlertNotificationCommand{IsDefault: true, Name: "default", Type: "email", OrgId: 1, SendReminder: true, Frequency: "10s", Settings: simplejson.New()} - otherOrg := m.CreateAlertNotificationCommand{Name: "default", Type: "email", OrgId: 2, SendReminder: true, Frequency: "10s", Settings: simplejson.New()} + otherOrg := models.CreateAlertNotificationCommand{Name: "default", Type: "email", OrgId: 2, SendReminder: true, Frequency: "10s", Settings: simplejson.New()} So(CreateAlertNotificationCommand(&cmd1), ShouldBeNil) So(CreateAlertNotificationCommand(&cmd2), ShouldBeNil) @@ -188,7 +273,7 @@ func TestAlertNotificationSQLAccess(t *testing.T) { So(CreateAlertNotificationCommand(&otherOrg), ShouldBeNil) Convey("search", func() { - query := &m.GetAlertNotificationsToSendQuery{ + query := &models.GetAlertNotificationsToSendQuery{ Ids: []int64{cmd1.Result.Id, cmd2.Result.Id, 112341231}, OrgId: 1, } @@ -199,7 +284,7 @@ func TestAlertNotificationSQLAccess(t *testing.T) { }) Convey("all", func() { - query := &m.GetAllAlertNotificationsQuery{ + query := &models.GetAllAlertNotificationsQuery{ OrgId: 1, } From 5566a61aef2aa3dc750a2408688324d4c6bb9b95 Mon Sep 17 00:00:00 2001 From: bergquist Date: Wed, 17 Oct 2018 09:53:03 +0200 Subject: [PATCH 6/7] alerting: tests default value for disable resolve message --- pkg/services/sqlstore/alert_notification_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/services/sqlstore/alert_notification_test.go b/pkg/services/sqlstore/alert_notification_test.go index 1b922783bcb..680685a6d22 100644 --- a/pkg/services/sqlstore/alert_notification_test.go +++ b/pkg/services/sqlstore/alert_notification_test.go @@ -219,6 +219,7 @@ func TestAlertNotificationSQLAccess(t *testing.T) { So(cmd.Result.OrgId, ShouldNotEqual, 0) So(cmd.Result.Type, ShouldEqual, "email") So(cmd.Result.Frequency, ShouldEqual, 10*time.Second) + So(cmd.Result.DisableResolvedMessage, ShouldBeFalse) Convey("Cannot save Alert Notification with the same name", func() { err = CreateAlertNotificationCommand(cmd) From 70385119bc29e0cf8fa6b501b18938a4e7b03c79 Mon Sep 17 00:00:00 2001 From: bergquist Date: Wed, 17 Oct 2018 10:41:18 +0200 Subject: [PATCH 7/7] removes d in disableResolvedMessage --- pkg/api/dtos/alerting.go | 52 +++++++++---------- pkg/models/alert_notifications.go | 52 +++++++++---------- pkg/services/alerting/interfaces.go | 2 +- pkg/services/alerting/notifiers/base.go | 42 +++++++-------- pkg/services/alerting/notifiers/base_test.go | 2 +- pkg/services/sqlstore/alert_notification.go | 28 +++++----- .../sqlstore/alert_notification_test.go | 20 +++---- pkg/services/sqlstore/migrations/alert_mig.go | 4 +- .../alerting/NotificationsEditCtrl.ts | 2 +- .../alerting/partials/notification_edit.html | 14 ++--- 10 files changed, 109 insertions(+), 109 deletions(-) diff --git a/pkg/api/dtos/alerting.go b/pkg/api/dtos/alerting.go index 33f10c805ab..c037831f341 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, + DisableResolveMessage: notification.DisableResolveMessage, + 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"` + DisableResolveMessage bool `json:"disableResolveMessage"` + 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"` + DisableResolveMessage bool `json:"disableResolveMessage"` + 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 f9327a4d4bc..e0fd12937ed 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"` + DisableResolveMessage bool `json:"disableResolveMessage"` + 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"` - DisableResolvedMessage bool `json:"disableResolvedMessage"` - Frequency string `json:"frequency"` - IsDefault bool `json:"isDefault"` - Settings *simplejson.Json `json:"settings"` + Name string `json:"name" binding:"Required"` + Type string `json:"type" binding:"Required"` + SendReminder bool `json:"sendReminder"` + DisableResolveMessage bool `json:"disableResolveMessage"` + 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"` + DisableResolveMessage bool `json:"disableResolveMessage"` + 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/interfaces.go b/pkg/services/alerting/interfaces.go index 7cd0e6ffca8..040d0991861 100644 --- a/pkg/services/alerting/interfaces.go +++ b/pkg/services/alerting/interfaces.go @@ -27,7 +27,7 @@ type Notifier interface { GetNotifierId() int64 GetIsDefault() bool GetSendReminder() bool - GetDisableResolvedMessage() bool + GetDisableResolveMessage() bool GetFrequency() time.Duration } diff --git a/pkg/services/alerting/notifiers/base.go b/pkg/services/alerting/notifiers/base.go index 1b14059b1e6..d141d6cd257 100644 --- a/pkg/services/alerting/notifiers/base.go +++ b/pkg/services/alerting/notifiers/base.go @@ -14,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 + DisableResolveMessage bool + Frequency time.Duration log log.Logger } @@ -34,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, - DisableResolvedMessage: model.DisableResolvedMessage, - Frequency: model.Frequency, - log: log.New("alerting.notifier." + model.Name), + Id: model.Id, + Name: model.Name, + IsDeault: model.IsDefault, + Type: model.Type, + UploadImage: uploadImage, + SendReminder: model.SendReminder, + DisableResolveMessage: model.DisableResolveMessage, + Frequency: model.Frequency, + log: log.New("alerting.notifier." + model.Name), } } @@ -84,8 +84,8 @@ func (n *NotifierBase) ShouldNotify(ctx context.Context, context *alerting.EvalC } } - // Do not notify when state is OK if DisableResolvedMessage is set to true - if context.Rule.State == models.AlertStateOK && n.DisableResolvedMessage { + // Do not notify when state is OK if DisableResolveMessage is set to true + if context.Rule.State == models.AlertStateOK && n.DisableResolveMessage { return false } @@ -112,8 +112,8 @@ func (n *NotifierBase) GetSendReminder() bool { return n.SendReminder } -func (n *NotifierBase) GetDisableResolvedMessage() bool { - return n.DisableResolvedMessage +func (n *NotifierBase) GetDisableResolveMessage() bool { + return n.DisableResolveMessage } func (n *NotifierBase) GetFrequency() time.Duration { diff --git a/pkg/services/alerting/notifiers/base_test.go b/pkg/services/alerting/notifiers/base_test.go index 281d0bfb238..5062828cb4f 100644 --- a/pkg/services/alerting/notifiers/base_test.go +++ b/pkg/services/alerting/notifiers/base_test.go @@ -182,7 +182,7 @@ func TestBaseNotifier(t *testing.T) { Convey("default value should be false for backwards compatibility", func() { base := NewNotifierBase(model) - So(base.DisableResolvedMessage, ShouldBeFalse) + So(base.DisableResolveMessage, ShouldBeFalse) }) }) } diff --git a/pkg/services/sqlstore/alert_notification.go b/pkg/services/sqlstore/alert_notification.go index d82d5814869..afe6269510f 100644 --- a/pkg/services/sqlstore/alert_notification.go +++ b/pkg/services/sqlstore/alert_notification.go @@ -66,7 +66,7 @@ func GetAlertNotificationsToSend(query *m.GetAlertNotificationsToSendQuery) erro alert_notification.updated, alert_notification.settings, alert_notification.is_default, - alert_notification.disable_resolved_message, + alert_notification.disable_resolve_message, alert_notification.send_reminder, alert_notification.frequency FROM alert_notification @@ -107,7 +107,7 @@ func getAlertNotificationInternal(query *m.GetAlertNotificationsQuery, sess *DBS alert_notification.updated, alert_notification.settings, alert_notification.is_default, - alert_notification.disable_resolved_message, + alert_notification.disable_resolve_message, alert_notification.send_reminder, alert_notification.frequency FROM alert_notification @@ -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, - DisableResolvedMessage: cmd.DisableResolvedMessage, - Frequency: frequency, - Created: time.Now(), - Updated: time.Now(), - IsDefault: cmd.IsDefault, + OrgId: cmd.OrgId, + Name: cmd.Name, + Type: cmd.Type, + Settings: cmd.Settings, + SendReminder: cmd.SendReminder, + DisableResolveMessage: cmd.DisableResolveMessage, + Frequency: frequency, + Created: time.Now(), + Updated: time.Now(), + IsDefault: cmd.IsDefault, } if _, err = sess.MustCols("send_reminder").Insert(alertNotification); err != nil { @@ -213,7 +213,7 @@ func UpdateAlertNotification(cmd *m.UpdateAlertNotificationCommand) error { current.Type = cmd.Type current.IsDefault = cmd.IsDefault current.SendReminder = cmd.SendReminder - current.DisableResolvedMessage = cmd.DisableResolvedMessage + current.DisableResolveMessage = cmd.DisableResolveMessage if current.SendReminder { if cmd.Frequency == "" { @@ -228,7 +228,7 @@ func UpdateAlertNotification(cmd *m.UpdateAlertNotificationCommand) error { current.Frequency = frequency } - sess.UseBool("is_default", "send_reminder") + sess.UseBool("is_default", "send_reminder", "disable_resolve_message") if affected, err := sess.ID(cmd.Id).Update(current); err != nil { return err diff --git a/pkg/services/sqlstore/alert_notification_test.go b/pkg/services/sqlstore/alert_notification_test.go index 680685a6d22..629a6292eb5 100644 --- a/pkg/services/sqlstore/alert_notification_test.go +++ b/pkg/services/sqlstore/alert_notification_test.go @@ -219,7 +219,7 @@ func TestAlertNotificationSQLAccess(t *testing.T) { So(cmd.Result.OrgId, ShouldNotEqual, 0) So(cmd.Result.Type, ShouldEqual, "email") So(cmd.Result.Frequency, ShouldEqual, 10*time.Second) - So(cmd.Result.DisableResolvedMessage, ShouldBeFalse) + So(cmd.Result.DisableResolveMessage, ShouldBeFalse) Convey("Cannot save Alert Notification with the same name", func() { err = CreateAlertNotificationCommand(cmd) @@ -228,20 +228,20 @@ func TestAlertNotificationSQLAccess(t *testing.T) { Convey("Can update alert notification", func() { newCmd := &models.UpdateAlertNotificationCommand{ - Name: "NewName", - Type: "webhook", - OrgId: cmd.Result.OrgId, - SendReminder: true, - DisableResolvedMessage: true, - Frequency: "60s", - Settings: simplejson.New(), - Id: cmd.Result.Id, + Name: "NewName", + Type: "webhook", + OrgId: cmd.Result.OrgId, + SendReminder: true, + DisableResolveMessage: true, + Frequency: "60s", + Settings: simplejson.New(), + Id: cmd.Result.Id, } err := UpdateAlertNotification(newCmd) So(err, ShouldBeNil) So(newCmd.Result.Name, ShouldEqual, "NewName") So(newCmd.Result.Frequency, ShouldEqual, 60*time.Second) - So(newCmd.Result.DisableResolvedMessage, ShouldBeTrue) + So(newCmd.Result.DisableResolveMessage, ShouldBeTrue) }) Convey("Can update alert notification to disable sending of reminders", func() { diff --git a/pkg/services/sqlstore/migrations/alert_mig.go b/pkg/services/sqlstore/migrations/alert_mig.go index 7add7ee3e86..198a47b50ff 100644 --- a/pkg/services/sqlstore/migrations/alert_mig.go +++ b/pkg/services/sqlstore/migrations/alert_mig.go @@ -71,8 +71,8 @@ func addAlertMigrations(mg *Migrator) { mg.AddMigration("Add column send_reminder", NewAddColumnMigration(alert_notification, &Column{ 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: "0", + mg.AddMigration("Add column disable_resolve_message", NewAddColumnMigration(alert_notification, &Column{ + Name: "disable_resolve_message", Type: DB_Bool, Nullable: false, Default: "0", })) mg.AddMigration("add index alert_notification org_id & name", NewAddIndexMigration(alert_notification, alert_notification.Indices[0])) diff --git a/public/app/features/alerting/NotificationsEditCtrl.ts b/public/app/features/alerting/NotificationsEditCtrl.ts index 92392559480..2607121bb0e 100644 --- a/public/app/features/alerting/NotificationsEditCtrl.ts +++ b/public/app/features/alerting/NotificationsEditCtrl.ts @@ -12,7 +12,7 @@ export class AlertNotificationEditCtrl { defaults: any = { type: 'email', sendReminder: false, - disableResolvedMessage: false, + disableResolveMessage: false, frequency: '15m', settings: { httpMethod: 'POST', diff --git a/public/app/features/alerting/partials/notification_edit.html b/public/app/features/alerting/partials/notification_edit.html index 1ecab3c3e3c..b2cd2f21e4d 100644 --- a/public/app/features/alerting/partials/notification_edit.html +++ b/public/app/features/alerting/partials/notification_edit.html @@ -21,28 +21,28 @@ + label="Disable Resolve Message" + label-class="width-14" + checked="ctrl.model.disableResolveMessage" + tooltip="Disable the resolve message [OK] that is sent when alerting state returns to false">