From 56e2243f3be7e859666ce0e4e1a8b8b94444f8d4 Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Thu, 12 Aug 2010 12:15:48 +0800 Subject: Use generic email confirmation object Rather than having a UserPerson-specific confirmation, add an EmailConfirmation object to allow multiple types of confirmations (eg, opt-out requests in future). To do this, we use a view (patchwork.views.confirm) that will call the type-specific view with the confirmation object. Also, add tests to check that the User/Person linkage system works. Signed-off-by: Jeremy Kerr --- apps/patchwork/models.py | 29 ++++---- apps/patchwork/tests/__init__.py | 1 + apps/patchwork/tests/confirm.py | 67 ++++++++++++++++++ apps/patchwork/tests/user.py | 121 ++++++++++++++++++++++++++++++++ apps/patchwork/urls.py | 3 +- apps/patchwork/views/base.py | 24 ++++++- apps/patchwork/views/user.py | 24 ++++--- lib/sql/grant-all.mysql.sql | 2 +- lib/sql/grant-all.postgres.sql | 4 +- lib/sql/migration/008-confirmations.sql | 11 +++ templates/patchwork/confirm-error.html | 19 +++++ templates/patchwork/user-link.mail | 2 +- 12 files changed, 274 insertions(+), 33 deletions(-) create mode 100644 apps/patchwork/tests/confirm.py create mode 100644 apps/patchwork/tests/user.py create mode 100644 lib/sql/migration/008-confirmations.sql create mode 100644 templates/patchwork/confirm-error.html diff --git a/apps/patchwork/models.py b/apps/patchwork/models.py index 6c8fc71..ee6748f 100644 --- a/apps/patchwork/models.py +++ b/apps/patchwork/models.py @@ -373,34 +373,29 @@ class BundlePatch(models.Model): unique_together = [('bundle', 'patch')] ordering = ['order'] -class UserPersonConfirmation(models.Model): - user = models.ForeignKey(User) +class EmailConfirmation(models.Model): + validity = datetime.timedelta(days = 30) + type = models.CharField(max_length = 20, choices = [ + ('userperson', 'User-Person association'), + ]) email = models.CharField(max_length = 200) + user = models.ForeignKey(User, null = True) key = HashField() - date = models.DateTimeField(default=datetime.datetime.now) + date = models.DateTimeField(default = datetime.datetime.now) active = models.BooleanField(default = True) - def confirm(self): - if not self.active: - return - person = None - try: - person = Person.objects.get(email__iexact = self.email) - except Exception: - pass - if not person: - person = Person(email = self.email) - - person.link_to_user(self.user) - person.save() + def deactivate(self): self.active = False self.save() + def is_valid(self): + return self.date + self.validity > datetime.datetime.now() + def save(self): max = 1 << 32 if self.key == '': str = '%s%s%d' % (self.user, self.email, random.randint(0, max)) self.key = self._meta.get_field('key').construct(str).hexdigest() - super(UserPersonConfirmation, self).save() + super(EmailConfirmation, self).save() diff --git a/apps/patchwork/tests/__init__.py b/apps/patchwork/tests/__init__.py index 68fe563..9618d1f 100644 --- a/apps/patchwork/tests/__init__.py +++ b/apps/patchwork/tests/__init__.py @@ -23,3 +23,4 @@ from patchwork.tests.bundles import * from patchwork.tests.mboxviews import * from patchwork.tests.updates import * from patchwork.tests.filters import * +from patchwork.tests.confirm import * diff --git a/apps/patchwork/tests/confirm.py b/apps/patchwork/tests/confirm.py new file mode 100644 index 0000000..fad5125 --- /dev/null +++ b/apps/patchwork/tests/confirm.py @@ -0,0 +1,67 @@ +# Patchwork - automated patch tracking system +# Copyright (C) 2011 Jeremy Kerr +# +# This file is part of the Patchwork package. +# +# Patchwork is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# Patchwork is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Patchwork; if not, write to the Free Software +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + +import unittest +from django.test import TestCase +from django.contrib.auth.models import User +from django.core.urlresolvers import reverse +from patchwork.models import EmailConfirmation, Person + +def _confirmation_url(conf): + return reverse('patchwork.views.confirm', kwargs = {'key': conf.key}) + +class TestUser(object): + username = 'testuser' + email = 'test@example.com' + secondary_email = 'test2@example.com' + password = None + + def __init__(self): + self.password = User.objects.make_random_password() + self.user = User.objects.create_user(self.username, + self.email, self.password) + +class InvalidConfirmationTest(TestCase): + def setUp(self): + EmailConfirmation.objects.all().delete() + Person.objects.all().delete() + self.user = TestUser() + self.conf = EmailConfirmation(type = 'userperson', + email = self.user.secondary_email, + user = self.user.user) + self.conf.save() + + def testInactiveConfirmation(self): + self.conf.active = False + self.conf.save() + response = self.client.get(_confirmation_url(self.conf)) + self.assertEquals(response.status_code, 200) + self.assertTemplateUsed(response, 'patchwork/confirm-error.html') + self.assertEqual(response.context['error'], 'inactive') + self.assertEqual(response.context['conf'], self.conf) + + def testExpiredConfirmation(self): + self.conf.date -= self.conf.validity + self.conf.save() + response = self.client.get(_confirmation_url(self.conf)) + self.assertEquals(response.status_code, 200) + self.assertTemplateUsed(response, 'patchwork/confirm-error.html') + self.assertEqual(response.context['error'], 'expired') + self.assertEqual(response.context['conf'], self.conf) + diff --git a/apps/patchwork/tests/user.py b/apps/patchwork/tests/user.py new file mode 100644 index 0000000..c9e5be3 --- /dev/null +++ b/apps/patchwork/tests/user.py @@ -0,0 +1,121 @@ +# Patchwork - automated patch tracking system +# Copyright (C) 2010 Jeremy Kerr +# +# This file is part of the Patchwork package. +# +# Patchwork is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# Patchwork is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Patchwork; if not, write to the Free Software +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + +import unittest +from django.test import TestCase +from django.test.client import Client +from django.core import mail +from django.core.urlresolvers import reverse +from django.contrib.auth.models import User +from patchwork.models import EmailConfirmation, Person +from patchwork.utils import userprofile_register_callback + +def _confirmation_url(conf): + return reverse('patchwork.views.confirm', kwargs = {'key': conf.key}) + +class TestUser(object): + username = 'testuser' + email = 'test@example.com' + secondary_email = 'test2@example.com' + password = None + + def __init__(self): + self.password = User.objects.make_random_password() + self.user = User.objects.create_user(self.username, + self.email, self.password) + userprofile_register_callback(self.user) + +class UserPersonRequestTest(TestCase): + def setUp(self): + self.user = TestUser() + self.client.login(username = self.user.username, + password = self.user.password) + EmailConfirmation.objects.all().delete() + + def testUserPersonRequestForm(self): + response = self.client.get('/user/link/') + self.assertEquals(response.status_code, 200) + self.assertTrue(response.context['linkform']) + + def testUserPersonRequestEmpty(self): + response = self.client.post('/user/link/', {'email': ''}) + self.assertEquals(response.status_code, 200) + self.assertTrue(response.context['linkform']) + self.assertFormError(response, 'linkform', 'email', + 'This field is required.') + + def testUserPersonRequestInvalid(self): + response = self.client.post('/user/link/', {'email': 'foo'}) + self.assertEquals(response.status_code, 200) + self.assertTrue(response.context['linkform']) + self.assertFormError(response, 'linkform', 'email', + 'Enter a valid e-mail address.') + + def testUserPersonRequestValid(self): + response = self.client.post('/user/link/', + {'email': self.user.secondary_email}) + self.assertEquals(response.status_code, 200) + self.assertTrue(response.context['confirmation']) + + # check that we have a confirmation saved + self.assertEquals(EmailConfirmation.objects.count(), 1) + conf = EmailConfirmation.objects.all()[0] + self.assertEquals(conf.user, self.user.user) + self.assertEquals(conf.email, self.user.secondary_email) + self.assertEquals(conf.type, 'userperson') + + # check that an email has gone out... + self.assertEquals(len(mail.outbox), 1) + msg = mail.outbox[0] + self.assertEquals(msg.subject, 'Patchwork email address confirmation') + self.assertTrue(self.user.secondary_email in msg.to) + self.assertTrue(_confirmation_url(conf) in msg.body) + + # ...and that the URL is valid + response = self.client.get(_confirmation_url(conf)) + self.assertEquals(response.status_code, 200) + self.assertTemplateUsed(response, 'patchwork/user-link-confirm.html') + +class UserPersonConfirmTest(TestCase): + def setUp(self): + EmailConfirmation.objects.all().delete() + Person.objects.all().delete() + self.user = TestUser() + self.client.login(username = self.user.username, + password = self.user.password) + self.conf = EmailConfirmation(type = 'userperson', + email = self.user.secondary_email, + user = self.user.user) + self.conf.save() + + def testUserPersonConfirm(self): + self.assertEquals(Person.objects.count(), 1) + response = self.client.get(_confirmation_url(self.conf)) + self.assertEquals(response.status_code, 200) + + # check that the Person object has been created and linked + self.assertEquals(Person.objects.count(), 2) + person = Person.objects.get(email = self.user.secondary_email) + self.assertEquals(person.email, self.user.secondary_email) + self.assertEquals(person.user, self.user.user) + + # check that the confirmation has been marked as inactive. We + # need to reload the confirmation to check this. + conf = EmailConfirmation.objects.get(pk = self.conf.pk) + self.assertEquals(conf.active, False) diff --git a/apps/patchwork/urls.py b/apps/patchwork/urls.py index b49b4e1..27c79fd 100644 --- a/apps/patchwork/urls.py +++ b/apps/patchwork/urls.py @@ -44,13 +44,14 @@ urlpatterns = patterns('', 'patchwork.views.bundle.mbox'), (r'^user/link/$', 'patchwork.views.user.link'), - (r'^user/link/(?P[^/]+)/$', 'patchwork.views.user.link_confirm'), (r'^user/unlink/(?P[^/]+)/$', 'patchwork.views.user.unlink'), # public view for bundles (r'^bundle/(?P[^/]*)/(?P[^/]*)/$', 'patchwork.views.bundle.public'), + (r'^confirm/(?P[0-9a-f]+)/$', 'patchwork.views.confirm'), + # submitter autocomplete (r'^submitter/$', 'patchwork.views.submitter_complete'), diff --git a/apps/patchwork/views/base.py b/apps/patchwork/views/base.py index c0e68ed..1539472 100644 --- a/apps/patchwork/views/base.py +++ b/apps/patchwork/views/base.py @@ -18,7 +18,7 @@ # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA -from patchwork.models import Patch, Project, Person +from patchwork.models import Patch, Project, Person, EmailConfirmation from django.shortcuts import render_to_response, get_object_or_404 from django.http import HttpResponse, HttpResponseRedirect, Http404 from patchwork.requestcontext import PatchworkRequestContext @@ -58,6 +58,28 @@ def pwclient(request): response.write(render_to_string('patchwork/pwclient', context)) return response +def confirm(request, key): + import patchwork.views.user + views = { + 'userperson': patchwork.views.user.link_confirm, + } + + conf = get_object_or_404(EmailConfirmation, key = key) + if conf.type not in views: + raise Http404 + + if conf.active and conf.is_valid(): + return views[conf.type](request, conf) + + context = PatchworkRequestContext(request) + context['conf'] = conf + if not conf.active: + context['error'] = 'inactive' + elif not conf.is_valid(): + context['error'] = 'expired' + + return render_to_response('patchwork/confirm-error.html', context) + def submitter_complete(request): search = request.GET.get('q', '') response = HttpResponse(mimetype = "text/plain") diff --git a/apps/patchwork/views/user.py b/apps/patchwork/views/user.py index 1ae3c2d..759a6e3 100644 --- a/apps/patchwork/views/user.py +++ b/apps/patchwork/views/user.py @@ -22,8 +22,7 @@ from django.contrib.auth.decorators import login_required from patchwork.requestcontext import PatchworkRequestContext from django.shortcuts import render_to_response, get_object_or_404 from django.http import HttpResponseRedirect -from patchwork.models import Project, Bundle, Person, UserPersonConfirmation, \ - State +from patchwork.models import Project, Bundle, Person, EmailConfirmation, State from patchwork.forms import UserProfileForm, UserPersonLinkForm from patchwork.filters import DelegateFilter from patchwork.views import generic_list @@ -61,7 +60,8 @@ def link(request): if request.method == 'POST': form = UserPersonLinkForm(request.POST) if form.is_valid(): - conf = UserPersonConfirmation(user = request.user, + conf = EmailConfirmation(type = 'userperson', + user = request.user, email = form.cleaned_data['email']) conf.save() context['confirmation'] = conf @@ -83,15 +83,19 @@ def link(request): return render_to_response('patchwork/user-link.html', context) @login_required -def link_confirm(request, key): +def link_confirm(request, conf): context = PatchworkRequestContext(request) - confirmation = get_object_or_404(UserPersonConfirmation, key = key) - errors = confirmation.confirm() - if errors: - context['errors'] = errors - else: - context['person'] = Person.objects.get(email = confirmation.email) + try: + person = Person.objects.get(email__iexact = conf.email) + except Person.DoesNotExist: + person = Person(email = conf.email) + + person.link_to_user(conf.user) + person.save() + conf.deactivate() + + context['person'] = person return render_to_response('patchwork/user-link-confirm.html', context) diff --git a/lib/sql/grant-all.mysql.sql b/lib/sql/grant-all.mysql.sql index 4dd6efb..f60c6b8 100644 --- a/lib/sql/grant-all.mysql.sql +++ b/lib/sql/grant-all.mysql.sql @@ -12,7 +12,7 @@ GRANT SELECT, UPDATE, INSERT, DELETE ON auth_user_groups TO 'www-data'@localhost GRANT SELECT, UPDATE, INSERT, DELETE ON auth_group TO 'www-data'@localhost; GRANT SELECT, UPDATE, INSERT, DELETE ON auth_user_user_permissions TO 'www-data'@localhost; GRANT SELECT, UPDATE, INSERT, DELETE ON auth_permission TO 'www-data'@localhost; -GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_userpersonconfirmation TO 'www-data'@localhost; +GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_emailconfirmation TO 'www-data'@localhost; GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_state TO 'www-data'@localhost; GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_comment TO 'www-data'@localhost; GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_person TO 'www-data'@localhost; diff --git a/lib/sql/grant-all.postgres.sql b/lib/sql/grant-all.postgres.sql index 6a1a47d..47c4ad3 100644 --- a/lib/sql/grant-all.postgres.sql +++ b/lib/sql/grant-all.postgres.sql @@ -13,7 +13,7 @@ GRANT SELECT, UPDATE, INSERT, DELETE ON auth_group, auth_user_user_permissions, auth_permission, - patchwork_userpersonconfirmation, + patchwork_emailconfirmation, patchwork_state, patchwork_comment, patchwork_person, @@ -43,7 +43,7 @@ GRANT SELECT, UPDATE ON patchwork_person_id_seq, patchwork_project_id_seq, patchwork_state_id_seq, - patchwork_userpersonconfirmation_id_seq, + patchwork_emailconfirmation_id_seq, patchwork_userprofile_id_seq, patchwork_userprofile_maintainer_projects_id_seq, registration_registrationprofile_id_seq diff --git a/lib/sql/migration/008-confirmations.sql b/lib/sql/migration/008-confirmations.sql new file mode 100644 index 0000000..89437a2 --- /dev/null +++ b/lib/sql/migration/008-confirmations.sql @@ -0,0 +1,11 @@ +BEGIN; +ALTER TABLE "patchwork_userpersonconfirmation" + RENAME TO "patchwork_emailconfirmation"; +ALTER SEQUENCE "patchwork_userpersonconfirmation_id_seq" + RENAME TO "patchwork_emailconfirmation_id_seq"; +ALTER TABLE "patchwork_emailconfirmation" + ALTER COLUMN "user_id" DROP NOT NULL, + ADD COLUMN "type" varchar(20) NOT NULL DEFAULT 'userperson'; +ALTER TABLE "patchwork_emailconfirmation" + ALTER COLUMN "type" DROP DEFAULT; +COMMIT; diff --git a/templates/patchwork/confirm-error.html b/templates/patchwork/confirm-error.html new file mode 100644 index 0000000..81292e2 --- /dev/null +++ b/templates/patchwork/confirm-error.html @@ -0,0 +1,19 @@ +{% extends "base.html" %} + +{% block title %}Confirmation{% endblock %} +{% block heading %}Confirmation{% endblock %} + + +{% block body %} + +{% if error == 'inactive' %} +

