From 4c5ca6d0869dd057d8a5666646727bab89416c72 Mon Sep 17 00:00:00 2001 From: Christopher Pappas Date: Mon, 6 May 2019 17:14:45 -0400 Subject: [PATCH] ENT-1887 | Adding logic for new business marketing footer url construction, while maintaining backwards compatibility Fixing quality test Testsing out a default value for the sake of jenkins tests. will revert Fix footer test from being flaky Moving an import statement ran isort. adding a test. fixing 1 quality issue Quality cleanups Attempting more quality fixes adding back in config variable name for default value Adding ENTERPRISE_MARKETING_FOOTER_QUERY_PARAMS to common settings Changing marketing_url logic to only concatenate enterprise url to root if the enterprise url is relative (starts with a /) quality fixes --- common/djangoapps/edxmako/shortcuts.py | 6 +++++ lms/djangoapps/branding/api.py | 22 +++++++++++++++--- lms/djangoapps/branding/tests/test_api.py | 28 +++++++++++++++++++---- lms/envs/common.py | 2 ++ lms/envs/devstack_docker.py | 2 ++ lms/envs/production.py | 4 ++++ lms/envs/test.py | 6 +++++ 7 files changed, 62 insertions(+), 8 deletions(-) diff --git a/common/djangoapps/edxmako/shortcuts.py b/common/djangoapps/edxmako/shortcuts.py index d187cd3540..442482538d 100644 --- a/common/djangoapps/edxmako/shortcuts.py +++ b/common/djangoapps/edxmako/shortcuts.py @@ -52,6 +52,12 @@ def marketing_link(name): # special case for when we only want the root marketing URL if name == 'ROOT': return marketing_urls.get('ROOT') + # special case for new enterprise marketing url with custom tracking query params + if name == 'ENTERPRISE': + enterprise_url = marketing_urls.get(name) + # if url is not relative, then return it without joining to root + if not enterprise_url.startswith('/'): + return enterprise_url # Using urljoin here allows us to enable a marketing site and set # a site ROOT, but still specify absolute URLs for other marketing # URLs in the MKTG_URLS setting diff --git a/lms/djangoapps/branding/api.py b/lms/djangoapps/branding/api.py index 3135ef5ddc..4309f048e3 100644 --- a/lms/djangoapps/branding/api.py +++ b/lms/djangoapps/branding/api.py @@ -13,7 +13,6 @@ the marketing site and blog). """ import logging -import urlparse from django.conf import settings from django.contrib.staticfiles.storage import staticfiles_storage @@ -22,9 +21,15 @@ from django.utils.translation import ugettext as _ from branding.models import BrandingApiConfig from edxmako.shortcuts import marketing_link - from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers +try: + from urllib import urlencode + import urlparse +except ImportError: + from urllib.parse import urlencode # pylint: disable=ungrouped-imports + import urllib.parse as urlparse + log = logging.getLogger("edx.footer") EMPTY_URL = '#' @@ -311,13 +316,24 @@ def _footer_legal_links(language=settings.LANGUAGE_CODE): ] +def _add_enterprise_marketing_footer_query_params(url): + """Add query params to url if they exist in the settings""" + params = settings.ENTERPRISE_MARKETING_FOOTER_QUERY_PARAMS + if params: + return "{url}/?{params}".format( + url=url, + params=urlencode(params), + ) + return url + + def _footer_business_links(language=settings.LANGUAGE_CODE): """Return the business links to display in the footer. """ platform_name = configuration_helpers.get_value('platform_name', settings.PLATFORM_NAME) links = [ ("about", (marketing_link("ABOUT"), _("About"))), ("enterprise", ( - marketing_link("ENTERPRISE"), + _add_enterprise_marketing_footer_query_params(marketing_link("ENTERPRISE")), _(u"{platform_name} for Business").format(platform_name=platform_name) )), ] diff --git a/lms/djangoapps/branding/tests/test_api.py b/lms/djangoapps/branding/tests/test_api.py index 4827116dc4..1cea4e9980 100644 --- a/lms/djangoapps/branding/tests/test_api.py +++ b/lms/djangoapps/branding/tests/test_api.py @@ -8,7 +8,7 @@ from django.urls import reverse from django.test import TestCase from django.test.utils import override_settings -from branding.api import get_footer, get_home_url, get_logo_url +from branding.api import _footer_business_links, get_footer, get_home_url, get_logo_url from edxmako.shortcuts import marketing_link from openedx.core.djangoapps.site_configuration.tests.test_util import ( @@ -54,8 +54,25 @@ class TestHeader(TestCase): class TestFooter(TestCase): - maxDiff = None """Test retrieving the footer. """ + maxDiff = None + + @mock.patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}) + @mock.patch.dict('django.conf.settings.MKTG_URLS', { + "ROOT": "https://edx.org", + "ENTERPRISE": "/enterprise" + }) + @override_settings(ENTERPRISE_MARKETING_FOOTER_QUERY_PARAMS={}, PLATFORM_NAME='\xe9dX') + def test_footer_business_links_no_marketing_query_params(self): + """ + Enterprise marketing page values returned should be a concatenation of ROOT and + ENTERPRISE marketing url values when ENTERPRISE_MARKETING_FOOTER_QUERY_PARAMS + is not set. + """ + + business_links = _footer_business_links() + assert business_links[0]['url'] == 'https://edx.org/enterprise' + @mock.patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}) @mock.patch.dict('django.conf.settings.MKTG_URLS', { "ROOT": "https://edx.org", @@ -74,18 +91,19 @@ class TestFooter(TestCase): "ACCESSIBILITY": "/accessibility", "AFFILIATES": '/affiliate-program', "MEDIA_KIT": "/media-kit", - "ENTERPRISE": "/enterprise" + "ENTERPRISE": "https://business.edx.org" }) @override_settings(PLATFORM_NAME='\xe9dX') def test_get_footer(self): actual_footer = get_footer(is_secure=True) + business_url = 'https://business.edx.org/?utm_campaign=edX.org+Referral&utm_source=edX.org&utm_medium=Footer' expected_footer = { 'copyright': '\xa9 \xe9dX. All rights reserved except where noted. ' ' EdX, Open edX and their respective logos are ' 'trademarks or registered trademarks of edX Inc.', 'navigation_links': [ {'url': 'https://edx.org/about-us', 'name': 'about', 'title': 'About'}, - {'url': 'https://edx.org/enterprise', 'name': 'enterprise', 'title': '\xe9dX for Business'}, + {'url': 'https://business.edx.org', 'name': 'enterprise', 'title': '\xe9dX for Business'}, {'url': 'https://edx.org/edx-blog', 'name': 'blog', 'title': 'Blog'}, {'url': 'https://edx.org/news-announcements', 'name': 'news', 'title': 'News'}, {'url': 'https://support.example.com', 'name': 'help-center', 'title': 'Help Center'}, @@ -95,7 +113,7 @@ class TestFooter(TestCase): ], 'business_links': [ {'url': 'https://edx.org/about-us', 'name': 'about', 'title': 'About'}, - {'url': 'https://edx.org/enterprise', 'name': 'enterprise', 'title': '\xe9dX for Business'}, + {'url': business_url, 'name': 'enterprise', 'title': '\xe9dX for Business'}, {'url': 'https://edx.org/affiliate-program', 'name': 'affiliates', 'title': 'Affiliates'}, {'url': 'http://open.edx.org', 'name': 'openedx', 'title': 'Open edX'}, {'url': 'https://edx.org/careers', 'name': 'careers', 'title': 'Careers'}, diff --git a/lms/envs/common.py b/lms/envs/common.py index c5471478dd..67699b03c7 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -3352,6 +3352,8 @@ SYSTEM_TO_FEATURE_ROLE_MAPPING = { DATA_CONSENT_SHARE_CACHE_TIMEOUT = None # Never expire +ENTERPRISE_MARKETING_FOOTER_QUERY_PARAMS = {} + ############## Settings for Course Enrollment Modes ###################### # The min_price key refers to the minimum price allowed for an instance # of a particular type of course enrollment mode. This is not to be confused diff --git a/lms/envs/devstack_docker.py b/lms/envs/devstack_docker.py index ec86e7edcc..a795d41d12 100644 --- a/lms/envs/devstack_docker.py +++ b/lms/envs/devstack_docker.py @@ -72,6 +72,8 @@ MKTG_URLS = { 'WHAT_IS_VERIFIED_CERT': '/verified-certificate', } +ENTERPRISE_MARKETING_FOOTER_QUERY_PARAMS = {} + CREDENTIALS_SERVICE_USERNAME = 'credentials_worker' COURSE_CATALOG_API_URL = 'http://edx.devstack.discovery:18381/api/v1/' diff --git a/lms/envs/production.py b/lms/envs/production.py index 00919da01d..9e9efe918e 100644 --- a/lms/envs/production.py +++ b/lms/envs/production.py @@ -316,6 +316,10 @@ ENABLE_COMPREHENSIVE_THEMING = ENV_TOKENS.get('ENABLE_COMPREHENSIVE_THEMING', EN # Marketing link overrides MKTG_URL_LINK_MAP.update(ENV_TOKENS.get('MKTG_URL_LINK_MAP', {})) +ENTERPRISE_MARKETING_FOOTER_QUERY_PARAMS = ENV_TOKENS.get( + 'ENTERPRISE_MARKETING_FOOTER_QUERY_PARAMS', + ENTERPRISE_MARKETING_FOOTER_QUERY_PARAMS +) # Intentional defaults. SUPPORT_SITE_LINK = ENV_TOKENS.get('SUPPORT_SITE_LINK', SUPPORT_SITE_LINK) diff --git a/lms/envs/test.py b/lms/envs/test.py index af02d1e50a..4f279f0bab 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -12,6 +12,7 @@ sessions. Assumes structure: # We intentionally define lots of variables that aren't used, and # want to import all variables from base settings files # pylint: disable=wildcard-import, unused-wildcard-import +from collections import OrderedDict from django.utils.translation import ugettext_lazy @@ -324,6 +325,11 @@ MKTG_URL_LINK_MAP = { SUPPORT_SITE_LINK = 'https://support.example.com' PASSWORD_RESET_SUPPORT_LINK = 'https://support.example.com/password-reset-help.html' ACTIVATION_EMAIL_SUPPORT_LINK = 'https://support.example.com/activation-email-help.html' +ENTERPRISE_MARKETING_FOOTER_QUERY_PARAMS = OrderedDict([ + ("utm_campaign", "edX.org Referral"), + ("utm_source", "edX.org"), + ("utm_medium", "Footer"), +]) ############################ STATIC FILES ############################# DEFAULT_FILE_STORAGE = 'django.core.files.storage.FileSystemStorage'