diff --git a/pkg/api/alerting.go b/pkg/api/alerting.go index c61e373c148..0a63e897297 100644 --- a/pkg/api/alerting.go +++ b/pkg/api/alerting.go @@ -10,6 +10,7 @@ import ( "github.com/grafana/grafana/pkg/services/alerting" "github.com/grafana/grafana/pkg/services/guardian" "github.com/grafana/grafana/pkg/services/search" + "github.com/grafana/grafana/pkg/util" ) func ValidateOrgAlert(c *models.ReqContext) { @@ -287,13 +288,12 @@ func UpdateAlertNotification(c *models.ReqContext, cmd models.UpdateAlertNotific } if err := bus.Dispatch(&cmd); err != nil { + if err == models.ErrAlertNotificationNotFound { + return Error(404, err.Error(), err) + } return Error(500, "Failed to update alert notification", err) } - if cmd.Result == nil { - return Error(404, "Alert notification not found", nil) - } - query := models.GetAlertNotificationsQuery{ OrgId: c.OrgId, Id: cmd.Id, @@ -316,13 +316,12 @@ func UpdateAlertNotificationByUID(c *models.ReqContext, cmd models.UpdateAlertNo } if err := bus.Dispatch(&cmd); err != nil { + if err == models.ErrAlertNotificationNotFound { + return Error(404, err.Error(), nil) + } return Error(500, "Failed to update alert notification", err) } - if cmd.Result == nil { - return Error(404, "Alert notification not found", nil) - } - query := models.GetAlertNotificationsWithUidQuery{ OrgId: cmd.OrgId, Uid: cmd.Uid, @@ -390,6 +389,9 @@ func DeleteAlertNotification(c *models.ReqContext) Response { } if err := bus.Dispatch(&cmd); err != nil { + if err == models.ErrAlertNotificationNotFound { + return Error(404, err.Error(), nil) + } return Error(500, "Failed to delete alert notification", err) } @@ -403,10 +405,16 @@ func DeleteAlertNotificationByUID(c *models.ReqContext) Response { } if err := bus.Dispatch(&cmd); err != nil { + if err == models.ErrAlertNotificationNotFound { + return Error(404, err.Error(), nil) + } return Error(500, "Failed to delete alert notification", err) } - return Success("Notification deleted") + return JSON(200, util.DynMap{ + "message": "Notification deleted", + "id": cmd.DeletedAlertNotificationId, + }) } //POST /api/alert-notifications/test diff --git a/pkg/models/alert_notifications.go b/pkg/models/alert_notifications.go index e7a7486b9bd..2d0886e4d53 100644 --- a/pkg/models/alert_notifications.go +++ b/pkg/models/alert_notifications.go @@ -9,6 +9,7 @@ import ( ) var ( + ErrAlertNotificationNotFound = errors.New("Alert notification not found") ErrNotificationFrequencyNotFound = errors.New("Notification frequency not specified") ErrAlertNotificationStateNotFound = errors.New("alert notification state not found") ErrAlertNotificationStateVersionConflict = errors.New("alert notification state update version conflict") @@ -94,6 +95,8 @@ type DeleteAlertNotificationCommand struct { type DeleteAlertNotificationWithUidCommand struct { Uid string OrgId int64 + + DeletedAlertNotificationId int64 } type GetAlertNotificationUidQuery struct { diff --git a/pkg/services/sqlstore/alert_notification.go b/pkg/services/sqlstore/alert_notification.go index 71d00fa96d1..f8a6d24e9e1 100644 --- a/pkg/services/sqlstore/alert_notification.go +++ b/pkg/services/sqlstore/alert_notification.go @@ -33,9 +33,18 @@ func init() { func DeleteAlertNotification(cmd *models.DeleteAlertNotificationCommand) error { return inTransaction(func(sess *DBSession) error { sql := "DELETE FROM alert_notification WHERE alert_notification.org_id = ? AND alert_notification.id = ?" - if _, err := sess.Exec(sql, cmd.OrgId, cmd.Id); err != nil { + res, err := sess.Exec(sql, cmd.OrgId, cmd.Id) + if err != nil { return err } + rowsAffected, err := res.RowsAffected() + if err != nil { + return err + } + + if rowsAffected == 0 { + return models.ErrAlertNotificationNotFound + } if _, err := sess.Exec("DELETE FROM alert_notification_state WHERE alert_notification_state.org_id = ? AND alert_notification_state.notifier_id = ?", cmd.OrgId, cmd.Id); err != nil { return err @@ -51,14 +60,17 @@ func DeleteAlertNotificationWithUid(cmd *models.DeleteAlertNotificationWithUidCo return err } - if existingNotification.Result != nil { - deleteCommand := &models.DeleteAlertNotificationCommand{ - Id: existingNotification.Result.Id, - OrgId: existingNotification.Result.OrgId, - } - if err := bus.Dispatch(deleteCommand); err != nil { - return err - } + if existingNotification.Result == nil { + return models.ErrAlertNotificationNotFound + } + + cmd.DeletedAlertNotificationId = existingNotification.Result.Id + deleteCommand := &models.DeleteAlertNotificationCommand{ + Id: existingNotification.Result.Id, + OrgId: existingNotification.Result.OrgId, + } + if err := bus.Dispatch(deleteCommand); err != nil { + return err } return nil @@ -367,6 +379,10 @@ func UpdateAlertNotification(cmd *models.UpdateAlertNotificationCommand) error { return err } + if current.Id == 0 { + return models.ErrAlertNotificationNotFound + } + // check if name exists sameNameQuery := &models.GetAlertNotificationsQuery{OrgId: cmd.OrgId, Name: cmd.Name} if err := getAlertNotificationInternal(sameNameQuery, sess); err != nil { @@ -433,7 +449,7 @@ func UpdateAlertNotificationWithUid(cmd *models.UpdateAlertNotificationWithUidCo current := getAlertNotificationWithUidQuery.Result if current == nil { - return fmt.Errorf("Cannot update, alert notification uid %s doesn't exist", cmd.Uid) + return models.ErrAlertNotificationNotFound } if cmd.NewUid == "" { diff --git a/pkg/services/sqlstore/alert_notification_test.go b/pkg/services/sqlstore/alert_notification_test.go index ddef553d481..6cc105c5826 100644 --- a/pkg/services/sqlstore/alert_notification_test.go +++ b/pkg/services/sqlstore/alert_notification_test.go @@ -390,5 +390,87 @@ func TestAlertNotificationSQLAccess(t *testing.T) { So(result, ShouldBeNil) }) }) + + Convey("Cannot update non-existing Alert Notification", func() { + updateCmd := &models.UpdateAlertNotificationCommand{ + Name: "NewName", + Type: "webhook", + OrgId: 1, + SendReminder: true, + DisableResolveMessage: true, + Frequency: "60s", + Settings: simplejson.New(), + Id: 1, + } + err := UpdateAlertNotification(updateCmd) + So(err, ShouldEqual, models.ErrAlertNotificationNotFound) + + Convey("using UID", func() { + updateWithUidCmd := &models.UpdateAlertNotificationWithUidCommand{ + Name: "NewName", + Type: "webhook", + OrgId: 1, + SendReminder: true, + DisableResolveMessage: true, + Frequency: "60s", + Settings: simplejson.New(), + Uid: "uid", + NewUid: "newUid", + } + err := UpdateAlertNotificationWithUid(updateWithUidCmd) + So(err, ShouldEqual, models.ErrAlertNotificationNotFound) + }) + }) + + Convey("Can delete Alert Notification", func() { + cmd := &models.CreateAlertNotificationCommand{ + Name: "ops update", + Type: "email", + OrgId: 1, + SendReminder: false, + Settings: simplejson.New(), + } + + err := CreateAlertNotificationCommand(cmd) + So(err, ShouldBeNil) + + deleteCmd := &models.DeleteAlertNotificationCommand{ + Id: cmd.Result.Id, + OrgId: 1, + } + err = DeleteAlertNotification(deleteCmd) + So(err, ShouldBeNil) + + Convey("using UID", func() { + err := CreateAlertNotificationCommand(cmd) + So(err, ShouldBeNil) + + deleteWithUidCmd := &models.DeleteAlertNotificationWithUidCommand{ + Uid: cmd.Result.Uid, + OrgId: 1, + } + err = DeleteAlertNotificationWithUid(deleteWithUidCmd) + So(err, ShouldBeNil) + So(deleteWithUidCmd.DeletedAlertNotificationId, ShouldEqual, cmd.Result.Id) + }) + }) + + Convey("Cannot delete non-existing Alert Notification", func() { + deleteCmd := &models.DeleteAlertNotificationCommand{ + Id: 1, + OrgId: 1, + } + err := DeleteAlertNotification(deleteCmd) + So(err, ShouldEqual, models.ErrAlertNotificationNotFound) + + Convey("using UID", func() { + deleteWithUidCmd := &models.DeleteAlertNotificationWithUidCommand{ + Uid: "uid", + OrgId: 1, + } + err = DeleteAlertNotificationWithUid(deleteWithUidCmd) + So(err, ShouldEqual, models.ErrAlertNotificationNotFound) + }) + }) }) }