This confirmation has already been processed; you've probably visited this +page before.

+{% endif %} + +{% if error == 'expired' %} +

The confirmation has expired. If you'd still like to perform the +{{conf.get_type_display}} process, you'll need to resubmit the request.

+{% endif %} + +{% endblock %} diff --git a/templates/patchwork/user-link.mail b/templates/patchwork/user-link.mail index 5f74d3b..c483181 100644 --- a/templates/patchwork/user-link.mail +++ b/templates/patchwork/user-link.mail @@ -7,6 +7,6 @@ This email is to confirm that you own the email address: So that you can add it to your patchwork profile. You can confirm this email address by visiting the url: - http://{{site.domain}}{% url patchwork.views.user.link_confirm key=confirmation.key %} + http://{{site.domain}}{% url patchwork.views.confirm key=confirmation.key %} Happy patchworking. -- cgit v1.2.3 From c2c6a408c7764fa29389ce160f52776c9308d50a Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Sun, 31 Oct 2010 19:29:29 -0400 Subject: registration: use EmailConfimation rather than separate registration app Since we have infrastructure for email confirmations, we no longer need the separate registration app. Requires a migration script, which will delete all inactive users, including those newly added and pending confirmation. Use carefully. Signed-off-by: Jeremy Kerr --- apps/patchwork/forms.py | 43 +++--- apps/patchwork/models.py | 4 +- apps/patchwork/tests/__init__.py | 2 + apps/patchwork/tests/registration.py | 150 +++++++++++++++++++++ apps/patchwork/tests/user.py | 11 +- apps/patchwork/tests/utils.py | 2 +- apps/patchwork/urls.py | 18 +++ apps/patchwork/views/base.py | 1 + apps/patchwork/views/user.py | 54 +++++++- apps/settings.py | 5 +- apps/urls.py | 10 -- docs/INSTALL | 11 -- lib/sql/grant-all.mysql.sql | 1 - lib/sql/grant-all.postgres.sql | 6 +- lib/sql/migration/009-drop-registrationprofile.sql | 27 ++++ templates/base.html | 2 +- templates/patchwork/activation_email.txt | 11 ++ templates/patchwork/activation_email_subject.txt | 1 + templates/patchwork/help/about.html | 4 - templates/patchwork/login.html | 27 ++++ templates/patchwork/logout.html | 8 ++ templates/patchwork/registration-confirm.html | 13 ++ templates/patchwork/registration_form.html | 121 +++++++++++++++++ templates/registration/activate.html | 13 -- templates/registration/activation_email.txt | 11 -- .../registration/activation_email_subject.txt | 1 - templates/registration/login.html | 27 ---- templates/registration/logout.html | 8 -- templates/registration/registration_complete.html | 13 -- templates/registration/registration_form.html | 122 ----------------- 30 files changed, 471 insertions(+), 256 deletions(-) create mode 100644 apps/patchwork/tests/registration.py create mode 100644 lib/sql/migration/009-drop-registrationprofile.sql create mode 100644 templates/patchwork/activation_email.txt create mode 100644 templates/patchwork/activation_email_subject.txt create mode 100644 templates/patchwork/login.html create mode 100644 templates/patchwork/logout.html create mode 100644 templates/patchwork/registration-confirm.html create mode 100644 templates/patchwork/registration_form.html delete mode 100644 templates/registration/activate.html delete mode 100644 templates/registration/activation_email.txt delete mode 100644 templates/registration/activation_email_subject.txt delete mode 100644 templates/registration/login.html delete mode 100644 templates/registration/logout.html delete mode 100644 templates/registration/registration_complete.html delete mode 100644 templates/registration/registration_form.html diff --git a/apps/patchwork/forms.py b/apps/patchwork/forms.py index 1ff2bd0..f83c27a 100644 --- a/apps/patchwork/forms.py +++ b/apps/patchwork/forms.py @@ -22,34 +22,33 @@ from django.contrib.auth.models import User from django import forms from patchwork.models import Patch, State, Bundle, UserProfile -from registration.forms import RegistrationFormUniqueEmail -from registration.models import RegistrationProfile -class RegistrationForm(RegistrationFormUniqueEmail): +class RegistrationForm(forms.Form): first_name = forms.CharField(max_length = 30, required = False) last_name = forms.CharField(max_length = 30, required = False) - username = forms.CharField(max_length=30, label=u'Username') + username = forms.RegexField(regex = r'^\w+$', max_length=30, + label=u'Username') email = forms.EmailField(max_length=100, label=u'Email address') password = forms.CharField(widget=forms.PasswordInput(), label='Password') - password1 = forms.BooleanField(required = False) - password2 = forms.BooleanField(required = False) - - def save(self, profile_callback = None): - user = RegistrationProfile.objects.create_inactive_user( \ - username = self.cleaned_data['username'], - password = self.cleaned_data['password'], - email = self.cleaned_data['email'], - profile_callback = profile_callback) - user.first_name = self.cleaned_data.get('first_name', '') - user.last_name = self.cleaned_data.get('last_name', '') - user.save() - - # saving the userprofile causes the firstname/lastname to propagate - # to the person objects. - user.get_profile().save() - - return user + + def clean_username(self): + value = self.cleaned_data['username'] + try: + user = User.objects.get(username__iexact = value) + except User.DoesNotExist: + return self.cleaned_data['username'] + raise forms.ValidationError('This username is already taken. ' + \ + 'Please choose another.') + + def clean_email(self): + value = self.cleaned_data['email'] + try: + user = User.objects.get(email__iexact = value) + except User.DoesNotExist: + return self.cleaned_data['email'] + raise forms.ValidationError('This email address is already in use ' + \ + 'for the account "%s".\n' % user.username) def clean(self): return self.cleaned_data diff --git a/apps/patchwork/models.py b/apps/patchwork/models.py index ee6748f..806875b 100644 --- a/apps/patchwork/models.py +++ b/apps/patchwork/models.py @@ -21,6 +21,7 @@ from django.db import models from django.contrib.auth.models import User from django.core.urlresolvers import reverse from django.contrib.sites.models import Site +from django.conf import settings from patchwork.parser import hash_patch import re @@ -374,9 +375,10 @@ class BundlePatch(models.Model): ordering = ['order'] class EmailConfirmation(models.Model): - validity = datetime.timedelta(days = 30) + validity = datetime.timedelta(days = settings.CONFIRMATION_VALIDITY_DAYS) type = models.CharField(max_length = 20, choices = [ ('userperson', 'User-Person association'), + ('registration', 'Registration'), ]) email = models.CharField(max_length = 200) user = models.ForeignKey(User, null = True) diff --git a/apps/patchwork/tests/__init__.py b/apps/patchwork/tests/__init__.py index 9618d1f..db096d8 100644 --- a/apps/patchwork/tests/__init__.py +++ b/apps/patchwork/tests/__init__.py @@ -24,3 +24,5 @@ from patchwork.tests.mboxviews import * from patchwork.tests.updates import * from patchwork.tests.filters import * from patchwork.tests.confirm import * +from patchwork.tests.registration import * +from patchwork.tests.user import * diff --git a/apps/patchwork/tests/registration.py b/apps/patchwork/tests/registration.py new file mode 100644 index 0000000..18b781f --- /dev/null +++ b/apps/patchwork/tests/registration.py @@ -0,0 +1,150 @@ +# Patchwork - automated patch tracking system +# Copyright (C) 2010 Jeremy Kerr +# +# This file is part of the Patchwork package. +# +# Patchwork is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# Patchwork is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Patchwork; if not, write to the Free Software +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + +import unittest +from django.test import TestCase +from django.test.client import Client +from django.core import mail +from django.core.urlresolvers import reverse +from django.contrib.auth.models import User +from patchwork.models import EmailConfirmation, Person +from patchwork.tests.utils import create_user + +def _confirmation_url(conf): + return reverse('patchwork.views.confirm', kwargs = {'key': conf.key}) + +class TestUser(object): + firstname = 'Test' + lastname = 'User' + username = 'testuser' + email = 'test@example.com' + password = 'foobar' + +class RegistrationTest(TestCase): + def setUp(self): + self.user = TestUser() + self.client = Client() + self.default_data = {'username': self.user.username, + 'first_name': self.user.firstname, + 'last_name': self.user.lastname, + 'email': self.user.email, + 'password': self.user.password} + self.required_error = 'This field is required.' + self.invalid_error = 'Enter a valid value.' + + def testRegistrationForm(self): + response = self.client.get('/register/') + self.assertEquals(response.status_code, 200) + self.assertTemplateUsed(response, 'patchwork/registration_form.html') + + def testBlankFields(self): + for field in ['username', 'email', 'password']: + data = self.default_data.copy() + del data[field] + response = self.client.post('/register/', data) + self.assertEquals(response.status_code, 200) + self.assertFormError(response, 'form', field, self.required_error) + + def testInvalidUsername(self): + data = self.default_data.copy() + data['username'] = 'invalid user' + response = self.client.post('/register/', data) + self.assertEquals(response.status_code, 200) + self.assertFormError(response, 'form', 'username', self.invalid_error) + + def testExistingUsername(self): + user = create_user() + data = self.default_data.copy() + data['username'] = user.username + response = self.client.post('/register/', data) + self.assertEquals(response.status_code, 200) + self.assertFormError(response, 'form', 'username', + 'This username is already taken. Please choose another.') + + def testExistingEmail(self): + user = create_user() + data = self.default_data.copy() + data['email'] = user.email + response = self.client.post('/register/', data) + self.assertEquals(response.status_code, 200) + self.assertFormError(response, 'form', 'email', + 'This email address is already in use ' + \ + 'for the account "%s".\n' % user.username) + + def testValidRegistration(self): + response = self.client.post('/register/', self.default_data) + self.assertEquals(response.status_code, 200) + self.assertContains(response, 'confirmation email has been sent') + + # check for presence of an inactive user object + users = User.objects.filter(username = self.user.username) + self.assertEquals(users.count(), 1) + user = users[0] + self.assertEquals(user.username, self.user.username) + self.assertEquals(user.email, self.user.email) + self.assertEquals(user.is_active, False) + + # check for confirmation object + confs = EmailConfirmation.objects.filter(user = user, + type = 'registration') + self.assertEquals(len(confs), 1) + conf = confs[0] + self.assertEquals(conf.email, self.user.email) + + # check for a sent mail + self.assertEquals(len(mail.outbox), 1) + msg = mail.outbox[0] + self.assertEquals(msg.subject, 'Patchwork account confirmation') + self.assertTrue(self.user.email in msg.to) + self.assertTrue(_confirmation_url(conf) in msg.body) + + # ...and that the URL is valid + response = self.client.get(_confirmation_url(conf)) + self.assertEquals(response.status_code, 200) + +class RegistrationConfirmationTest(TestCase): + + def setUp(self): + self.user = TestUser() + self.default_data = {'username': self.user.username, + 'first_name': self.user.firstname, + 'last_name': self.user.lastname, + 'email': self.user.email, + 'password': self.user.password} + + def testRegistrationConfirmation(self): + self.assertEqual(EmailConfirmation.objects.count(), 0) + response = self.client.post('/register/', self.default_data) + self.assertEquals(response.status_code, 200) + self.assertContains(response, 'confirmation email has been sent') + + self.assertEqual(EmailConfirmation.objects.count(), 1) + conf = EmailConfirmation.objects.filter()[0] + self.assertFalse(conf.user.is_active) + self.assertTrue(conf.active) + + response = self.client.get(_confirmation_url(conf)) + self.assertEquals(response.status_code, 200) + self.assertTemplateUsed(response, 'patchwork/registration-confirm.html') + + conf = EmailConfirmation.objects.get(pk = conf.pk) + self.assertTrue(conf.user.is_active) + self.assertFalse(conf.active) + + diff --git a/apps/patchwork/tests/user.py b/apps/patchwork/tests/user.py index c9e5be3..e96e6c5 100644 --- a/apps/patchwork/tests/user.py +++ b/apps/patchwork/tests/user.py @@ -22,9 +22,9 @@ from django.test import TestCase from django.test.client import Client from django.core import mail from django.core.urlresolvers import reverse +from django.conf import settings from django.contrib.auth.models import User from patchwork.models import EmailConfirmation, Person -from patchwork.utils import userprofile_register_callback def _confirmation_url(conf): return reverse('patchwork.views.confirm', kwargs = {'key': conf.key}) @@ -39,7 +39,6 @@ class TestUser(object): self.password = User.objects.make_random_password() self.user = User.objects.create_user(self.username, self.email, self.password) - userprofile_register_callback(self.user) class UserPersonRequestTest(TestCase): def setUp(self): @@ -119,3 +118,11 @@ class UserPersonConfirmTest(TestCase): # need to reload the confirmation to check this. conf = EmailConfirmation.objects.get(pk = self.conf.pk) self.assertEquals(conf.active, False) + +class UserLoginRedirectTest(TestCase): + + def testUserLoginRedirect(self): + url = '/user/' + response = self.client.get(url) + self.assertRedirects(response, settings.LOGIN_URL + '?next=' + url) + diff --git a/apps/patchwork/tests/utils.py b/apps/patchwork/tests/utils.py index f1c95e8..1cb5dfb 100644 --- a/apps/patchwork/tests/utils.py +++ b/apps/patchwork/tests/utils.py @@ -59,7 +59,7 @@ class defaults(object): _user_idx = 1 def create_user(): global _user_idx - userid = 'test-%d' % _user_idx + userid = 'test%d' % _user_idx email = '%s@example.com' % userid _user_idx += 1 diff --git a/apps/patchwork/urls.py b/apps/patchwork/urls.py index 27c79fd..6810e3e 100644 --- a/apps/patchwork/urls.py +++ b/apps/patchwork/urls.py @@ -19,6 +19,7 @@ from django.conf.urls.defaults import * from django.conf import settings +from django.contrib.auth import views as auth_views urlpatterns = patterns('', # Example: @@ -46,6 +47,23 @@ urlpatterns = patterns('', (r'^user/link/$', 'patchwork.views.user.link'), (r'^user/unlink/(?P[^/]+)/$', 'patchwork.views.user.unlink'), + # password change + url(r'^user/password-change/$', auth_views.password_change, + name='auth_password_change'), + url(r'^user/password-change/done/$', auth_views.password_change_done, + name='auth_password_change_done'), + + # login/logout + url(r'^user/login/$', auth_views.login, + {'template_name': 'patchwork/login.html'}, + name = 'auth_login'), + url(r'^user/logout/$', auth_views.logout, + {'template_name': 'patchwork/logout.html'}, + name = 'auth_logout'), + + # registration + (r'^register/', 'patchwork.views.user.register'), + # public view for bundles (r'^bundle/(?P[^/]*)/(?P[^/]*)/$', 'patchwork.views.bundle.public'), diff --git a/apps/patchwork/views/base.py b/apps/patchwork/views/base.py index 1539472..590a3b6 100644 --- a/apps/patchwork/views/base.py +++ b/apps/patchwork/views/base.py @@ -62,6 +62,7 @@ def confirm(request, key): import patchwork.views.user views = { 'userperson': patchwork.views.user.link_confirm, + 'registration': patchwork.views.user.register_confirm, } conf = get_object_or_404(EmailConfirmation, key = key) diff --git a/apps/patchwork/views/user.py b/apps/patchwork/views/user.py index 759a6e3..3d28f4b 100644 --- a/apps/patchwork/views/user.py +++ b/apps/patchwork/views/user.py @@ -21,9 +21,12 @@ from django.contrib.auth.decorators import login_required from patchwork.requestcontext import PatchworkRequestContext from django.shortcuts import render_to_response, get_object_or_404 +from django.contrib import auth +from django.contrib.sites.models import Site from django.http import HttpResponseRedirect from patchwork.models import Project, Bundle, Person, EmailConfirmation, State -from patchwork.forms import UserProfileForm, UserPersonLinkForm +from patchwork.forms import UserProfileForm, UserPersonLinkForm, \ + RegistrationForm from patchwork.filters import DelegateFilter from patchwork.views import generic_list from django.template.loader import render_to_string @@ -31,6 +34,55 @@ from django.conf import settings from django.core.mail import send_mail import django.core.urlresolvers +def register(request): + context = PatchworkRequestContext(request) + if request.method == 'POST': + form = RegistrationForm(request.POST) + if form.is_valid(): + data = form.cleaned_data + # create inactive user + user = auth.models.User.objects.create_user(data['username'], + data['email'], + data['password']) + user.is_active = False; + user.first_name = data.get('first_name', '') + user.last_name = data.get('last_name', '') + user.save() + + # create confirmation + conf = EmailConfirmation(type = 'registration', user = user, + email = user.email) + conf.save() + + # send email + mail_ctx = {'site': Site.objects.get_current(), + 'confirmation': conf} + + subject = render_to_string('patchwork/activation_email_subject.txt', + mail_ctx).replace('\n', ' ').strip() + + message = render_to_string('patchwork/activation_email.txt', + mail_ctx) + + send_mail(subject, message, settings.DEFAULT_FROM_EMAIL, + [conf.email]) + + # setting 'confirmation' in the template indicates success + context['confirmation'] = conf + + else: + form = RegistrationForm() + + return render_to_response('patchwork/registration_form.html', + { 'form': form }, + context_instance=context) + +def register_confirm(request, conf): + conf.user.is_active = True + conf.user.save() + conf.deactivate() + return render_to_response('patchwork/registration-confirm.html') + @login_required def profile(request): context = PatchworkRequestContext(request) diff --git a/apps/settings.py b/apps/settings.py index f56da70..8f091d0 100644 --- a/apps/settings.py +++ b/apps/settings.py @@ -64,7 +64,7 @@ MIDDLEWARE_CLASSES = ( ROOT_URLCONF = 'apps.urls' -LOGIN_URL = '/accounts/login' +LOGIN_URL = '/user/login/' LOGIN_REDIRECT_URL = '/user/' # If you change the ROOT_DIR setting in your local_settings.py, you'll need to @@ -96,13 +96,12 @@ INSTALLED_APPS = ( 'django.contrib.sites', 'django.contrib.admin', 'patchwork', - 'registration', ) DEFAULT_PATCHES_PER_PAGE = 100 DEFAULT_FROM_EMAIL = 'Patchwork ' -ACCOUNT_ACTIVATION_DAYS = 7 +CONFIRMATION_VALIDITY_DAYS = 7 # Set to True to enable the Patchwork XML-RPC interface ENABLE_XMLRPC = False diff --git a/apps/urls.py b/apps/urls.py index 3894708..4ddef9e 100644 --- a/apps/urls.py +++ b/apps/urls.py @@ -23,9 +23,6 @@ from django.conf.urls.defaults import * from django.conf import settings from django.contrib import admin -from registration.views import register -from patchwork.forms import RegistrationForm - admin.autodiscover() htdocs = os.path.join(settings.ROOT_DIR, 'htdocs') @@ -34,13 +31,6 @@ urlpatterns = patterns('', # Example: (r'^', include('patchwork.urls')), - # override the default registration form - url(r'^accounts/register/$', - register, {'form_class': RegistrationForm}, - name='registration_register'), - - (r'^accounts/', include('registration.urls')), - # Uncomment this for admin: (r'^admin/', include(admin.site.urls)), diff --git a/docs/INSTALL b/docs/INSTALL index 4c178ef..6a1a0bf 100644 --- a/docs/INSTALL +++ b/docs/INSTALL @@ -81,17 +81,6 @@ in brackets): cd ../python ln -s ../packages/django/django ./django - We also use the django-registration infrastructure from - http://bitbucket.org/ubernostrum/django-registration/. Your distro - may provide the django-registration python module (in Ubuntu/Debian it's - called 'python-django-registration'). If not, download the module - and symlink it to lib/python/ : - - cd lib/packages/ - hg clone http://bitbucket.org/ubernostrum/django-registration/ - cd ../python - ln -s ../packages/django-registration/registration ./registration - We also use some Javascript libraries: cd lib/packages diff --git a/lib/sql/grant-all.mysql.sql b/lib/sql/grant-all.mysql.sql index f60c6b8..a3d758c 100644 --- a/lib/sql/grant-all.mysql.sql +++ b/lib/sql/grant-all.mysql.sql @@ -22,7 +22,6 @@ GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_project TO 'www-data'@localhos GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_bundle TO 'www-data'@localhost; GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_bundle_patches TO 'www-data'@localhost; GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_patch TO 'www-data'@localhost; -GRANT SELECT, UPDATE, INSERT, DELETE ON registration_registrationprofile TO 'www-data'@localhost; -- allow the mail user (in this case, 'nobody') to add patches GRANT INSERT, SELECT ON patchwork_patch TO 'nobody'@localhost; diff --git a/lib/sql/grant-all.postgres.sql b/lib/sql/grant-all.postgres.sql index 47c4ad3..591ffd0 100644 --- a/lib/sql/grant-all.postgres.sql +++ b/lib/sql/grant-all.postgres.sql @@ -22,8 +22,7 @@ GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_project, patchwork_bundle, patchwork_bundlepatch, - patchwork_patch, - registration_registrationprofile + patchwork_patch TO "www-data"; GRANT SELECT, UPDATE ON auth_group_id_seq, @@ -45,8 +44,7 @@ GRANT SELECT, UPDATE ON patchwork_state_id_seq, patchwork_emailconfirmation_id_seq, patchwork_userprofile_id_seq, - patchwork_userprofile_maintainer_projects_id_seq, - registration_registrationprofile_id_seq + patchwork_userprofile_maintainer_projects_id_seq TO "www-data"; -- allow the mail user (in this case, 'nobody') to add patches diff --git a/lib/sql/migration/009-drop-registrationprofile.sql b/lib/sql/migration/009-drop-registrationprofile.sql new file mode 100644 index 0000000..f1c2b43 --- /dev/null +++ b/lib/sql/migration/009-drop-registrationprofile.sql @@ -0,0 +1,27 @@ +BEGIN; + +DELETE FROM registration_registrationprofile; + +-- unlink users who have contributed + +UPDATE patchwork_person SET user_id = NULL + WHERE user_id IN (SELECT id FROM auth_user WHERE is_active = False) + AND id IN (SELECT DISTINCT submitter_id FROM patchwork_comment); + +-- remove persons who only have a user linkage + +DELETE FROM patchwork_person WHERE user_id IN + (SELECT id FROM auth_user WHERE is_active = False); + +-- delete profiles + +DELETE FROM patchwork_userprofile WHERE user_id IN + (SELECT id FROM auth_user WHERE is_active = False); + +-- delete inactive users + +DELETE FROM auth_user WHERE is_active = False; + +DROP TABLE registration_registrationprofile; + +COMMIT; diff --git a/templates/base.html b/templates/base.html index e14470e..9e80dca 100644 --- a/templates/base.html +++ b/templates/base.html @@ -30,7 +30,7 @@ {% else %} login
- register + register {% endif %}
diff --git a/templates/patchwork/activation_email.txt b/templates/patchwork/activation_email.txt new file mode 100644 index 0000000..e918e5f --- /dev/null +++ b/templates/patchwork/activation_email.txt @@ -0,0 +1,11 @@ +Hi, + +This email is to confirm your account on the patchwork patch-tracking +system. You can activate your account by visiting the url: + + http://{{site.domain}}{% url patchwork.views.confirm key=confirmation.key %} + +If you didn't request a user account on patchwork, then you can ignore +this mail. + +Happy patchworking. diff --git a/templates/patchwork/activation_email_subject.txt b/templates/patchwork/activation_email_subject.txt new file mode 100644 index 0000000..c409f38 --- /dev/null +++ b/templates/patchwork/activation_email_subject.txt @@ -0,0 +1 @@ +Patchwork account confirmation diff --git a/templates/patchwork/help/about.html b/templates/patchwork/help/about.html index edc381e..0d784d7 100644 --- a/templates/patchwork/help/about.html +++ b/templates/patchwork/help/about.html @@ -11,10 +11,6 @@

