Alerting API: send 404 not found error and enrich delete with UID endpoint…

Alerting API: send 404 not found error and enrich delete with UID endpoint response with alert notification ID (#27550)

* Alerting API: Send back 404 not found error for update and delete endpoints

* Alerting API: send back alert notification id for delete with uid endpoint

pkg/api/alerting.go

@@ -10,6 +10,7 @@ import (
 	"github.com/grafana/grafana/pkg/services/alerting"
 	"github.com/grafana/grafana/pkg/services/alerting"
 	"github.com/grafana/grafana/pkg/services/guardian"
 	"github.com/grafana/grafana/pkg/services/guardian"
 	"github.com/grafana/grafana/pkg/services/search"
 	"github.com/grafana/grafana/pkg/services/search"
+	"github.com/grafana/grafana/pkg/util"
 )
 )
 
 
 func ValidateOrgAlert(c *models.ReqContext) {
 func ValidateOrgAlert(c *models.ReqContext) {
@@ -287,11 +288,10 @@ func UpdateAlertNotification(c *models.ReqContext, cmd models.UpdateAlertNotific
 	}
 	}
 
 
 	if err := bus.Dispatch(&cmd); err != nil {
 	if err := bus.Dispatch(&cmd); err != nil {
-		return Error(500, "Failed to update alert notification", err)
+		if err == models.ErrAlertNotificationNotFound {
+			return Error(404, err.Error(), err)
 		}
 		}
-
-	if cmd.Result == nil {
-		return Error(404, "Alert notification not found", nil)
+		return Error(500, "Failed to update alert notification", err)
 	}
 	}
 
 
 	query := models.GetAlertNotificationsQuery{
 	query := models.GetAlertNotificationsQuery{
@@ -316,11 +316,10 @@ func UpdateAlertNotificationByUID(c *models.ReqContext, cmd models.UpdateAlertNo
 	}
 	}
 
 
 	if err := bus.Dispatch(&cmd); err != nil {
 	if err := bus.Dispatch(&cmd); err != nil {
-		return Error(500, "Failed to update alert notification", err)
+		if err == models.ErrAlertNotificationNotFound {
+			return Error(404, err.Error(), nil)
 		}
 		}
-
-	if cmd.Result == nil {
-		return Error(404, "Alert notification not found", nil)
+		return Error(500, "Failed to update alert notification", err)
 	}
 	}
 
 
 	query := models.GetAlertNotificationsWithUidQuery{
 	query := models.GetAlertNotificationsWithUidQuery{
@@ -390,6 +389,9 @@ func DeleteAlertNotification(c *models.ReqContext) Response {
 	}
 	}
 
 
 	if err := bus.Dispatch(&cmd); err != nil {
 	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 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 := 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 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
 //POST /api/alert-notifications/test

pkg/models/alert_notifications.go

@@ -9,6 +9,7 @@ import (
 )
 )
 
 
 var (
 var (
+	ErrAlertNotificationNotFound                = errors.New("Alert notification not found")
 	ErrNotificationFrequencyNotFound            = errors.New("Notification frequency not specified")
 	ErrNotificationFrequencyNotFound            = errors.New("Notification frequency not specified")
 	ErrAlertNotificationStateNotFound           = errors.New("alert notification state not found")
 	ErrAlertNotificationStateNotFound           = errors.New("alert notification state not found")
 	ErrAlertNotificationStateVersionConflict    = errors.New("alert notification state update version conflict")
 	ErrAlertNotificationStateVersionConflict    = errors.New("alert notification state update version conflict")
@@ -94,6 +95,8 @@ type DeleteAlertNotificationCommand struct {
 type DeleteAlertNotificationWithUidCommand struct {
 type DeleteAlertNotificationWithUidCommand struct {
 	Uid   string
 	Uid   string
 	OrgId int64
 	OrgId int64
+
+	DeletedAlertNotificationId int64
 }
 }
 
 
 type GetAlertNotificationUidQuery struct {
 type GetAlertNotificationUidQuery struct {

pkg/services/sqlstore/alert_notification.go

@@ -33,10 +33,19 @@ func init() {
 func DeleteAlertNotification(cmd *models.DeleteAlertNotificationCommand) error {
 func DeleteAlertNotification(cmd *models.DeleteAlertNotificationCommand) error {
 	return inTransaction(func(sess *DBSession) error {
 	return inTransaction(func(sess *DBSession) error {
 		sql := "DELETE FROM alert_notification WHERE alert_notification.org_id = ? AND alert_notification.id = ?"
 		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
 			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 {
 		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
 			return err
 		}
 		}
@@ -51,7 +60,11 @@ func DeleteAlertNotificationWithUid(cmd *models.DeleteAlertNotificationWithUidCo
 		return err
 		return err
 	}
 	}
 
 
-	if existingNotification.Result != nil {
+	if existingNotification.Result == nil {
+		return models.ErrAlertNotificationNotFound
+	}
+
+	cmd.DeletedAlertNotificationId = existingNotification.Result.Id
 	deleteCommand := &models.DeleteAlertNotificationCommand{
 	deleteCommand := &models.DeleteAlertNotificationCommand{
 		Id:    existingNotification.Result.Id,
 		Id:    existingNotification.Result.Id,
 		OrgId: existingNotification.Result.OrgId,
 		OrgId: existingNotification.Result.OrgId,
@@ -59,7 +72,6 @@ func DeleteAlertNotificationWithUid(cmd *models.DeleteAlertNotificationWithUidCo
 	if err := bus.Dispatch(deleteCommand); err != nil {
 	if err := bus.Dispatch(deleteCommand); err != nil {
 		return err
 		return err
 	}
 	}
-	}
 
 
 	return nil
 	return nil
 }
 }
@@ -367,6 +379,10 @@ func UpdateAlertNotification(cmd *models.UpdateAlertNotificationCommand) error {
 			return err
 			return err
 		}
 		}
 
 
+		if current.Id == 0 {
+			return models.ErrAlertNotificationNotFound
+		}
+
 		// check if name exists
 		// check if name exists
 		sameNameQuery := &models.GetAlertNotificationsQuery{OrgId: cmd.OrgId, Name: cmd.Name}
 		sameNameQuery := &models.GetAlertNotificationsQuery{OrgId: cmd.OrgId, Name: cmd.Name}
 		if err := getAlertNotificationInternal(sameNameQuery, sess); err != nil {
 		if err := getAlertNotificationInternal(sameNameQuery, sess); err != nil {
@@ -433,7 +449,7 @@ func UpdateAlertNotificationWithUid(cmd *models.UpdateAlertNotificationWithUidCo
 	current := getAlertNotificationWithUidQuery.Result
 	current := getAlertNotificationWithUidQuery.Result
 
 
 	if current == nil {
 	if current == nil {
-		return fmt.Errorf("Cannot update, alert notification uid %s doesn't exist", cmd.Uid)
+		return models.ErrAlertNotificationNotFound
 	}
 	}
 
 
 	if cmd.NewUid == "" {
 	if cmd.NewUid == "" {

pkg/services/sqlstore/alert_notification_test.go

@@ -390,5 +390,87 @@ func TestAlertNotificationSQLAccess(t *testing.T) {
 				So(result, ShouldBeNil)
 				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)
+			})
+		})
 	})
 	})
 }
 }