Merge pull request #13038 from OmarIthawi/edraak/bulk-email-from-addr

Translatable bulk_email from Address based on platform`s default lang

cms/templates/js/mock/mock-settings-page.underscore

@@ -103,7 +103,7 @@
         <option value="" selected> - </option>
         <option value="en">English</option>
       </select>
-      <span class="tip tip-stacked">Identify the course language here. This is used to assist users find courses that are taught in a specific language.</span>
+      <span class="tip tip-stacked">Identify the course language here. This is used to assist users find courses that are taught in a specific language. It is also used to localize the 'From:' field in bulk emails.</span>
     </li>
 
   </ol>

cms/templates/settings.html

@@ -284,7 +284,7 @@ CMS.URL.UPLOAD_ASSET = '${upload_asset_url | n, js_escaped_string}'
                   <option value="${lang}">${label}</option>
                 % endfor
               </select>
-              <span class="tip tip-stacked">${_("Identify the course language here. This is used to assist users find courses that are taught in a specific language.")}</span>
+              <span class="tip tip-stacked">${_("Identify the course language here. This is used to assist users find courses that are taught in a specific language. It is also used to localize the 'From:' field in bulk emails.")}</span>
             </li>
           </ol>
         </div>

lms/djangoapps/bulk_email/admin.py

@@ -37,6 +37,7 @@ Other tags that may be used (surrounded by one curly brace on each side):
 {platform_name}        : the name of the platform
 {course_title}         : the name of the course
 {course_root}          : the URL path to the root of the course
+{course_language}      : the course language. The default is None.
 {course_url}           : the course's full URL
 {email}                : the user's email address
 {account_settings_url} : URL at which users can change account preferences

lms/djangoapps/bulk_email/tasks.py

@@ -35,6 +35,7 @@ from django.contrib.auth.models import User
 from django.core.mail import EmailMultiAlternatives, get_connection
 from django.core.mail.message import forbid_multi_line_headers
 from django.core.urlresolvers import reverse
+from django.utils.translation import override as override_language, ugettext as _
 
 from bulk_email.models import CourseEmail, Optout
 from courseware.courses import get_course
@@ -109,6 +110,7 @@ def _get_course_email_context(course):
     email_context = {
         'course_title': course_title,
         'course_root': course_root,
+        'course_language': course.language,
         'course_url': course_url,
         'course_image_url': image_url,
         'course_end_date': course_end_date,
@@ -350,7 +352,7 @@ def _filter_optouts_from_recipients(to_list, course_id):
     return to_list, num_optout
 
 
-def _get_source_address(course_id, course_title, truncate=True):
+def _get_source_address(course_id, course_title, course_language, truncate=True):
     """
     Calculates an email address to be used as the 'from-address' for sent emails.
 
@@ -373,7 +375,17 @@ def _get_source_address(course_id, course_title, truncate=True):
     # character appears.
     course_name = re.sub(r"[^\w.-]", '_', course_id.course)
 
-    from_addr_format = u'"{course_title}" Course Staff <{course_name}-{from_email}>'
+    # Use course.language if present
+    language = course_language if course_language else settings.LANGUAGE_CODE
+    with override_language(language):
+        # RFC2821 requires the byte order of the email address to be the name then email
+        #   e.g. "John Doe <email@example.com>"
+        # Although the display will be flipped in RTL languages, the byte order is still the same.
+        from_addr_format = u'{name} {email}'.format(
+            # Translators: Bulk email from address e.g. ("Physics 101" Course Staff)
+            name=_('"{course_title}" Course Staff'),
+            email=u'<{course_name}-{from_email}>',
+        )
 
     def format_address(course_title_no_quotes):
         """
@@ -475,10 +487,11 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas
         subtask_status.increment(skipped=num_optout)
 
     course_title = global_email_context['course_title']
+    course_language = global_email_context['course_language']
 
     # use the email from address in the CourseEmail, if it is present, otherwise compute it
     from_addr = course_email.from_addr if course_email.from_addr else \
-        _get_source_address(course_email.course_id, course_title)
+        _get_source_address(course_email.course_id, course_title, course_language)
 
     # use the CourseEmailTemplate that was associated with the CourseEmail
     course_email_template = course_email.get_template()

lms/djangoapps/bulk_email/tests/test_email.py

@@ -8,6 +8,7 @@ from mock import patch, Mock
 from nose.plugins.attrib import attr
 import os
 from unittest import skipIf
+import ddt
 
 from django.conf import settings
 from django.core import mail
@@ -15,6 +16,7 @@ from django.core.mail.message import forbid_multi_line_headers
 from django.core.urlresolvers import reverse
 from django.core.management import call_command
 from django.test.utils import override_settings
+from django.utils.translation import get_language
 
 from bulk_email.models import Optout, BulkEmailFlag
 from bulk_email.tasks import _get_source_address, _get_course_email_context
@@ -132,6 +134,99 @@ class EmailSendFromDashboardTestCase(SharedModuleStoreTestCase):
         BulkEmailFlag.objects.all().delete()
 
 
+class SendEmailWithMockedUgettextMixin(object):
+    """
+    Mock uggetext for EmailSendFromDashboardTestCase.
+    """
+    def send_email(self):
+        """
+        Sends a dummy email to check the `from_addr` translation.
+        """
+        test_email = {
+            'action': 'send',
+            'send_to': '["myself"]',
+            'subject': 'test subject for myself',
+            'message': 'test message for myself'
+        }
+
+        def mock_ugettext(text):
+            """
+            Mocks ugettext to return the lang code with the original string.
+
+            e.g.
+
+            >>> mock_ugettext('Hello') == '@AR Hello@'
+            """
+            return u'@{lang} {text}@'.format(
+                lang=get_language().upper(),
+                text=text,
+            )
+
+        with patch('bulk_email.tasks._', side_effect=mock_ugettext):
+            self.client.post(self.send_mail_url, test_email)
+
+        return mail.outbox[0]
+
+
+@attr(shard=1)
+@patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': False})
+@ddt.ddt
+class LocalizedFromAddressPlatformLangTestCase(SendEmailWithMockedUgettextMixin, EmailSendFromDashboardTestCase):
+    """
+    Tests to ensure that the bulk email has the "From" address localized according to LANGUAGE_CODE.
+    """
+    @override_settings(LANGUAGE_CODE='en')
+    def test_english_platform(self):
+        """
+        Ensures that the source-code language (English) works well.
+        """
+        self.assertIsNone(self.course.language)  # Sanity check
+        message = self.send_email()
+        self.assertRegexpMatches(message.from_email, '.*Course Staff.*')
+
+    @override_settings(LANGUAGE_CODE='eo')
+    def test_esperanto_platform(self):
+        """
+        Tests the fake Esperanto language to ensure proper gettext calls.
+        """
+        self.assertIsNone(self.course.language)  # Sanity check
+        message = self.send_email()
+        self.assertRegexpMatches(message.from_email, '@EO .* Course Staff@')
+
+
+@attr(shard=1)
+@patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': False})
+@ddt.ddt
+class LocalizedFromAddressCourseLangTestCase(SendEmailWithMockedUgettextMixin, EmailSendFromDashboardTestCase):
+    """
+    Test if the bulk email "From" address uses the course.language if present instead of LANGUAGE_CODE.
+
+    This is similiar to LocalizedFromAddressTestCase but creating a different test case to allow
+    changing the class-wide course object.
+    """
+
+    @classmethod
+    def setUpClass(cls):
+        """
+        Creates a different course.
+        """
+        super(LocalizedFromAddressCourseLangTestCase, cls).setUpClass()
+        course_title = u"ẗëṡẗ ｲэ"
+        cls.course = CourseFactory.create(
+            display_name=course_title,
+            language='ar',
+            default_store=ModuleStoreEnum.Type.split
+        )
+
+    @override_settings(LANGUAGE_CODE='eo')
+    def test_esperanto_platform_arabic_course(self):
+        """
+        The course language should override the platform's.
+        """
+        message = self.send_email()
+        self.assertRegexpMatches(message.from_email, '@AR .* Course Staff@')
+
+
 @attr(shard=1)
 @patch('bulk_email.models.html_to_text', Mock(return_value='Mocking CourseEmail.text_message', autospec=True))
 class TestEmailSendFromDashboardMockedHtmlToText(EmailSendFromDashboardTestCase):
@@ -394,7 +489,7 @@ class TestEmailSendFromDashboardMockedHtmlToText(EmailSendFromDashboardTestCase)
         instructor = InstructorFactory(course_key=course.id)
 
         unexpected_from_addr = _get_source_address(
-            course.id, course.display_name, truncate=False
+            course.id, course.display_name, course_language=None, truncate=False
         )
         __, encoded_unexpected_from_addr = forbid_multi_line_headers(
             "from", unexpected_from_addr, 'utf-8'

lms/djangoapps/bulk_email/tests/test_tasks.py

@@ -440,6 +440,7 @@ class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase):
         result = _get_course_email_context(self.course)
         self.assertIn('course_title', result)
         self.assertIn('course_root', result)
+        self.assertIn('course_language', result)
         self.assertIn('course_url', result)
         self.assertIn('course_image_url', result)
         self.assertIn('course_end_date', result)