Merge pull request #2773 from jezdez/bug1056950

fix bug 1056950 - only send welcome email when at least one email address is verified

kuma/users/models.py

@@ -6,7 +6,7 @@
 from django.db import models
 from django.utils.functional import cached_property
 
-from allauth.account.signals import user_signed_up
+from allauth.account.signals import user_signed_up, email_confirmed
 from allauth.socialaccount.signals import social_account_removed
 import constance.config
 from jsonfield import JSONField
@@ -190,7 +190,21 @@ def create_user_profile(sender, instance, created, **kwargs):
 @receiver(user_signed_up)
 def on_user_signed_up(sender, request, user, **kwargs):
     if switch_is_active('welcome_email'):
-        send_welcome_email.delay(user.pk, request.locale)
+        # only send if the user has already verified at least one email address
+        if user.emailaddress_set.filter(verified=True).exists():
+            send_welcome_email.delay(user.pk, request.locale)
+
+
+@receiver(email_confirmed)
+def on_email_confirmed(sender, request, email_address, **kwargs):
+    if switch_is_active('welcome_email'):
+        # only send if the user has exactly one verified (the given)
+        # email address, in other words if it was just confirmed
+        if not (email_address.user
+                             .emailaddress_set.exclude(pk=email_address.pk)
+                                              .exists()):
+            send_welcome_email.delay(email_address.user.pk, request.locale)
+
 
 
 @receiver(social_account_removed)

kuma/users/tests/test_adapters.py

@@ -1,4 +1,3 @@
-import mock
 from nose.plugins.attrib import attr
 from nose.tools import eq_, assert_raises
 

kuma/users/tests/test_tasks.py

@@ -1,9 +1,14 @@
 import mock
 from nose.tools import eq_, ok_
+from waffle import Switch
 
 from django.conf import settings
 from django.contrib.auth.models import User
 from django.core import mail
+from django.test import RequestFactory
+
+from allauth.account.models import EmailAddress
+from allauth.account.signals import user_signed_up
 
 from kuma.users.tasks import send_welcome_email
 from sumo.tests import TestCase
