diff --git a/CHANGELOG.md b/CHANGELOG.md index e4f5bad2..e59d9f9f 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/hc/api/migrations/0072_auto_20200701_1007.py b/hc/api/migrations/0072_auto_20200701_1007.py new file mode 100644 index 00000000..d3ede3c2 --- /dev/null +++ b/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'), + ), + ] diff --git a/hc/api/models.py b/hc/api/models.py index 5c1b6ca6..749a93bc 100644 --- a/hc/api/models.py +++ b/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) diff --git a/hc/api/tests/test_notify.py b/hc/api/tests/test_notify.py index 36772d05..d8649cd1 100644 --- a/hc/api/tests/test_notify.py +++ b/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 - # without logging a notification: - self.assertEqual(Notification.objects.count(), 0) - self.assertEqual(len(mail.outbox), 0) + # If an email is not verified, it should say so in the notification: + n = Notification.objects.get() + self.assertEqual(n.error, "Email not verified") def test_email_checks_up_down_flags(self): payload = {"value": "alice@example.org", "up": True, "down": False} diff --git a/hc/api/transports.py b/hc/api/transports.py index e41b6fb2..302447e7 100644 --- a/hc/api/transports.py +++ b/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: diff --git a/hc/front/tests/test_channels.py b/hc/front/tests/test_channels.py index d6ea3113..7beb42a8 100644 --- a/hc/front/tests/test_channels.py +++ b/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" diff --git a/hc/front/tests/test_send_test_notification.py b/hc/front/tests/test_send_test_notification.py index 7a4ceb94..7918e323 100644 --- a/hc/front/tests/test_send_test_notification.py +++ b/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 diff --git a/hc/front/views.py b/hc/front/views.py index 9d793a4b..263742c7 100644 --- a/hc/front/views.py +++ b/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": - error = channel.transport.notify(dummy, channel.get_unsub_link()) - else: - error = channel.transport.notify(dummy) + # Delete all older test notifications for this channel + Notification.objects.filter(channel=channel, owner=None).delete() + + # Send the test notification + error = channel.notify(dummy, is_test=True) if error: messages.warning(request, "Could not send a test notification. %s" % error) diff --git a/templates/front/channels.html b/templates/front/channels.html index 6201e9d8..753d92b8 100644 --- a/templates/front/channels.html +++ b/templates/front/channels.html @@ -103,11 +103,7 @@ {% if ch.kind == "email" and not ch.email_verified %} - {% if n and n.error %} - Disabled - {% else %} - Unconfirmed - {% endif %} + Unconfirmed {% elif ch.kind == "hipchat" or ch.kind == "pagerteam" %} Retired {% else %}