[fix bug 989110] Send email notification to Council for a budget requ…

…est.

* Remove send_mail_task.
* Refactor send_remo_mail.

remo/base/forms.py

@@ -3,7 +3,7 @@
 from django import forms
 from django.contrib import messages
 
-from remo.base.tasks import send_mail_task
+from remo.base.tasks import send_remo_mail
 from remo.profiles.models import FunctionalArea, UserProfile
 
 
@@ -28,16 +28,13 @@ def __init__(self, users, *args, **kwargs):
         Dynamically set fields for the recipients of the mail.
         """
         super(EmailUsersForm, self).__init__(*args, **kwargs)
-        recipients = users.values_list('first_name', 'last_name', 'email')
-
-        for first_name, last_name, email in recipients:
-            field_name = '%s %s <%s>' % (first_name, last_name, email)
+        for user in users:
             # Insert method is used to override the order of form fields
             form_widget = forms.CheckboxInput(
                 attrs={'class': 'input-text-big'})
-            self.fields.insert(0, field_name,
+            self.fields.insert(0, str(user.id),
                                forms.BooleanField(
-                                   label=field_name,
+                                   label=user,
                                    initial=True,
                                    required=False,
                                    widget=form_widget))
@@ -48,12 +45,13 @@ def send_mail(self, request):
         for field in self.fields:
             if (isinstance(self.fields[field], forms.BooleanField) and
                     self.cleaned_data[field]):
-                recipients_list.append(field)
+                recipients_list.append(long(field))
+
         if recipients_list:
             from_email = '%s <%s>' % (request.user.get_full_name(),
                                       request.user.email)
-            send_mail_task.delay(sender=from_email,
-                                 recipients=recipients_list,
+            send_remo_mail.delay(sender=from_email,
+                                 recipients_list=recipients_list,
                                  subject=self.cleaned_data['subject'],
                                  message=self.cleaned_data['body'])
             messages.success(request, 'Email sent successfully.')
@@ -71,17 +69,13 @@ class EmailRepsForm(BaseEmailUsersFrom):
 
     def send_email(self, request, users):
         """Send mail to recipients list."""
-        recipients = users.values_list('first_name', 'last_name', 'email')
-        recipients_list = []
-        for first_name, last_name, email in recipients:
-            recipient = '%s %s <%s>' % (first_name, last_name, email)
-            recipients_list.append(recipient)
+        recipients = users.values_list('id', flat=True)
 
-        if recipients_list:
+        if recipients:
             from_email = '%s <%s>' % (request.user.get_full_name(),
                                       request.user.email)
-            send_mail_task.delay(sender=from_email,
-                                 recipients=recipients_list,
+            send_remo_mail.delay(sender=from_email,
+                                 recipients_list=recipients,
                                  subject=self.cleaned_data['subject'],
                                  message=self.cleaned_data['body'])
             messages.success(request, 'Email sent successfully.')

remo/base/tasks.py

@@ -1,18 +1,55 @@
+from django.conf import settings
+from django.contrib.auth.models import User
 from django.core.mail import EmailMessage
+from django.core.validators import email_re
+from django.template.loader import render_to_string
 
 from celery.task import task
 
 
 @task
-def send_mail_task(sender, recipients, subject, message):
-    """Send email from /sender/ to /recipients/ with /subject/ and
+def send_remo_mail(subject, recipients_list, sender=None,
+                   message=None, email_template=None, data=None,
+                   headers=None):
+    """Send email from /sender/ to /recipients_list/ with /subject/ and
     /message/ as body.
 
     """
+    # Make sure that there is either a message or a template
+    if not message and not email_template:
+        return
+    # Make sure that there is a recipient
+    if not recipients_list:
+        return
+    if not headers:
+        headers = {}
+    if not sender:
+        sender = settings.FROM_EMAIL
+    if not data:
+        data = {}
+    data.update({'SITE_URL': settings.SITE_URL,
+                 'FROM_EMAIL': settings.FROM_EMAIL})
 
     # Make sure subject is one line.
     subject = subject.replace('\n', ' ')
 
