alerting: only check frequency when not send once

pkg/api/alerting.go

@@ -192,17 +192,7 @@ func GetAlertNotifications(c *m.ReqContext) Response {
 	result := make([]*dtos.AlertNotification, 0)
 
 	for _, notification := range query.Result {
-		result = append(result, &dtos.AlertNotification{
-			Id:         notification.Id,
-			Name:       notification.Name,
-			Type:       notification.Type,
-			IsDefault:  notification.IsDefault,
-			Created:    notification.Created,
-			Updated:    notification.Updated,
-			Frequency:  notification.Frequency.String(),
-			NotifyOnce: notification.NotifyOnce,
-			Settings:   notification.Settings,
-		})
+		result = append(result, dtos.NewAlertNotification(notification))
 	}
 
 	return JSON(200, result)
@@ -218,19 +208,7 @@ func GetAlertNotificationByID(c *m.ReqContext) Response {
 		return Error(500, "Failed to get alert notifications", err)
 	}
 
-	result := &dtos.AlertNotification{
-		Id:         query.Result.Id,
-		Name:       query.Result.Name,
-		Type:       query.Result.Type,
-		IsDefault:  query.Result.IsDefault,
-		Created:    query.Result.Created,
-		Updated:    query.Result.Updated,
-		Frequency:  query.Result.Frequency.String(),
-		NotifyOnce: query.Result.NotifyOnce,
-		Settings:   query.Result.Settings,
-	}
-
-	return JSON(200, result)
+	return JSON(200, dtos.NewAlertNotification(query.Result))
 }
 
 func CreateAlertNotification(c *m.ReqContext, cmd m.CreateAlertNotificationCommand) Response {
@@ -240,19 +218,7 @@ func CreateAlertNotification(c *m.ReqContext, cmd m.CreateAlertNotificationComma
 		return Error(500, "Failed to create alert notification", err)
 	}
 
-	result := &dtos.AlertNotification{
-		Id:         cmd.Result.Id,
-		Name:       cmd.Result.Name,
-		Type:       cmd.Result.Type,
-		IsDefault:  cmd.Result.IsDefault,
-		Created:    cmd.Result.Created,
-		Updated:    cmd.Result.Updated,
-		Frequency:  cmd.Result.Frequency.String(),
-		NotifyOnce: cmd.Result.NotifyOnce,
-		Settings:   cmd.Result.Settings,
-	}
-
-	return JSON(200, result)
+	return JSON(200, dtos.NewAlertNotification(cmd.Result))
 }
 
 func UpdateAlertNotification(c *m.ReqContext, cmd m.UpdateAlertNotificationCommand) Response {
@@ -262,19 +228,7 @@ func UpdateAlertNotification(c *m.ReqContext, cmd m.UpdateAlertNotificationComma
 		return Error(500, "Failed to update alert notification", err)
 	}
 
-	result := &dtos.AlertNotification{
-		Id:         cmd.Result.Id,
-		Name:       cmd.Result.Name,
-		Type:       cmd.Result.Type,
-		IsDefault:  cmd.Result.IsDefault,
-		Created:    cmd.Result.Created,
-		Updated:    cmd.Result.Updated,
-		Frequency:  cmd.Result.Frequency.String(),
-		NotifyOnce: cmd.Result.NotifyOnce,
-		Settings:   cmd.Result.Settings,
-	}
-
-	return JSON(200, result)
+	return JSON(200, dtos.NewAlertNotification(cmd.Result))
 }
 
 func DeleteAlertNotification(c *m.ReqContext) Response {

pkg/api/alerting_test.go

@@ -2,6 +2,7 @@ package api
 
 import (
 	"testing"
+	"time"
 
 	"github.com/grafana/grafana/pkg/api/dtos"
 	"github.com/grafana/grafana/pkg/bus"
@@ -11,6 +12,24 @@ import (
 	. "github.com/smartystreets/goconvey/convey"
 )
 
+func TestRemoveZeroUnitsFromInterval(t *testing.T) {
+	tcs := []struct {
+		interval time.Duration
+		expected string
+	}{
+		{interval: time.Duration(time.Hour), expected: "1h"},
+		{interval: time.Duration(time.Hour + time.Minute), expected: "1h1m"},
+		{interval: time.Duration((time.Hour * 10) + time.Minute), expected: "10h1m"},
+	}
+
+	for _, tc := range tcs {
+		got := removeZeroesFromDuration(tc.interval)
+		if got != tc.expected {
+			t.Errorf("expected %s got %s internval: %v", tc.expected, got, tc.interval)
+		}
+	}
+}
+
 func TestAlertingApiEndpoint(t *testing.T) {
 	Convey("Given an alert in a dashboard with an acl", t, func() {
 

pkg/api/dtos/alerting.go

@@ -1,26 +1,51 @@
 package dtos
 
 import (
+	"strings"
 	"time"
 
 	"github.com/grafana/grafana/pkg/components/null"
 	"github.com/grafana/grafana/pkg/components/simplejson"
-	m "github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/models"
 )
 
 type AlertRule struct {
-	Id             int64            `json:"id"`
-	DashboardId    int64            `json:"dashboardId"`
-	PanelId        int64            `json:"panelId"`
-	Name           string           `json:"name"`
-	Message        string           `json:"message"`
-	State          m.AlertStateType `json:"state"`
-	NewStateDate   time.Time        `json:"newStateDate"`
-	EvalDate       time.Time        `json:"evalDate"`
-	EvalData       *simplejson.Json `json:"evalData"`
-	ExecutionError string           `json:"executionError"`
-	Url            string           `json:"url"`
-	CanEdit        bool             `json:"canEdit"`
+	Id             int64                 `json:"id"`
+	DashboardId    int64                 `json:"dashboardId"`
+	PanelId        int64                 `json:"panelId"`
+	Name           string                `json:"name"`
+	Message        string                `json:"message"`
+	State          models.AlertStateType `json:"state"`
+	NewStateDate   time.Time             `json:"newStateDate"`
+	EvalDate       time.Time             `json:"evalDate"`
+	EvalData       *simplejson.Json      `json:"evalData"`
+	ExecutionError string                `json:"executionError"`
+	Url            string                `json:"url"`
+	CanEdit        bool                  `json:"canEdit"`
+}
+
+func removeZeroesFromDuration(interval time.Duration) string {
+	frequency := interval.String()
+
+	frequency = strings.Replace(frequency, "0h", "", 1)
+	frequency = strings.Replace(frequency, "0m", "", 1)
+	frequency = strings.Replace(frequency, "0s", "", 1)
+
+	return frequency
+}
+
+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:  removeZeroesFromDuration(notification.Frequency),
+		NotifyOnce: notification.NotifyOnce,
+		Settings:   notification.Settings,
+	}
 }
 
 type AlertNotification struct {
@@ -42,7 +67,7 @@ type AlertTestCommand struct {
 
 type AlertTestResult struct {
 	Firing         bool                  `json:"firing"`
-	State          m.AlertStateType      `json:"state"`
+	State          models.AlertStateType `json:"state"`
 	ConditionEvals string                `json:"conditionEvals"`
 	TimeMs         string                `json:"timeMs"`
 	Error          string                `json:"error,omitempty"`

pkg/services/sqlstore/alert_notification.go

@@ -144,14 +144,16 @@ func CreateAlertNotificationCommand(cmd *m.CreateAlertNotificationCommand) error
 			return fmt.Errorf("Alert notification name %s already exists", cmd.Name)
 		}
 
-		if cmd.Frequency == "" {
-			return fmt.Errorf("Alert notification frequency required")
-		}
-
 		var frequency time.Duration
-		frequency, err = time.ParseDuration(cmd.Frequency)
-		if err != nil {
-			return err
+		if !cmd.NotifyOnce {
+			if cmd.Frequency == "" {
+				return m.ErrNotificationFrequencyNotFound
+			}
+
+			frequency, err = time.ParseDuration(cmd.Frequency)
+			if err != nil {
+				return err
+			}
 		}
 
 		alertNotification := &m.AlertNotification{
@@ -200,22 +202,25 @@ func UpdateAlertNotification(cmd *m.UpdateAlertNotificationCommand) error {
 		current.IsDefault = cmd.IsDefault
 		current.NotifyOnce = cmd.NotifyOnce
 
-		if cmd.Frequency == "" {
-			return m.ErrNotificationFrequencyNotFound
-		}
+		if !current.NotifyOnce {
+			if cmd.Frequency == "" {
+				return m.ErrNotificationFrequencyNotFound
+			}
 
-		frequency, err := time.ParseDuration(cmd.Frequency)
-		if err != nil {
-			return err
+			frequency, err := time.ParseDuration(cmd.Frequency)
+			if err != nil {
+				return err
+			}
+
+			current.Frequency = frequency
 		}
-		current.Frequency = frequency
 
 		sess.UseBool("is_default", "notify_once")
 
 		if affected, err := sess.ID(cmd.Id).Update(current); err != nil {
 			return err
 		} else if affected == 0 {
-			return fmt.Errorf("Could not find alert notification")
+			return fmt.Errorf("Could not update alert notification")
 		}
 
 		cmd.Result = &current

pkg/services/sqlstore/alert_notification_test.go

@@ -11,7 +11,6 @@ import (
 func TestAlertNotificationSQLAccess(t *testing.T) {
 	Convey("Testing Alert notification sql access", t, func() {
 		InitTestDB(t)
-		var err error
 
 		Convey("Alert notifications should be empty", func() {
 			cmd := &m.GetAlertNotificationsQuery{
@@ -24,6 +23,58 @@ func TestAlertNotificationSQLAccess(t *testing.T) {
 			So(cmd.Result, ShouldBeNil)
 		})
 
+		Convey("Cannot save alert notifier with notitfyonce = false", func() {
+			cmd := &m.CreateAlertNotificationCommand{
+				Name:       "ops",
+				Type:       "email",
+				OrgId:      1,
+				NotifyOnce: false,
+				Settings:   simplejson.New(),
+			}
+
+			Convey("and missing frequency", func() {
+				err := CreateAlertNotificationCommand(cmd)
+				So(err, ShouldEqual, m.ErrNotificationFrequencyNotFound)
+			})
+
+			Convey("invalid frequency", func() {
+				cmd.Frequency = "invalid duration"
+
+				err := CreateAlertNotificationCommand(cmd)
+				So(err.Error(), ShouldEqual, "time: invalid duration invalid duration")
+			})
+		})
+
+		Convey("Cannot update alert notifier with notitfyonce = false", func() {
+			cmd := &m.CreateAlertNotificationCommand{
+				Name:       "ops update",
+				Type:       "email",
+				OrgId:      1,
+				NotifyOnce: true,
+				Settings:   simplejson.New(),
+			}
+
+			err := CreateAlertNotificationCommand(cmd)
+			So(err, ShouldBeNil)
+
+			updateCmd := &m.UpdateAlertNotificationCommand{
+				Id:         cmd.Result.Id,
+				NotifyOnce: false,
+			}
+
+			Convey("and missing frequency", func() {
+				err := UpdateAlertNotification(updateCmd)
+				So(err, ShouldEqual, m.ErrNotificationFrequencyNotFound)
+			})
+
+			Convey("invalid frequency", func() {
+				updateCmd.Frequency = "invalid duration"
+
+				err := UpdateAlertNotification(updateCmd)
+				So(err.Error(), ShouldEqual, "time: invalid duration invalid duration")
+			})
+		})
+
 		Convey("Can save Alert Notification", func() {
 			cmd := &m.CreateAlertNotificationCommand{
 				Name:       "ops",
@@ -34,7 +85,7 @@ func TestAlertNotificationSQLAccess(t *testing.T) {
 				Settings:   simplejson.New(),
 			}
 
-			err = CreateAlertNotificationCommand(cmd)
+			err := CreateAlertNotificationCommand(cmd)
 			So(err, ShouldBeNil)
 			So(cmd.Result.Id, ShouldNotEqual, 0)
 			So(cmd.Result.OrgId, ShouldNotEqual, 0)