From 58a363e9d459b55f8881ed572350ec1fa28c1c24 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Fri, 7 Jun 2019 15:25:04 -0400 Subject: [PATCH 1/6] Remove microsites from commerce and theming. --- lms/djangoapps/commerce/tests/test_utils.py | 6 +++--- openedx/core/djangoapps/theming/helpers.py | 10 +--------- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/lms/djangoapps/commerce/tests/test_utils.py b/lms/djangoapps/commerce/tests/test_utils.py index 1f2619cde2..1d6147a826 100644 --- a/lms/djangoapps/commerce/tests/test_utils.py +++ b/lms/djangoapps/commerce/tests/test_utils.py @@ -83,9 +83,9 @@ class EcommerceServiceTests(TestCase): self.assertTrue(is_enabled) @patch('openedx.core.djangoapps.theming.helpers.is_request_in_themed_site') - def test_is_enabled_for_microsites(self, is_microsite): - """Verify that is_enabled() returns True if used for a microsite.""" - is_microsite.return_value = True + def test_is_enabled_for_sites(self, is_site): + """Verify that is_enabled() returns True if used for a site.""" + is_site.return_value = True is_enabled = EcommerceService().is_enabled(self.user) self.assertTrue(is_enabled) diff --git a/openedx/core/djangoapps/theming/helpers.py b/openedx/core/djangoapps/theming/helpers.py index a069cdcf1e..bc0b971cb3 100644 --- a/openedx/core/djangoapps/theming/helpers.py +++ b/openedx/core/djangoapps/theming/helpers.py @@ -11,7 +11,6 @@ from logging import getLogger import crum from django.conf import settings -from microsite_configuration import microsite from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.theming.helpers_dirs import ( Theme, @@ -28,15 +27,8 @@ logger = getLogger(__name__) # pylint: disable=invalid-name @request_cached() def get_template_path(relative_path, **kwargs): """ - This is a proxy function to hide microsite_configuration behind comprehensive theming. - The calculated value is cached for the lifetime of the current request. """ - # We need to give priority to theming over microsites - # So, we apply microsite override only if there is no associated site theme - # and associated microsite is present. - if not current_request_has_associated_site_theme() and microsite.is_request_in_microsite(): - relative_path = microsite.get_template_path(relative_path, **kwargs) return relative_path @@ -45,7 +37,7 @@ def is_request_in_themed_site(): This is a proxy function to hide microsite_configuration behind comprehensive theming. """ # We need to give priority to theming/site-configuration over microsites - return configuration_helpers.is_site_configuration_enabled() or microsite.is_request_in_microsite() + return configuration_helpers.is_site_configuration_enabled() def get_template(uri): From f8c54212f4c6760d3d0afa83570073141c60f27c Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Fri, 7 Jun 2019 15:35:49 -0400 Subject: [PATCH 2/6] Remove more microsites from theming. --- openedx/core/djangoapps/theming/helpers.py | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/openedx/core/djangoapps/theming/helpers.py b/openedx/core/djangoapps/theming/helpers.py index bc0b971cb3..ae517f2ecf 100644 --- a/openedx/core/djangoapps/theming/helpers.py +++ b/openedx/core/djangoapps/theming/helpers.py @@ -45,10 +45,7 @@ def get_template(uri): This is a proxy function to hide microsite_configuration behind comprehensive theming. :param uri: uri of the template """ - # We need to give priority to theming over microsites - # So, we apply microsite template override only when there is no associated theme, - if not current_request_has_associated_site_theme(): - return microsite.get_template(uri) + return None def get_template_path_with_theme(relative_path): @@ -324,15 +321,9 @@ def is_comprehensive_theming_enabled(): Returns: (bool): True if comprehensive theming is enabled else False """ - # We need to give priority to theming over microsites if settings.ENABLE_COMPREHENSIVE_THEMING and current_request_has_associated_site_theme(): return True - # Disable theming for microsites - # Microsite configurations take priority over the default site theme. - if microsite.is_request_in_microsite(): - return False - return settings.ENABLE_COMPREHENSIVE_THEMING From 762f20385ef4ce5ac299b47081097b1eae79c32b Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Tue, 11 Jun 2019 09:39:17 -0400 Subject: [PATCH 3/6] Fix theming test helpers. --- .../djangoapps/theming/tests/test_helpers.py | 103 +----------------- 1 file changed, 5 insertions(+), 98 deletions(-) diff --git a/openedx/core/djangoapps/theming/tests/test_helpers.py b/openedx/core/djangoapps/theming/tests/test_helpers.py index 28bf55e8d0..0250581a68 100644 --- a/openedx/core/djangoapps/theming/tests/test_helpers.py +++ b/openedx/core/djangoapps/theming/tests/test_helpers.py @@ -86,33 +86,14 @@ class TestHelpers(TestCase): "openedx.core.djangoapps.theming.helpers.current_request_has_associated_site_theme", Mock(return_value=True), ): - with patch( - "openedx.core.djangoapps.theming.helpers.microsite.is_request_in_microsite", - Mock(return_value=True), - ): - self.assertTrue(theming_helpers.is_comprehensive_theming_enabled()) - - # Theming is enabled, there is no SiteTheme record but there is microsite configuration for the current site. - with patch( - "openedx.core.djangoapps.theming.helpers.current_request_has_associated_site_theme", - Mock(return_value=False), - ): - with patch( - "openedx.core.djangoapps.theming.helpers.microsite.is_request_in_microsite", - Mock(return_value=True), - ): - self.assertFalse(theming_helpers.is_comprehensive_theming_enabled()) + self.assertTrue(theming_helpers.is_comprehensive_theming_enabled()) # Theming is enabled, there is neither a SiteTheme record nor microsite configuration for the current site. with patch( "openedx.core.djangoapps.theming.helpers.current_request_has_associated_site_theme", Mock(return_value=False), ): - with patch( - "openedx.core.djangoapps.theming.helpers.microsite.is_request_in_microsite", - Mock(return_value=False), - ): - self.assertTrue(theming_helpers.is_comprehensive_theming_enabled()) + self.assertTrue(theming_helpers.is_comprehensive_theming_enabled()) with override_settings(ENABLE_COMPREHENSIVE_THEMING=False): # Theming is disabled, there is a SiteTheme record and microsite configuration for the current site. @@ -120,11 +101,7 @@ class TestHelpers(TestCase): "openedx.core.djangoapps.theming.helpers.current_request_has_associated_site_theme", Mock(return_value=True), ): - with patch( - "openedx.core.djangoapps.theming.helpers.microsite.is_request_in_microsite", - Mock(return_value=True), - ): - self.assertFalse(theming_helpers.is_comprehensive_theming_enabled()) + self.assertFalse(theming_helpers.is_comprehensive_theming_enabled()) # Theming is disabled, there is no SiteTheme record but # there is microsite configuration for the current site. @@ -132,84 +109,14 @@ class TestHelpers(TestCase): "openedx.core.djangoapps.theming.helpers.current_request_has_associated_site_theme", Mock(return_value=False), ): - with patch( - "openedx.core.djangoapps.theming.helpers.microsite.is_request_in_microsite", - Mock(return_value=True), - ): - self.assertFalse(theming_helpers.is_comprehensive_theming_enabled()) + self.assertFalse(theming_helpers.is_comprehensive_theming_enabled()) # Theming is disabled, there is neither a SiteTheme record nor microsite configuration for the current site. with patch( "openedx.core.djangoapps.theming.helpers.current_request_has_associated_site_theme", Mock(return_value=False), ): - with patch( - "openedx.core.djangoapps.theming.helpers.microsite.is_request_in_microsite", - Mock(return_value=False), - ): - self.assertFalse(theming_helpers.is_comprehensive_theming_enabled()) - - def test_get_template(self): - """ - Tests to make sure the get_template function works as expected. - """ - # if the current site has associated SiteTheme then get_template should return None - with patch( - "openedx.core.djangoapps.theming.helpers.current_request_has_associated_site_theme", - Mock(return_value=True), - ): - with patch( - "openedx.core.djangoapps.theming.helpers.microsite.is_request_in_microsite", - Mock(return_value=True), - ): - with patch("microsite_configuration.microsite.TEMPLATES_BACKEND") as mock_microsite_backend: - mock_microsite_backend.get_template = Mock(return_value="/microsite/about.html") - self.assertIsNone(theming_helpers.get_template("about.html")) - - # if the current site does not have associated SiteTheme then get_template should return microsite override - with patch( - "openedx.core.djangoapps.theming.helpers.current_request_has_associated_site_theme", - Mock(return_value=False), - ): - with patch( - "openedx.core.djangoapps.theming.helpers.microsite.is_request_in_microsite", - Mock(return_value=True), - ): - with patch("microsite_configuration.microsite.TEMPLATES_BACKEND") as mock_microsite_backend: - mock_microsite_backend.get_template = Mock(return_value="/microsite/about.html") - self.assertEqual(theming_helpers.get_template("about.html"), "/microsite/about.html") - - def test_get_template_path(self): - """ - Tests to make sure the get_template_path function works as expected. - """ - # if the current site has associated SiteTheme then get_template_path should return the argument as is. - with patch( - "openedx.core.djangoapps.theming.helpers.current_request_has_associated_site_theme", - Mock(return_value=True), - ): - with patch( - "openedx.core.djangoapps.theming.helpers.microsite.is_request_in_microsite", - Mock(return_value=True), - ): - with patch("microsite_configuration.microsite.TEMPLATES_BACKEND") as mock_microsite_backend: - mock_microsite_backend.get_template = Mock(return_value="/microsite/about.html") - self.assertEqual(theming_helpers.get_template_path("about.html"), "about.html") - - RequestCache.clear_all_namespaces() - - # if the current site does not have associated SiteTheme then get_template_path should return microsite override - with patch( - "openedx.core.djangoapps.theming.helpers.current_request_has_associated_site_theme", - Mock(return_value=False), - ): - with patch( - "openedx.core.djangoapps.theming.helpers.microsite.is_request_in_microsite", - Mock(return_value=True), - ): - with patch("microsite_configuration.microsite.TEMPLATES_BACKEND") as mock_microsite_backend: - mock_microsite_backend.get_template_path = Mock(return_value="/microsite/about.html") - self.assertEqual(theming_helpers.get_template_path("about.html"), "/microsite/about.html") + self.assertFalse(theming_helpers.is_comprehensive_theming_enabled()) @skip_unless_lms From fe3d33091f6d345f0da8bbe1a178d9bc89d70508 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Tue, 11 Jun 2019 11:41:36 -0400 Subject: [PATCH 4/6] More test fixes. --- .../tests/backends/test_filebased.py | 172 ------------------ .../theming/tests/test_microsites.py | 52 ------ 2 files changed, 224 deletions(-) delete mode 100644 common/djangoapps/microsite_configuration/tests/backends/test_filebased.py delete mode 100644 openedx/core/djangoapps/theming/tests/test_microsites.py diff --git a/common/djangoapps/microsite_configuration/tests/backends/test_filebased.py b/common/djangoapps/microsite_configuration/tests/backends/test_filebased.py deleted file mode 100644 index e96e67bd21..0000000000 --- a/common/djangoapps/microsite_configuration/tests/backends/test_filebased.py +++ /dev/null @@ -1,172 +0,0 @@ -""" -Test Microsite filebased backends. -""" -from __future__ import absolute_import - -import six -import unittest -from mock import patch - -from django.test import TestCase -from django.conf import settings -from django.urls import reverse - -from microsite_configuration.backends.base import ( - BaseMicrositeBackend, - BaseMicrositeTemplateBackend, -) -from microsite_configuration import microsite -from student.tests.factories import CourseEnrollmentFactory, UserFactory -from xmodule.modulestore.tests.factories import CourseFactory -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase - - -@patch( - 'microsite_configuration.microsite.BACKEND', - microsite.get_backend( - 'microsite_configuration.backends.filebased.FilebasedMicrositeBackend', BaseMicrositeBackend - ) -) -class FilebasedMicrositeBackendTests(TestCase): - """ - Go through and test the FilebasedMicrositeBackend class - """ - def setUp(self): - super(FilebasedMicrositeBackendTests, self).setUp() - self.microsite_subdomain = 'test-site' - - def tearDown(self): - super(FilebasedMicrositeBackendTests, self).tearDown() - microsite.clear() - - def test_get_value(self): - """ - Tests microsite.get_value works as expected. - """ - microsite.set_by_domain(self.microsite_subdomain) - self.assertEqual(microsite.get_value('platform_name'), 'Test Site') - - def test_is_request_in_microsite(self): - """ - Tests microsite.is_request_in_microsite works as expected. - """ - microsite.set_by_domain(self.microsite_subdomain) - self.assertTrue(microsite.is_request_in_microsite()) - - def test_has_override_value(self): - """ - Tests microsite.has_override_value works as expected. - """ - microsite.set_by_domain(self.microsite_subdomain) - self.assertTrue(microsite.has_override_value('platform_name')) - - def test_get_value_for_org(self): - """ - Tests microsite.get_value_for_org works as expected. - """ - microsite.set_by_domain(self.microsite_subdomain) - self.assertEqual( - microsite.get_value_for_org('TestSiteX', 'platform_name'), - 'Test Site' - ) - - # if no config is set - microsite.clear() - with patch('django.conf.settings.MICROSITE_CONFIGURATION', False): - self.assertEqual( - microsite.get_value_for_org('TestSiteX', 'platform_name', 'Default Value'), - 'Default Value' - ) - - def test_get_all_orgs(self): - """ - Tests microsite.get_all_orgs works as expected. - """ - microsite.set_by_domain(self.microsite_subdomain) - self.assertEqual( - microsite.get_all_orgs(), - set(['TestSiteX', 'LogistrationX']) - ) - - # if no config is set - microsite.clear() - with patch('django.conf.settings.MICROSITE_CONFIGURATION', False): - self.assertEqual( - microsite.get_all_orgs(), - set() - ) - - def test_clear(self): - """ - Tests microsite.clear works as expected. - """ - microsite.set_by_domain(self.microsite_subdomain) - self.assertEqual( - microsite.get_value('platform_name'), - 'Test Site' - ) - microsite.clear() - self.assertIsNone(microsite.get_value('platform_name')) - - def test_get_all_configs(self): - """ - Tests microsite.get_all_config works as expected. - """ - microsite.set_by_domain(self.microsite_subdomain) - configs = microsite.get_all_config() - self.assertEqual(len(list(configs.keys())), 3) - - def test_set_config_by_domain(self): - """ - Tests microsite.set_config_by_domain works as expected. - """ - microsite.clear() - # if microsite config does not exist default config should be used - microsite.set_by_domain('unknown') - self.assertEqual(microsite.get_value('university'), 'default_university') - - def test_has_configuration_set(self): - """ - Tests microsite.has_configuration_set works as expected. - """ - self.assertTrue(microsite.BACKEND.has_configuration_set()) - - with patch('django.conf.settings.MICROSITE_CONFIGURATION', {}): - self.assertFalse(microsite.BACKEND.has_configuration_set()) - - -@patch( - 'microsite_configuration.microsite.TEMPLATES_BACKEND', - microsite.get_backend( - 'microsite_configuration.backends.filebased.FilebasedMicrositeTemplateBackend', BaseMicrositeTemplateBackend - ) -) -@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') -class FilebasedMicrositeTemplateBackendTests(ModuleStoreTestCase): - """ - Go through and test the FilebasedMicrositeTemplateBackend class - """ - def setUp(self): - super(FilebasedMicrositeTemplateBackendTests, self).setUp() - self.microsite_subdomain = 'test-site' - self.course = CourseFactory.create() - self.user = UserFactory.create(username="Bob", email="bob@example.com", password="edx") - self.client.login(username=self.user.username, password="edx") - - def test_get_template_path(self): - """ - Tests get template path works for both relative and absolute paths. - """ - microsite.set_by_domain(self.microsite_subdomain) - CourseEnrollmentFactory( - course_id=self.course.id, - user=self.user - ) - - response = self.client.get( - reverse('syllabus', args=[six.text_type(self.course.id)]), - HTTP_HOST=settings.MICROSITE_TEST_HOSTNAME, - ) - - self.assertContains(response, "Microsite relative path template contents") - self.assertContains(response, "Microsite absolute path template contents") diff --git a/openedx/core/djangoapps/theming/tests/test_microsites.py b/openedx/core/djangoapps/theming/tests/test_microsites.py deleted file mode 100644 index 45ac476ef7..0000000000 --- a/openedx/core/djangoapps/theming/tests/test_microsites.py +++ /dev/null @@ -1,52 +0,0 @@ -""" - Tests for microsites and comprehensive themes. -""" -from __future__ import absolute_import - -from django.conf import settings -from django.contrib.sites.models import Site -from django.test import TestCase - -from openedx.core.djangoapps.theming.models import SiteTheme -from openedx.core.djangolib.testing.utils import skip_unless_lms - - -@skip_unless_lms -class TestComprehensiveThemeLMS(TestCase): - """ - Test html, sass and static file overrides for comprehensive themes. - """ - def __add_site_theme__(self, domain, theme): - """ - Add a Site and SiteTheme record for the given domain and theme - Args: - domain: domain to which attach the new Site - theme: theme to apply on the new site - """ - site, __ = Site.objects.get_or_create(domain=domain, name=domain) - SiteTheme.objects.get_or_create(site=site, theme_dir_name=theme) - - def test_theme_footer(self): - """ - Test that theme footer is used instead of microsite footer. - """ - # Add SiteTheme with the same domain name as microsite - self.__add_site_theme__(domain=settings.MICROSITE_TEST_HOSTNAME, theme="test-theme") - - # Test that requesting on a host, where both theme and microsite is applied - # theme gets priority over microsite. - resp = self.client.get('/', HTTP_HOST=settings.MICROSITE_TEST_HOSTNAME) - self.assertEqual(resp.status_code, 200) - # This string comes from footer.html of test-theme - self.assertContains(resp, "This is a footer for test-theme.") - - def test_microsite_footer(self): - """ - Test that microsite footer is used instead of default theme footer. - """ - # Test that if theming is enabled but there is no SiteTheme for the current site, then - # DEFAULT_SITE_THEME does not interfere with microsites - resp = self.client.get('/', HTTP_HOST=settings.MICROSITE_TEST_HOSTNAME) - self.assertEqual(resp.status_code, 200) - # This string comes from footer.html of test_site, which is a microsite - self.assertContains(resp, "This is a Test Site footer") From 6604cf104eb2d11d47f71d4d211cda3665cc7662 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Tue, 11 Jun 2019 12:03:46 -0400 Subject: [PATCH 5/6] Remove more microsites code. --- .../courseware/tests/test_microsites.py | 333 ------------------ 1 file changed, 333 deletions(-) delete mode 100644 lms/djangoapps/courseware/tests/test_microsites.py diff --git a/lms/djangoapps/courseware/tests/test_microsites.py b/lms/djangoapps/courseware/tests/test_microsites.py deleted file mode 100644 index 04f3fca4b8..0000000000 --- a/lms/djangoapps/courseware/tests/test_microsites.py +++ /dev/null @@ -1,333 +0,0 @@ -""" -Tests related to the Site Configuration feature -""" - -from __future__ import absolute_import - -from contextlib import contextmanager - -from bs4 import BeautifulSoup -from django.conf import settings -from django.test.utils import override_settings -from django.urls import reverse -from mock import patch -from six import text_type -from six.moves import range - -from course_modes.models import CourseMode -from courseware.tests.helpers import LoginEnrollmentTestCase -from xmodule.course_module import CATALOG_VISIBILITY_CATALOG_AND_ABOUT, CATALOG_VISIBILITY_NONE -from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase -from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory - - -class TestSites(SharedModuleStoreTestCase, LoginEnrollmentTestCase): - """ - This is testing of the Site Configuration feature - """ - STUDENT_INFO = [('view@test.com', 'foo'), ('view2@test.com', 'foo')] - ENABLED_SIGNALS = ['course_published'] - - @classmethod - def setUpClass(cls): - super(TestSites, cls).setUpClass() - cls.course = CourseFactory.create( - display_name='Robot_Super_Course', - org='TestSiteX', - emit_signals=True, - ) - cls.chapter0 = ItemFactory.create(parent_location=cls.course.location, display_name='Overview') - cls.chapter9 = ItemFactory.create(parent_location=cls.course.location, display_name='factory_chapter') - cls.section0 = ItemFactory.create(parent_location=cls.chapter0.location, display_name='Welcome') - cls.section9 = ItemFactory.create(parent_location=cls.chapter9.location, display_name='factory_section') - - cls.course_outside_site = CourseFactory.create( - display_name='Robot_Course_Outside_Site', - org='FooX', - emit_signals=True, - ) - - # have a course which explicitly sets visibility in catalog to False - cls.course_hidden_visibility = CourseFactory.create( - display_name='Hidden_course', - org='TestSiteX', - catalog_visibility=CATALOG_VISIBILITY_NONE, - emit_signals=True, - ) - - # have a course which explicitly sets visibility in catalog and about to true - cls.course_with_visibility = CourseFactory.create( - display_name='visible_course', - org='TestSiteX', - course="foo", - catalog_visibility=CATALOG_VISIBILITY_CATALOG_AND_ABOUT, - emit_signals=True, - ) - - def setup_users(self): - # Create student accounts and activate them. - for i in range(len(self.STUDENT_INFO)): - email, password = self.STUDENT_INFO[i] - username = 'u{0}'.format(i) - self.create_account(username, email, password) - self.activate_user(email) - - @override_settings(SITE_NAME=settings.MICROSITE_TEST_HOSTNAME) - def test_site_anonymous_homepage_content(self): - """ - Verify that the homepage, when accessed via a Site domain, returns - HTML that reflects the Site branding elements - """ - - resp = self.client.get('/', HTTP_HOST=settings.MICROSITE_TEST_HOSTNAME) - self.assertEqual(resp.status_code, 200) - - # assert various branding definitions on this Site - # as per the configuration and Site overrides - - self.assertContains(resp, 'This is a Test Site Overlay') # Overlay test message - self.assertContains(resp, 'test_site/images/header-logo.png') # logo swap - self.assertContains(resp, 'test_site/css/test_site') # css override - self.assertContains(resp, 'Test Site') # page title - - # assert that test course display name is visible - self.assertContains(resp, 'Robot_Super_Course') - - # assert that test course with 'visible_in_catalog' to True is showing up - self.assertContains(resp, 'visible_course') - - # assert that test course that is outside current configured site is not visible - self.assertNotContains(resp, 'Robot_Course_Outside_Site') - - # assert that a course that has visible_in_catalog=False is not visible - self.assertNotContains(resp, 'Hidden_course') - - # assert that footer template has been properly overriden on homepage - self.assertContains(resp, 'This is a Test Site footer') - - # assert that the edX partners section is not in the HTML - self.assertNotContains(resp, '
') - - # assert that the edX partners tag line is not in the HTML - self.assertNotContains(resp, 'Explore free courses from') - - def test_no_configuration_anonymous_homepage_content(self): - """ - Make sure we see the right content on the homepage if there is no site configuration defined. - """ - - resp = self.client.get('/') - self.assertEqual(resp.status_code, 200) - - # assert various branding definitions on this Site ARE NOT VISIBLE - - self.assertNotContains(resp, 'This is a Test Site Overlay') # Overlay test message - self.assertNotContains(resp, 'test_site/images/header-logo.png') # logo swap - self.assertNotContains(resp, 'test_site/css/test_site') # css override - self.assertNotContains(resp, 'Test Site') # page title - - # assert that test course display name IS NOT VISIBLE - self.assertNotContains(resp, 'Robot_Super_Course') - - # assert that test course that is outside site IS VISIBLE - self.assertContains(resp, 'Robot_Course_Outside_Site') - - # assert that footer template has been properly overriden on homepage - self.assertNotContains(resp, 'This is a Test Site footer') - - @override_settings(SITE_NAME=settings.MICROSITE_TEST_HOSTNAME) - def test_site_homepage_course_max(self): - """ - Verify that the number of courses displayed on the homepage honors - the HOMEPAGE_COURSE_MAX setting. - """ - @contextmanager - def homepage_course_max_site_config(limit): - """Temporarily set the microsite HOMEPAGE_COURSE_MAX setting to desired value.""" - with patch.dict(settings.MICROSITE_CONFIGURATION, { - 'test_site': dict( - settings.MICROSITE_CONFIGURATION['test_site'], - HOMEPAGE_COURSE_MAX=limit, - ) - }): - yield - - def assert_displayed_course_count(response, expected_count): - """Assert that the number of courses displayed matches the expectation.""" - soup = BeautifulSoup(response.content, 'html.parser') - courses = soup.find_all(class_='course') - self.assertEqual(len(courses), expected_count) - - # By default the number of courses on the homepage is not limited. - # We should see both courses and no link to all courses. - resp = self.client.get('/', HTTP_HOST=settings.MICROSITE_TEST_HOSTNAME) - self.assertEqual(resp.status_code, 200) - assert_displayed_course_count(resp, 2) - self.assertNotContains(resp, 'View all Courses') - - # With the limit set to 5, we should still see both courses and no link to all courses. - with homepage_course_max_site_config(5): - resp = self.client.get('/', HTTP_HOST=settings.MICROSITE_TEST_HOSTNAME) - self.assertEqual(resp.status_code, 200) - assert_displayed_course_count(resp, 2) - self.assertNotContains(resp, 'View all Courses') - - # With the limit set to 2, we should still see both courses and no link to all courses. - with homepage_course_max_site_config(2): - resp = self.client.get('/', HTTP_HOST=settings.MICROSITE_TEST_HOSTNAME) - self.assertEqual(resp.status_code, 200) - assert_displayed_course_count(resp, 2) - self.assertNotContains(resp, 'View all Courses') - - # With the limit set to 1, we should only see one course. - # We should also see the link to all courses. - with homepage_course_max_site_config(1): - resp = self.client.get('/', HTTP_HOST=settings.MICROSITE_TEST_HOSTNAME) - self.assertEqual(resp.status_code, 200) - assert_displayed_course_count(resp, 1) - self.assertContains(resp, 'View all Courses') - - # If no site configuration is set, the limit falls back to settings.HOMEPAGE_COURSE_MAX. - with override_settings(HOMEPAGE_COURSE_MAX=1): - resp = self.client.get('/', HTTP_HOST=settings.MICROSITE_TEST_HOSTNAME) - self.assertEqual(resp.status_code, 200) - assert_displayed_course_count(resp, 1) - self.assertContains(resp, 'View all Courses') - - # Site configuration takes precedence over settings when both are set. - with homepage_course_max_site_config(2), override_settings(HOMEPAGE_COURSE_MAX=1): - resp = self.client.get('/', HTTP_HOST=settings.MICROSITE_TEST_HOSTNAME) - self.assertEqual(resp.status_code, 200) - assert_displayed_course_count(resp, 2) - self.assertNotContains(resp, 'View all Courses') - - @override_settings(SITE_NAME=settings.MICROSITE_TEST_HOSTNAME) - def test_site_anonymous_copyright_content(self): - """ - Verify that the copyright, when accessed via a Site domain, returns - the expected 200 response - """ - - resp = self.client.get('/copyright', HTTP_HOST=settings.MICROSITE_TEST_HOSTNAME) - self.assertEqual(resp.status_code, 200) - - self.assertContains(resp, 'This is a copyright page for an Open edX site.') - - def test_not_site_anonymous_copyright_content(self): - """ - Verify that the copyright page does not exist if we are not in a configured site. - """ - - resp = self.client.get('/copyright') - self.assertEqual(resp.status_code, 404) - - def test_no_redirect_on_homepage_when_no_enrollments(self): - """ - Verify that a user going to homepage will not redirect if he/she has no course enrollments - """ - self.setup_users() - - email, password = self.STUDENT_INFO[0] - self.login(email, password) - resp = self.client.get(reverse('root'), HTTP_HOST=settings.MICROSITE_TEST_HOSTNAME) - self.assertEquals(resp.status_code, 200) - - def test_no_redirect_on_homepage_when_has_enrollments(self): - """ - Verify that a user going to homepage will not redirect to dashboard if he/she has - a course enrollment - """ - self.setup_users() - - email, password = self.STUDENT_INFO[0] - self.login(email, password) - self.enroll(self.course, True) - - resp = self.client.get(reverse('root'), HTTP_HOST=settings.MICROSITE_TEST_HOSTNAME) - self.assertEquals(resp.status_code, 200) - - def test_site_course_enrollment(self): - """ - Enroll user in a course scoped in a Site and one course outside of a Site - and make sure that they are only visible in the right Dashboards - """ - self.setup_users() - - email, password = self.STUDENT_INFO[1] - self.login(email, password) - self.enroll(self.course, True) - self.enroll(self.course_outside_site, True) - - # Access the site dashboard and make sure the right courses appear - resp = self.client.get(reverse('dashboard'), HTTP_HOST=settings.MICROSITE_TEST_HOSTNAME) - self.assertContains(resp, 'Robot_Super_Course') - self.assertNotContains(resp, 'Robot_Course_Outside_Site') - - # Now access the non-site dashboard and make sure the right courses appear - resp = self.client.get(reverse('dashboard')) - self.assertNotContains(resp, 'Robot_Super_Course') - self.assertContains(resp, 'Robot_Course_Outside_Site') - - def test_site_course_custom_tabs(self): - """ - Enroll user in a course scoped in a Site and make sure that - template with tabs is overridden - """ - self.setup_users() - - email, password = self.STUDENT_INFO[1] - self.login(email, password) - self.enroll(self.course, True) - - resp = self.client.get(reverse('courseware', args=[text_type(self.course.id)]), - HTTP_HOST=settings.MICROSITE_TEST_HOSTNAME) - self.assertContains(resp, 'Test Site Tab:') - - @override_settings(SITE_NAME=settings.MICROSITE_TEST_HOSTNAME) - def test_visible_about_page_settings(self): - """ - Make sure the Site is honoring the visible_about_page permissions that is - set in configuration - """ - url = reverse('about_course', args=[text_type(self.course_with_visibility.id)]) - resp = self.client.get(url, HTTP_HOST=settings.MICROSITE_TEST_HOSTNAME) - self.assertEqual(resp.status_code, 200) - - url = reverse('about_course', args=[text_type(self.course_hidden_visibility.id)]) - resp = self.client.get(url, HTTP_HOST=settings.MICROSITE_TEST_HOSTNAME) - self.assertEqual(resp.status_code, 404) - - @override_settings(SITE_NAME=settings.MICROSITE_TEST_HOSTNAME) - def test_paid_course_registration(self): - """ - Make sure that Site overrides on the ENABLE_SHOPPING_CART and - ENABLE_PAID_COURSE_ENROLLMENTS are honored - """ - course_mode = CourseMode( - course_id=self.course_with_visibility.id, - mode_slug=CourseMode.DEFAULT_MODE_SLUG, - mode_display_name=CourseMode.DEFAULT_MODE_SLUG, - min_price=10, - ) - course_mode.save() - - # first try on the non site, which - # should pick up the global configuration (where ENABLE_PAID_COURSE_REGISTRATIONS = False) - url = reverse('about_course', args=[text_type(self.course_with_visibility.id)]) - resp = self.client.get(url) - self.assertEqual(resp.status_code, 200) - self.assertIn(u"Enroll Now", resp.content.decode(resp.charset)) - self.assertNotIn(u"Add {} to Cart ($10)".format( - self.course_with_visibility.id.course), - resp.content.decode(resp.charset) - ) - - # now try on the site - url = reverse('about_course', args=[text_type(self.course_with_visibility.id)]) - resp = self.client.get(url, HTTP_HOST=settings.MICROSITE_TEST_HOSTNAME) - self.assertEqual(resp.status_code, 200) - self.assertNotIn(u"Enroll Now", resp.content.decode(resp.charset)) - self.assertIn(u"Add {} to Cart ($10 USD)".format( - self.course_with_visibility.id.course - ), resp.content.decode(resp.charset)) - self.assertIn('$("#add_to_cart_post").click', resp.content) From 0c7ad64350373ca9484127086c3a64a0d381f912 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Fri, 14 Jun 2019 15:20:18 -0400 Subject: [PATCH 6/6] Clean up theming tests and microsites. --- openedx/core/djangoapps/theming/helpers.py | 8 ------ .../djangoapps/theming/tests/test_helpers.py | 28 ++++++------------- 2 files changed, 8 insertions(+), 28 deletions(-) diff --git a/openedx/core/djangoapps/theming/helpers.py b/openedx/core/djangoapps/theming/helpers.py index ae517f2ecf..f65ceb4566 100644 --- a/openedx/core/djangoapps/theming/helpers.py +++ b/openedx/core/djangoapps/theming/helpers.py @@ -40,14 +40,6 @@ def is_request_in_themed_site(): return configuration_helpers.is_site_configuration_enabled() -def get_template(uri): - """ - This is a proxy function to hide microsite_configuration behind comprehensive theming. - :param uri: uri of the template - """ - return None - - def get_template_path_with_theme(relative_path): """ Returns template path in current site's theme if it finds one there otherwise returns same path. diff --git a/openedx/core/djangoapps/theming/tests/test_helpers.py b/openedx/core/djangoapps/theming/tests/test_helpers.py index 0250581a68..bb920ff1cb 100644 --- a/openedx/core/djangoapps/theming/tests/test_helpers.py +++ b/openedx/core/djangoapps/theming/tests/test_helpers.py @@ -68,27 +68,23 @@ class TestHelpers(TestCase): Tests to make sure the is_comprehensive_theming_enabled function works as expected. Here are different scenarios that we need to test - 1. Theming is enabled, there is a SiteTheme record and microsite configuration for the current site. + 1. Theming is enabled and there is a SiteTheme record. is_comprehensive_theming_enabled should return True - 2. Theming is enabled, there is no SiteTheme record but there is microsite configuration for the current site. + 2. Theming is enabled and there is no SiteTheme record. is_comprehensive_theming_enabled should return False - 3. Theming is enabled, there is neither a SiteTheme record nor microsite configuration for the current site. - is_comprehensive_theming_enabled should return True - 4. Theming is disabled, there is a SiteTheme record and microsite configuration for the current site. + 3. Theming is disabled, there is a SiteTheme record for the current site. is_comprehensive_theming_enabled should return False - 5. Theming is disabled, there is no SiteTheme record but there is microsite configuration for the current site. - is_comprehensive_theming_enabled should return False - 6. Theming is disabled, there is neither a SiteTheme record nor microsite configuration for the current site. + 4. Theming is disabled, there is no SiteTheme record. is_comprehensive_theming_enabled should return False """ - # Theming is enabled, there is a SiteTheme record and microsite configuration for the current site + # Theming is enabled, there is a SiteTheme record with patch( "openedx.core.djangoapps.theming.helpers.current_request_has_associated_site_theme", Mock(return_value=True), ): self.assertTrue(theming_helpers.is_comprehensive_theming_enabled()) - # Theming is enabled, there is neither a SiteTheme record nor microsite configuration for the current site. + # Theming is enabled, there is not a SiteTheme record with patch( "openedx.core.djangoapps.theming.helpers.current_request_has_associated_site_theme", Mock(return_value=False), @@ -96,22 +92,14 @@ class TestHelpers(TestCase): self.assertTrue(theming_helpers.is_comprehensive_theming_enabled()) with override_settings(ENABLE_COMPREHENSIVE_THEMING=False): - # Theming is disabled, there is a SiteTheme record and microsite configuration for the current site. + # Theming is disabled, there is a SiteTheme record with patch( "openedx.core.djangoapps.theming.helpers.current_request_has_associated_site_theme", Mock(return_value=True), ): self.assertFalse(theming_helpers.is_comprehensive_theming_enabled()) - # Theming is disabled, there is no SiteTheme record but - # there is microsite configuration for the current site. - with patch( - "openedx.core.djangoapps.theming.helpers.current_request_has_associated_site_theme", - Mock(return_value=False), - ): - self.assertFalse(theming_helpers.is_comprehensive_theming_enabled()) - - # Theming is disabled, there is neither a SiteTheme record nor microsite configuration for the current site. + # Theming is disabled, there is no SiteTheme record with patch( "openedx.core.djangoapps.theming.helpers.current_request_has_associated_site_theme", Mock(return_value=False),