Patchwork is built on the django web framework.

-

Patchwork includes the django-registration -application.

-

Icons from the Sweetie icon set. {% endblock %} diff --git a/templates/patchwork/login.html b/templates/patchwork/login.html new file mode 100644 index 0000000..2dfc2a7 --- /dev/null +++ b/templates/patchwork/login.html @@ -0,0 +1,27 @@ +{% extends "base.html" %} + +{% block title %}Login{% endblock %} +{% block heading %}Login{% endblock %} + + +{% block body %} +

+{% csrf_token %} + + + + + {% if error %} + + + + {% endif %} + {{ form }} + + + +
login
{{ error }}
+ +
+
+{% endblock %} diff --git a/templates/patchwork/logout.html b/templates/patchwork/logout.html new file mode 100644 index 0000000..f030aee --- /dev/null +++ b/templates/patchwork/logout.html @@ -0,0 +1,8 @@ +{% extends "base.html" %} + +{% block title %}Logout{% endblock %} +{% block heading %}Logout{% endblock %} + +{% block body %} +

Logged out

+{% endblock %} diff --git a/templates/patchwork/registration-confirm.html b/templates/patchwork/registration-confirm.html new file mode 100644 index 0000000..f0cc39f --- /dev/null +++ b/templates/patchwork/registration-confirm.html @@ -0,0 +1,13 @@ +{% extends "base.html" %} + +{% block title %}Registration{% endblock %} +{% block heading %}Registration{% endblock %} + +{% block body %} +