-    email = EmailMessage(subject=subject, body=message, from_email=sender,
-                         to=recipients, cc=[sender])
-    email.send()
+    for recipient in recipients_list:
+        to = ''
+        if (isinstance(recipient, long) and
+                User.objects.filter(pk=recipient).exists()):
+            user = User.objects.get(pk=recipient)
+            to = '%s <%s>' % (user.get_full_name(), user.email)
+            ctx_data = {'user': user,
+                        'userprofile': user.userprofile}
+            data.update(ctx_data)
+        elif email_re.match(recipient):
+            to = recipient
+        else:
+            return
+
+        if email_template:
+            message = render_to_string(email_template, data)
+
+        email = EmailMessage(subject=subject, body=message, from_email=sender,
+                             to=[to], cc=[sender], headers=headers)
+        email.send()

remo/base/tests/test_forms.py

@@ -46,12 +46,16 @@ def test_send_mail(self, fake_messages):
 
         form.send_email(request, reps)
 
-        eq_(len(mail.outbox), 1)
+        eq_(len(mail.outbox), 20)
 
         address = lambda u: '%s %s <%s>' % (u.first_name, u.last_name, u.email)
         recipients = map(address, reps)
 
-        eq_(set(mail.outbox[0].to), set(recipients))
-        eq_(mail.outbox[0].subject, data['subject'])
-        eq_(mail.outbox[0].body, data['body'])
+        receivers = []
+        for i in range(0, len(mail.outbox)):
+            eq_(mail.outbox[i].subject, data['subject'])
+            eq_(mail.outbox[i].body, data['body'])
+            receivers.append(mail.outbox[i].to[0])
+
+        eq_(set(receivers), set(recipients))
         fake_messages.assert_called_with(ANY, 'Email sent successfully.')

remo/base/tests/test_views.py

@@ -257,14 +257,11 @@ def test_view_dashboard_page(self):
         self.assertTemplateUsed(response, 'dashboard_reps.html')
 
     def test_email_my_mentees_mentor_with_send_True(self):
-        """Email mentees when mentor and checkbox
-        is checked."""
+        """Email mentees when mentor and checkbox is checked."""
         c = Client()
         c.login(username='mentor', password='passwd')
         rep1 = User.objects.get(first_name='Nick')
