Improved support for multiple organization names

Updates copy used in the checkbox label, constructs strings within the view instead of passing through a separate context variable, and alleviates unipain

common/djangoapps/util/cache.py

@@ -5,6 +5,7 @@ invalidation. Import these instead of django.core.cache.
 Note that 'default' is being preserved for user session caching, which we're
 not migrating so as not to inconvenience users by logging them all out.
 """
+import urllib
 from functools import wraps
 
 from django.core import cache
@@ -56,7 +57,13 @@ def cache_if_anonymous(*get_parameters):
 
                 # Include the values of GET parameters in the cache key.
                 for get_parameter in get_parameters:
-                    cache_key = cache_key + '.' + unicode(request.GET.get(get_parameter))
+                    parameter_value = request.GET.get(get_parameter)
+                    if parameter_value is not None:
+                        # urlencode expects data to be of type str, and doesn't deal well with Unicode data
+                        # since it doesn't provide a way to specify an encoding.
+                        cache_key = cache_key + '.' + urllib.urlencode({
+                            get_parameter: unicode(parameter_value).encode('utf-8')
+                        })
 
                 response = cache.get(cache_key)  # pylint: disable=maybe-no-member
                 if not response:

lms/djangoapps/courseware/tests/test_views.py

@@ -6,6 +6,7 @@ import cgi
 from datetime import datetime
 from pytz import UTC
 import unittest
+import ddt
 
 from django.conf import settings
 from django.contrib.auth.models import User, AnonymousUser
@@ -74,6 +75,7 @@ class TestJumpTo(TestCase):
         self.assertEqual(response.status_code, 404)
 
 
+@ddt.ddt
 @override_settings(MODULESTORE=TEST_DATA_MOCK_MODULESTORE)
 class ViewsTestCase(TestCase):
     """
@@ -97,9 +99,8 @@ class ViewsTestCase(TestCase):
         chapter = 'Overview'
         self.chapter_url = '%s/%s/%s' % ('/courses', self.course_key, chapter)
 
-        # For marketing email opt-in
-        self.organization_full_name = u"𝖀𝖒𝖇𝖗𝖊𝖑𝖑𝖆 𝕮𝖔𝖗𝖕𝖔𝖗𝖆𝖙𝖎𝖔𝖓"
-        self.organization_html = "<p>'+Umbrella/Corporation+'</p>"
+        self.org = u"ꜱᴛᴀʀᴋ ɪɴᴅᴜꜱᴛʀɪᴇꜱ"
+        self.org_html = "<p>'+Stark/Industries+'</p>"
 
     @unittest.skipUnless(settings.FEATURES.get('ENABLE_SHOPPING_CART'), "Shopping Cart not enabled in settings")
     @patch.dict(settings.FEATURES, {'ENABLE_PAID_COURSE_REGISTRATION': True})
@@ -266,23 +267,41 @@ class ViewsTestCase(TestCase):
     def test_course_mktg_about_coming_soon(self):
         # We should not be able to find this course
         url = reverse('mktg_about_course', kwargs={'course_id': 'no/course/here'})
-        response = self.client.get(url, {'organization_full_name': self.organization_full_name})
+        response = self.client.get(url, {'org': self.org})
         self.assertIn('Coming Soon', response.content)
 
         # Verify that the checkbox is not displayed
         self._email_opt_in_checkbox(response)
 
     @patch.dict(settings.FEATURES, {'ENABLE_MKTG_EMAIL_OPT_IN': True})
-    def test_course_mktg_register(self):
-        response = self._load_mktg_about(organization_full_name=self.organization_full_name)
+    @ddt.data(
+        # One organization name
+        (u"ꜱᴛᴀʀᴋ ɪɴᴅᴜꜱᴛʀɪᴇꜱ", u"ꜱᴛᴀʀᴋ ɪɴᴅᴜꜱᴛʀɪᴇꜱ"),
+        # Two organization names
+        (",".join([u"ꜱᴛᴀʀᴋ ɪɴᴅᴜꜱᴛʀɪᴇꜱ"] * 2), u"ꜱᴛᴀʀᴋ ɪɴᴅᴜꜱᴛʀɪᴇꜱ" + " and " + u"ꜱᴛᴀʀᴋ ɪɴᴅᴜꜱᴛʀɪᴇꜱ"),
+        # Three organization names
+        (",".join([u"ꜱᴛᴀʀᴋ ɪɴᴅᴜꜱᴛʀɪᴇꜱ"] * 3), u"ꜱᴛᴀʀᴋ ɪɴᴅᴜꜱᴛʀɪᴇꜱ" + ", " + u"ꜱᴛᴀʀᴋ ɪɴᴅᴜꜱᴛʀɪᴇꜱ" + ", " + "and " + u"ꜱᴛᴀʀᴋ ɪɴᴅᴜꜱᴛʀɪᴇꜱ")
+    )
+    @ddt.unpack
+    def test_course_mktg_register(self, org, org_name_string):
+        response = self._load_mktg_about(org=org)
         self.assertIn('Enroll in', response.content)
         self.assertNotIn('and choose your student track', response.content)
 
         # Verify that the checkbox is displayed