Registraton confirmed!

+ +

Your patchwork registration is complete. Head over to your profile to start using +patchwork's extra features.

+ +{% endblock %} diff --git a/templates/patchwork/registration_form.html b/templates/patchwork/registration_form.html new file mode 100644 index 0000000..3a314b8 --- /dev/null +++ b/templates/patchwork/registration_form.html @@ -0,0 +1,121 @@ +{% extends "base.html" %} + +{% block title %}Registration{% endblock %} +{% block heading %}Registration{% endblock %} + + +{% block body %} + +{% if confirmation and not error %} +

Registration successful!

+

A confirmation email has been sent to {{ confirmation.email }}. You'll + need to visit the link provided in that email to confirm your + registration.

+

+{% else %} +

By creating a patchwork account, you can:

+

    +
  • create "bundles" of patches
    • +
  • update the state of your own patches
    • +
+
+{% csrf_token %} + + + + + {% if error %} + + + + {% endif %} + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
register
{{ error }}
{{ form.first_name.label_tag }} +{% if form.first_name.errors %} + {{ form.first_name.errors }} +{% endif %} + {{ form.first_name }} +{% if form.first_name.help_text %} +
{{ form.first_name.help_text }}
+{% endif %} +
{{ form.last_name.label_tag }} +{% if form.last_name.errors %} + {{ form.last_name.errors }} +{% endif %} + {{ form.last_name }} +{% if form.last_name.help_text %} +
{{ form.last_name.help_text }}
+{% endif %} +
+ Your name is used to identify you on the site +
{{ form.email.label_tag }} +{% if form.email.errors %} + {{ form.email.errors }} +{% endif %} + {{ form.email }} +{% if form.email.help_text %} +
{{ form.email.help_text }}
+{% endif %} +
+ Patchwork will send a confirmation email to this address +
{{ form.username.label_tag }} +{% if form.username.errors %} + {{ form.username.errors }} +{% endif %} + {{ form.username }} +{% if form.username.help_text %} +
{{ form.username.help_text }}
+{% endif %} +
{{ form.password.label_tag }} +{% if form.password.errors %} + {{ form.password.errors }} +{% endif %} + {{ form.password }} +{% if form.password.help_text %} +
{{ form.password.help_text }}
+{% endif %} +
+ +
+
+{% endif %} + +{% endblock %} diff --git a/templates/registration/activate.html b/templates/registration/activate.html deleted file mode 100644 index f0cc39f..0000000 --- a/templates/registration/activate.html +++ /dev/null @@ -1,13 +0,0 @@ -{% extends "base.html" %} - -{% block title %}Registration{% endblock %} -{% block heading %}Registration{% endblock %} - -{% block body %} -

Registraton confirmed!

- -

Your patchwork registration is complete. Head over to your profile to start using -patchwork's extra features.

- -{% endblock %} diff --git a/templates/registration/activation_email.txt b/templates/registration/activation_email.txt deleted file mode 100644 index 6b1477d..0000000 --- a/templates/registration/activation_email.txt +++ /dev/null @@ -1,11 +0,0 @@ -Hi, - -This email is to confirm your account on the patchwork patch-tracking -system. You can activate your account by visiting the url: - - http://{{site.domain}}{% url registration_activate activation_key=activation_key %} - -If you didn't request a user account on patchwork, then you can ignore -this mail. - -Happy patchworking. diff --git a/templates/registration/activation_email_subject.txt b/templates/registration/activation_email_subject.txt deleted file mode 100644 index c409f38..0000000 --- a/templates/registration/activation_email_subject.txt +++ /dev/null @@ -1 +0,0 @@ -Patchwork account confirmation diff --git a/templates/registration/login.html b/templates/registration/login.html deleted file mode 100644 index 2dfc2a7..0000000 --- a/templates/registration/login.html +++ /dev/null @@ -1,27 +0,0 @@ -{% extends "base.html" %} - -{% block title %}Login{% endblock %} -{% block heading %}Login{% endblock %} - - -{% block body %} -
-{% csrf_token %} - - - - - {% if error %} - - - - {% endif %} - {{ form }} - - - -
login
{{ error }}
- -
-
-{% endblock %} diff --git a/templates/registration/logout.html b/templates/registration/logout.html deleted file mode 100644 index f030aee..0000000 --- a/templates/registration/logout.html +++ /dev/null @@ -1,8 +0,0 @@ -{% extends "base.html" %} - -{% block title %}Logout{% endblock %} -{% block heading %}Logout{% endblock %} - -{% block body %} -

Logged out

-{% endblock %} diff --git a/templates/registration/registration_complete.html b/templates/registration/registration_complete.html deleted file mode 100644 index a89c116..0000000 --- a/templates/registration/registration_complete.html +++ /dev/null @@ -1,13 +0,0 @@ -{% extends "base.html" %} - -{% block title %}Registration{% endblock %} -{% block heading %}Registration{% endblock %} - -{% block body %} - -

Registration successful!

-

A confirmation email has been sent to your email address. You'll - need to visit the link provided in that email to activate your - patchwork account.

- -{% endblock %} diff --git a/templates/registration/registration_form.html b/templates/registration/registration_form.html deleted file mode 100644 index e2b17c1..0000000 --- a/templates/registration/registration_form.html +++ /dev/null @@ -1,122 +0,0 @@ -{% extends "base.html" %} - -{% block title %}Registration{% endblock %} -{% block heading %}Registration{% endblock %} - - -{% block body %} - -{% if request and not error %} -

Registration successful!