-        data = {'%s %s <%s>' % (rep1.first_name,
-                                rep1.last_name,
-                                rep1.email): 'True',
+        data = {'%s' % (rep1.id): 'True',
                 'subject': 'This is subject',
                 'body': ('This is my body\n',
                          'Multiline of course')}
@@ -278,14 +275,11 @@ def test_email_my_mentees_mentor_with_send_True(self):
         eq_(len(mail.outbox[0].cc), 1)
 
     def test_email_my_mentees_mentor_with_send_False(self):
-        """Email mentees when mentor and checkbox is
-        not checked."""
+        """Email mentees when mentor and checkbox is not checked."""
         c = Client()
         c.login(username='mentor', password='passwd')
         rep1 = User.objects.get(first_name='Nick')
-        data = {'%s %s <%s>' % (rep1.first_name,
-                                rep1.last_name,
-                                rep1.email): 'False',
+        data = {'%s' % (rep1.id): 'False',
                 'subject': 'This is subject',
                 'body': ('This is my body\n',
                          'Multiline of course')}

remo/events/models.py

@@ -16,10 +16,10 @@
 from uuslug import uuslug as slugify
 
 from remo.base.models import GenericActiveManager
+from remo.base.tasks import send_remo_mail
 from remo.base.utils import add_permissions_to_groups
 from remo.profiles.models import FunctionalArea
 from remo.remozilla.models import Bug
-from remo.reports.tasks import send_remo_mail
 
 
 SIMILAR_EVENTS = 3
@@ -227,4 +227,5 @@ def email_event_owner_on_add_comment(sender, instance, **kwargs):
     if owner.userprofile.receive_email_on_add_event_comment:
         subject = subject % (instance.user.get_full_name(),
                              instance.event.name)
-        send_remo_mail.delay([owner.id], subject, email_template, ctx_data)
+        send_remo_mail.delay(subject=subject, recipients_list=[owner.id],
+                             email_template=email_template, data=ctx_data)

remo/events/tests/test_views.py

@@ -630,9 +630,7 @@ def test_email_event_attendees(self, mock_success):
         reps = event.attendees.all()
         valid_data = dict()
         for rep in reps:
-            field_name = '%s %s <%s>' % (rep.first_name,
-                                         rep.last_name,
-                                         rep.email)
+            field_name = '%s' % rep.id
             valid_data[field_name] = 'True'
 
         valid_data['subject'] = 'This is the mail subject'
@@ -644,11 +642,11 @@ def test_email_event_attendees(self, mock_success):
         self.assertTemplateUsed(response, 'view_event.html')
 
         mock_success.assert_called_with(ANY, 'Email sent successfully.')
-        eq_(len(mail.outbox), 1)
+        eq_(len(mail.outbox), 4)
 
-        email = mail.outbox[0]
-        eq_(len(email.to), 4)
-        eq_(len(email.cc), 1)
+        for i in range(0, len(mail.outbox)):
+            eq_(len(mail.outbox[i].cc), 1)
+            eq_(len(mail.outbox[i].to), 1)
 
     @mock.patch('remo.events.views.iri_to_uri', wraps=iri_to_uri)
     def test_view_redirect_list_events(self, mocked_uri):

remo/profiles/models.py

@@ -16,8 +16,8 @@
 from uuslug import uuslug as slugify
 
 from remo.base.models import GenericActiveManager
+from remo.base.tasks import send_remo_mail
 from remo.base.utils import add_permissions_to_groups
-from remo.reports.tasks import send_remo_mail
 
 DISPLAY_NAME_MAX_LENGTH = 50
 
@@ -269,8 +269,8 @@ def email_mentor_notification(sender, instance, raw, **kwargs):
                           instance.mentor.id]
             ctx_data = {'rep_user': instance.user,
                         'new_mentor': instance.mentor}
-            send_remo_mail.delay(recipients, subject, email_template,
-                                 ctx_data)
+            send_remo_mail.delay(recipients_list=recipients, subject=subject,
+                                 email_template=email_template, data=ctx_data)
             statsd.incr('profiles.change_mentor')
 
 

remo/reports/models.py

@@ -16,14 +16,14 @@
 from remo.base.utils import (add_permissions_to_groups,
                              get_object_or_none)
 from remo.base.models import GenericActiveManager
+from remo.base.tasks import send_remo_mail
 from remo.base.utils import daterange, get_date
 from remo.events.helpers import get_event_link
 from remo.events.models import Attendance as EventAttendance, Event
 from remo.profiles.models import FunctionalArea
 from remo.reports import (ACTIVITY_CAMPAIGN, ACTIVITY_EVENT_ATTEND,
                           ACTIVITY_EVENT_CREATE, READONLY_ACTIVITIES,
                           VERIFIABLE_ACTIVITIES)
-from remo.reports.tasks import send_remo_mail
 
 
 @receiver(post_migrate, dispatch_uid='report_set_groups_signal')
@@ -405,8 +405,8 @@ def email_commenters_on_add_ng_report_comment(sender, instance, **kwargs):
                         'comment': instance.comment,
                         'created_on': instance.created_on}
             subject = subject.format(instance.user.get_full_name(), report)
-            send_remo_mail.delay([user_id], subject,
-                                 email_template, ctx_data)
+            send_remo_mail.delay(subject=subject, recipients_list=[user_id],
+                                 email_template=email_template, data=ctx_data)
 
 
 @receiver(pre_delete, sender=NGReport,

remo/reports/tasks.py

@@ -16,28 +16,6 @@
 DIGEST_SUBJECT = 'Your mentee activity for {date}'
 
 
-@task()
-def send_remo_mail(user_ids_list, subject, email_template, data=None):
-    """Send to user_list emails based rendered using email_template
-    and populated with data.
-
-    """
-    if not data:
-        data = {}
-
-    data.update({'SITE_URL': settings.SITE_URL,
-                 'FROM_EMAIL': settings.FROM_EMAIL})
-
-    for user_id in user_ids_list:
-        if User.objects.filter(pk=user_id).exists():
-            user = User.objects.get(pk=user_id)
-            ctx_data = {'user': user,
-                        'userprofile': user.userprofile}
-            ctx_data.update(data)
-            message = render_to_string(email_template, ctx_data)
-            send_mail(subject, message, settings.FROM_EMAIL, [user.email])
-
-
 @task()
 def send_report_digest():
     from remo.reports.models import NGReport

remo/reports/tests/test_models.py

@@ -57,7 +57,7 @@ def test_different_current_longest_streak(self):
         today = now().date()
         past_day = now().date() - datetime.timedelta(days=30)
         user = UserFactory.create()
-        #longest streak
+        # longest streak
         for i in range(0, 3):
             NGReportFactory.create(
                 user=user, report_date=past_day - datetime.timedelta(days=i))
@@ -246,7 +246,8 @@ def test_comment_one_user(self):
                                       comment='This is a comment')
 
         eq_(len(mail.outbox), 1)
