From b06a2a6b772e984c140b59b7e5a6cd3cbde1d991 Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Fri, 16 Jun 2017 09:20:36 +0200 Subject: [PATCH 1/2] Fixed notifications. see #1132 --- .../management/commands/subcommands/_mx.py | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/modoboa/admin/management/commands/subcommands/_mx.py b/modoboa/admin/management/commands/subcommands/_mx.py index 740f514d4..7b3b9d407 100644 --- a/modoboa/admin/management/commands/subcommands/_mx.py +++ b/modoboa/admin/management/commands/subcommands/_mx.py @@ -147,25 +147,26 @@ class CheckMXRecords(BaseCommand): models.DNSBLResult.objects.bulk_create(to_create) if not alerts: return - emails = options["email"] + emails = list(options["email"]) if not options["skip_admin_emails"]: emails.extend( domain.admins.exclude(email="").values_list("email", flat=True) ) if not len(emails): return - content = render_to_string( - "admin/notifications/domain_in_dnsbl.html", { - "domain": domain, "alerts": alerts - }) - subject = _("[modoboa] DNSBL issue(s) for domain {}").format( - domain.name) - for email in emails: - status, msg = email_utils.sendmail_simple( - self.sender, email, - subject=subject, content=content) - if not status: - print(msg) + for domain, providers in list(alerts.items()): + content = render_to_string( + "admin/notifications/domain_in_dnsbl.html", { + "domain": domain, "alerts": providers + }) + subject = _("[modoboa] DNSBL issue(s) for domain {}").format( + domain.name) + for email in emails: + status, msg = email_utils.sendmail_simple( + self.sender, email, + subject=subject, content=content.strip()) + if not status: + print(msg) def check_valid_mx(self, domain, mx_list, **options): """Check that domain's MX record exist. @@ -195,7 +196,7 @@ class CheckMXRecords(BaseCommand): ) if not alerts: return - emails = options["email"] + emails = list(options["email"]) if not options["skip_admin_emails"]: emails.extend( domain.admins.exclude(email="").values_list("email", flat=True) @@ -211,7 +212,7 @@ class CheckMXRecords(BaseCommand): for email in emails: status, msg = email_utils.sendmail_simple( self.sender, email, - subject=subject, content=content) + subject=subject, content=content.strip()) if not status: print(msg) From e34dda90ae6b64ed8aeffabc673d2d4dadc9604b Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Fri, 16 Jun 2017 11:04:23 +0200 Subject: [PATCH 2/2] Increased coverage. see #1132 --- .../management/commands/subcommands/_mx.py | 49 ++++++++++--------- modoboa/admin/tests/test_mx.py | 14 +++++- 2 files changed, 38 insertions(+), 25 deletions(-) diff --git a/modoboa/admin/management/commands/subcommands/_mx.py b/modoboa/admin/management/commands/subcommands/_mx.py index 7b3b9d407..092641c61 100644 --- a/modoboa/admin/management/commands/subcommands/_mx.py +++ b/modoboa/admin/management/commands/subcommands/_mx.py @@ -11,6 +11,8 @@ import gevent from gevent import socket from django.conf import settings +from django.core import mail +from django.core.mail import EmailMessage from django.core.management.base import BaseCommand from django.template.loader import render_to_string from django.utils.functional import cached_property @@ -131,19 +133,24 @@ class CheckMXRecords(BaseCommand): result = "" if not results[mx] else results[mx] dnsbl_result = models.DNSBLResult.objects.filter( domain=domain, provider=provider, mx=mx).first() + trigger = False if dnsbl_result is None: to_create.append( models.DNSBLResult( domain=domain, provider=provider, mx=mx, status=result) ) + if result: + trigger = True else: - if not dnsbl_result.status and result: - if domain not in alerts: - alerts[domain] = [] - alerts[domain].append((provider, mx)) dnsbl_result.status = result dnsbl_result.save() + if not dnsbl_result.status and result: + trigger = True + if trigger: + if domain not in alerts: + alerts[domain] = [] + alerts[domain].append((provider, mx)) models.DNSBLResult.objects.bulk_create(to_create) if not alerts: return @@ -154,19 +161,19 @@ class CheckMXRecords(BaseCommand): ) if not len(emails): return - for domain, providers in list(alerts.items()): - content = render_to_string( - "admin/notifications/domain_in_dnsbl.html", { - "domain": domain, "alerts": providers - }) - subject = _("[modoboa] DNSBL issue(s) for domain {}").format( - domain.name) - for email in emails: - status, msg = email_utils.sendmail_simple( - self.sender, email, - subject=subject, content=content.strip()) - if not status: - print(msg) + with mail.get_connection() as connection: + for domain, providers in list(alerts.items()): + content = render_to_string( + "admin/notifications/domain_in_dnsbl.html", { + "domain": domain, "alerts": providers + }) + subject = _("[modoboa] DNSBL issue(s) for domain {}").format( + domain.name) + msg = EmailMessage( + subject, content.strip(), self.sender, emails, + connection=connection + ) + msg.send() def check_valid_mx(self, domain, mx_list, **options): """Check that domain's MX record exist. @@ -209,12 +216,8 @@ class CheckMXRecords(BaseCommand): }) subject = _("[modoboa] MX issue(s) for domain {}").format( domain.name) - for email in emails: - status, msg = email_utils.sendmail_simple( - self.sender, email, - subject=subject, content=content.strip()) - if not status: - print(msg) + msg = EmailMessage(subject, content.strip(), self.sender, emails) + msg.send() def check_domain(self, domain, timeout=3, ttl=7200, **options): """Check specified domain.""" diff --git a/modoboa/admin/tests/test_mx.py b/modoboa/admin/tests/test_mx.py index aa455f500..792027418 100644 --- a/modoboa/admin/tests/test_mx.py +++ b/modoboa/admin/tests/test_mx.py @@ -2,6 +2,9 @@ from __future__ import unicode_literals +from mock import patch + +from django.core import mail from django.core import management from django.core.urlresolvers import reverse from django.test import override_settings @@ -65,6 +68,7 @@ class MXTestCase(ModoTestCase): "modo", "check_mx", "--domain", "toto.com") +@override_settings(DNSBL_PROVIDERS=["zen.spamhaus.org"]) class DNSBLTestCase(ModoTestCase): """TestCase for DNSBL related features.""" @@ -80,7 +84,6 @@ class DNSBLTestCase(ModoTestCase): name="modoboa.com", enable_dns_checks=False) models.DNSBLResult.objects.all().delete() - @override_settings(DNSBL_PROVIDERS=["zen.spamhaus.org"]) def test_management_command(self): """Check that command works fine.""" self.assertEqual(models.DNSBLResult.objects.count(), 0) @@ -95,7 +98,14 @@ class DNSBLTestCase(ModoTestCase): self.assertFalse(self.domain.uses_a_reserved_tld) self.assertTrue(self.domain2.uses_a_reserved_tld) - @override_settings(DNSBL_PROVIDERS=["zen.spamhaus.org"]) + @patch("gevent.socket.gethostbyname") + def test_notifications(self, mock_gethostbyname): + """Check notifications.""" + mock_gethostbyname.return_value = "1.2.3.4" + management.call_command( + "modo", "check_mx", "--email", "user@example.test") + self.assertEqual(len(mail.outbox), 2) + def test_management_command_no_dnsbl(self): """Check that command works fine without dnsbl.""" self.assertEqual(models.DNSBLResult.objects.count(), 0)