-        self._email_opt_in_checkbox(response, self.organization_full_name)
+        self._email_opt_in_checkbox(response, org_name_string)
 
     @patch.dict(settings.FEATURES, {'ENABLE_MKTG_EMAIL_OPT_IN': True})
-    def test_course_mktg_register_multiple_modes(self):
+    @ddt.data(
+        # One organization name
+        (u"ꜱᴛᴀʀᴋ ɪɴᴅᴜꜱᴛʀɪᴇꜱ", u"ꜱᴛᴀʀᴋ ɪɴᴅᴜꜱᴛʀɪᴇꜱ"),
+        # Two organization names
+        (",".join([u"ꜱᴛᴀʀᴋ ɪɴᴅᴜꜱᴛʀɪᴇꜱ"] * 2), u"ꜱᴛᴀʀᴋ ɪɴᴅᴜꜱᴛʀɪᴇꜱ" + " and " + u"ꜱᴛᴀʀᴋ ɪɴᴅᴜꜱᴛʀɪᴇꜱ"),
+        # Three organization names
+        (",".join([u"ꜱᴛᴀʀᴋ ɪɴᴅᴜꜱᴛʀɪᴇꜱ"] * 3), u"ꜱᴛᴀʀᴋ ɪɴᴅᴜꜱᴛʀɪᴇꜱ" + ", " + u"ꜱᴛᴀʀᴋ ɪɴᴅᴜꜱᴛʀɪᴇꜱ" + ", " + "and " + u"ꜱᴛᴀʀᴋ ɪɴᴅᴜꜱᴛʀɪᴇꜱ")
+    )
+    @ddt.unpack
+    def test_course_mktg_register_multiple_modes(self, org, org_name_string):
         CourseMode.objects.get_or_create(
             mode_slug='honor',
             mode_display_name='Honor Code Certificate',
@@ -294,12 +313,12 @@ class ViewsTestCase(TestCase):
             course_id=self.course_key
         )
 
-        response = self._load_mktg_about(organization_full_name=self.organization_full_name)
+        response = self._load_mktg_about(org=org)
         self.assertIn('Enroll in', response.content)
         self.assertIn('and choose your student track', response.content)
 
         # Verify that the checkbox is displayed
-        self._email_opt_in_checkbox(response, self.organization_full_name)
+        self._email_opt_in_checkbox(response, org_name_string)
 
         # clean up course modes
         CourseMode.objects.all().delete()
@@ -317,18 +336,18 @@ class ViewsTestCase(TestCase):
     def test_course_mktg_opt_in_disabled(self):
         # Pass an organization name as a GET parameter, even though the email
         # opt-in feature is disabled.
-        response = self._load_mktg_about(organization_full_name=self.organization_full_name)
+        response = self._load_mktg_about(org=self.org)
 
         # Verify that the checkbox is not displayed
         self._email_opt_in_checkbox(response)
 
     @patch.dict(settings.FEATURES, {'ENABLE_MKTG_EMAIL_OPT_IN': True})
     def test_course_mktg_organization_html(self):
-        response = self._load_mktg_about(organization_full_name=self.organization_html)
+        response = self._load_mktg_about(org=self.org_html)
 
         # Verify that the checkbox is displayed with the organization name
         # in the label escaped as expected.
-        self._email_opt_in_checkbox(response, cgi.escape(self.organization_html))
+        self._email_opt_in_checkbox(response, cgi.escape(self.org_html))
 
     @patch.dict(settings.FEATURES, {'IS_EDX_DOMAIN': True})
     def test_mktg_about_language_edx_domain(self):
@@ -385,12 +404,13 @@ class ViewsTestCase(TestCase):
         response = self.client.get(url)
         self.assertFalse('<script>' in response.content)
 
