From 67e5f625149b02c245a277798ad3129494b9ec89 Mon Sep 17 00:00:00 2001
From: bergquist <carl.bergquist@gmail.com>
Date: Tue, 2 Oct 2018 14:23:18 +0200
Subject: [PATCH] move version conflict logging for mark as complete to
 sqlstore package

---
 pkg/services/alerting/notifier.go               | 17 +++--------------
 pkg/services/sqlstore/alert_notification.go     |  5 ++++-
 .../sqlstore/alert_notification_test.go         |  9 +++++----
 3 files changed, 12 insertions(+), 19 deletions(-)

diff --git a/pkg/services/alerting/notifier.go b/pkg/services/alerting/notifier.go
index 6a9b2d0b3ef..0f3290c0dbe 100644
--- a/pkg/services/alerting/notifier.go
+++ b/pkg/services/alerting/notifier.go
@@ -3,7 +3,6 @@ package alerting
 import (
 	"errors"
 	"fmt"
-	"time"
 
 	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/components/imguploader"
@@ -60,15 +59,14 @@ func (n *notificationService) SendIfNeeded(context *EvalContext) error {
 
 func (n *notificationService) sendAndMarkAsComplete(evalContext *EvalContext, notifierState *notifierState) error {
 	notifier := notifierState.notifier
+
 	n.log.Debug("Sending notification", "type", notifier.GetType(), "id", notifier.GetNotifierId(), "isDefault", notifier.GetIsDefault())
 	metrics.M_Alerting_Notification_Sent.WithLabelValues(notifier.GetType()).Inc()
 
 	err := notifier.Notify(evalContext)
 
 	if err != nil {
-		n.log.Error("failed to send notification", "id", notifier.GetNotifierId())
-	} else {
-		notifierState.state.UpdatedAt = time.Now().UTC().Unix()
+		n.log.Error("failed to send notification", "id", notifier.GetNotifierId(), "error", err)
 	}
 
 	if evalContext.IsTestRun {
@@ -80,16 +78,7 @@ func (n *notificationService) sendAndMarkAsComplete(evalContext *EvalContext, no
 		Version: notifierState.state.Version,
 	}
 
-	if err = bus.DispatchCtx(evalContext.Ctx, cmd); err != nil {
-		if err == m.ErrAlertNotificationStateVersionConflict {
-			n.log.Error("notification state out of sync", "id", notifier.GetNotifierId())
-			return nil
-		}
-
-		return err
-	}
-
-	return nil
+	return bus.DispatchCtx(evalContext.Ctx, cmd)
 }
 
 func (n *notificationService) sendNotification(evalContext *EvalContext, notifierState *notifierState) error {
diff --git a/pkg/services/sqlstore/alert_notification.go b/pkg/services/sqlstore/alert_notification.go
index 069e6213b03..8815f4c4bd7 100644
--- a/pkg/services/sqlstore/alert_notification.go
+++ b/pkg/services/sqlstore/alert_notification.go
@@ -259,7 +259,10 @@ func SetAlertNotificationStateToCompleteCommand(ctx context.Context, cmd *m.SetA
 		}
 
 		if current.Version != version {
-			return m.ErrAlertNotificationStateVersionConflict
+			sqlog.Error(
+				`notification state out of sync. the notification 
+				is marked as complete but has been modified between 
+				set as pending and completion.`, "notifierId", current.NotifierId)
 		}
 
 		return nil
diff --git a/pkg/services/sqlstore/alert_notification_test.go b/pkg/services/sqlstore/alert_notification_test.go
index df360d14efa..ed682bae5c6 100644
--- a/pkg/services/sqlstore/alert_notification_test.go
+++ b/pkg/services/sqlstore/alert_notification_test.go
@@ -77,19 +77,20 @@ func TestAlertNotificationSQLAccess(t *testing.T) {
 						So(query3.Result.UpdatedAt, ShouldEqual, now.Unix())
 					})
 
-					Convey("Update existing state to completed should update database, but return version mismatch", func() {
+					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: 1000,
+							Version: unknownVersion,
 						}
 						err := SetAlertNotificationStateToCompleteCommand(context.Background(), &cmd)
-						So(err, ShouldEqual, models.ErrAlertNotificationStateVersionConflict)
+						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, 1001)
+						So(query3.Result.Version, ShouldEqual, unknownVersion+1)
 						So(query3.Result.State, ShouldEqual, models.AlertNotificationStateCompleted)
 						So(query3.Result.UpdatedAt, ShouldEqual, now.Unix())
 					})