From 12bf5c225a98121a9198ffd54a80b609e8364657 Mon Sep 17 00:00:00 2001
From: bergquist <carl.bergquist@gmail.com>
Date: Fri, 15 Jun 2018 16:27:20 +0200
Subject: [PATCH] tests for defaultShouldNotify

---
 pkg/services/alerting/notifiers/base.go      |  4 ++
 pkg/services/alerting/notifiers/base_test.go | 45 +++++++++++++++++---
 pkg/services/sqlstore/alert_notification.go  |  2 +-
 3 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/pkg/services/alerting/notifiers/base.go b/pkg/services/alerting/notifiers/base.go
index 8450816f97e..8178054f4d3 100644
--- a/pkg/services/alerting/notifiers/base.go
+++ b/pkg/services/alerting/notifiers/base.go
@@ -78,6 +78,10 @@ func (n *NotifierBase) ShouldNotify(c *alerting.EvalContext) bool {
 		return false
 	}
 
+	// this currently serves two purposes.
+	// 1. make sure failed notifications try again
+	// 2. make sure we send notifications if no previous exist
+	// this should be refactored //Carl Bergquist
 	if !cmd.Result.Success {
 		return true
 	}
diff --git a/pkg/services/alerting/notifiers/base_test.go b/pkg/services/alerting/notifiers/base_test.go
index b7395030e5b..96c80cf03bc 100644
--- a/pkg/services/alerting/notifiers/base_test.go
+++ b/pkg/services/alerting/notifiers/base_test.go
@@ -13,30 +13,61 @@ import (
 
 func TestShouldSendAlertNotification(t *testing.T) {
 	tcs := []struct {
-		prevState m.AlertStateType
-		newState  m.AlertStateType
-		expected  bool
+		name         string
+		prevState    m.AlertStateType
+		newState     m.AlertStateType
+		expected     bool
+		sendReminder bool
 	}{
 		{
+			name:      "pending -> ok should not trigger an notification",
 			newState:  m.AlertStatePending,
 			prevState: m.AlertStateOK,
 			expected:  false,
 		},
 		{
+			name:      "ok -> alerting should trigger an notification",
 			newState:  m.AlertStateOK,
 			prevState: m.AlertStateAlerting,
 			expected:  true,
 		},
+		{
+			name:      "ok -> pending should not trigger an notification",
+			newState:  m.AlertStateOK,
+			prevState: m.AlertStatePending,
+			expected:  false,
+		},
+		{
+			name:         "ok -> ok should not trigger an notification",
+			newState:     m.AlertStateOK,
+			prevState:    m.AlertStateOK,
+			expected:     false,
+			sendReminder: false,
+		},
+		{
+			name:         "ok -> alerting should not trigger an notification",
+			newState:     m.AlertStateOK,
+			prevState:    m.AlertStateAlerting,
+			expected:     true,
+			sendReminder: true,
+		},
+		{
+			name:         "ok -> ok with reminder should not trigger an notification",
+			newState:     m.AlertStateOK,
+			prevState:    m.AlertStateOK,
+			expected:     false,
+			sendReminder: true,
+		},
 	}
 
 	for _, tc := range tcs {
-		context := alerting.NewEvalContext(context.TODO(), &alerting.Rule{
+		evalContext := alerting.NewEvalContext(context.TODO(), &alerting.Rule{
 			State: tc.newState,
 		})
-		context.Rule.State = tc.prevState
+		evalContext.Rule.State = tc.prevState
 		timeNow := time.Now()
-		if defaultShouldNotify(context, true, 0, &timeNow) != tc.expected {
-			t.Errorf("expected %v to return %v", tc, tc.expected)
+		if defaultShouldNotify(evalContext, true, 0, &timeNow) != tc.expected {
+			t.Errorf("failed %s. expected %+v to return %v", tc.name, tc, tc.expected)
 		}
 	}
 }
diff --git a/pkg/services/sqlstore/alert_notification.go b/pkg/services/sqlstore/alert_notification.go
index 26362dcb750..5f08a70eff3 100644
--- a/pkg/services/sqlstore/alert_notification.go
+++ b/pkg/services/sqlstore/alert_notification.go
@@ -262,7 +262,7 @@ func GetLatestNotification(ctx context.Context, cmd *m.GetLatestNotificationQuer
 
 func CleanNotificationJournal(ctx context.Context, cmd *m.CleanNotificationJournalCommand) error {
 	return inTransactionCtx(ctx, func(sess *DBSession) error {
-		sql := "DELETE FROM alert_notification_journal WHERE notification_journal.org_id = ? AND alert_notification_journal.alert_id = ? AND alert_notification_journal.notifier_id = ?"
+		sql := "DELETE FROM alert_notification_journal WHERE alert_notification_journal.org_id = ? AND alert_notification_journal.alert_id = ? AND alert_notification_journal.notifier_id = ?"
 		_, err := sess.Exec(sql, cmd.OrgId, cmd.AlertId, cmd.NotifierId)
 		return err
 	})