-    def _load_mktg_about(self, language=None, organization_full_name=None):
+    def _load_mktg_about(self, language=None, org=None):
         """Retrieve the marketing about button (iframed into the marketing site)
         and return the HTTP response.
 
         Keyword Args:
             language (string): If provided, send this in the 'Accept-Language' HTTP header.
+            org (string): If provided, send the string as a GET parameter.
 
         Returns:
             Response
@@ -406,19 +426,19 @@ class ViewsTestCase(TestCase):
             headers['HTTP_ACCEPT_LANGUAGE'] = language
 
         url = reverse('mktg_about_course', kwargs={'course_id': unicode(self.course_key)})
-        if organization_full_name:
-            return self.client.get(url, {'organization_full_name': organization_full_name}, **headers)
+        if org:
+            return self.client.get(url, {'org': org}, **headers)
         else:
             return self.client.get(url, **headers)
 
-    def _email_opt_in_checkbox(self, response, organization_full_name=None):
+    def _email_opt_in_checkbox(self, response, org_name_string=None):
         """Check if the email opt-in checkbox appears in the response content."""
         checkbox_html = '<input id="email-opt-in" type="checkbox" name="opt-in" class="email-opt-in" value="true" checked>'
-        if organization_full_name:
+        if org_name_string:
             # Verify that the email opt-in checkbox appears, and that the expected
             # organization name is displayed.
             self.assertContains(response, checkbox_html, html=True)
-            self.assertContains(response, organization_full_name)
+            self.assertContains(response, org_name_string)
         else:
             # Verify that the email opt-in checkbox does not appear
             self.assertNotContains(response, checkbox_html, html=True)

lms/djangoapps/courseware/views.py

@@ -11,6 +11,7 @@ from datetime import datetime
 from collections import defaultdict
 from django.utils import translation
 from django.utils.translation import ugettext as _
+from django.utils.translation import ungettext
 
 from django.conf import settings
 from django.core.context_processors import csrf
@@ -803,7 +804,7 @@ def course_about(request, course_id):
 
 
 @ensure_csrf_cookie
-@cache_if_anonymous('organization_full_name')
+@cache_if_anonymous('org')
 @ensure_valid_course_key
 def mktg_course_about(request, course_id):
     """This is the button that gets put into an iframe on the Drupal site."""
@@ -844,10 +845,37 @@ def mktg_course_about(request, course_id):
     }
 
     if settings.FEATURES.get('ENABLE_MKTG_EMAIL_OPT_IN'):
-        # Drupal will pass the organization's full name as a GET parameter. If no full name
-        # is provided, the marketing iframe won't show the email opt-in checkbox.
-        organization_full_name = request.GET.get('organization_full_name')
-        context['organization_full_name'] = cgi.escape(organization_full_name) if organization_full_name else organization_full_name
+        # Drupal will pass organization names using a GET parameter, as follows:
+        #     ?org=Harvard
+        #     ?org=Harvard,MIT
+        # If no full names are provided, the marketing iframe won't show the
+        # email opt-in checkbox.
+        org = request.GET.get('org')
+        if org:
+            org_list = org.split(',')
+            # HTML-escape the provided organization names
+            org_list = [cgi.escape(org) for org in org_list]
+            if len(org_list) > 1:
+                if len(org_list) > 2:
+                    # Translators: The join of three or more institution names (e.g., Harvard, MIT, and Dartmouth).
+                    org_name_string = _("{first_institutions}, and {last_institution}").format(
+                        first_institutions=u", ".join(org_list[:-1]),
+                        last_institution=org_list[-1]
+                    )
+                else:
+                    # Translators: The join of two institution names (e.g., Harvard and MIT).
+                    org_name_string = _("{first_institution} and {second_institution}").format(
+                        first_institution=org_list[0],
+                        second_institution=org_list[1]
+                    )
+            else:
+                org_name_string = org_list[0]
+
+            context['checkbox_label'] = ungettext(
+                "I would like to receive email from {institution_series} and learn about its other programs.",
+                "I would like to receive email from {institution_series} and learn about their other programs.",
+                len(org_list)
+            ).format(institution_series=org_name_string)
 
     # The edx.org marketing site currently displays only in English.
     # To avoid displaying a different language in the register / access button,

lms/templates/courseware/mktg_course_about.html

@@ -93,17 +93,11 @@
         </a>
 
         % if settings.FEATURES.get('ENABLE_MKTG_EMAIL_OPT_IN'):
-          ## We only display the email opt-in checkbox if we've been given a valid full name (i.e., not None)
-          % if organization_full_name:
+          ## We only display the email opt-in checkbox if we've been given one or more organization names.
+          % if checkbox_label:
             <p class="form-field">
               <input id="email-opt-in" type="checkbox" name="opt-in" class="email-opt-in" value="true" checked>
-              <label for="email-opt-in" class="register-emails">
-                ## Translators: This line appears next a checkbox which users can leave checked or uncheck in order
-                ## to indicate whether they want to receive emails from the organization offering the course.
-                ${_("I would like to receive email about other {organization_full_name} programs and offers.").format(
-                  organization_full_name=organization_full_name
-                )}
-              </label>
+              <label for="email-opt-in" class="register-emails">${checkbox_label}</label>
             </p>
           % endif
         % endif