-

A confirmation email has been sent to {{ request.email }}. You'll - need to visit the link provided in that email to confirm your - registration.

- 
{{email}}
-

-{% else %} -

By creating a patchwork account, you can:

-

    -
  • create "bundles" of patches
    • -
  • update the state of your own patches
    • -
-
-{% csrf_token %} - - - - - {% if error %} - - - - {% endif %} - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
register
{{ error }}
{{ form.first_name.label_tag }} -{% if form.first_name.errors %} - {{ form.first_name.errors }} -{% endif %} - {{ form.first_name }} -{% if form.first_name.help_text %} -
{{ form.first_name.help_text }}
-{% endif %} -
{{ form.last_name.label_tag }} -{% if form.last_name.errors %} - {{ form.last_name.errors }} -{% endif %} - {{ form.last_name }} -{% if form.last_name.help_text %} -
{{ form.last_name.help_text }}
-{% endif %} -
- Your name is used to identify you on the site -
{{ form.email.label_tag }} -{% if form.email.errors %} - {{ form.email.errors }} -{% endif %} - {{ form.email }} -{% if form.email.help_text %} -
{{ form.email.help_text }}
-{% endif %} -
- Patchwork will send a confirmation email to this address -
{{ form.username.label_tag }} -{% if form.username.errors %} - {{ form.username.errors }} -{% endif %} - {{ form.username }} -{% if form.username.help_text %} -
{{ form.username.help_text }}
-{% endif %} -
{{ form.password.label_tag }} -{% if form.password.errors %} - {{ form.password.errors }} -{% endif %} - {{ form.password }} -{% if form.password.help_text %} -
{{ form.password.help_text }}
-{% endif %} -
- -
-
-{% endif %} - -{% endblock %} -- cgit v1.2.3 From 41f19b6643b44768dc06561c992c04ed6148477d Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Wed, 11 Aug 2010 14:16:28 +0800 Subject: Add email opt-out system We're going to start generating emails on patchwork updates, so firstly allow people to opt-out of all patchwork communications. We do this with a 'mail settings' interface, allowing non-registered users to set preferences on their email address. Logged-in users can do this through the user profile view. Signed-off-by: Jeremy Kerr --- apps/patchwork/forms.py | 5 +- apps/patchwork/models.py | 5 + apps/patchwork/tests/__init__.py | 1 + apps/patchwork/tests/mail_settings.py | 302 ++++++++++++++++++++++++++++++++ apps/patchwork/urls.py | 5 + apps/patchwork/views/base.py | 4 +- apps/patchwork/views/mail.py | 119 +++++++++++++ apps/patchwork/views/user.py | 11 +- lib/sql/grant-all.mysql.sql | 1 + lib/sql/grant-all.postgres.sql | 3 +- lib/sql/migration/010-optout-tables.sql | 5 + templates/base.html | 2 + templates/patchwork/mail-form.html | 38 ++++ templates/patchwork/mail-settings.html | 37 ++++ templates/patchwork/optin-request.html | 50 ++++++ templates/patchwork/optin-request.mail | 12 ++ templates/patchwork/optin.html | 19 ++ templates/patchwork/optout-request.html | 51 ++++++ templates/patchwork/optout-request.mail | 12 ++ templates/patchwork/optout.html | 22 +++ templates/patchwork/profile.html | 36 ++-- 21 files changed, 725 insertions(+), 15 deletions(-) create mode 100644 apps/patchwork/tests/mail_settings.py create mode 100644 apps/patchwork/views/mail.py create mode 100644 lib/sql/migration/010-optout-tables.sql create mode 100644 templates/patchwork/mail-form.html create mode 100644 templates/patchwork/mail-settings.html create mode 100644 templates/patchwork/optin-request.html create mode 100644 templates/patchwork/optin-request.mail create mode 100644 templates/patchwork/optin.html create mode 100644 templates/patchwork/optout-request.html create mode 100644 templates/patchwork/optout-request.mail create mode 100644 templates/patchwork/optout.html diff --git a/apps/patchwork/forms.py b/apps/patchwork/forms.py index f83c27a..d5e51a2 100644 --- a/apps/patchwork/forms.py +++ b/apps/patchwork/forms.py @@ -227,5 +227,8 @@ class MultiplePatchForm(forms.Form): instance.save() return instance -class UserPersonLinkForm(forms.Form): +class EmailForm(forms.Form): email = forms.EmailField(max_length = 200) + +UserPersonLinkForm = EmailForm +OptinoutRequestForm = EmailForm diff --git a/apps/patchwork/models.py b/apps/patchwork/models.py index 806875b..f21d073 100644 --- a/apps/patchwork/models.py +++ b/apps/patchwork/models.py @@ -379,6 +379,7 @@ class EmailConfirmation(models.Model): type = models.CharField(max_length = 20, choices = [ ('userperson', 'User-Person association'), ('registration', 'Registration'), + ('optout', 'Email opt-out'), ]) email = models.CharField(max_length = 200) user = models.ForeignKey(User, null = True) @@ -400,4 +401,8 @@ class EmailConfirmation(models.Model): self.key = self._meta.get_field('key').construct(str).hexdigest() super(EmailConfirmation, self).save() +class EmailOptout(models.Model): + email = models.CharField(max_length = 200, primary_key = True) + def __unicode__(self): + return self.email diff --git a/apps/patchwork/tests/__init__.py b/apps/patchwork/tests/__init__.py index db096d8..0b56fc1 100644 --- a/apps/patchwork/tests/__init__.py +++ b/apps/patchwork/tests/__init__.py @@ -26,3 +26,4 @@ from patchwork.tests.filters import * from patchwork.tests.confirm import * from patchwork.tests.registration import * from patchwork.tests.user import * +from patchwork.tests.mail_settings import * diff --git a/apps/patchwork/tests/mail_settings.py b/apps/patchwork/tests/mail_settings.py new file mode 100644 index 0000000..36dc5cc --- /dev/null +++ b/apps/patchwork/tests/mail_settings.py @@ -0,0 +1,302 @@ +# Patchwork - automated patch tracking system +# Copyright (C) 2010 Jeremy Kerr +# +# This file is part of the Patchwork package. +# +# Patchwork is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# Patchwork is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Patchwork; if not, write to the Free Software +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + +import unittest +import re +from django.test import TestCase +from django.test.client import Client +from django.core import mail +from django.core.urlresolvers import reverse +from django.contrib.auth.models import User +from patchwork.models import EmailOptout, EmailConfirmation, Person +from patchwork.tests.utils import create_user + +class MailSettingsTest(TestCase): + view = 'patchwork.views.mail.settings' + url = reverse(view) + + def testMailSettingsGET(self): + response = self.client.get(self.url) + self.assertEquals(response.status_code, 200) + self.assertTrue(response.context['form']) + + def testMailSettingsPOST(self): + email = u'foo@example.com' + response = self.client.post(self.url, {'email': email}) + self.assertEquals(response.status_code, 200) + self.assertTemplateUsed(response, 'patchwork/mail-settings.html') + self.assertEquals(response.context['email'], email) + + def testMailSettingsPOSTEmpty(self): + response = self.client.post(self.url, {'email': ''}) + self.assertEquals(response.status_code, 200) + self.assertTemplateUsed(response, 'patchwork/mail-form.html') + self.assertFormError(response, 'form', 'email', + 'This field is required.') + + def testMailSettingsPOSTInvalid(self): + response = self.client.post(self.url, {'email': 'foo'}) + self.assertEquals(response.status_code, 200) + self.assertTemplateUsed(response, 'patchwork/mail-form.html') + self.assertFormError(response, 'form', 'email', + 'Enter a valid e-mail address.') + + def testMailSettingsPOSTOptedIn(self): + email = u'foo@example.com' + response = self.client.post(self.url, {'email': email}) + self.assertEquals(response.status_code, 200) + self.assertTemplateUsed(response, 'patchwork/mail-settings.html') + self.assertEquals(response.context['is_optout'], False) + self.assertTrue('may' in response.content) + optout_url = reverse('patchwork.views.mail.optout') + self.assertTrue(('action="%s"' % optout_url) in response.content) + + def testMailSettingsPOSTOptedOut(self): + email = u'foo@example.com' + EmailOptout(email = email).save() + response = self.client.post(self.url, {'email': email}) + self.assertEquals(response.status_code, 200) + self.assertTemplateUsed(response, 'patchwork/mail-settings.html') + self.assertEquals(response.context['is_optout'], True) + self.assertTrue('may not' in response.content) + optin_url = reverse('patchwork.views.mail.optin') + self.assertTrue(('action="%s"' % optin_url) in response.content) + +class OptoutRequestTest(TestCase): + view = 'patchwork.views.mail.optout' + url = reverse(view) + + def testOptOutRequestGET(self): + response = self.client.get(self.url) + self.assertRedirects(response, reverse('patchwork.views.mail.settings')) + + def testOptoutRequestValidPOST(self): + email = u'foo@example.com' + response = self.client.post(self.url, {'email': email}) + + # check for a confirmation object + self.assertEquals(EmailConfirmation.objects.count(), 1) + conf = EmailConfirmation.objects.get(email = email) + + # check confirmation page + self.assertEquals(response.status_code, 200) + self.assertEquals(response.context['confirmation'], conf) + self.assertTrue(email in response.content) + + # check email + url = reverse('patchwork.views.confirm', kwargs = {'key': conf.key}) + self.assertEquals(len(mail.outbox), 1) + msg = mail.outbox[0] + self.assertEquals(msg.to, [email]) + self.assertEquals(msg.subject, 'Patchwork opt-out confirmation') + self.assertTrue(url in msg.body) + + def testOptoutRequestInvalidPOSTEmpty(self): + response = self.client.post(self.url, {'email': ''}) + self.assertEquals(response.status_code, 200) + self.assertFormError(response, 'form', 'email', + 'This field is required.') + self.assertTrue(response.context['error']) + self.assertTrue('email_sent' not in response.context) + self.assertEquals(len(mail.outbox), 0) + + def testOptoutRequestInvalidPOSTNonEmail(self): + response = self.client.post(self.url, {'email': 'foo'}) + self.assertEquals(response.status_code, 200) + self.assertFormError(response, 'form', 'email', + 'Enter a valid e-mail address.') + self.assertTrue(response.context['error']) + self.assertTrue('email_sent' not in response.context) + self.assertEquals(len(mail.outbox), 0) + +class OptoutTest(TestCase): + view = 'patchwork.views.mail.optout' + url = reverse(view) + + def setUp(self): + self.email = u'foo@example.com' + self.conf = EmailConfirmation(type = 'optout', email = self.email) + self.conf.save() + + def testOptoutValidHash(self): + url = reverse('patchwork.views.confirm', + kwargs = {'key': self.conf.key}) + response = self.client.get(url) + + self.assertEquals(response.status_code, 200) + self.assertTemplateUsed(response, 'patchwork/optout.html') + self.assertTrue(self.email in response.content) + + # check that we've got an optout in the list + self.assertEquals(EmailOptout.objects.count(), 1) + self.assertEquals(EmailOptout.objects.all()[0].email, self.email) + + # check that the confirmation is now inactive + self.assertFalse(EmailConfirmation.objects.get( + pk = self.conf.pk).active) + + +class OptoutPreexistingTest(OptoutTest): + """Test that a duplicated opt-out behaves the same as the initial one""" + def setUp(self): + super(OptoutPreexistingTest, self).setUp() + EmailOptout(email = self.email).save() + +class OptinRequestTest(TestCase): + view = 'patchwork.views.mail.optin' + url = reverse(view) + + def setUp(self): + self.email = u'foo@example.com' + EmailOptout(email = self.email).save() + + def testOptInRequestGET(self): + response = self.client.get(self.url) + self.assertRedirects(response, reverse('patchwork.views.mail.settings')) + + def testOptInRequestValidPOST(self): + response = self.client.post(self.url, {'email': self.email}) + + # check for a confirmation object + self.assertEquals(EmailConfirmation.objects.count(), 1) + conf = EmailConfirmation.objects.get(email = self.email) + + # check confirmation page + self.assertEquals(response.status_code, 200) + self.assertEquals(response.context['confirmation'], conf) + self.assertTrue(self.email in response.content) + + # check email + url = reverse('patchwork.views.confirm', kwargs = {'key': conf.key}) + self.assertEquals(len(mail.outbox), 1) + msg = mail.outbox[0] + self.assertEquals(msg.to, [self.email]) + self.assertEquals(msg.subject, 'Patchwork opt-in confirmation') + self.assertTrue(url in msg.body) + + def testOptoutRequestInvalidPOSTEmpty(self): + response = self.client.post(self.url, {'email': ''}) + self.assertEquals(response.status_code, 200) + self.assertFormError(response, 'form', 'email', + 'This field is required.') + self.assertTrue(response.context['error']) + self.assertTrue('email_sent' not in response.context) + self.assertEquals(len(mail.outbox), 0) + + def testOptoutRequestInvalidPOSTNonEmail(self): + response = self.client.post(self.url, {'email': 'foo'}) + self.assertEquals(response.status_code, 200) + self.assertFormError(response, 'form', 'email', + 'Enter a valid e-mail address.') + self.assertTrue(response.context['error']) + self.assertTrue('email_sent' not in response.context) + self.assertEquals(len(mail.outbox), 0) + +class OptinTest(TestCase): + + def setUp(self): + self.email = u'foo@example.com' + self.optout = EmailOptout(email = self.email) + self.optout.save() + self.conf = EmailConfirmation(type = 'optin', email = self.email) + self.conf.save() + + def testOptinValidHash(self): + url = reverse('patchwork.views.confirm', + kwargs = {'key': self.conf.key}) + response = self.client.get(url) + + self.assertEquals(response.status_code, 200) + self.assertTemplateUsed(response, 'patchwork/optin.html') + self.assertTrue(self.email in response.content) + + # check that there's no optout remaining + self.assertEquals(EmailOptout.objects.count(), 0) + + # check that the confirmation is now inactive + self.assertFalse(EmailConfirmation.objects.get( + pk = self.conf.pk).active) + +class OptinWithoutOptoutTest(TestCase): + """Test an opt-in with no existing opt-out""" + view = 'patchwork.views.mail.optin' + url = reverse(view) + + def testOptInWithoutOptout(self): + email = u'foo@example.com' + response = self.client.post(self.url, {'email': email}) + + # check for an error message + self.assertEquals(response.status_code, 200) + self.assertTrue(bool(response.context['error'])) + self.assertTrue('not on the patchwork opt-out list' in response.content) + +class UserProfileOptoutFormTest(TestCase): + """Test that the correct optin/optout forms appear on the user profile + page, for logged-in users""" + + view = 'patchwork.views.user.profile' + url = reverse(view) + optout_url = reverse('patchwork.views.mail.optout') + optin_url = reverse('patchwork.views.mail.optin') + form_re_template = (']*action="%(url)s"[^>]*>' + '.*?]*value="%(email)s"[^>]*>.*?' + '') + secondary_email = 'test2@example.com' + + def setUp(self): + self.user = create_user() + self.client.login(username = self.user.username, + password = self.user.username) + + def _form_re(self, url, email): + return re.compile(self.form_re_template % {'url': url, 'email': email}, + re.DOTALL) + + def testMainEmailOptoutForm(self): + form_re = self._form_re(self.optout_url, self.user.email) + response = self.client.get(self.url) + self.assertEquals(response.status_code, 200) + self.assertTrue(form_re.search(response.content) is not None) + + def testMainEmailOptinForm(self): + EmailOptout(email = self.user.email).save() + form_re = self._form_re(self.optin_url, self.user.email) + response = self.client.get(self.url) + self.assertEquals(response.status_code, 200) + self.assertTrue(form_re.search(response.content) is not None) + + def testSecondaryEmailOptoutForm(self): + p = Person(email = self.secondary_email, user = self.user) + p.save() + + form_re = self._form_re(self.optout_url, p.email) + response = self.client.get(self.url) + self.assertEquals(response.status_code, 200) + self.assertTrue(form_re.search(response.content) is not None) + + def testSecondaryEmailOptinForm(self): + p = Person(email = self.secondary_email, user = self.user) + p.save() + EmailOptout(email = p.email).save() + + form_re = self._form_re(self.optin_url, self.user.email) + response = self.client.get(self.url) + self.assertEquals(response.status_code, 200) + self.assertTrue(form_re.search(response.content) is not None) diff --git a/apps/patchwork/urls.py b/apps/patchwork/urls.py index 6810e3e..10fc3b9 100644 --- a/apps/patchwork/urls.py +++ b/apps/patchwork/urls.py @@ -73,6 +73,11 @@ urlpatterns = patterns('', # submitter autocomplete (r'^submitter/$', 'patchwork.views.submitter_complete'), + # email setup + (r'^mail/$', 'patchwork.views.mail.settings'), + (r'^mail/optout/$', 'patchwork.views.mail.optout'), + (r'^mail/optin/$', 'patchwork.views.mail.optin'), + # help! (r'^help/(?P.*)$', 'patchwork.views.help'), ) diff --git a/apps/patchwork/views/base.py b/apps/patchwork/views/base.py index 590a3b6..82c0368 100644 --- a/apps/patchwork/views/base.py +++ b/apps/patchwork/views/base.py @@ -59,10 +59,12 @@ def pwclient(request): return response def confirm(request, key): - import patchwork.views.user + import patchwork.views.user, patchwork.views.mail views = { 'userperson': patchwork.views.user.link_confirm, 'registration': patchwork.views.user.register_confirm, + 'optout': patchwork.views.mail.optout_confirm, + 'optin': patchwork.views.mail.optin_confirm, } conf = get_object_or_404(EmailConfirmation, key = key) diff --git a/apps/patchwork/views/mail.py b/apps/patchwork/views/mail.py new file mode 100644 index 0000000..aebba34 --- /dev/null +++ b/apps/patchwork/views/mail.py @@ -0,0 +1,119 @@ +# Patchwork - automated patch tracking system +# Copyright (C) 2010 Jeremy Kerr +# +# This file is part of the Patchwork package. +# +# Patchwork is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# Patchwork is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Patchwork; if not, write to the Free Software +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + +from patchwork.requestcontext import PatchworkRequestContext +from patchwork.models import EmailOptout, EmailConfirmation +from patchwork.forms import OptinoutRequestForm, EmailForm +from django.shortcuts import render_to_response +from django.template.loader import render_to_string +from django.conf import settings as conf_settings +from django.core.mail import send_mail +from django.core.urlresolvers import reverse +from django.http import HttpResponseRedirect + +def settings(request): + context = PatchworkRequestContext(request) + if request.method == 'POST': + form = EmailForm(data = request.POST) + if form.is_valid(): + email = form.cleaned_data['email'] + is_optout = EmailOptout.objects.filter(email = email).count() > 0 + context.update({ + 'email': email, + 'is_optout': is_optout, + }) + return render_to_response('patchwork/mail-settings.html', context) + + else: + form = EmailForm() + context['form'] = form + return render_to_response('patchwork/mail-form.html', context) + +def optout_confirm(request, conf): + context = PatchworkRequestContext(request) + + email = conf.email.strip().lower() + # silently ignore duplicated optouts + if EmailOptout.objects.filter(email = email).count() == 0: + optout = EmailOptout(email = email) + optout.save() + + conf.deactivate() + context['email'] = conf.email + + return render_to_response('patchwork/optout.html', context) + +def optin_confirm(request, conf): + context = PatchworkRequestContext(request) + + email = conf.email.strip().lower() + EmailOptout.objects.filter(email = email).delete() + + conf.deactivate() + context['email'] = conf.email + + return render_to_response('patchwork/optin.html', context) + +def optinout(request, action, description): + context = PatchworkRequestContext(request) + + mail_template = 'patchwork/%s-request.mail' % action + html_template = 'patchwork/%s-request.html' % action + + if request.method != 'POST': + return HttpResponseRedirect(reverse(settings)) + + form = OptinoutRequestForm(data = request.POST) + if not form.is_valid(): + context['error'] = ('There was an error in the %s form. ' + + 'Please review the form and re-submit.') % \ + description + context['form'] = form + return render_to_response(html_template, context) + + email = form.cleaned_data['email'] + if action == 'optin' and \ + EmailOptout.objects.filter(email = email).count() == 0: + context['error'] = ('The email address %s is not on the ' + + 'patchwork opt-out list, so you don\'t ' + + 'need to opt back in') % email + context['form'] = form + return render_to_response(html_template, context) + + conf = EmailConfirmation(type = action, email = email) + conf.save() + context['confirmation'] = conf + mail = render_to_string(mail_template, context) + try: + send_mail('Patchwork %s confirmation' % description, mail, + conf_settings.DEFAULT_FROM_EMAIL, [email]) + context['email'] = mail + context['email_sent'] = True + except Exception, ex: + context['error'] = 'An error occurred during confirmation . ' + \ + 'Please try again later.' + context['admins'] = conf_settings.ADMINS + + return render_to_response(html_template, context) + +def optout(request): + return optinout(request, 'optout', 'opt-out') + +def optin(request): + return optinout(request, 'optin', 'opt-in') diff --git a/apps/patchwork/views/user.py b/apps/patchwork/views/user.py index 3d28f4b..4a0e845 100644 --- a/apps/patchwork/views/user.py +++ b/apps/patchwork/views/user.py @@ -24,7 +24,8 @@ from django.shortcuts import render_to_response, get_object_or_404 from django.contrib import auth from django.contrib.sites.models import Site from django.http import HttpResponseRedirect -from patchwork.models import Project, Bundle, Person, EmailConfirmation, State +from patchwork.models import Project, Bundle, Person, EmailConfirmation, \ + State, EmailOptout from patchwork.forms import UserProfileForm, UserPersonLinkForm, \ RegistrationForm from patchwork.filters import DelegateFilter @@ -99,7 +100,13 @@ def profile(request): context['bundles'] = Bundle.objects.filter(owner = request.user) context['profileform'] = form - people = Person.objects.filter(user = request.user) + optout_query = '%s.%s IN (SELECT %s FROM %s)' % ( + Person._meta.db_table, + Person._meta.get_field('email').column, + EmailOptout._meta.get_field('email').column, + EmailOptout._meta.db_table) + people = Person.objects.filter(user = request.user) \ + .extra(select = {'is_optout': optout_query}) context['linked_emails'] = people context['linkform'] = UserPersonLinkForm() diff --git a/lib/sql/grant-all.mysql.sql b/lib/sql/grant-all.mysql.sql index a3d758c..c272e1e 100644 --- a/lib/sql/grant-all.mysql.sql +++ b/lib/sql/grant-all.mysql.sql @@ -22,6 +22,7 @@ GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_project TO 'www-data'@localhos GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_bundle TO 'www-data'@localhost; GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_bundle_patches TO 'www-data'@localhost; GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_patch TO 'www-data'@localhost; +GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_emailoptout TO 'www-data'@localhost; -- allow the mail user (in this case, 'nobody') to add patches GRANT INSERT, SELECT ON patchwork_patch TO 'nobody'@localhost; diff --git a/lib/sql/grant-all.postgres.sql b/lib/sql/grant-all.postgres.sql index 591ffd0..9b6c862 100644 --- a/lib/sql/grant-all.postgres.sql +++ b/lib/sql/grant-all.postgres.sql @@ -22,7 +22,8 @@ GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_project, patchwork_bundle, patchwork_bundlepatch, - patchwork_patch + patchwork_patch, + patchwork_emailoptout TO "www-data"; GRANT SELECT, UPDATE ON auth_group_id_seq, diff --git a/lib/sql/migration/010-optout-tables.sql b/lib/sql/migration/010-optout-tables.sql new file mode 100644 index 0000000..0a5d835 --- /dev/null +++ b/lib/sql/migration/010-optout-tables.sql @@ -0,0 +1,5 @@ +BEGIN; +CREATE TABLE "patchwork_emailoptout" ( + "email" varchar(200) NOT NULL PRIMARY KEY +); +COMMIT; diff --git a/templates/base.html b/templates/base.html index 9e80dca..d3b8e67 100644 --- a/templates/base.html +++ b/templates/base.html @@ -31,6 +31,8 @@ login
register +
+ mail settings {% endif %}
diff --git a/templates/patchwork/mail-form.html b/templates/patchwork/mail-form.html new file mode 100644 index 0000000..d71b2fb --- /dev/null +++ b/templates/patchwork/mail-form.html @@ -0,0 +1,38 @@ +{% extends "base.html" %} + +{% block title %}mail settings{% endblock %} +{% block heading %}mail settings{% endblock %} + +{% block body %} + +

