move version conflict logging for mark as complete to sqlstore package

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 {

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

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())
 					})