From 8f6182aabf126cae48cfdc6347b4a5e7c8d36d76 Mon Sep 17 00:00:00 2001 From: Douglas Hall Date: Tue, 20 Sep 2016 17:32:58 -0400 Subject: [PATCH 1/2] Allow for site override of MKTG_URLS setting in index view --- common/djangoapps/edxmako/shortcuts.py | 28 +++++++++++++++---- common/djangoapps/edxmako/tests.py | 8 +++--- lms/djangoapps/branding/views.py | 6 +++- .../tests/test_configuration_overrides.py | 4 --- 4 files changed, 32 insertions(+), 14 deletions(-) diff --git a/common/djangoapps/edxmako/shortcuts.py b/common/djangoapps/edxmako/shortcuts.py index 939b1ceccf..c3279058bb 100644 --- a/common/djangoapps/edxmako/shortcuts.py +++ b/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() ) ] ) diff --git a/common/djangoapps/edxmako/tests.py b/common/djangoapps/edxmako/tests.py index 8634916d5a..3172c84cda 100644 --- a/common/djangoapps/edxmako/tests.py +++ b/common/djangoapps/edxmako/tests.py @@ -27,12 +27,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 +42,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 +54,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 diff --git a/lms/djangoapps/branding/views.py b/lms/djangoapps/branding/views.py index b73b2f8b5d..e6c21c5fce 100644 --- a/lms/djangoapps/branding/views.py +++ b/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') diff --git a/lms/djangoapps/shoppingcart/tests/test_configuration_overrides.py b/lms/djangoapps/shoppingcart/tests/test_configuration_overrides.py index 1b68e2c0cb..5025c56450 100644 --- a/lms/djangoapps/shoppingcart/tests/test_configuration_overrides.py +++ b/lms/djangoapps/shoppingcart/tests/test_configuration_overrides.py @@ -117,8 +117,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,8 +134,6 @@ 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.client.login(username=self.user.username, password="password") response = self.client.get(reverse("dashboard")) From 1d1952c8d3b98a4b440a9ae14d4c86af06bb49db Mon Sep 17 00:00:00 2001 From: Douglas Hall Date: Thu, 22 Sep 2016 17:08:48 -0400 Subject: [PATCH 2/2] Added SiteMixin for tests that need to test Site specific functionality --- common/djangoapps/edxmako/tests.py | 1 - lms/djangoapps/branding/tests/test_views.py | 41 ++++++++++++++++ .../tests/test_configuration_overrides.py | 26 +++------- .../site_configuration/tests/factories.py | 12 +++++ .../site_configuration/tests/mixins.py | 47 +++++++++++++++++++ 5 files changed, 106 insertions(+), 21 deletions(-) create mode 100644 openedx/core/djangoapps/site_configuration/tests/mixins.py diff --git a/common/djangoapps/edxmako/tests.py b/common/djangoapps/edxmako/tests.py index 3172c84cda..4e945b5af3 100644 --- a/common/djangoapps/edxmako/tests.py +++ b/common/djangoapps/edxmako/tests.py @@ -1,4 +1,3 @@ - from mock import patch, Mock import unittest import ddt diff --git a/lms/djangoapps/branding/tests/test_views.py b/lms/djangoapps/branding/tests/test_views.py index 991f528809..5364a0f82b 100644 --- a/lms/djangoapps/branding/tests/test_views.py +++ b/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) diff --git a/lms/djangoapps/shoppingcart/tests/test_configuration_overrides.py b/lms/djangoapps/shoppingcart/tests/test_configuration_overrides.py index 5025c56450..da85df5b66 100644 --- a/lms/djangoapps/shoppingcart/tests/test_configuration_overrides.py +++ b/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) @@ -135,6 +120,7 @@ class TestOrderHistoryOnSiteDashboard(ModuleStoreTestCase): self.assertNotIn(receipt_url_donation, content) 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}) diff --git a/openedx/core/djangoapps/site_configuration/tests/factories.py b/openedx/core/djangoapps/site_configuration/tests/factories.py index 7a5dee1ea9..1e2cfd6359 100644 --- a/openedx/core/djangoapps/site_configuration/tests/factories.py +++ b/openedx/core/djangoapps/site_configuration/tests/factories.py @@ -1,6 +1,7 @@ """ 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' diff --git a/openedx/core/djangoapps/site_configuration/tests/mixins.py b/openedx/core/djangoapps/site_configuration/tests/mixins.py new file mode 100644 index 0000000000..6ee2470636 --- /dev/null +++ b/openedx/core/djangoapps/site_configuration/tests/mixins.py @@ -0,0 +1,47 @@ +""" +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)