From 0a9a39e4b18b6aa46fa998be62f83e87b3a15359 Mon Sep 17 00:00:00 2001 From: Omar Al-Ithawi Date: Tue, 7 Jun 2016 10:51:14 +0300 Subject: [PATCH 1/4] Translatable bulk_email from Address based on platform`s default language --- lms/djangoapps/bulk_email/tasks.py | 8 +- lms/djangoapps/bulk_email/tests/test_email.py | 85 +++++++++++++++++++ 2 files changed, 92 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/bulk_email/tasks.py b/lms/djangoapps/bulk_email/tasks.py index e6dca1f895..6a8f788f8c 100644 --- a/lms/djangoapps/bulk_email/tasks.py +++ b/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 @@ -373,7 +374,12 @@ 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}>' + with override_language(settings.LANGUAGE_CODE): + 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): """ diff --git a/lms/djangoapps/bulk_email/tests/test_email.py b/lms/djangoapps/bulk_email/tests/test_email.py index 1834eedcb4..4d7653238b 100644 --- a/lms/djangoapps/bulk_email/tests/test_email.py +++ b/lms/djangoapps/bulk_email/tests/test_email.py @@ -16,6 +16,7 @@ from django.core.urlresolvers import reverse from django.core.management import call_command from django.test.utils import override_settings +from django.utils import translation from bulk_email.models import Optout, BulkEmailFlag from bulk_email.tasks import _get_source_address, _get_course_email_context from openedx.core.djangoapps.course_groups.models import CourseCohort @@ -132,6 +133,90 @@ class EmailSendFromDashboardTestCase(SharedModuleStoreTestCase): BulkEmailFlag.objects.all().delete() +@attr(shard=1) +@patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': False}) +class TestLocalizedFromAddress(EmailSendFromDashboardTestCase): + + original_ugettext = None + mocked_lang = 'ar' + + def setUp(self): + super(TestLocalizedFromAddress, self).setUp() + + translations = translation.trans_real._translations # pylint: disable=protected-access + + with translation.override(self.mocked_lang): + # In order to undo it later + self.original_ugettext = translations[self.mocked_lang].ugettext + + mocked_ugettext = self.get_mocked_ugettext(self.mocked_lang) + translations[self.mocked_lang].ugettext = mocked_ugettext + + def get_mocked_ugettext(self, lang_code): + """ + Mocks ugettext to return the lang code with the original string. + + e.g. + + >>> ugettext = self.mock_ugettext('ar') + >>> ugettext('Hello') == '@AR Hello@' + """ + def mocked_ugettext(msg): + """ + A mock of ugettext to isolate it from the real `.mo` files. + """ + return u'@{} {}@'.format(lang_code.upper(), msg) + + return mocked_ugettext + + 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' + } + + self.client.post(self.send_mail_url, test_email) + + return mail.outbox[0] + + @override_settings(LANGUAGE_CODE='en') + def test_english_platform(self): + """ + Test if the email `from` is localized to the platform's preference. + """ + + message = self.send_email() + + self.assertNotRegexpMatches( + message.from_email, + '@.* Course Staff@' + ) + + @override_settings(LANGUAGE_CODE='ar') + def test_arabic_platform(self): + """ + Test if the email `from` is localized to the platform's preference. + """ + + message = self.send_email() + + self.assertRegexpMatches( + message.from_email, + '@AR .* Course Staff@' + ) + + def tearDown(self): + super(TestLocalizedFromAddress, self).tearDown() + + translations = translation.trans_real._translations # pylint: disable=protected-access + translations[self.mocked_lang].ugettext = self.original_ugettext + + @attr(shard=1) @patch('bulk_email.models.html_to_text', Mock(return_value='Mocking CourseEmail.text_message', autospec=True)) class TestEmailSendFromDashboardMockedHtmlToText(EmailSendFromDashboardTestCase): From d29453204d3589ff546cc60edcd2507c41287ae3 Mon Sep 17 00:00:00 2001 From: Omar Al-Ithawi Date: Wed, 5 Apr 2017 12:34:17 +0300 Subject: [PATCH 2/4] Simplified the test --- lms/djangoapps/bulk_email/tests/test_email.py | 79 ++++++------------- 1 file changed, 22 insertions(+), 57 deletions(-) diff --git a/lms/djangoapps/bulk_email/tests/test_email.py b/lms/djangoapps/bulk_email/tests/test_email.py index 4d7653238b..9c1fa2d543 100644 --- a/lms/djangoapps/bulk_email/tests/test_email.py +++ b/lms/djangoapps/bulk_email/tests/test_email.py @@ -15,8 +15,8 @@ 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 django.utils import translation from bulk_email.models import Optout, BulkEmailFlag from bulk_email.tasks import _get_source_address, _get_course_email_context from openedx.core.djangoapps.course_groups.models import CourseCohort @@ -136,39 +136,6 @@ class EmailSendFromDashboardTestCase(SharedModuleStoreTestCase): @attr(shard=1) @patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': False}) class TestLocalizedFromAddress(EmailSendFromDashboardTestCase): - - original_ugettext = None - mocked_lang = 'ar' - - def setUp(self): - super(TestLocalizedFromAddress, self).setUp() - - translations = translation.trans_real._translations # pylint: disable=protected-access - - with translation.override(self.mocked_lang): - # In order to undo it later - self.original_ugettext = translations[self.mocked_lang].ugettext - - mocked_ugettext = self.get_mocked_ugettext(self.mocked_lang) - translations[self.mocked_lang].ugettext = mocked_ugettext - - def get_mocked_ugettext(self, lang_code): - """ - Mocks ugettext to return the lang code with the original string. - - e.g. - - >>> ugettext = self.mock_ugettext('ar') - >>> ugettext('Hello') == '@AR Hello@' - """ - def mocked_ugettext(msg): - """ - A mock of ugettext to isolate it from the real `.mo` files. - """ - return u'@{} {}@'.format(lang_code.upper(), msg) - - return mocked_ugettext - def send_email(self): """ Sends a dummy email to check the `from_addr` translation. @@ -180,41 +147,39 @@ class TestLocalizedFromAddress(EmailSendFromDashboardTestCase): 'message': 'test message for myself' } - self.client.post(self.send_mail_url, test_email) + 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] @override_settings(LANGUAGE_CODE='en') def test_english_platform(self): """ - Test if the email `from` is localized to the platform's preference. + Ensures that the source-code language (English) works well. """ - message = self.send_email() + self.assertRegexpMatches(message.from_email, '.*Course Staff.*') - self.assertNotRegexpMatches( - message.from_email, - '@.* Course Staff@' - ) - - @override_settings(LANGUAGE_CODE='ar') - def test_arabic_platform(self): + @override_settings(LANGUAGE_CODE='eo') + def test_esperanto_platform(self): """ - Test if the email `from` is localized to the platform's preference. + Tests the fake Esperanto language to ensure proper gettext calls. """ - message = self.send_email() - - self.assertRegexpMatches( - message.from_email, - '@AR .* Course Staff@' - ) - - def tearDown(self): - super(TestLocalizedFromAddress, self).tearDown() - - translations = translation.trans_real._translations # pylint: disable=protected-access - translations[self.mocked_lang].ugettext = self.original_ugettext + self.assertRegexpMatches(message.from_email, '@EO .* Course Staff@') @attr(shard=1) From 3739cc4549df5bdbbad9e6d730457f118c487540 Mon Sep 17 00:00:00 2001 From: Omar Al-Ithawi Date: Wed, 5 Apr 2017 14:29:43 +0300 Subject: [PATCH 3/4] Make the course lang override platform lang --- lms/djangoapps/bulk_email/admin.py | 1 + lms/djangoapps/bulk_email/tasks.py | 13 +++-- lms/djangoapps/bulk_email/tests/test_email.py | 53 +++++++++++++++++-- lms/djangoapps/bulk_email/tests/test_tasks.py | 1 + 4 files changed, 61 insertions(+), 7 deletions(-) diff --git a/lms/djangoapps/bulk_email/admin.py b/lms/djangoapps/bulk_email/admin.py index e42f7f535f..ac6937a7da 100644 --- a/lms/djangoapps/bulk_email/admin.py +++ b/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 diff --git a/lms/djangoapps/bulk_email/tasks.py b/lms/djangoapps/bulk_email/tasks.py index 6a8f788f8c..f64da3e29c 100644 --- a/lms/djangoapps/bulk_email/tasks.py +++ b/lms/djangoapps/bulk_email/tasks.py @@ -110,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, @@ -351,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. @@ -374,7 +375,12 @@ def _get_source_address(course_id, course_title, truncate=True): # character appears. course_name = re.sub(r"[^\w.-]", '_', course_id.course) - with override_language(settings.LANGUAGE_CODE): + # 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 " + # 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'), @@ -481,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() diff --git a/lms/djangoapps/bulk_email/tests/test_email.py b/lms/djangoapps/bulk_email/tests/test_email.py index 9c1fa2d543..6d2f5680b1 100644 --- a/lms/djangoapps/bulk_email/tests/test_email.py +++ b/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 @@ -133,9 +134,10 @@ class EmailSendFromDashboardTestCase(SharedModuleStoreTestCase): BulkEmailFlag.objects.all().delete() -@attr(shard=1) -@patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': False}) -class TestLocalizedFromAddress(EmailSendFromDashboardTestCase): +class SendEmailWithMockedUgettextMixin(object): + """ + Mock uggetext for EmailSendFromDashboardTestCase. + """ def send_email(self): """ Sends a dummy email to check the `from_addr` translation. @@ -165,11 +167,20 @@ class TestLocalizedFromAddress(EmailSendFromDashboardTestCase): 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.*') @@ -178,10 +189,44 @@ class TestLocalizedFromAddress(EmailSendFromDashboardTestCase): """ 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): @@ -444,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' diff --git a/lms/djangoapps/bulk_email/tests/test_tasks.py b/lms/djangoapps/bulk_email/tests/test_tasks.py index c5eca9edd8..f01d85eb33 100644 --- a/lms/djangoapps/bulk_email/tests/test_tasks.py +++ b/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) From 8fe16541c0b3c83caf4afc55a0b78a9d1c65224f Mon Sep 17 00:00:00 2001 From: Omar Al-Ithawi Date: Wed, 5 Apr 2017 16:40:24 +0300 Subject: [PATCH 4/4] Update the tip for the course language field --- cms/templates/js/mock/mock-settings-page.underscore | 2 +- cms/templates/settings.html | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cms/templates/js/mock/mock-settings-page.underscore b/cms/templates/js/mock/mock-settings-page.underscore index 064e80ac27..c03fb697a2 100644 --- a/cms/templates/js/mock/mock-settings-page.underscore +++ b/cms/templates/js/mock/mock-settings-page.underscore @@ -103,7 +103,7 @@ - Identify the course language here. This is used to assist users find courses that are taught in a specific language. + 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. diff --git a/cms/templates/settings.html b/cms/templates/settings.html index 1663a12476..2a5c51b887 100644 --- a/cms/templates/settings.html +++ b/cms/templates/settings.html @@ -284,7 +284,7 @@ CMS.URL.UPLOAD_ASSET = '${upload_asset_url | n, js_escaped_string}' % endfor - ${_("Identify the course language here. This is used to assist users find courses that are taught in a specific language.")} + ${_("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.")}