You can configure patchwork to send you mail on certain events, +or block automated mail altogether. Enter your email address to +view or change your email settings.

+ +
+{% csrf_token %} + +{% if form.errors %} + + + +{% endif %} + + + + + + + +
+ There was an error accessing your mail settings: +
{{ form.email.label_tag }} + {{form.email}} + {{form.email.errors}} +
+ +
+
+ + +{% endblock %} diff --git a/templates/patchwork/mail-settings.html b/templates/patchwork/mail-settings.html new file mode 100644 index 0000000..303139a --- /dev/null +++ b/templates/patchwork/mail-settings.html @@ -0,0 +1,37 @@ +{% extends "base.html" %} + +{% block title %}mail settings{% endblock %} +{% block heading %}mail settings{% endblock %} + +{% block body %} +

Settings for {{email}}:

+ + + + +{% if is_optout %} + + + +{% else %} + + +{% endif %} + +
Opt-out listPatchwork may not send automated notifications to + this address. +
+ {% csrf_token %} + + +
+ 		Patchwork may send automated notifications to + this address. +
+ {% csrf_token %} + + +
+
+ +{% endblock %} diff --git a/templates/patchwork/optin-request.html b/templates/patchwork/optin-request.html new file mode 100644 index 0000000..63a4e12 --- /dev/null +++ b/templates/patchwork/optin-request.html @@ -0,0 +1,50 @@ +{% extends "base.html" %} + +{% block title %}opt-in{% endblock %} +{% block heading %}opt-in{% endblock %} + +{% block body %} +{% if email_sent %} +

