From 8fe8e0f605ca0c969d9c4929841e716d25128f80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C4=93teris=20Caune?= Date: Sat, 26 Dec 2020 18:11:36 +0200 Subject: [PATCH] Update alert email template: more information, less styling Fixes: #348 --- hc/accounts/models.py | 11 ++ hc/api/tests/test_notify.py | 52 --------- hc/api/tests/test_notify_email.py | 151 ++++++++++++++++++++++++++ hc/api/transports.py | 19 ++-- hc/front/views.py | 2 +- templates/emails/alert-body-html.html | 122 +++++++++++++++++---- 6 files changed, 271 insertions(+), 86 deletions(-) create mode 100644 hc/api/tests/test_notify_email.py diff --git a/hc/accounts/models.py b/hc/accounts/models.py index a67e9396..878944d3 100644 --- a/hc/accounts/models.py +++ b/hc/accounts/models.py @@ -343,6 +343,14 @@ class Project(models.Model): break return status + def get_n_down(self): + result = 0 + for check in self.check_set.all(): + if check.get_status() == "down": + result += 1 + + return result + def have_channel_issues(self): errors = list(self.channel_set.values_list("last_error", flat=True)) @@ -363,6 +371,9 @@ class Project(models.Model): frag = urlencode({self.api_key_readonly: str(self)}, quote_via=quote) return reverse("hc-dashboard") + "#" + frag + def checks_url(self): + return settings.SITE_ROOT + reverse("hc-checks", args=[self.code]) + class Member(models.Model): user = models.ForeignKey(User, models.CASCADE, related_name="memberships") diff --git a/hc/api/tests/test_notify.py b/hc/api/tests/test_notify.py index 637966da..a5679eee 100644 --- a/hc/api/tests/test_notify.py +++ b/hc/api/tests/test_notify.py @@ -329,58 +329,6 @@ class NotifyTestCase(BaseTestCase): "get", "http://foo.com", headers=headers, timeout=5 ) - def test_email(self): - self._setup_data("email", "alice@example.org") - self.channel.notify(self.check) - - n = Notification.objects.get() - self.assertEqual(n.error, "") - - # And email should have been sent - self.assertEqual(len(mail.outbox), 1) - - email = mail.outbox[0] - self.assertEqual(email.to[0], "alice@example.org") - self.assertTrue("X-Status-Url" in email.extra_headers) - self.assertTrue("List-Unsubscribe" in email.extra_headers) - self.assertTrue("List-Unsubscribe-Post" in email.extra_headers) - - def test_email_transport_handles_json_value(self): - payload = {"value": "alice@example.org", "up": True, "down": True} - self._setup_data("email", json.dumps(payload)) - self.channel.notify(self.check) - - # And email should have been sent - self.assertEqual(len(mail.outbox), 1) - - email = mail.outbox[0] - self.assertEqual(email.to[0], "alice@example.org") - - 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 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} - self._setup_data("email", json.dumps(payload)) - self.channel.notify(self.check) - - # This channel should not notify on "down" events: - self.assertEqual(Notification.objects.count(), 0) - self.assertEqual(len(mail.outbox), 0) - - def test_email_handles_amperstand(self): - self._setup_data("email", "alice@example.org") - self.check.name = "Foo & Bar" - self.channel.notify(self.check) - - email = mail.outbox[0] - self.assertEqual(email.subject, "DOWN | Foo & Bar") - @patch("hc.api.transports.requests.request") def test_pd(self, mock_post): self._setup_data("pd", "123") diff --git a/hc/api/tests/test_notify_email.py b/hc/api/tests/test_notify_email.py new file mode 100644 index 00000000..cbcc4408 --- /dev/null +++ b/hc/api/tests/test_notify_email.py @@ -0,0 +1,151 @@ +# coding: utf-8 + +from datetime import timedelta as td +import json + +from django.core import mail +from django.utils.timezone import now +from hc.api.models import Channel, Check, Notification, Ping +from hc.test import BaseTestCase + + +class NotifyTestCase(BaseTestCase): + def setUp(self): + super().setUp() + + self.check = Check(project=self.project) + self.check.name = "Daily Backup" + self.check.desc = "Line 1\nLine2" + self.check.tags = "foo bar" + self.check.status = "down" + self.check.last_ping = now() - td(minutes=61) + self.check.n_pings = 112233 + self.check.save() + + self.ping = Ping(owner=self.check) + self.ping.remote_addr = "1.2.3.4" + self.ping.body = "Body Line 1\nBody Line 2" + self.ping.save() + + self.channel = Channel(project=self.project) + self.channel.kind = "email" + self.channel.value = "alice@example.org" + self.channel.email_verified = True + self.channel.save() + self.channel.checks.add(self.check) + + def test_email(self): + self.channel.notify(self.check) + + n = Notification.objects.get() + self.assertEqual(n.error, "") + + # And email should have been sent + self.assertEqual(len(mail.outbox), 1) + + email = mail.outbox[0] + self.assertEqual(email.to[0], "alice@example.org") + self.assertTrue("X-Status-Url" in email.extra_headers) + self.assertTrue("List-Unsubscribe" in email.extra_headers) + self.assertTrue("List-Unsubscribe-Post" in email.extra_headers) + + html = email.alternatives[0][0] + self.assertIn("Daily Backup", html) + self.assertIn("Line 1
Line2", html) + self.assertIn("Alices Project", html) + self.assertIn("foo", html) + self.assertIn("bar", html) + self.assertIn("1 day", html) + self.assertIn("from 1.2.3.4", html) + self.assertIn("112233", html) + self.assertIn("Body Line 1
Body Line 2", html) + + def test_it_shows_cron_schedule(self): + self.check.kind = "cron" + self.check.schedule = "0 18-23,0-8 * * *" + self.check.save() + + self.channel.notify(self.check) + + email = mail.outbox[0] + html = email.alternatives[0][0] + + self.assertIn("0 18-23,0-8 * * *", html) + + def test_it_truncates_long_body(self): + self.ping.body = "X" * 10000 + ", and the rest gets cut off" + self.ping.save() + + self.channel.notify(self.check) + + email = mail.outbox[0] + html = email.alternatives[0][0] + + self.assertIn("[truncated]", html) + self.assertNotIn("the rest gets cut off", html) + + def test_it_handles_missing_ping_object(self): + self.ping.delete() + + self.channel.notify(self.check) + + email = mail.outbox[0] + html = email.alternatives[0][0] + + self.assertIn("Daily Backup", html) + + def test_it_handles_missing_profile(self): + self.channel.value = "alice+notifications@example.org" + self.channel.save() + + self.channel.notify(self.check) + + email = mail.outbox[0] + self.assertEqual(email.to[0], "alice+notifications@example.org") + + html = email.alternatives[0][0] + self.assertIn("Daily Backup", html) + self.assertNotIn("Projects Overview", html) + + def test_email_transport_handles_json_value(self): + payload = {"value": "alice@example.org", "up": True, "down": True} + self.channel.value = json.dumps(payload) + self.channel.save() + + self.channel.notify(self.check) + + # And email should have been sent + self.assertEqual(len(mail.outbox), 1) + + email = mail.outbox[0] + self.assertEqual(email.to[0], "alice@example.org") + + def test_it_reports_unverified_email(self): + self.channel.email_verified = False + self.channel.save() + + self.channel.notify(self.check) + + # 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} + self.channel.value = json.dumps(payload) + self.channel.save() + + self.channel.notify(self.check) + + # This channel should not notify on "down" events: + self.assertEqual(Notification.objects.count(), 0) + self.assertEqual(len(mail.outbox), 0) + + def test_email_handles_amperstand(self): + self.check.name = "Foo & Bar" + self.check.save() + + self.channel.notify(self.check) + + email = mail.outbox[0] + self.assertEqual(email.subject, "DOWN | Foo & Bar") diff --git a/hc/api/transports.py b/hc/api/transports.py index 4da12981..ca5ca3ac 100644 --- a/hc/api/transports.py +++ b/hc/api/transports.py @@ -68,21 +68,20 @@ class Email(Transport): "List-Unsubscribe-Post": "List-Unsubscribe=One-Click", } + from hc.accounts.models import Profile + + # If this email address has an associated account, include + # a summary of projects the account has access to try: - # Look up the sorting preference for this email address - p = Profile.objects.get(user__email=self.channel.email_value) - sort = p.sort + profile = Profile.objects.get(user__email=self.channel.email_value) + projects = list(profile.projects()) except Profile.DoesNotExist: - # Default sort order is by check's creation time - sort = "created" + projects = None - # list() executes the query, to avoid DB access while - # rendering a template ctx = { "check": check, - "checks": list(self.checks()), - "sort": sort, - "now": timezone.now(), + "ping": check.ping_set.order_by("created").last(), + "projects": projects, "unsub_link": unsub_link, } diff --git a/hc/front/views.py b/hc/front/views.py index f4ece3c6..7d045a8a 100644 --- a/hc/front/views.py +++ b/hc/front/views.py @@ -848,7 +848,7 @@ def unsubscribe_email(request, code, signed_token): def send_test_notification(request, code): channel, rw = _get_channel_for_user(request, code) - dummy = Check(name="TEST", status="down") + dummy = Check(name="TEST", status="down", project=channel.project) dummy.last_ping = timezone.now() - td(days=1) dummy.n_pings = 42 diff --git a/templates/emails/alert-body-html.html b/templates/emails/alert-body-html.html index 54bf31e4..a99a5bb4 100644 --- a/templates/emails/alert-body-html.html +++ b/templates/emails/alert-body-html.html @@ -1,32 +1,108 @@ -{% extends "emails/base.html" %} -{% load hc_extras %} -{% block content %} - -The check {{ check.name_then_code|mangle_link }} -has gone {{ check.status|upper }}. -
- -{% if check.status == "down" and check.desc %} -Additional notes:

