diff --git a/src/postorius/models.py b/src/postorius/models.py index 7dcb8cd..2280ec6 100644 --- a/src/postorius/models.py +++ b/src/postorius/models.py @@ -196,16 +196,20 @@ email_str = email_str.encode('utf-8') activation_key = hashlib.sha1( str(random.random())+email_str).hexdigest() + # Make now tz naive (we don't care about the timezone) + now = datetime.now().replace(tzinfo=None) # Either update an existing profile record for the given email address try: profile = self.get(email=email) profile.activation_key = activation_key - profile.created = datetime.now() + profile.created = now profile.save() # ... or create a new one. except AddressConfirmationProfile.DoesNotExist: - profile = self.create( - email=email, activation_key=activation_key, user=user) + profile = self.create(email=email, + activation_key=activation_key, + user=user, + created=now) return profile @@ -216,7 +220,7 @@ """ email = models.EmailField() activation_key = models.CharField(max_length=40) - created = models.DateTimeField(auto_now_add=True) + created = models.DateTimeField() user = models.ForeignKey(User) objects = AddressConfirmationProfileManager() @@ -235,7 +239,8 @@ """ expiration_delta = getattr( settings, 'EMAIL_CONFIRMATION_EXPIRATION_DELTA', timedelta(days=1)) - age = datetime.now() - self.created + age = datetime.now().replace(tzinfo=None) - \ + self.created.replace(tzinfo=None) return age > expiration_delta def _create_host_url(self, request): diff --git a/src/postorius/tests/test_address_activation.py b/src/postorius/tests/test_address_activation.py index 7c53a34..02fef05 100644 --- a/src/postorius/tests/test_address_activation.py +++ b/src/postorius/tests/test_address_activation.py @@ -10,19 +10,19 @@ from django.test.utils import override_settings from django.utils import unittest from mailmanclient._client import _Connection -from mock import patch +from mock import patch, call from postorius.forms import AddressActivationForm from postorius.models import AddressConfirmationProfile from postorius import views -from postorius.views.user import AddressActivationView +from postorius.views.user import AddressActivationView, address_activation_link class TestAddressActivationForm(unittest.TestCase): """ Test the activation form. """ - + def test_valid_email_is_valid(self): data = { 'email': 'les@example.org', @@ -30,7 +30,7 @@ } form = AddressActivationForm(data) self.assertTrue(form.is_valid()) - + def test_identical_emails_are_invalid(self): data = { 'email': 'les@example.org', @@ -38,7 +38,7 @@ } form = AddressActivationForm(data) self.assertFalse(form.is_valid()) - + def test_invalid_email_is_not_valid(self): data = { 'email': 'les@example', @@ -52,7 +52,7 @@ """ Tests to make sure the view is properly connected, renders the form correctly and starts the actual address activation process if a valid - form is submitted. + form is submitted. """ def setUp(self): @@ -60,7 +60,7 @@ # We don't use Client().login because it triggers the browserid dance. self.user = User.objects.create_user( username='les', email='les@example.org', password='secret') - self.client = Client() + self.client = Client() self.client.post(reverse('user_login'), {'username': 'les', 'password': 'secret'}) @@ -80,9 +80,9 @@ self.assertTrue('form' in response.context) def test_view_renders_correct_template(self): - # The view should render the user_address_activation template. + # The view should render the user_address_activation template. response = self.client.get(reverse('address_activation')) - self.assertTrue('postorius/user_address_activation.html' + self.assertTrue('postorius/user_address_activation.html' in [t.name for t in response.templates]) def test_post_invalid_form_shows_error_msg(self): @@ -100,13 +100,13 @@ 'email': 'new_address@example.org', 'user_email': self.user.email}) self.assertEqual(mock_send_confirmation_link.call_count, 1) - self.assertTrue('postorius/user_address_activation_sent.html' + self.assertTrue('postorius/user_address_activation_sent.html' in [t.name for t in response.templates]) class TestAddressConfirmationProfile(unittest.TestCase): """ - Test the confirmation of an email address activation (validating token, + Test the confirmation of an email address activation (validating token, expiration, Mailman API calls etc.). """ @@ -131,7 +131,7 @@ self.assertTrue(type(self.profile.created), datetime) def test_no_duplicate_profiles(self): - # Creating a new profile returns an existing record + # Creating a new profile returns an existing record # (if one exists), instead of creating a new one. new_profile = AddressConfirmationProfile.objects.create_profile( u'les@example.org', @@ -144,14 +144,14 @@ 'Address Confirmation Profile for les@example.org') def test_profile_not_expired_default_setting(self): - # A profile created less then a day ago is not expired by default. + # A profile created less then a day ago is not expired by default. delta = timedelta(hours=23) now = datetime.now() self.profile.created = now - delta self.assertFalse(self.profile.is_expired) def test_profile_is_expired_default_setting(self): - # A profile older than 1 day is expired by default. + # A profile older than 1 day is expired by default. delta = timedelta(days=1, hours=1) now = datetime.now() self.profile.created = now - delta @@ -173,24 +173,18 @@ def test_confirmation_link(self): # The profile obj can send out a confirmation email. # set the activation key to a fixed string for testing - self.profile.activation_key = '6323fba0097781fdb887cfc37a1122ee7c8bb0b0' + self.profile.activation_key = \ + '6323fba0097781fdb887cfc37a1122ee7c8bb0b0' self.profile.send_confirmation_link(self.request) self.assertEqual(mail.outbox[0].to[0], u'les@example.org') self.assertEqual(mail.outbox[0].subject, u'Confirmation needed') - self.assertEqual(mail.outbox[0].body, """\ -Please click the link below to add your email address to your mailing list -profile at http://testserver: + self.assertTrue(self.profile.activation_key in mail.outbox[0].body) -http://testserver/postorius/users/address_activation/\ -6323fba0097781fdb887cfc37a1122ee7c8bb0b0/ - -Thanks! -""") class TestAddressActivationLinkSuccess(unittest.TestCase): """ - This tests the activation link view if the key is valid and the profile is - not expired. + This tests the activation link view if the key is valid and the profile is + not expired. """ def setUp(self): @@ -199,20 +193,24 @@ username='ler', email=u'ler@mailman.mostdesirable.org', password='pwd') self.profile = AddressConfirmationProfile.objects.create_profile( - 'les@mailman.mostdesirable.org', self.user) - self.profile.activation_key = '6323fba0097781fdb887cfc37a1122ee7c8bb0b0' + u'les@mailman.mostdesirable.org', self.user) + self.profile.activation_key = \ + u'6323fba0097781fdb887cfc37a1122ee7c8bb0b0' self.profile.save() def tearDown(self): self.profile.delete() self.user.delete() - + @patch.object(views.user, '_add_address') - def test_mailman(self, mock_call): - # An activation key pointing to a valid profile adds the address to the user - self.response = Client().get( - reverse('address_activation_link', kwargs={ + def test_mailman(self, _add_address_mock): + # An activation key pointing to a valid profile adds the address + # to the user. + request = RequestFactory().get(reverse( + 'address_activation_link', kwargs={ 'activation_key': '6323fba0097781fdb887cfc37a1122ee7c8bb0b0'})) - self.assertEqual(mock_call.call_count, 1) - - + address_activation_link( + request, '6323fba0097781fdb887cfc37a1122ee7c8bb0b0') + expected_calls = [call(request, u'ler@mailman.mostdesirable.org', + u'les@mailman.mostdesirable.org')] + self.assertEqual(_add_address_mock.mock_calls, expected_calls) diff --git a/src/postorius/views/user.py b/src/postorius/views/user.py index 1701b59..fac5df2 100644 --- a/src/postorius/views/user.py +++ b/src/postorius/views/user.py @@ -249,7 +249,7 @@ profile.send_confirmation_link(request) except SMTPException: messages.error(request, 'The email confirmation message could ' - 'not be sent.') + 'not be sent. %s' % profile.activation_key) return render_to_response('postorius/user_address_activation_sent.html', context_instance=RequestContext(request)) return render_to_response('postorius/user_address_activation.html', @@ -393,12 +393,13 @@ context_instance=RequestContext(request)) -def _add_address(user_email, address): +def _add_address(request, user_email, address): + # Add an address to a user record in mailman. try: mailman_user = utils.get_client().get_user(user_email) mailman_user.add_address(address) - except (MailmanApiError, MailmanConnectionError): - pass + except (MailmanApiError, MailmanConnectionError) as e: + messages.error(request, 'The address could not be added.') def address_activation_link(request, activation_key): @@ -411,7 +412,7 @@ profile = AddressConfirmationProfile.objects.get( activation_key=activation_key) if not profile.is_expired: - _add_address(profile.user.email, profile.email) + _add_address(request, profile.user.email, profile.email) except profile.DoesNotExist: pass return render_to_response('postorius/user_address_activation_link.html',