Implement code review changes

pkg/models/alert_notifications.go

@@ -1,11 +1,16 @@
 package models
 
 import (
+	"errors"
 	"time"
 
 	"github.com/grafana/grafana/pkg/components/simplejson"
 )
 
+var (
+	ErrNotificationFrequencyNotFound = errors.New("Notification frequency not specified")
+)
+
 type AlertNotification struct {
 	Id         int64            `json:"id"`
 	OrgId      int64            `json:"-"`

pkg/services/alerting/engine.go

@@ -10,9 +10,7 @@ import (
 	tlog "github.com/opentracing/opentracing-go/log"
 
 	"github.com/benbjohnson/clock"
-	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/log"
-	m "github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/registry"
 	"github.com/grafana/grafana/pkg/services/rendering"
 	"github.com/grafana/grafana/pkg/setting"
@@ -207,18 +205,6 @@ func (e *AlertingService) processJob(attemptID int, attemptChan chan int, cancel
 		}
 
 		evalContext.Rule.State = evalContext.GetNewState()
-		if evalContext.Rule.State == m.AlertStateOK && evalContext.PrevAlertState != m.AlertStateOK {
-			for _, notifierId := range evalContext.Rule.Notifications {
-				cmd := &m.CleanNotificationJournalCommand{
-					AlertId:    evalContext.Rule.Id,
-					NotifierId: notifierId,
-					OrgId:      evalContext.Rule.OrgId,
-				}
-				if err := bus.Dispatch(cmd); err != nil {
-					e.log.Error("Failed to clean up old notification records", "notifier", notifierId, "alert", evalContext.Rule.Id, "Error", err)
-				}
-			}
-		}
 		e.resultHandler.Handle(evalContext)
 		span.Finish()
 		e.log.Debug("Job Execution completed", "timeMs", evalContext.GetDurationMs(), "alertId", evalContext.Rule.Id, "name", evalContext.Rule.Name, "firing", evalContext.Firing, "attemptID", attemptID)

pkg/services/alerting/result_handler.go

@@ -88,6 +88,18 @@ func (handler *DefaultResultHandler) Handle(evalContext *EvalContext) error {
 		}
 	}
 
+	if evalContext.Rule.State == m.AlertStateOK && evalContext.PrevAlertState != m.AlertStateOK {
+		for _, notifierId := range evalContext.Rule.Notifications {
+			cmd := &m.CleanNotificationJournalCommand{
+				AlertId:    evalContext.Rule.Id,
+				NotifierId: notifierId,
+				OrgId:      evalContext.Rule.OrgId,
+			}
+			if err := bus.Dispatch(cmd); err != nil {
+				handler.log.Error("Failed to clean up old notification records", "notifier", notifierId, "alert", evalContext.Rule.Id, "Error", err)
+			}
+		}
+	}
 	handler.notifier.SendIfNeeded(evalContext)
 
 	return nil

pkg/services/sqlstore/alert_notification.go

@@ -148,8 +148,9 @@ func CreateAlertNotificationCommand(cmd *m.CreateAlertNotificationCommand) error
 			return fmt.Errorf("Alert notification frequency required")
 		}
 
-		frequency, err_convert := time.ParseDuration(cmd.Frequency)
+		var frequency time.Duration
-		if err_convert != nil {
+		frequency, err = time.ParseDuration(cmd.Frequency)
+		if err != nil {
 			return err
 		}
 
@@ -200,7 +201,7 @@ func UpdateAlertNotification(cmd *m.UpdateAlertNotificationCommand) error {
 		current.NotifyOnce = cmd.NotifyOnce
 
 		if cmd.Frequency == "" {
-			return fmt.Errorf("Alert notification frequency required")
+			return m.ErrNotificationFrequencyNotFound
 		}
 
 		frequency, err_convert := time.ParseDuration(cmd.Frequency)

public/app/features/alerting/notification_edit_ctrl.ts

@@ -12,6 +12,7 @@ export class AlertNotificationEditCtrl {
   defaults: any = {
     type: 'email',
     notifyOnce: true,
+    frequency: '15m',
     settings: {
       httpMethod: 'POST',
       autoResolve: true,

public/app/features/alerting/partials/notification_edit.html

@@ -18,10 +18,6 @@
           </select>
         </div>
       </div>
-      <div class="gf-form">
-        <a class="gf-form-label width-12" ng-click="ctrl.model.notifyOnce = !ctrl.model.notifyOnce;">{{ ctrl.model.notifyOnce ? 'Notify on state change' : 'Notify at most every' }}</a>
-        <input class="gf-form-input max-width-15" type="text" ng-model="ctrl.model.frequency" ng-if="!ctrl.model.notifyOnce"></input>
-      </div>
       <gf-form-switch
           class="gf-form"
           label="Send on all alerts"
@@ -36,6 +32,17 @@
           checked="ctrl.model.settings.uploadImage"
           tooltip="Captures an image and include it in the notification">
       </gf-form-switch>
+      <gf-form-switch
+          class="gf-form"
+          label="Notify once"
+          label-class="width-12"
+          checked="ctrl.model.notifyOnce"
+          tooltip="Choose to either notify on state change or at every interval">
+      </gf-form-switch>
+      <div class="gf-form">
+        <span class="gf-form-label width-12" ng-if="!ctrl.model.notifyOnce">Notify every</span>
+        <input class="gf-form-input max-width-15" type="text" required ng-model="ctrl.model.frequency" required ng-if="!ctrl.model.notifyOnce"></input>
+      </div>
     </div>
 
     <div class="gf-form-group" ng-include src="ctrl.notifierTemplateId">