-
- {{ check.desc|linebreaksbr|urlize }} -
+{% load hc_extras humanize %} + +

+ "{{ check.name_then_code }}" is {{ check.status|upper }}. + View on {% site_name %}… +

+ + +{% if check.desc %} +

+ Description
+ {{ check.desc|linebreaksbr }} +

{% endif %} -
-A summary of your checks: -
+{% cycle '' '' as trtr silent %} + + + {% if check.project.name %} + + {{ trtr|safe }} {% cycle trtr %} + {% endif %} + + {% if check.tags_list %} + + {{ trtr|safe }} {% cycle trtr %} + {% endif %} + + {% if check.kind == "simple" %} + + {{ trtr|safe }} {% cycle trtr %} + {% endif %} -{% include "emails/summary-html.html" %} + {% if check.kind == "cron" %} + + {% if trttr %}Yo!{% endif %} + {{ trtr|safe }} {% cycle trtr %} + {% endif %} -Thanks,
-The {% site_name %} Team + {% if ping %} + + {% if trttr %}Yo!{% endif %} + {{ trtr|safe }} {% cycle trtr %} + {% endif %} -{% endblock %} + + +
+ Project
+ {{ check.project.name }} + 		+ Tags
+ {% for tag in check.tags_list %} + {{ tag }} + {% endfor %} + 		+ Period
+ {{ check.timeout|hc_duration }} + 		+ Schedule
+ {{ check.schedule }} + 		+ Last Ping
+ {{ ping.created|naturaltime }}{% if ping.remote_addr %}, from {{ ping.remote_addr }}{% endif %} + 		+ Total Pings
+ {{ check.n_pings }} + {% if check.created %}(since {{ check.created|date:'M j, Y' }}){% endif %} +
+ +{% if ping.body %} +

Last Ping Body

+
{{ ping.body|slice:":10000"|linebreaksbr }}{% if ping.body|length > 10000 %} [truncated]{% endif %}
+{% endif %} + +{% if projects %} +

Projects Overview

+ + {% for project in projects %} + + + + + {% endfor %} +
+ {{ project }} + + {% with project.get_n_down as n_down %} + {% if n_down %} + {{ n_down }} check{{ n_down|pluralize }} down + {% else %} + OK, all checks up + {% endif %} + {% endwith %} +
+{% endif %} -{% block unsub %} -
+

+—
+{% site_name %}
+ {% if check.project.name %} + Unsubscribe from "{{ check.project.name }}" notifications + {% else %} Unsubscribe + {% endif %} -{% endblock %} +