From: Jeremy Kerr Date: Tue, 29 Mar 2011 14:18:54 +0000 (+0800) Subject: notifications: Add code to send notifications X-Git-Url: https://git.ozlabs.org/?p=patchwork;a=commitdiff_plain;h=f94d40159168d0811de576328b77fd2a553039af notifications: Add code to send notifications Add a function (patchwork.utils.send_notifications) to process the PatchChangeNotification queue. We try to group mail to the same sender, by waiting settings.NOTIFICATION_DELAY_MINUTES to allow other notifications to arrive. Signed-off-by: Jeremy Kerr --- diff --git a/apps/patchwork/bin/patchwork-cron.py b/apps/patchwork/bin/patchwork-cron.py new file mode 100755 index 0000000..e9bd0c1 --- /dev/null +++ b/apps/patchwork/bin/patchwork-cron.py @@ -0,0 +1,13 @@ +#!/usr/bin/env python + +import sys +from patchwork.utils import send_notifications + +def main(args): + errors = send_notifications() + for (recipient, error) in errors: + print "Failed sending to %s: %s" % (recipient.email, ex) + +if __name__ == '__main__': + sys.exit(main(sys.argv)) + diff --git a/apps/patchwork/tests/notifications.py b/apps/patchwork/tests/notifications.py index c4df1b0..ae37988 100644 --- a/apps/patchwork/tests/notifications.py +++ b/apps/patchwork/tests/notifications.py @@ -17,11 +17,15 @@ # along with Patchwork; if not, write to the Free Software # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA +import datetime from django.test import TestCase from django.core.urlresolvers import reverse +from django.core import mail +from django.conf import settings from django.db.utils import IntegrityError from patchwork.models import Patch, State, PatchChangeNotification from patchwork.tests.utils import defaults, create_maintainer +from patchwork.utils import send_notifications class PatchNotificationModelTest(TestCase): """Tests for the creation & update of the PatchChangeNotification model""" @@ -115,3 +119,111 @@ class PatchNotificationModelTest(TestCase): self.patch.save() self.assertEqual(PatchChangeNotification.objects.count(), 0) +class PatchNotificationEmailTest(TestCase): + + def setUp(self): + self.project = defaults.project + self.project.send_notifications = True + self.project.save() + self.submitter = defaults.patch_author_person + self.submitter.save() + self.patch = Patch(project = self.project, msgid = 'testpatch', + name = 'testpatch', content = '', + submitter = self.submitter) + self.patch.save() + + def tearDown(self): + self.patch.delete() + self.submitter.delete() + self.project.delete() + + def _expireNotifications(self, **kwargs): + timestamp = datetime.datetime.now() - \ + datetime.timedelta(minutes = + settings.NOTIFICATION_DELAY_MINUTES + 1) + + qs = PatchChangeNotification.objects.all() + if kwargs: + qs = qs.filter(**kwargs) + + qs.update(last_modified = timestamp) + + def testNoNotifications(self): + self.assertEquals(send_notifications(), []) + + def testNoReadyNotifications(self): + """ We shouldn't see immediate notifications""" + PatchChangeNotification(patch = self.patch, + orig_state = self.patch.state).save() + + errors = send_notifications() + self.assertEquals(errors, []) + self.assertEquals(len(mail.outbox), 0) + + def testNotifications(self): + PatchChangeNotification(patch = self.patch, + orig_state = self.patch.state).save() + self._expireNotifications() + + errors = send_notifications() + self.assertEquals(errors, []) + self.assertEquals(len(mail.outbox), 1) + msg = mail.outbox[0] + self.assertEquals(msg.to, [self.submitter.email]) + self.assertTrue(self.patch.get_absolute_url() in msg.body) + + def testNotificationMerge(self): + patches = [self.patch, + Patch(project = self.project, msgid = 'testpatch-2', + name = 'testpatch 2', content = '', + submitter = self.submitter)] + + for patch in patches: + patch.save() + PatchChangeNotification(patch = patch, + orig_state = patch.state).save() + + self.assertEquals(PatchChangeNotification.objects.count(), len(patches)) + self._expireNotifications() + errors = send_notifications() + self.assertEquals(errors, []) + self.assertEquals(len(mail.outbox), 1) + msg = mail.outbox[0] + self.assertTrue(patches[0].get_absolute_url() in msg.body) + self.assertTrue(patches[1].get_absolute_url() in msg.body) + + def testUnexpiredNotificationMerge(self): + """Test that when there are multiple pending notifications, with + at least one within the notification delay, that other notifications + are held""" + patches = [self.patch, + Patch(project = self.project, msgid = 'testpatch-2', + name = 'testpatch 2', content = '', + submitter = self.submitter)] + + for patch in patches: + patch.save() + PatchChangeNotification(patch = patch, + orig_state = patch.state).save() + + self.assertEquals(PatchChangeNotification.objects.count(), len(patches)) + self._expireNotifications() + + # update one notification, to bring it out of the notification delay + patches[0].state = State.objects.exclude(pk = patches[0].state.pk)[0] + patches[0].save() + + # the updated notification should prevent the other from being sent + errors = send_notifications() + self.assertEquals(errors, []) + self.assertEquals(len(mail.outbox), 0) + + # expire the updated notification + self._expireNotifications() + + errors = send_notifications() + self.assertEquals(errors, []) + self.assertEquals(len(mail.outbox), 1) + msg = mail.outbox[0] + self.assertTrue(patches[0].get_absolute_url() in msg.body) + self.assertTrue(patches[1].get_absolute_url() in msg.body) diff --git a/apps/patchwork/utils.py b/apps/patchwork/utils.py index e41ffb6..94b3f53 100644 --- a/apps/patchwork/utils.py +++ b/apps/patchwork/utils.py @@ -18,8 +18,17 @@ # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA -from patchwork.models import Bundle, Project, BundlePatch +import itertools +import datetime from django.shortcuts import get_object_or_404 +from django.template.loader import render_to_string +from django.contrib.sites.models import Site +from django.conf import settings +from django.core.mail import EmailMessage +from django.db.models import Max +from patchwork.forms import MultiplePatchForm +from patchwork.models import Bundle, Project, BundlePatch, UserProfile, \ + PatchChangeNotification def get_patch_ids(d, prefix = 'patch_id'): ids = [] @@ -137,3 +146,51 @@ def set_bundle(user, project, action, data, patches, context): bundle.save() return [] + +def send_notifications(): + date_limit = datetime.datetime.now() - \ + datetime.timedelta(minutes = + settings.NOTIFICATION_DELAY_MINUTES) + + # This gets funky: we want to filter out any notifications that should + # be grouped with other notifications that aren't ready to go out yet. To + # do that, we join back onto PatchChangeNotification (PCN -> Patch -> + # Person -> Patch -> max(PCN.last_modified)), filtering out any maxima + # that are with the date_limit. + qs = PatchChangeNotification.objects \ + .annotate(m = Max('patch__submitter__patch__patchchangenotification' + '__last_modified')) \ + .filter(m__lt = date_limit) + + groups = itertools.groupby(qs.order_by('patch__submitter'), + lambda n: n.patch.submitter) + + errors = [] + + for (recipient, notifications) in groups: + notifications = list(notifications) + context = { + 'site': Site.objects.get_current(), + 'person': recipient, + 'notifications': notifications, + } + subject = render_to_string( + 'patchwork/patch-change-notification-subject.text', + context).strip() + content = render_to_string('patchwork/patch-change-notification.mail', + context) + + message = EmailMessage(subject = subject, body = content, + from_email = settings.DEFAULT_FROM_EMAIL, + to = [recipient.email], + headers = {'Precedence': 'bulk'}) + + try: + message.send() + except ex: + errors.append((recipient, ex)) + continue + + PatchChangeNotification.objects.filter(pk__in = notifications).delete() + + return errors diff --git a/apps/settings.py b/apps/settings.py index 8f091d0..d5595e0 100644 --- a/apps/settings.py +++ b/apps/settings.py @@ -103,6 +103,8 @@ DEFAULT_FROM_EMAIL = 'Patchwork ' CONFIRMATION_VALIDITY_DAYS = 7 +NOTIFICATION_DELAY_MINUTES = 10 + # Set to True to enable the Patchwork XML-RPC interface ENABLE_XMLRPC = False diff --git a/templates/patchwork/patch-change-notification-subject.text b/templates/patchwork/patch-change-notification-subject.text new file mode 100644 index 0000000..02ee55b --- /dev/null +++ b/templates/patchwork/patch-change-notification-subject.text @@ -0,0 +1 @@ +Patch update notification: {{notifications|length}} patch{{notifications|length|pluralize:"es"}} updated diff --git a/templates/patchwork/patch-change-notification.mail b/templates/patchwork/patch-change-notification.mail new file mode 100644 index 0000000..d86a6af --- /dev/null +++ b/templates/patchwork/patch-change-notification.mail @@ -0,0 +1,19 @@ +Hello, + +The following patch{{notifications|length|pluralize:"es"}} (submitted by you) {{notifications|length|pluralize:"has,have"}} been updated in patchwork: +{% for notification in notifications %} + * {{notification.patch.name}} + - http://{{site.domain}}{{notification.patch.get_absolute_url}} + was: {{notification.orig_state}} + now: {{notification.patch.state}} +{% endfor %} +This email is a notification only - you do not need to respond. + +Happy patchworking. + +-- + +This is an automated mail sent by the patchwork system at +{{site.domain}}. To stop receiving these notifications, edit +your mail settings at: + http://{{site.domain}}{% url patchwork.views.mail.settings %}