Opt-in confirmation email sent

+

An opt-in confirmation mail has been sent to +{{confirmation.email}}, containing a link. Please click on +that link to confirm your opt-in.

+{% else %} +{% if error %} +

{{error}}

+{% endif %} + +{% if form %} +

This form allows you to opt-in to automated email from patchwork. Use +this if you have previously opted-out of patchwork mail, but now want to +received notifications from patchwork.

+When you submit it, an email will be sent to your address with a link to click +to finalise the opt-in. Patchwork does this to prevent someone opting you in +without your consent.

+
+{% csrf_token %} +{{form.email.errors}} +
+{{form.email.label_tag}}: {{form.email}} +
+ +
+{% endif %} + +{% if error and admins %} +

If you are having trouble opting in, please email +{% for admin in admins %} +{% if admins|length > 1 and forloop.last %} or {% endif %} +{{admin.0}} <{{admin.1}}>{% if admins|length > 2 and not forloop.last %}, {% endif %} +{% endfor %} +{% endif %} + +{% endif %} + +{% if user.is_authenticated %} +

Return to your user +profile.

+{% endif %} + +{% endblock %} diff --git a/templates/patchwork/optin-request.mail b/templates/patchwork/optin-request.mail new file mode 100644 index 0000000..34dd2c7 --- /dev/null +++ b/templates/patchwork/optin-request.mail @@ -0,0 +1,12 @@ +Hi, + +This email is to confirm that you would like to opt-in to automated +email from the patchwork system at {{site.domain}}. + +To complete the opt-in process, visit: + + http://{{site.domain}}{% url patchwork.views.confirm key=confirmation.key %} + +If you didn't request this opt-in, you don't need to do anything. + +Happy patchworking. diff --git a/templates/patchwork/optin.html b/templates/patchwork/optin.html new file mode 100644 index 0000000..f7c0c04 --- /dev/null +++ b/templates/patchwork/optin.html @@ -0,0 +1,19 @@ +{% extends "base.html" %} + +{% block title %}opt-in{% endblock %} +{% block heading %}opt-in{% endblock %} + +{% block body %} + +

Opt-in complete. You have sucessfully opted back in to +automated email from this patchwork system, using the address +{{email}}.

+

If you later decide that you no longer want to receive automated mail from +patchwork, just visit http://{{site.domain}}{% url patchwork.views.mail.settings %}, or +visit the main patchwork page and navigate from there.

+{% if user.is_authenticated %} +

Return to your user +profile.

+{% endif %} +{% endblock %} diff --git a/templates/patchwork/optout-request.html b/templates/patchwork/optout-request.html new file mode 100644 index 0000000..dbdf250 --- /dev/null +++ b/templates/patchwork/optout-request.html @@ -0,0 +1,51 @@ +{% extends "base.html" %} + +{% block title %}opt-out{% endblock %} +{% block heading %}opt-out{% endblock %} + +{% block body %} +{% if email_sent %} +

Opt-out confirmation email sent

+

An opt-out confirmation mail has been sent to +{{confirmation.email}}, containing a link. Please click on +that link to confirm your opt-out.

+{% else %} +{% if error %} +

{{error}}

+{% endif %} + +{% if form %} +

This form allows you to opt-out of automated email from patchwork.

+

If you opt-out of email, Patchwork may still email you if you do certain +actions yourself (such as create a new patchwork account), but will not send +you unsolicited email.

+When you submit it, one email will be sent to your address with a link to click +to finalise the opt-out. Patchwork does this to prevent someone opting you out +without your consent.

+
+{% csrf_token %} +{{form.email.errors}} +
+{{form.email.label_tag}}: {{form.email}} +
+ +
+{% endif %} + +{% if error and admins %} +

If you are having trouble opting out, please email +{% for admin in admins %} +{% if admins|length > 1 and forloop.last %} or {% endif %} +{{admin.0}} <{{admin.1}}>{% if admins|length > 2 and not forloop.last %}, {% endif %} +{% endfor %} +{% endif %} + +{% endif %} + +{% if user.is_authenticated %} +

Return to your user +profile.

+{% endif %} + +{% endblock %} diff --git a/templates/patchwork/optout-request.mail b/templates/patchwork/optout-request.mail new file mode 100644 index 0000000..f896e3c --- /dev/null +++ b/templates/patchwork/optout-request.mail @@ -0,0 +1,12 @@ +Hi, + +This email is to confirm that you would like to opt-out from all email +from the patchwork system at {{site.domain}}. + +To complete the opt-out process, visit: + + http://{{site.domain}}{% url patchwork.views.confirm key=confirmation.key %} + +If you didn't request this opt-out, you don't need to do anything. + +Happy patchworking. diff --git a/templates/patchwork/optout.html b/templates/patchwork/optout.html new file mode 100644 index 0000000..6b97806 --- /dev/null +++ b/templates/patchwork/optout.html @@ -0,0 +1,22 @@ +{% extends "base.html" %} + +{% block title %}opt-out{% endblock %} +{% block heading %}opt-out{% endblock %} + +{% block body %} + +

Opt-out complete. You have successfully opted-out of +automated notifications from this patchwork system, from the address +{{email}}

+

Please note that you may still receive email from other patchwork setups at +different sites, as they are run independently. You may need to opt-out of +those separately.

+

If you later decide to receive mail from patchwork, just visit +http://{{site.domain}}{% url patchwork.views.mail.settings %}, or +visit the main patchwork page and navigate from there.

+{% if user.is_authenticated %} +

Return to your user +profile.

+{% endif %} +{% endblock %} diff --git a/templates/patchwork/profile.html b/templates/patchwork/profile.html index 44df921..130b947 100644 --- a/templates/patchwork/profile.html +++ b/templates/patchwork/profile.html @@ -40,34 +40,50 @@ Contributor to

The following email addresses are associated with this patchwork account. Adding alternative addresses allows patchwork to group contributions that you have made under different addresses.

+

The "notify?" column allows you to opt-in or -out of automated +patchwork notification emails. Setting it to "no" will disable automated +notifications for that address.

Adding a new email address will send a confirmation email to that address.