@@ -29,3 +34,84 @@ def test_dont_send_untranslated_language_email(self,
         send_welcome_email(u.pk, 'de')
 
         eq_([], mail.outbox)
+
+    def test_welcome_mail_for_verified_email(self):
+        Switch.objects.get_or_create(name='welcome_email', active=True)
+        user = User.objects.create_user('welcome',
+                                        'welcome@tester.com',
+                                        'welcome')
+        request = RequestFactory().get('/')
+        request.locale = 'en-US'
+        user_signed_up.send(sender=user.__class__, request=request, user=user)
+
+        # no email sent
+        eq_(len(mail.outbox), 0)
+
+        EmailAddress.objects.create(user=user,
+                                    email='welcome@tester.com',
+                                    verified=True)
+
+        user_signed_up.send(sender=user.__class__, request=request, user=user)
+
+        # only one email, the welcome email, is sent, no confirmation needed
+        eq_(len(mail.outbox), 1)
+        welcome_email = mail.outbox[0]
+        expected_to = [user.email]
+        eq_(expected_to, welcome_email.to)
+        ok_(u'utm_campaign=welcome' in welcome_email.body)
+
+    def test_welcome_mail_for_unverified_email(self):
+        Switch.objects.get_or_create(name='welcome_email', active=True)
+        user = User.objects.create_user('welcome2',
+                                        'welcome2@tester.com',
+                                        'welcome2')
+        email_address = EmailAddress.objects.create(user=user,
+                                                    email='welcome2@tester.com',
+                                                    verified=False)
+        request = RequestFactory().get('/')
+        request.locale = 'en-US'
+
+        # emulate the phase in which the request for email confirmation is
+        # sent as the user's email address is not verified
+        email_address.send_confirmation(request)
+
+        # only one email, the confirmation email is sent
+        eq_(1, email_address.emailconfirmation_set.count())
+        eq_(len(mail.outbox), 1)
+        confirm_email = mail.outbox[0]
+        expected_to = [email_address.email]
+        eq_(expected_to, confirm_email.to)
+        ok_('Confirm' in confirm_email.subject)
+
+        # then get the email confirmation and confirm it by emulating
+        # clicking on the confirm link
+        email_confirmation = email_address.emailconfirmation_set.all()[0]
+        email_confirmation.confirm(request)
+
+        # a second email, the welcome email, is sent
+        eq_(len(mail.outbox), 2)
+        welcome_email = mail.outbox[1]
+        expected_to = [email_address.email]
+        eq_(expected_to, welcome_email.to)
+        ok_('utm_campaign=welcome' in welcome_email.body)
+
+        # now add second unverified email address to the user
+        # and check if the usual confirmation email is sent out
+        email_address2 = EmailAddress.objects.create(user=user,
+                                                     email='welcome3@tester.com',
+                                                     verified=False)
+        email_address2.send_confirmation(request)
+        eq_(1, email_address2.emailconfirmation_set.count())
+        eq_(len(mail.outbox), 3)
+        confirm_email2 = mail.outbox[2]
+        expected_to = [email_address2.email]
+        eq_(expected_to, confirm_email2.to)
+        ok_('Confirm' in confirm_email2.subject)
+
+        # but this time there should no welcome email be sent as there
+        # is already a verified email address
+        email_confirmation2 = email_address2.emailconfirmation_set.all()[0]
+        email_confirmation2.confirm(request)
+        # no increase in number of emails
+        eq_(len(mail.outbox), 3)
+        ok_('Confirm' in mail.outbox[2].subject)

kuma/users/tests/test_views.py

@@ -11,7 +11,6 @@
 from allauth.socialaccount.models import SocialAccount
 
 from devmo.tests import mock_lookup_user, LocalizingClient
-from sumo.helpers import urlparams
 from sumo.tests import TestCase
 from sumo.urlresolvers import reverse
 from ..models import UserProfile, UserBan
@@ -124,7 +123,8 @@ def _get_current_form_field_values(self, doc):
         form = dict()
         for fn in ('email', 'fullname', 'title', 'organization', 'location',
                    'irc_nickname', 'bio', 'interests'):
-            form[fn] = doc.find('#profile-edit *[name="profile-%s"]' % fn).val()
+            form[fn] = doc.find('#profile-edit *[name="profile-%s"]' %
+                                fn).val()
         form['country'] = 'us'
         form['format'] = 'html'
         return form
@@ -322,7 +322,8 @@ def test_profile_edit_websites(self, unsubscribe, subscribe, lookup_user):
         doc = pq(r.content)
         for k, v in test_sites.items():
             eq_(v,
-                doc.find('#profile-edit *[name="profile-websites_%s"]' % k).val())
+                doc.find('#profile-edit *[name="profile-websites_%s"]' %
+                         k).val())
 
         # Come up with some bad sites, either invalid URL or bad URL prefix
         bad_sites = {
@@ -598,15 +599,13 @@ def test_persona_signup_create_django_user(self):
                 'status': 'okay',
                 'email': self.persona_signup_email,
             }
-            r = self.client.post(reverse('persona_login'),
-                                 follow=True)
+            self.client.post(reverse('persona_login'), follow=True)
             data = {'username': self.persona_signup_username,
                     'email': self.persona_signup_email}
-            r = self.client.post(
-                reverse('socialaccount_signup',
-                        locale=settings.WIKI_DEFAULT_LANGUAGE),
-                data=data,
-                follow=True)
+            self.client.post(reverse('socialaccount_signup',
+                                     locale=settings.WIKI_DEFAULT_LANGUAGE),
+                             data=data,
+                             follow=True)
             new_count = User.objects.count()
             # Did we get a new user?
             eq_(old_count + 1, new_count)
@@ -637,18 +636,17 @@ def test_persona_signup_create_socialaccount(self):
                 'status': 'okay',
                 'email': self.persona_signup_email,
             }
-            r = self.client.post(reverse('persona_login'),
-                                 follow=True)
+            self.client.post(reverse('persona_login'), follow=True)
             data = {'username': self.persona_signup_username,
                     'email': self.persona_signup_email}
-            r = self.client.post(
-                reverse('socialaccount_signup',
-                        locale=settings.WIKI_DEFAULT_LANGUAGE),
-                data=data,
-                follow=True)
+            self.client.post(reverse('socialaccount_signup',
+                                     locale=settings.WIKI_DEFAULT_LANGUAGE),
+                             data=data,
+                             follow=True)
             socialaccount = None
             try:
-                socialaccount = SocialAccount.objects.order_by('-date_joined')[0]
+                socialaccount = (SocialAccount.objects
+                                              .order_by('-date_joined')[0])
             except IndexError:
                 pass
             ok_(socialaccount is not None)