-        eq_(reporter.email, mail.outbox[0].to[0])
+        eq_('%s <%s>' % (reporter.get_full_name(), reporter.email),
+            mail.outbox[0].to[0])
         msg = ('[Report] User {0} commented on {1}'
                .format(commenter.get_full_name(), report))
         eq_(mail.outbox[0].subject, msg)
@@ -282,8 +283,11 @@ def test_comment_multiple_users(self):
                                       comment='This is a comment')
 
         eq_(len(mail.outbox), 3)
-        recipients = [reporter.email, users_with_comments[0].email,
-                      users_with_comments[1].email]
+        recipients = ['%s <%s>' % (reporter.get_full_name(), reporter.email),
+                      '%s <%s>' % (users_with_comments[0].get_full_name(),
+                                   users_with_comments[0].email),
+                      '%s <%s>' % (users_with_comments[1].get_full_name(),
+                                   users_with_comments[1].email)]
         receivers = [mail.outbox[0].to[0], mail.outbox[1].to[0],
                      mail.outbox[2].to[0]]
         eq_(set(recipients), set(receivers))

remo/voting/cron.py

@@ -6,7 +6,7 @@
 
 from cronjobs import register
 
-from remo.reports.tasks import send_remo_mail
+from remo.base.tasks import send_remo_mail
 from remo.voting.models import Poll
 
 # Intervals in seconds
@@ -36,8 +36,9 @@ def poll_vote_reminder():
                        'for "%s" now!' % poll.name)
             template_reminder = 'emails/voting_vote_reminder.txt'
             ctx_data = {'poll': poll}
-            send_remo_mail.delay(recipients, subject,
-                                 template_reminder, ctx_data)
+            send_remo_mail.delay(subject=subject, recipients_list=recipients,
+                                 email_template=template_reminder,
+                                 data=ctx_data)
             Poll.objects.filter(pk=poll.pk).update(last_notification=now())
 
 

remo/voting/models.py

@@ -13,6 +13,7 @@
 from south.signals import post_migrate
 from uuslug import uuslug
 
+from remo.base.tasks import send_remo_mail
 from remo.base.utils import add_permissions_to_groups
 from remo.remozilla.models import Bug
 from remo.voting.tasks import send_voting_mail
@@ -21,6 +22,7 @@
 # Voting period in days
 BUDGET_REQUEST_PERIOD_START = 3
 BUDGET_REQUEST_PERIOD_END = 6
+BUGZILLA_URL = 'https://bugzilla.mozilla.org/show_bug.cgi?id='
 
 
 class Poll(models.Model):
@@ -147,6 +149,18 @@ def poll_email_reminder(sender, instance, raw, **kwargs):
     end_template = 'emails/voting_results_reminder.txt'
 
     if not instance.task_start_id or instance.is_future_voting:
+        if instance.automated_poll:
+            template = 'emails/review_budget_notify_council.txt'
+            subject = ('[Bug %s] Budget request discussion'
+                       % str(instance.bug_id))
+            data = {'bug': instance.bug,
+                    'BUGZILLA_URL': BUGZILLA_URL}
+            send_remo_mail.delay(
+                subject=subject, email_template=template,
+                recipients_list=[settings.REPS_COUNCIL_ALIAS],
+                data=data,
+                headers={'Reply-To': settings.REPS_COUNCIL_ALIAS})
+
         start_reminder = send_voting_mail.apply_async(
             eta=instance.start, kwargs={'voting_id': instance.id,
                                         'subject': subject_start,