Sending a test notification updates Channel.last_error. Fixes #391

CHANGELOG.md

@@ -13,6 +13,7 @@ All notable changes to this project will be documented in this file.
 
 ### Bug Fixes
 - Removing Pager Team integration, project appears to be discontinued
+- Sending a test notification updates Channel.last_error (#391)
 
 ## v1.15.0 - 2020-06-04
 

hc/api/migrations/0072_auto_20200701_1007.py

@@ -0,0 +1,19 @@
+# Generated by Django 3.0.7 on 2020-07-01 10:07
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('api', '0071_check_manual_resume'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='notification',
+            name='owner',
+            field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, to='api.Check'),
+        ),
+    ]

hc/api/models.py

@@ -459,11 +459,18 @@ class Channel(models.Model):
         else:
             raise NotImplementedError("Unknown channel kind: %s" % self.kind)
 
-    def notify(self, check):
+    def notify(self, check, is_test=False):
         if self.transport.is_noop(check):
             return "no-op"
 
-        n = Notification(owner=check, channel=self)
+        n = Notification(channel=self)
+        if is_test:
+            # When sending a test notification we leave the owner field null.
+            # (the passed check is a dummy, unsaved Check instance)
+            pass
+        else:
+            n.owner = check
+
         n.check_status = check.status
         n.error = "Sending"
         n.save()
@@ -735,7 +742,7 @@ class Notification(models.Model):
         get_latest_by = "created"
 
     code = models.UUIDField(default=uuid.uuid4, null=True, editable=False)
-    owner = models.ForeignKey(Check, models.CASCADE)
+    owner = models.ForeignKey(Check, models.CASCADE, null=True)
     check_status = models.CharField(max_length=6)
     channel = models.ForeignKey(Channel, models.CASCADE)
     created = models.DateTimeField(auto_now_add=True)

hc/api/tests/test_notify.py

@@ -327,14 +327,13 @@ class NotifyTestCase(BaseTestCase):
         email = mail.outbox[0]
         self.assertEqual(email.to[0], "alice@example.org")
 
-    def test_it_skips_unverified_email(self):
+    def test_it_reports_unverified_email(self):
         self._setup_data("email", "alice@example.org", email_verified=False)
         self.channel.notify(self.check)
 
-        # If an email is not verified, it should be skipped over
+        # If an email is not verified, it should say so in the notification:
-        # without logging a notification:
+        n = Notification.objects.get()
-        self.assertEqual(Notification.objects.count(), 0)
+        self.assertEqual(n.error, "Email not verified")
-        self.assertEqual(len(mail.outbox), 0)
 
     def test_email_checks_up_down_flags(self):
         payload = {"value": "alice@example.org", "up": True, "down": False}

hc/api/transports.py

@@ -88,9 +88,6 @@ class Email(Transport):
         emails.alert(self.channel.email_value, ctx, headers)
 
     def is_noop(self, check):
-        if not self.channel.email_verified:
-            return True
-
         if check.status == "down":
             return not self.channel.email_notify_down
         else:

hc/front/tests/test_channels.py

@@ -57,22 +57,6 @@ class ChannelsTestCase(BaseTestCase):
         self.assertEqual(r.status_code, 200)
         self.assertContains(r, "(normal priority)")
 
-    def test_it_shows_disabled_email(self):
-        check = Check(project=self.project, status="up")
-        check.save()
-
-        channel = Channel(project=self.project, kind="email")
-        channel.value = "alice@example.org"
-        channel.save()
-
-        n = Notification(owner=check, channel=channel, error="Invalid address")
-        n.save()
-
-        self.client.login(username="alice@example.org", password="password")
-        r = self.client.get(self.channels_url)
-        self.assertEqual(r.status_code, 200)
-        self.assertContains(r, "Disabled")
-
     def test_it_shows_unconfirmed_email(self):
         channel = Channel(project=self.project, kind="email")
         channel.value = "alice@example.org"

hc/front/tests/test_send_test_notification.py

@@ -2,7 +2,7 @@ import json
 from unittest.mock import patch
 
 from django.core import mail
-from hc.api.models import Channel
+from hc.api.models import Channel, Notification
 from hc.test import BaseTestCase
 
 
@@ -31,6 +31,34 @@ class SendTestNotificationTestCase(BaseTestCase):
         self.assertTrue("X-Bounce-Url" in email.extra_headers)
         self.assertTrue("List-Unsubscribe" in email.extra_headers)
 
+        # It should create a notification
+        n = Notification.objects.get()
+        self.assertEqual(n.channel, self.channel)
+        self.assertEqual(n.error, "")
+
+    def test_it_clears_channel_last_error(self):
+        self.channel.last_error = "Something went wrong"
+        self.channel.save()
+
+        self.client.login(username="alice@example.org", password="password")
+        self.client.post(self.url, {})
+
+        self.channel.refresh_from_db()
+        self.assertEqual(self.channel.last_error, "")
+
+    def test_it_sets_channel_last_error(self):
+        self.channel.email_verified = False
+        self.channel.save()
+
+        self.client.login(username="alice@example.org", password="password")
+        r = self.client.post(self.url, {}, follow=True)
+
+        self.assertContains(r, "Could not send a test notification")
+        self.assertContains(r, "Email not verified")
+
+        self.channel.refresh_from_db()
+        self.assertEqual(self.channel.last_error, "Email not verified")
+
     @patch("hc.api.transports.requests.request")
     def test_it_handles_webhooks_with_no_down_url(self, mock_get):
         mock_get.return_value.status_code = 200

hc/front/views.py

@@ -795,10 +795,11 @@ def send_test_notification(request, code):
             # send "TEST is UP" notification instead:
             dummy.status = "up"
 
-    if channel.kind == "email":
+    # Delete all older test notifications for this channel
-        error = channel.transport.notify(dummy, channel.get_unsub_link())
+    Notification.objects.filter(channel=channel, owner=None).delete()
-    else:
+
-        error = channel.transport.notify(dummy)
+    # Send the test notification
+    error = channel.notify(dummy, is_test=True)
 
     if error:
         messages.warning(request, "Could not send a test notification. %s" % error)

templates/front/channels.html

@@ -103,11 +103,7 @@
             </td>
             <td>
                 {% if ch.kind == "email" and not ch.email_verified %}
-                    {% if n and n.error %}
+                    <span class="label label-default">Unconfirmed</span>
-                        <span class="label label-danger">Disabled</span>
-                    {% else %}
-                        <span class="label label-default">Unconfirmed</span>
-                    {% endif %}
                 {% elif ch.kind == "hipchat" or ch.kind == "pagerteam" %}
                     Retired
                 {% else %}