From 9b89bd3245a90cdb8b78c28bf52cda4e8cf4ec6f Mon Sep 17 00:00:00 2001 From: Matjaz Gregoric Date: Sun, 24 Jan 2016 19:28:30 +0100 Subject: [PATCH] Make comprehensive theme work with django templates. Comprehensive theming did not work with django templates (used by course wiki). The reason it didn't work was that in order for the theme to work, theme template folder has to be added to django template dirs setting *before* django startup. After django startup, modifying `settings.DEFAULT_TEMPLATE_ENGINE['DIRS']` has no effect, because at that point the template engine is already initialized with a copy of the template dirs list. Instead of running the theme startup code as an autostartup hook, we manually run it *before* `django.setup()`. This is fine because theme startup code doesn't have to do anything else besides modifying some settings and doesn't actually need django to be initialized. --- cms/startup.py | 7 +++ .../tests/test_comprehensive_theming.py | 45 +++++++++++++++++++ lms/startup.py | 6 +++ openedx/core/djangoapps/theming/core.py | 16 +++---- openedx/core/djangoapps/theming/startup.py | 14 ------ openedx/core/djangoapps/theming/test_util.py | 10 +++-- 6 files changed, 72 insertions(+), 26 deletions(-) create mode 100644 lms/djangoapps/course_wiki/tests/test_comprehensive_theming.py delete mode 100644 openedx/core/djangoapps/theming/startup.py diff --git a/cms/startup.py b/cms/startup.py index da77a07ddb..55244c9045 100644 --- a/cms/startup.py +++ b/cms/startup.py @@ -14,6 +14,8 @@ from monkey_patch import third_party_auth import xmodule.x_module import cms.lib.xblock.runtime +from openedx.core.djangoapps.theming.core import enable_comprehensive_theme + def run(): """ @@ -21,6 +23,11 @@ def run(): """ third_party_auth.patch() + # Comprehensive theming needs to be set up before django startup, + # because modifying django template paths after startup has no effect. + if settings.COMPREHENSIVE_THEME_DIR: + enable_comprehensive_theme(settings.COMPREHENSIVE_THEME_DIR) + django.setup() autostartup() diff --git a/lms/djangoapps/course_wiki/tests/test_comprehensive_theming.py b/lms/djangoapps/course_wiki/tests/test_comprehensive_theming.py new file mode 100644 index 0000000000..f763510efb --- /dev/null +++ b/lms/djangoapps/course_wiki/tests/test_comprehensive_theming.py @@ -0,0 +1,45 @@ +""" +Tests for wiki middleware. +""" +from django.conf import settings +from django.test.client import Client +from nose.plugins.attrib import attr +from wiki.models import URLPath + +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory +from openedx.core.djangoapps.theming.test_util import with_comprehensive_theme + +from courseware.tests.factories import InstructorFactory +from course_wiki.views import get_or_create_root + + +@attr('shard_1') +class TestComprehensiveTheming(ModuleStoreTestCase): + """Tests for comprehensive theming of wiki pages.""" + + def setUp(self): + """Test setup.""" + super(TestComprehensiveTheming, self).setUp() + + self.wiki = get_or_create_root() + + self.course_math101 = CourseFactory.create(org='edx', number='math101', display_name='2014', + metadata={'use_unique_wiki_id': 'false'}) + self.course_math101_instructor = InstructorFactory(course_key=self.course_math101.id, username='instructor', + password='secret') + self.wiki_math101 = URLPath.create_article(self.wiki, 'math101', title='math101') + + self.client = Client() + self.client.login(username='instructor', password='secret') + + @with_comprehensive_theme(settings.REPO_ROOT / 'themes/red-theme') + def test_themed_footer(self): + """ + Tests that theme footer is used rather than standard + footer when comprehensive theme is enabled. + """ + response = self.client.get('/courses/edx/math101/2014/wiki/math101/') + self.assertEqual(response.status_code, 200) + # This string comes from themes/red-theme/lms/templates/footer.html + self.assertContains(response, "super-ugly") diff --git a/lms/startup.py b/lms/startup.py index a80c7938f4..ab7a20adc3 100644 --- a/lms/startup.py +++ b/lms/startup.py @@ -18,6 +18,7 @@ from monkey_patch import third_party_auth import xmodule.x_module import lms_xblock.runtime +from openedx.core.djangoapps.theming.core import enable_comprehensive_theme from microsite_configuration import microsite log = logging.getLogger(__name__) @@ -33,6 +34,11 @@ def run(): if settings.FEATURES.get('ENABLE_THIRD_PARTY_AUTH', False): enable_third_party_auth() + # Comprehensive theming needs to be set up before django startup, + # because modifying django template paths after startup has no effect. + if settings.COMPREHENSIVE_THEME_DIR: + enable_comprehensive_theme(settings.COMPREHENSIVE_THEME_DIR) + # 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(). diff --git a/openedx/core/djangoapps/theming/core.py b/openedx/core/djangoapps/theming/core.py index bd2e4b5d0d..ee3e854103 100644 --- a/openedx/core/djangoapps/theming/core.py +++ b/openedx/core/djangoapps/theming/core.py @@ -5,8 +5,6 @@ from path import Path from django.conf import settings -import edxmako - def comprehensive_theme_changes(theme_dir): """ @@ -20,14 +18,14 @@ def comprehensive_theme_changes(theme_dir): * 'settings': a dictionary of settings names and their new values. - * 'mako_paths': a list of directories to prepend to the edxmako - template lookup path. + * 'template_paths': a list of directories to prepend to template + lookup path. """ changes = { 'settings': {}, - 'mako_paths': [], + 'template_paths': [], } root = Path(settings.PROJECT_ROOT) if root.name == "": @@ -37,8 +35,7 @@ def comprehensive_theme_changes(theme_dir): templates_dir = component_dir / "templates" if templates_dir.isdir(): - changes['settings']['TEMPLATE_DIRS'] = [templates_dir] + settings.DEFAULT_TEMPLATE_ENGINE['DIRS'] - changes['mako_paths'].append(templates_dir) + changes['template_paths'].append(templates_dir) staticfiles_dir = component_dir / "static" if staticfiles_dir.isdir(): @@ -64,5 +61,6 @@ def enable_comprehensive_theme(theme_dir): # Use the changes for name, value in changes['settings'].iteritems(): setattr(settings, name, value) - for template_dir in changes['mako_paths']: - edxmako.paths.add_lookup('main', template_dir, prepend=True) + for template_dir in changes['template_paths']: + settings.DEFAULT_TEMPLATE_ENGINE['DIRS'].insert(0, template_dir) + settings.MAKO_TEMPLATES['main'].insert(0, template_dir) diff --git a/openedx/core/djangoapps/theming/startup.py b/openedx/core/djangoapps/theming/startup.py deleted file mode 100644 index 8642a76555..0000000000 --- a/openedx/core/djangoapps/theming/startup.py +++ /dev/null @@ -1,14 +0,0 @@ -""" -Startup code for Comprehensive Theming -""" - -from path import Path as path -from django.conf import settings - -from .core import enable_comprehensive_theme - - -def run(): - """Enable comprehensive theming, if we should.""" - if settings.COMPREHENSIVE_THEME_DIR: - enable_comprehensive_theme(theme_dir=path(settings.COMPREHENSIVE_THEME_DIR)) diff --git a/openedx/core/djangoapps/theming/test_util.py b/openedx/core/djangoapps/theming/test_util.py index 63220f94a7..a1c24d2ec7 100644 --- a/openedx/core/djangoapps/theming/test_util.py +++ b/openedx/core/djangoapps/theming/test_util.py @@ -9,6 +9,7 @@ import os.path from mock import patch from django.conf import settings +from django.template import Engine from django.test.utils import override_settings import edxmako @@ -35,11 +36,14 @@ def with_comprehensive_theme(theme_dir): @wraps(func) def _decorated(*args, **kwargs): # pylint: disable=missing-docstring with override_settings(COMPREHENSIVE_THEME_DIR=theme_dir, **changes['settings']): + default_engine = Engine.get_default() + dirs = default_engine.dirs[:] with edxmako.save_lookups(): - for template_dir in changes['mako_paths']: + for template_dir in changes['template_paths']: edxmako.paths.add_lookup('main', template_dir, prepend=True) - - return func(*args, **kwargs) + dirs.insert(0, template_dir) + with patch.object(default_engine, 'dirs', dirs): + return func(*args, **kwargs) return _decorated return _decorator