Merge pull request #13526 from edx/douglashall/JER-157/allow_for_site_override_of_mktg_urls_setting

JER-157 Allow for per-Site override of MKTG_URLS setting

common/djangoapps/edxmako/shortcuts.py

@@ -13,6 +13,7 @@
 #   limitations under the License.
 
 import logging
+from urlparse import urljoin
 
 from django.http import HttpResponse
 from django.template import Context
@@ -42,12 +43,20 @@ def marketing_link(name):
         'ENABLE_MKTG_SITE',
         settings.FEATURES.get('ENABLE_MKTG_SITE', False)
     )
+    marketing_urls = configuration_helpers.get_value(
+        'MKTG_URLS',
+        settings.MKTG_URLS
+    )
 
-    if enable_mktg_site and name in settings.MKTG_URLS:
+    if enable_mktg_site and name in marketing_urls:
         # special case for when we only want the root marketing URL
         if name == 'ROOT':
-            return settings.MKTG_URLS.get('ROOT')
-        return settings.MKTG_URLS.get('ROOT') + settings.MKTG_URLS.get(name)
+            return marketing_urls.get('ROOT')
+        # 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
+        # e.g. urljoin('http://marketing.com', 'http://open-edx.org/about') >>> 'http://open-edx.org/about'
+        return urljoin(marketing_urls.get('ROOT'), marketing_urls.get(name))
     # only link to the old pages when the marketing site isn't on
     elif not enable_mktg_site and name in link_map:
         # don't try to reverse disabled marketing links
@@ -75,9 +84,13 @@ def is_marketing_link_set(name):
         'ENABLE_MKTG_SITE',
         settings.FEATURES.get('ENABLE_MKTG_SITE', False)
     )
+    marketing_urls = configuration_helpers.get_value(
+        'MKTG_URLS',
+        settings.MKTG_URLS
+    )
 
     if enable_mktg_site:
-        return name in settings.MKTG_URLS
+        return name in marketing_urls
     else:
         return name in settings.MKTG_URL_LINK_MAP
 