- +
- - - - + + {% for email in linked_emails %} - {% ifnotequal email.email user.email %} + - {% endifnotequal %} {% endfor %} -
email -
{{ user.email }}actionnotify?
{{ email.email }} - {% ifnotequal user.email email.email %} + {% ifnotequal user.email email.email %}
{% csrf_token %}
{% endifnotequal %} + 		+ {% if email.is_optout %} +
+ No, + {% csrf_token %} + + +
+ {% else %} +
+ Yes, + {% csrf_token %} + + +
+ {% endif %} +
+
{% csrf_token %} {{ linkform.email }} -- cgit v1.2.3 From 798a73b8bfb41f742e78e481ab9c961556e117b3 Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Tue, 29 Mar 2011 11:58:39 +0800 Subject: models: Add PatchChangeNotification and record patch state changes Add a PatchChangeNotification model to keep track of changes to a patch's state. Hook this up to Patch's pre_save signal. Requires a DB schema upgrade. Signed-off-by: Jeremy Kerr --- apps/patchwork/models.py | 44 ++++++++ apps/patchwork/tests/__init__.py | 1 + apps/patchwork/tests/notifications.py | 117 +++++++++++++++++++++ lib/sql/grant-all.mysql.sql | 1 + lib/sql/grant-all.postgres.sql | 6 +- .../migration/011-patch-change-notifications.sql | 12 +++ 6 files changed, 179 insertions(+), 2 deletions(-) create mode 100644 apps/patchwork/tests/notifications.py create mode 100644 lib/sql/migration/011-patch-change-notifications.sql diff --git a/apps/patchwork/models.py b/apps/patchwork/models.py index f21d073..17a68db 100644 --- a/apps/patchwork/models.py +++ b/apps/patchwork/models.py @@ -64,6 +64,7 @@ class Project(models.Model): name = models.CharField(max_length=255, unique=True) listid = models.CharField(max_length=255, unique=True) listemail = models.CharField(max_length=200) + send_notifications = models.BooleanField() def __unicode__(self): return self.name @@ -406,3 +407,46 @@ class EmailOptout(models.Model): def __unicode__(self): return self.email + +class PatchChangeNotification(models.Model): + patch = models.ForeignKey(Patch, primary_key = True) + last_modified = models.DateTimeField(default = datetime.datetime.now) + orig_state = models.ForeignKey(State) + +def _patch_change_callback(sender, instance, **kwargs): + # we only want notification of modified patches + if instance.pk is None: + return + + if instance.project is None or not instance.project.send_notifications: + return + + try: + orig_patch = Patch.objects.get(pk = instance.pk) + except Patch.DoesNotExist: + return + + # If there's no interesting changes, abort without creating the + # notification + if orig_patch.state == instance.state: + return + + notification = None + try: + notification = PatchChangeNotification.objects.get(patch = instance) + except PatchChangeNotification.DoesNotExist: + pass + + if notification is None: + notification = PatchChangeNotification(patch = instance, + orig_state = orig_patch.state) + + elif notification.orig_state == instance.state: + # If we're back at the original state, there is no need to notify + notification.delete() + return + + notification.last_modified = datetime.datetime.now() + notification.save() + +models.signals.pre_save.connect(_patch_change_callback, sender = Patch) diff --git a/apps/patchwork/tests/__init__.py b/apps/patchwork/tests/__init__.py index 0b56fc1..8ae271a 100644 --- a/apps/patchwork/tests/__init__.py +++ b/apps/patchwork/tests/__init__.py @@ -27,3 +27,4 @@ from patchwork.tests.confirm import * from patchwork.tests.registration import * from patchwork.tests.user import * from patchwork.tests.mail_settings import * +from patchwork.tests.notifications import * diff --git a/apps/patchwork/tests/notifications.py b/apps/patchwork/tests/notifications.py new file mode 100644 index 0000000..c4df1b0 --- /dev/null +++ b/apps/patchwork/tests/notifications.py @@ -0,0 +1,117 @@ +# Patchwork - automated patch tracking system +# Copyright (C) 2011 Jeremy Kerr +# +# This file is part of the Patchwork package. +# +# Patchwork is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# Patchwork is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Patchwork; if not, write to the Free Software +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + +from django.test import TestCase +from django.core.urlresolvers import reverse +from django.db.utils import IntegrityError +from patchwork.models import Patch, State, PatchChangeNotification +from patchwork.tests.utils import defaults, create_maintainer + +class PatchNotificationModelTest(TestCase): + """Tests for the creation & update of the PatchChangeNotification model""" + + 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) + + def tearDown(self): + self.patch.delete() + self.submitter.delete() + self.project.delete() + + def testPatchCreation(self): + """Ensure we don't get a notification on create""" + self.patch.save() + self.assertEqual(PatchChangeNotification.objects.count(), 0) + + def testPatchUninterestingChange(self): + """Ensure we don't get a notification for "uninteresting" changes""" + self.patch.save() + self.patch.archived = True + self.patch.save() + self.assertEqual(PatchChangeNotification.objects.count(), 0) + + def testPatchChange(self): + """Ensure we get a notification for interesting patch changes""" + self.patch.save() + oldstate = self.patch.state + state = State.objects.exclude(pk = oldstate.pk)[0] + + self.patch.state = state + self.patch.save() + self.assertEqual(PatchChangeNotification.objects.count(), 1) + notification = PatchChangeNotification.objects.all()[0] + self.assertEqual(notification.patch, self.patch) + self.assertEqual(notification.orig_state, oldstate) + + def testNotificationCancelled(self): + """Ensure we cancel notifications that are no longer valid""" + self.patch.save() + oldstate = self.patch.state + state = State.objects.exclude(pk = oldstate.pk)[0] + + self.patch.state = state + self.patch.save() + self.assertEqual(PatchChangeNotification.objects.count(), 1) + + self.patch.state = oldstate + self.patch.save() + self.assertEqual(PatchChangeNotification.objects.count(), 0) + + def testNotificationUpdated(self): + """Ensure we update notifications when the patch has a second change, + but keep the original patch details""" + self.patch.save() + oldstate = self.patch.state + newstates = State.objects.exclude(pk = oldstate.pk)[:2] + + self.patch.state = newstates[0] + self.patch.save() + self.assertEqual(PatchChangeNotification.objects.count(), 1) + notification = PatchChangeNotification.objects.all()[0] + self.assertEqual(notification.orig_state, oldstate) + orig_timestamp = notification.last_modified + + self.patch.state = newstates[1] + self.patch.save() + self.assertEqual(PatchChangeNotification.objects.count(), 1) + notification = PatchChangeNotification.objects.all()[0] + self.assertEqual(notification.orig_state, oldstate) + self.assertTrue(notification.last_modified > orig_timestamp) + + def testProjectNotificationsDisabled(self): + """Ensure we don't see notifications created when a project is + configured not to send them""" + self.project.send_notifications = False + self.project.save() + + self.patch.save() + oldstate = self.patch.state + state = State.objects.exclude(pk = oldstate.pk)[0] + + self.patch.state = state + self.patch.save() + self.assertEqual(PatchChangeNotification.objects.count(), 0) + diff --git a/lib/sql/grant-all.mysql.sql b/lib/sql/grant-all.mysql.sql index c272e1e..1bff526 100644 --- a/lib/sql/grant-all.mysql.sql +++ b/lib/sql/grant-all.mysql.sql @@ -23,6 +23,7 @@ GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_bundle TO 'www-data'@localhost GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_bundle_patches TO 'www-data'@localhost; GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_patch TO 'www-data'@localhost; GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_emailoptout TO 'www-data'@localhost; +GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_patchchangenotification TO 'www-data'@localhost; -- allow the mail user (in this case, 'nobody') to add patches GRANT INSERT, SELECT ON patchwork_patch TO 'nobody'@localhost; diff --git a/lib/sql/grant-all.postgres.sql b/lib/sql/grant-all.postgres.sql index 9b6c862..72abb57 100644 --- a/lib/sql/grant-all.postgres.sql +++ b/lib/sql/grant-all.postgres.sql @@ -23,7 +23,8 @@ GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_bundle, patchwork_bundlepatch, patchwork_patch, - patchwork_emailoptout + patchwork_emailoptout, + patchwork_patchchangenotification TO "www-data"; GRANT SELECT, UPDATE ON auth_group_id_seq, @@ -45,7 +46,8 @@ GRANT SELECT, UPDATE ON patchwork_state_id_seq, patchwork_emailconfirmation_id_seq, patchwork_userprofile_id_seq, - patchwork_userprofile_maintainer_projects_id_seq + patchwork_userprofile_maintainer_projects_id_seq, + patchwork_patchchangenotification_id_seq TO "www-data"; -- allow the mail user (in this case, 'nobody') to add patches diff --git a/lib/sql/migration/011-patch-change-notifications.sql b/lib/sql/migration/011-patch-change-notifications.sql new file mode 100644 index 0000000..0a9b9b7 --- /dev/null +++ b/lib/sql/migration/011-patch-change-notifications.sql @@ -0,0 +1,12 @@ +BEGIN; +CREATE TABLE "patchwork_patchchangenotification" ( + "patch_id" integer NOT NULL PRIMARY KEY REFERENCES "patchwork_patch" ("id") DEFERRABLE INITIALLY DEFERRED, + "last_modified" timestamp with time zone NOT NULL, + "orig_state_id" integer NOT NULL REFERENCES "patchwork_state" ("id") DEFERRABLE INITIALLY DEFERRED +) +; +ALTER TABLE "patchwork_project" ADD COLUMN + "send_notifications" boolean NOT NULL DEFAULT False; +ALTER TABLE "patchwork_project" ALTER COLUMN + "send_notifications" DROP DEFAULT; +COMMIT; -- cgit v1.2.3 From f94d40159168d0811de576328b77fd2a553039af Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Tue, 29 Mar 2011 22:18:54 +0800 Subject: 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 --- apps/patchwork/bin/patchwork-cron.py | 13 +++ apps/patchwork/tests/notifications.py | 112 +++++++++++++++++++++ apps/patchwork/utils.py | 59 ++++++++++- apps/settings.py | 2 + .../patch-change-notification-subject.text | 1 + templates/patchwork/patch-change-notification.mail | 19 ++++ 6 files changed, 205 insertions(+), 1 deletion(-) create mode 100755 apps/patchwork/bin/patchwork-cron.py create mode 100644 templates/patchwork/patch-change-notification-subject.text create mode 100644 templates/patchwork/patch-change-notification.mail 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 %} -- cgit v1.2.3 From 017f73b059aadfce30ee29d2aceeab92739105a6 Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Thu, 14 Apr 2011 11:25:47 +0800 Subject: notifications: Add NOTIFICATION_FROM_EMAIL setting Allow a separate From: address for notificaton emails. Signed-off-by: Jeremy Kerr --- apps/patchwork/utils.py | 2 +- apps/settings.py | 1 + docs/INSTALL | 2 ++ 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/apps/patchwork/utils.py b/apps/patchwork/utils.py index 94b3f53..58edb19 100644 --- a/apps/patchwork/utils.py +++ b/apps/patchwork/utils.py @@ -181,7 +181,7 @@ def send_notifications(): context) message = EmailMessage(subject = subject, body = content, - from_email = settings.DEFAULT_FROM_EMAIL, + from_email = settings.NOTIFICATION_FROM_EMAIL, to = [recipient.email], headers = {'Precedence': 'bulk'}) diff --git a/apps/settings.py b/apps/settings.py index d5595e0..4432f3f 100644 --- a/apps/settings.py +++ b/apps/settings.py @@ -104,6 +104,7 @@ DEFAULT_FROM_EMAIL = 'Patchwork ' CONFIRMATION_VALIDITY_DAYS = 7 NOTIFICATION_DELAY_MINUTES = 10 +NOTIFICATION_FROM_EMAIL = DEFAULT_FROM_EMAIL # Set to True to enable the Patchwork XML-RPC interface ENABLE_XMLRPC = False diff --git a/docs/INSTALL b/docs/INSTALL index 6a1a0bf..050fc9f 100644 --- a/docs/INSTALL +++ b/docs/INSTALL @@ -104,6 +104,8 @@ in brackets): ADMINS TIME_ZONE LANGUAGE_CODE + DEFAULT_FROM_EMAIL + NOTIFICATION_FROM_EMAIL You can generate the SECRET_KEY with the following python code: -- cgit v1.2.3 From f1e5f6a2c9d737f12290f5bd5a934b74c362616f Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Thu, 14 Apr 2011 19:37:55 +0800 Subject: notifications: implement opt-out Check for opt-out status before sending notification mail. Signed-off-by: Jeremy Kerr --- apps/patchwork/models.py | 5 +++++ apps/patchwork/tests/notifications.py | 14 +++++++++++++- apps/patchwork/utils.py | 13 +++++++++++-- 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/apps/patchwork/models.py b/apps/patchwork/models.py index 17a68db..22062c2 100644 --- a/apps/patchwork/models.py +++ b/apps/patchwork/models.py @@ -408,6 +408,11 @@ class EmailOptout(models.Model): def __unicode__(self): return self.email + @classmethod + def is_optout(cls, email): + email = email.lower().strip() + return cls.objects.filter(email = email).count() > 0 + class PatchChangeNotification(models.Model): patch = models.ForeignKey(Patch, primary_key = True) last_modified = models.DateTimeField(default = datetime.datetime.now) diff --git a/apps/patchwork/tests/notifications.py b/apps/patchwork/tests/notifications.py index ae37988..f14b30b 100644 --- a/apps/patchwork/tests/notifications.py +++ b/apps/patchwork/tests/notifications.py @@ -23,7 +23,7 @@ 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.models import Patch, State, PatchChangeNotification, EmailOptout from patchwork.tests.utils import defaults, create_maintainer from patchwork.utils import send_notifications @@ -172,6 +172,18 @@ class PatchNotificationEmailTest(TestCase): self.assertEquals(msg.to, [self.submitter.email]) self.assertTrue(self.patch.get_absolute_url() in msg.body) + def testNotificationOptout(self): + """ensure opt-out addresses don't get notifications""" + PatchChangeNotification(patch = self.patch, + orig_state = self.patch.state).save() + self._expireNotifications() + + EmailOptout(email = self.submitter.email).save() + + errors = send_notifications() + self.assertEquals(errors, []) + self.assertEquals(len(mail.outbox), 0) + def testNotificationMerge(self): patches = [self.patch, Patch(project = self.project, msgid = 'testpatch-2', diff --git a/apps/patchwork/utils.py b/apps/patchwork/utils.py index 58edb19..5cb45e8 100644 --- a/apps/patchwork/utils.py +++ b/apps/patchwork/utils.py @@ -28,7 +28,7 @@ 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 + PatchChangeNotification, EmailOptout def get_patch_ids(d, prefix = 'patch_id'): ids = [] @@ -169,6 +169,15 @@ def send_notifications(): for (recipient, notifications) in groups: notifications = list(notifications) + + def delete_notifications(): + PatchChangeNotification.objects.filter( + pk__in = notifications).delete() + + if EmailOptout.is_optout(recipient.email): + delete_notifications() + continue + context = { 'site': Site.objects.get_current(), 'person': recipient, @@ -191,6 +200,6 @@ def send_notifications(): errors.append((recipient, ex)) continue - PatchChangeNotification.objects.filter(pk__in = notifications).delete() + delete_notifications() return errors -- cgit v1.2.3