From a36eb51a649c36f63a2199fd54989c68102db51f Mon Sep 17 00:00:00 2001 From: John Eskew Date: Fri, 27 Oct 2017 12:07:06 -0400 Subject: [PATCH 1/2] Move microsite config from startup.py to settings and AppConfig.ready. --- .../djangoapps/microsite_configuration/apps.py | 15 +++++++++++++++ lms/envs/common.py | 16 +++++++++++++++- lms/startup.py | 10 ---------- lms/tests.py | 7 +++++-- 4 files changed, 35 insertions(+), 13 deletions(-) create mode 100644 common/djangoapps/microsite_configuration/apps.py diff --git a/common/djangoapps/microsite_configuration/apps.py b/common/djangoapps/microsite_configuration/apps.py new file mode 100644 index 0000000000..6233628385 --- /dev/null +++ b/common/djangoapps/microsite_configuration/apps.py @@ -0,0 +1,15 @@ + +import logging +from django.apps import AppConfig +from .microsite import enable_microsites + +log = logging.getLogger(__name__) + + +class MicrositeConfigurationConfig(AppConfig): + name = 'microsite_configuration' + verbose_name = "Microsite Configuration" + + def ready(self): + # Mako requires the directories to be added after the django setup. + enable_microsites(log) diff --git a/lms/envs/common.py b/lms/envs/common.py index ae9079993d..4fb74b49c0 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -557,6 +557,8 @@ def _make_main_mako_templates(settings): for theme in get_themes_unchecked(themes_dirs, PROJECT_ROOT): if theme.themes_base_dir not in settings.MAIN_MAKO_TEMPLATES_BASE: settings.MAIN_MAKO_TEMPLATES_BASE.insert(0, theme.themes_base_dir) + if settings.FEATURES.get('USE_MICROSITES', False) and getattr(settings, "MICROSITE_CONFIGURATION", False): + settings.MAIN_MAKO_TEMPLATES_BASE.insert(0, settings.MICROSITE_ROOT_DIR) return settings.MAIN_MAKO_TEMPLATES_BASE MAKO_TEMPLATES['main'] = _make_main_mako_templates derived_dict_entry('MAKO_TEMPLATES', 'main') @@ -620,6 +622,18 @@ TEMPLATES = [ } ] DEFAULT_TEMPLATE_ENGINE = TEMPLATES[0] +DEFAULT_TEMPLATE_ENGINE_DIRS = DEFAULT_TEMPLATE_ENGINE['DIRS'][:] + + +def _add_microsite_dirs_to_default_template_engine(settings): + """ + Derives the final DEFAULT_TEMPLATE_ENGINE['DIRS'] setting from other settings. + """ + if settings.FEATURES.get('USE_MICROSITES', False) and getattr(settings, "MICROSITE_CONFIGURATION", False): + DEFAULT_TEMPLATE_ENGINE_DIRS.append(settings.MICROSITE_ROOT_DIR) + return DEFAULT_TEMPLATE_ENGINE_DIRS +DEFAULT_TEMPLATE_ENGINE['DIRS'] = _add_microsite_dirs_to_default_template_engine +derived_dict_entry('DEFAULT_TEMPLATE_ENGINE', 'DIRS') ############################################################################################### @@ -2176,7 +2190,7 @@ INSTALLED_APPS = [ 'openedx.core.djangoapps.dark_lang', # Microsite configuration - 'microsite_configuration', + 'microsite_configuration.apps.MicrositeConfigurationConfig', # RSS Proxy 'rss_proxy', diff --git a/lms/startup.py b/lms/startup.py index 83055939c9..0afc7f782b 100644 --- a/lms/startup.py +++ b/lms/startup.py @@ -21,8 +21,6 @@ import lms_xblock.runtime from startup_configurations.validate_config import validate_lms_config -from microsite_configuration import microsite - log = logging.getLogger(__name__) @@ -35,20 +33,12 @@ def run(): """ django_db_models_options.patch() - # We currently use 2 template rendering engines, mako and django_templates, - # and one of them (django templates), requires the directories be added - # before the django.setup(). - microsite.enable_microsites_pre_startup(log) - django.setup() autostartup() add_mimetypes() - # Mako requires the directories to be added after the django setup. - microsite.enable_microsites(log) - # In order to allow modules to use a handler url, we need to # monkey-patch the x_module library. # TODO: Remove this code when Runtimes are no longer created by modulestores diff --git a/lms/tests.py b/lms/tests.py index cddcc181d9..bab5c7df1e 100644 --- a/lms/tests.py +++ b/lms/tests.py @@ -1,5 +1,6 @@ """Tests for the lms module itself.""" +import logging import mimetypes from django.core.urlresolvers import reverse @@ -7,11 +8,13 @@ from django.test import TestCase from mock import patch from edxmako import LOOKUP, add_lookup -from lms import startup +from microsite_configuration import microsite from openedx.features.course_experience import course_home_url_name from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory +log = logging.getLogger(__name__) + class LmsModuleTests(TestCase): """ @@ -38,7 +41,7 @@ class TemplateLookupTests(TestCase): self.assertEqual(len([directory for directory in directories if 'external_module' in directory]), 1) # This should not clear the directories list - startup.enable_microsites() + microsite.enable_microsites(log) directories = LOOKUP['main'].directories self.assertEqual(len([directory for directory in directories if 'external_module' in directory]), 1) From 1a1c93e5e8f46c4946b75cc20e85b39f0c3058d0 Mon Sep 17 00:00:00 2001 From: John Eskew Date: Tue, 31 Oct 2017 16:57:17 -0400 Subject: [PATCH 2/2] Remove orphaned pre-startup code and tests. --- .../microsite_configuration/backends/base.py | 11 ------- .../microsite_configuration/microsite.py | 11 +------ .../tests/backends/test_base.py | 33 ------------------- .../tests/backends/test_database.py | 17 ---------- .../tests/test_microsites.py | 17 ---------- lms/startup.py | 8 ----- 6 files changed, 1 insertion(+), 96 deletions(-) diff --git a/common/djangoapps/microsite_configuration/backends/base.py b/common/djangoapps/microsite_configuration/backends/base.py index 43743ee935..a31e91bc57 100644 --- a/common/djangoapps/microsite_configuration/backends/base.py +++ b/common/djangoapps/microsite_configuration/backends/base.py @@ -281,17 +281,6 @@ class BaseMicrositeBackend(AbstractBaseMicrositeBackend): microsites_root ) - def enable_microsites_pre_startup(self, log): - """ - The TEMPLATE_ENGINE directory to search for microsite templates - in non-mako templates must be loaded before the django startup - """ - microsites_root = settings.MICROSITE_ROOT_DIR - - if self.has_configuration_set(): - settings.MAKO_TEMPLATES['main'].insert(0, microsites_root) - settings.DEFAULT_TEMPLATE_ENGINE['DIRS'].append(microsites_root) - class BaseMicrositeTemplateBackend(object): """ diff --git a/common/djangoapps/microsite_configuration/microsite.py b/common/djangoapps/microsite_configuration/microsite.py index 959c7bd93e..6ee2a049a7 100644 --- a/common/djangoapps/microsite_configuration/microsite.py +++ b/common/djangoapps/microsite_configuration/microsite.py @@ -17,7 +17,7 @@ __all__ = [ 'is_request_in_microsite', 'get_value', 'has_override_value', 'get_template_path', 'get_value_for_org', 'get_all_orgs', 'clear', 'set_by_domain', 'enable_microsites', 'get_all_config', - 'is_feature_enabled', 'enable_microsites_pre_startup', + 'is_feature_enabled', ] BACKEND = None @@ -102,15 +102,6 @@ def set_by_domain(domain): BACKEND.set_config_by_domain(domain) -def enable_microsites_pre_startup(log): - """ - Prepare the feature settings that must be enabled before django.setup() or - autostartup() during the startup script - """ - if is_feature_enabled(): - BACKEND.enable_microsites_pre_startup(log) - - def enable_microsites(log): """ Enable the use of microsites during the startup script diff --git a/common/djangoapps/microsite_configuration/tests/backends/test_base.py b/common/djangoapps/microsite_configuration/tests/backends/test_base.py index e8f138bac7..b07b4ab671 100644 --- a/common/djangoapps/microsite_configuration/tests/backends/test_base.py +++ b/common/djangoapps/microsite_configuration/tests/backends/test_base.py @@ -138,36 +138,3 @@ class AbstractBaseMicrositeBackendTests(TestCase): with self.assertRaises(NotImplementedError): backend.get_all_orgs() - - -@patch( - 'microsite_configuration.microsite.BACKEND', - microsite.get_backend( - 'microsite_configuration.backends.base.BaseMicrositeBackend', BaseMicrositeBackend - ) -) -class BaseMicrositeBackendTests(TestCase): - """ - Go through and test the BaseMicrositeBackend class for behavior which is not - overriden in subclasses - """ - def test_enable_microsites_pre_startup(self): - """ - Tests microsite.test_enable_microsites_pre_startup works as expected. - """ - # remove microsite root directory paths first - settings.DEFAULT_TEMPLATE_ENGINE['DIRS'] = [ - path for path in settings.DEFAULT_TEMPLATE_ENGINE['DIRS'] - if path != settings.MICROSITE_ROOT_DIR - ] - - with patch('microsite_configuration.backends.base.BaseMicrositeBackend.has_configuration_set', - return_value=False): - microsite.enable_microsites_pre_startup(log) - self.assertNotIn(settings.MICROSITE_ROOT_DIR, - settings.DEFAULT_TEMPLATE_ENGINE['DIRS']) - with patch('microsite_configuration.backends.base.BaseMicrositeBackend.has_configuration_set', - return_value=True): - microsite.enable_microsites_pre_startup(log) - self.assertIn(settings.MICROSITE_ROOT_DIR, - settings.DEFAULT_TEMPLATE_ENGINE['DIRS']) diff --git a/common/djangoapps/microsite_configuration/tests/backends/test_database.py b/common/djangoapps/microsite_configuration/tests/backends/test_database.py index 912a18a03f..ec13684390 100644 --- a/common/djangoapps/microsite_configuration/tests/backends/test_database.py +++ b/common/djangoapps/microsite_configuration/tests/backends/test_database.py @@ -102,23 +102,6 @@ class DatabaseMicrositeBackendTests(DatabaseMicrositeTestCase): microsite.clear() self.assertIsNone(microsite.get_value('platform_name')) - def test_enable_microsites_pre_startup(self): - """ - Tests microsite.test_enable_microsites_pre_startup works as expected. - """ - # remove microsite root directory paths first - settings.DEFAULT_TEMPLATE_ENGINE['DIRS'] = [ - path for path in settings.DEFAULT_TEMPLATE_ENGINE['DIRS'] - if path != settings.MICROSITE_ROOT_DIR - ] - with patch.dict('django.conf.settings.FEATURES', {'USE_MICROSITES': False}): - microsite.enable_microsites_pre_startup(log) - self.assertNotIn(settings.MICROSITE_ROOT_DIR, settings.DEFAULT_TEMPLATE_ENGINE['DIRS']) - with patch.dict('django.conf.settings.FEATURES', {'USE_MICROSITES': True}): - microsite.enable_microsites_pre_startup(log) - self.assertIn(settings.MICROSITE_ROOT_DIR, settings.DEFAULT_TEMPLATE_ENGINE['DIRS']) - self.assertIn(settings.MICROSITE_ROOT_DIR, settings.MAKO_TEMPLATES['main']) - @patch('edxmako.paths.add_lookup') def test_enable_microsites(self, add_lookup): """ diff --git a/common/djangoapps/microsite_configuration/tests/test_microsites.py b/common/djangoapps/microsite_configuration/tests/test_microsites.py index db0acdb30d..aa321d0bfb 100644 --- a/common/djangoapps/microsite_configuration/tests/test_microsites.py +++ b/common/djangoapps/microsite_configuration/tests/test_microsites.py @@ -81,20 +81,3 @@ class MicrositeTests(TestCase): ), DatabaseMicrositeBackend ) - - def test_enable_microsites_pre_startup(self): - """ - Tests microsite.test_enable_microsites_pre_startup is not used if the feature is turned off. - """ - # remove microsite root directory paths first - settings.DEFAULT_TEMPLATE_ENGINE['DIRS'] = [ - path for path in settings.DEFAULT_TEMPLATE_ENGINE['DIRS'] - if path != settings.MICROSITE_ROOT_DIR - ] - - with patch.dict('django.conf.settings.FEATURES', {'USE_MICROSITES': False}): - microsite.enable_microsites_pre_startup(log) - self.assertNotIn(settings.MICROSITE_ROOT_DIR, settings.DEFAULT_TEMPLATE_ENGINE['DIRS']) - with patch.dict('django.conf.settings.FEATURES', {'USE_MICROSITES': True}): - microsite.enable_microsites_pre_startup(log) - self.assertIn(settings.MICROSITE_ROOT_DIR, settings.DEFAULT_TEMPLATE_ENGINE['DIRS']) diff --git a/lms/startup.py b/lms/startup.py index 0afc7f782b..68947f7eab 100644 --- a/lms/startup.py +++ b/lms/startup.py @@ -66,11 +66,3 @@ def add_mimetypes(): mimetypes.add_type('application/x-font-opentype', '.otf') mimetypes.add_type('application/x-font-ttf', '.ttf') mimetypes.add_type('application/font-woff', '.woff') - - -def enable_microsites(): - """ - Calls the enable_microsites function in the microsite backend. - Here for backwards compatibility - """ - microsite.enable_microsites(log)