From f5767a961c3b07e8c4e49f41fbfc90d03f2b2fa0 Mon Sep 17 00:00:00 2001 From: Renzo Lucioni Date: Mon, 24 Nov 2014 15:42:46 -0500 Subject: [PATCH] Prep marketing iframe and relevant courseware view for email opt-in Feature flagged. Puts a checkbox in the iframe. The iframe uses an organization_full_name parameter forwarded from Drupal by the courseware views and POSTs an email_opt_in parameter to the student views, preserving it on 403. --- common/djangoapps/util/cache.py | 55 +++++++++----- lms/djangoapps/branding/views.py | 4 +- lms/djangoapps/courseware/tests/test_views.py | 74 +++++++++++++++++-- lms/djangoapps/courseware/views.py | 21 +++--- lms/djangoapps/static_template_view/views.py | 4 +- lms/envs/common.py | 3 + .../courseware/mktg_course_about.html | 30 +++++++- 7 files changed, 149 insertions(+), 42 deletions(-) diff --git a/common/djangoapps/util/cache.py b/common/djangoapps/util/cache.py index 7262fabd09..cede8acb71 100644 --- a/common/djangoapps/util/cache.py +++ b/common/djangoapps/util/cache.py @@ -20,8 +20,9 @@ except Exception: cache = cache.cache -def cache_if_anonymous(view_func): - """ +def cache_if_anonymous(*get_parameters): + """Cache a page for anonymous users. + Many of the pages in edX are identical when the user is not logged in, but should not be cached when the user is logged in (because of the navigation bar at the top with the username). @@ -31,32 +32,46 @@ def cache_if_anonymous(view_func): the cookie to the vary header, and so every page is cached seperately for each user (because each user has a different csrf token). + Optionally, provide a series of GET parameters as arguments to cache + pages with these GET parameters separately. + Note that this decorator should only be used on views that do not contain the csrftoken within the html. The csrf token can be included in the header by ordering the decorators as such: @ensure_csrftoken - @cache_if_anonymous + @cache_if_anonymous() def myView(request): """ - @wraps(view_func) - def _decorated(request, *args, **kwargs): - if not request.user.is_authenticated(): - #Use the cache - # same view accessed through different domain names may - # return different things, so include the domain name in the key. - domain = str(request.META.get('HTTP_HOST')) + '.' - cache_key = domain + "cache_if_anonymous." + get_language() + '.' + request.path - response = cache.get(cache_key) - if not response: - response = view_func(request, *args, **kwargs) - cache.set(cache_key, response, 60 * 3) + def decorator(view_func): + """The outer wrapper, used to allow the decorator to take optional + arguments. + """ + @wraps(view_func) + def wrapper(request, *args, **kwargs): + """The inner wrapper, which wraps the view function.""" + if not request.user.is_authenticated(): + #Use the cache + # same view accessed through different domain names may + # return different things, so include the domain name in the key. + domain = str(request.META.get('HTTP_HOST')) + '.' + cache_key = domain + "cache_if_anonymous." + get_language() + '.' + request.path - return response + # 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)) - else: - #Don't use the cache - return view_func(request, *args, **kwargs) + response = cache.get(cache_key) # pylint: disable=maybe-no-member + if not response: + response = view_func(request, *args, **kwargs) + cache.set(cache_key, response, 60 * 3) # pylint: disable=maybe-no-member - return _decorated + return response + + else: + #Don't use the cache + return view_func(request, *args, **kwargs) + + return wrapper + return decorator diff --git a/lms/djangoapps/branding/views.py b/lms/djangoapps/branding/views.py index 667ab7a9e0..b79df70f55 100644 --- a/lms/djangoapps/branding/views.py +++ b/lms/djangoapps/branding/views.py @@ -33,7 +33,7 @@ def get_course_enrollments(user): @ensure_csrf_cookie -@cache_if_anonymous +@cache_if_anonymous() def index(request): ''' Redirects to main page -- info page if user authenticated, or marketing if not @@ -81,7 +81,7 @@ def index(request): @ensure_csrf_cookie -@cache_if_anonymous +@cache_if_anonymous() def courses(request): """ Render the "find courses" page. If the marketing site is enabled, redirect diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index b13c77165a..7556530daa 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -3,6 +3,7 @@ Tests courseware views.py """ import unittest +import cgi from datetime import datetime from mock import MagicMock, patch, create_autospec @@ -99,6 +100,10 @@ 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 = "

'+Umbrella/Corporation+'

" + @unittest.skipUnless(settings.FEATURES.get('ENABLE_SHOPPING_CART'), "Shopping Cart not enabled in settings") @patch.dict(settings.FEATURES, {'ENABLE_PAID_COURSE_REGISTRATION': True}) def test_course_about_in_cart(self): @@ -256,17 +261,26 @@ class ViewsTestCase(TestCase): # generate/store a real password. self.assertEqual(chat_settings['password'], "johndoe@%s" % domain) + @patch.dict(settings.FEATURES, {'ENABLE_MKTG_EMAIL_OPT_IN': True}) def test_course_mktg_about_coming_soon(self): - # we should not be able to find this course + # 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) + response = self.client.get(url, {'organization_full_name': self.organization_full_name}) 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() + response = self._load_mktg_about(organization_full_name=self.organization_full_name) 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) + + @patch.dict(settings.FEATURES, {'ENABLE_MKTG_EMAIL_OPT_IN': True}) def test_course_mktg_register_multiple_modes(self): CourseMode.objects.get_or_create( mode_slug='honor', @@ -279,12 +293,42 @@ class ViewsTestCase(TestCase): course_id=self.course_key ) - response = self._load_mktg_about() + response = self._load_mktg_about(organization_full_name=self.organization_full_name) 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) + # clean up course modes CourseMode.objects.all().delete() + @patch.dict(settings.FEATURES, {'ENABLE_MKTG_EMAIL_OPT_IN': True}) + def test_course_mktg_no_organization_name(self): + # Don't pass an organization name as a GET parameter, even though the email + # opt-in feature is enabled. + response = response = self._load_mktg_about() + + # Verify that the checkbox is not displayed + self._email_opt_in_checkbox(response) + + @patch.dict(settings.FEATURES, {'ENABLE_MKTG_EMAIL_OPT_IN': False}) + 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) + + # 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) + + # 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)) + @patch.dict(settings.FEATURES, {'IS_EDX_DOMAIN': True}) def test_mktg_about_language_edx_domain(self): # Since we're in an edx-controlled domain, and our marketing site @@ -340,9 +384,8 @@ class ViewsTestCase(TestCase): response = self.client.get(url) self.assertFalse('