@@ -91,12 +104,17 @@ def marketing_link_context_processor(request):
     'MKTG_URL_' and whose values are the corresponding URLs as computed by the
     marketing_link method.
     """
+    marketing_urls = configuration_helpers.get_value(
+        'MKTG_URLS',
+        settings.MKTG_URLS
+    )
+
     return dict(
         [
             ("MKTG_URL_" + k, marketing_link(k))
             for k in (
                 settings.MKTG_URL_LINK_MAP.viewkeys() |
-                settings.MKTG_URLS.viewkeys()
+                marketing_urls.viewkeys()
             )
         ]
     )

common/djangoapps/edxmako/tests.py

-
 from mock import patch, Mock
 import unittest
 import ddt
@@ -27,12 +26,12 @@ class ShortcutsTests(UrlResetMixin, TestCase):
     """
     Test the edxmako shortcuts file
     """
-    @override_settings(MKTG_URLS={'ROOT': 'dummy-root', 'ABOUT': '/about-us'})
+    @override_settings(MKTG_URLS={'ROOT': 'https://dummy-root', 'ABOUT': '/about-us'})
     @override_settings(MKTG_URL_LINK_MAP={'ABOUT': 'login'})
     def test_marketing_link(self):
         # test marketing site on
         with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}):
-            expected_link = 'dummy-root/about-us'
+            expected_link = 'https://dummy-root/about-us'
             link = marketing_link('ABOUT')
             self.assertEquals(link, expected_link)
         # test marketing site off
@@ -42,7 +41,7 @@ class ShortcutsTests(UrlResetMixin, TestCase):
             link = marketing_link('ABOUT')
             self.assertEquals(link, expected_link)
 
-    @override_settings(MKTG_URLS={'ROOT': 'dummy-root', 'ABOUT': '/about-us'})
+    @override_settings(MKTG_URLS={'ROOT': 'https://dummy-root', 'ABOUT': '/about-us'})
     @override_settings(MKTG_URL_LINK_MAP={'ABOUT': 'login'})
     def test_is_marketing_link_set(self):
         # test marketing site on
@@ -54,7 +53,7 @@ class ShortcutsTests(UrlResetMixin, TestCase):
             self.assertTrue(is_marketing_link_set('ABOUT'))
             self.assertFalse(is_marketing_link_set('NOT_CONFIGURED'))
 
-    @override_settings(MKTG_URLS={'ROOT': 'dummy-root', 'ABOUT': '/about-us'})
+    @override_settings(MKTG_URLS={'ROOT': 'https://dummy-root', 'ABOUT': '/about-us'})
     @override_settings(MKTG_URL_LINK_MAP={'ABOUT': 'login'})
     def test_is_any_marketing_link_set(self):
         # test marketing site on

lms/djangoapps/branding/tests/test_views.py

@@ -10,7 +10,9 @@ import mock
 import ddt
 from config_models.models import cache
 from branding.models import BrandingApiConfig
+from openedx.core.djangoapps.site_configuration.tests.mixins import SiteMixin
 from openedx.core.djangoapps.theming.tests.test_util import with_comprehensive_theme_context
+from student.tests.factories import UserFactory
 
 
 @ddt.ddt
@@ -227,3 +229,42 @@ class TestFooter(TestCase):
             )
 
         return self.client.get(url, HTTP_ACCEPT=accepts)
+
+
+class TestIndex(SiteMixin, TestCase):
+    """ Test the index view """
+
+    def setUp(self):
+        """ Set up a user """
+        super(TestIndex, self).setUp()
+
+        patcher = mock.patch("student.models.tracker")
+        self.mock_tracker = patcher.start()
+        self.user = UserFactory.create()
+        self.user.set_password("password")
+        self.user.save()
+
+    def test_index_does_not_redirect_without_site_override(self):
+        """ Test index view does not redirect if MKTG_URLS['ROOT'] is not set """
+        response = self.client.get(reverse("root"))
+        self.assertEqual(response.status_code, 200)
+
+    def test_index_redirects_to_marketing_site_with_site_override(self):
+        """ Test index view redirects if MKTG_URLS['ROOT'] is set in SiteConfiguration """
+        self.use_site(self.site_other)
+        response = self.client.get(reverse("root"))
+        self.assertRedirects(
+            response,
+            self.site_configuration_other.values["MKTG_URLS"]["ROOT"],
+            fetch_redirect_response=False
+        )
+
+    def test_header_logo_links_to_marketing_site_with_site_override(self):
+        """
+        Test marketing site root link is included on dashboard page
+        if MKTG_URLS['ROOT'] is set in SiteConfiguration
+        """
+        self.use_site(self.site_other)
+        self.client.login(username=self.user.username, password="password")
+        response = self.client.get(reverse("dashboard"))
+        self.assertIn(self.site_configuration_other.values["MKTG_URLS"]["ROOT"], response.content)

lms/djangoapps/branding/views.py

@@ -76,7 +76,11 @@ def index(request):
     )
 
     if enable_mktg_site:
-        return redirect(settings.MKTG_URLS.get('ROOT'))
+        marketing_urls = configuration_helpers.get_value(
+            'MKTG_URLS',
+            settings.MKTG_URLS
+        )
+        return redirect(marketing_urls.get('ROOT'))
 
     domain = request.META.get('HTTP_HOST')
 

lms/djangoapps/shoppingcart/tests/test_configuration_overrides.py

@@ -2,8 +2,6 @@
 """
 Dashboard with Shopping Cart History tests with configuration overrides.
 """
-import mock
-
 from django.core.urlresolvers import reverse
 
 from mock import patch
@@ -15,24 +13,11 @@ from shoppingcart.models import (
 )
 from student.tests.factories import UserFactory
 from course_modes.models import CourseMode
-
-
-def fake_all_orgs(default=None):  # pylint: disable=unused-argument
-    """
-    Method to return fake orgs,
-    """
-    return set(['fakeX', 'fooX'])
-
-
-def fake_site(name, default=None):  # pylint: disable=unused-argument
-    """
-    Method to return a fake site name.
-    """
-    return 'fakeX'
+from openedx.core.djangoapps.site_configuration.tests.mixins import SiteMixin
 
 
 @patch.dict('django.conf.settings.FEATURES', {'ENABLE_PAID_COURSE_REGISTRATION': True})
-class TestOrderHistoryOnSiteDashboard(ModuleStoreTestCase):
+class TestOrderHistoryOnSiteDashboard(SiteMixin, ModuleStoreTestCase):
     """
     Test for dashboard order history site configuration overrides.
     """
@@ -76,7 +61,7 @@ class TestOrderHistoryOnSiteDashboard(ModuleStoreTestCase):
         self.foox_site_order_id = cart.id
 
         # Third Order with course not attributed to any site.
-        course3 = CourseFactory.create(org='otherorg', number='777', display_name='otherorg Course')
+        course3 = CourseFactory.create(org='fakeOtherX', number='777', display_name='fakeOtherX Course')
         course3_key = course3.id
         course3_mode = CourseMode(course_id=course3.id,
                                   mode_slug="honor",
@@ -90,7 +75,7 @@ class TestOrderHistoryOnSiteDashboard(ModuleStoreTestCase):
         self.order_id = cart.id
 
         # Fourth Order with course not attributed to any site but with a CertificateItem
-        course4 = CourseFactory.create(org='otherorg', number='888')
+        course4 = CourseFactory.create(org='fakeOtherX', number='888')
         course4_key = course4.id
         course4_mode = CourseMode(course_id=course4.id,
                                   mode_slug="verified",
@@ -104,7 +89,7 @@ class TestOrderHistoryOnSiteDashboard(ModuleStoreTestCase):
         self.certificate_order_id = cart.id
 
         # Fifth Order with course not attributed to any site but with a Donation
-        course5 = CourseFactory.create(org='otherorg', number='999')
+        course5 = CourseFactory.create(org='fakeOtherX', number='999')
         course5_key = course5.id
 
         cart = Order.get_cart_for_user(self.user)
@@ -117,8 +102,6 @@ class TestOrderHistoryOnSiteDashboard(ModuleStoreTestCase):
         cart.purchase(first='FirstNameTesting123', street1='StreetTesting123')
         self.courseless_donation_order_id = cart.id
 
-    @mock.patch("openedx.core.djangoapps.site_configuration.helpers.get_value", fake_site)
-    @mock.patch("openedx.core.djangoapps.site_configuration.helpers.get_all_orgs", fake_all_orgs)
     def test_shows_orders_with_current_site_courses_only(self):
         self.client.login(username=self.user.username, password="password")
         response = self.client.get(reverse("dashboard"))
@@ -136,9 +119,8 @@ class TestOrderHistoryOnSiteDashboard(ModuleStoreTestCase):
         self.assertNotIn(receipt_url_cert, content)
         self.assertNotIn(receipt_url_donation, content)
 
-    @mock.patch("openedx.core.djangoapps.site_configuration.helpers.get_value", mock.Mock(return_value=None))
-    @mock.patch("openedx.core.djangoapps.site_configuration.helpers.get_all_orgs", fake_all_orgs)
     def test_shows_orders_with_non_site_courses_only_when_no_configuration_override_exists(self):
+        self.use_site(self.site_other)
         self.client.login(username=self.user.username, password="password")
         response = self.client.get(reverse("dashboard"))
         receipt_url_course = reverse('shoppingcart.views.show_receipt', kwargs={'ordernum': self.fakex_site_order_id})

openedx/core/djangoapps/site_configuration/tests/factories.py

 """
 Model factories for unit testing views or models.
 """
+from django.contrib.sites.models import Site
 from factory.django import DjangoModelFactory
 
 from openedx.core.djangoapps.site_configuration.models import SiteConfiguration
@@ -15,3 +16,14 @@ class SiteConfigurationFactory(DjangoModelFactory):
 
     values = {}
     enabled = True
+
+
+class SiteFactory(DjangoModelFactory):
+    """
+    Factory class for Site model
+    """
+    class Meta(object):
+        model = Site
+
+    domain = 'testserver.fake'
+    name = 'testserver.fake'

openedx/core/djangoapps/site_configuration/tests/mixins.py

+"""
+Mixins for TestCase classes that need to account for multiple sites
+"""
+from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory, SiteFactory
+
+
+class SiteMixin(object):
+    """
+    Mixin for setting up Site framework models
+    """
+    def setUp(self):
+        super(SiteMixin, self).setUp()
+
+        self.site = SiteFactory.create()
+        self.site_configuration = SiteConfigurationFactory.create(
+            site=self.site,
+            values={
+                "SITE_NAME": self.site.domain,
+                "course_org_filter": "fakeX",
+            }
+        )
+
+        self.site_other = SiteFactory.create(
+            domain='testserver.fakeother',
+            name='testserver.fakeother'
+        )
+        self.site_configuration_other = SiteConfigurationFactory.create(
+            site=self.site_other,
+            values={
+                "SITE_NAME": self.site_other.domain,
+                "course_org_filter": "fakeOtherX",
+                "ENABLE_MKTG_SITE": True,
+                "MKTG_URLS": {
+                    "ROOT": "https://marketing.fakeother",
+                    "ABOUT": "/fake-about"
+                }
+            }
+        )
+
+        # Initialize client with default site domain
+        self.use_site(self.site)
+
+    def use_site(self, site):
+        """
+        # Initializes the test client with the domain of the given site
+        """
+        self.client = self.client_class(SERVER_NAME=site.domain)