From f6f29ca49e98845cbc9afcc5c70c8670ad9f69fe Mon Sep 17 00:00:00 2001 From: Chandrakant Gopalan Date: Thu, 31 Aug 2017 13:37:05 -0400 Subject: [PATCH] Allow theme template block overrides. This allows an overridding template from a theme to inherit from the same corresponding standard template. This is useful when you only want to override one or more named blocks, but otherwise make no modifications to the standard template. --- common/djangoapps/edxmako/paths.py | 52 +++++++++++-- .../lms/templates/courseware/courses.html | 8 ++ .../lms/templates/courseware/info.html | 2 + .../courseware/test-theme-custom.html | 2 + .../test-theme/lms/templates/dashboard.html | 9 +++ .../tests/test_theme_style_overrides.py | 75 +++++++++++++++++++ 6 files changed, 142 insertions(+), 6 deletions(-) create mode 100644 common/test/test-theme/lms/templates/courseware/courses.html create mode 100644 common/test/test-theme/lms/templates/courseware/info.html create mode 100644 common/test/test-theme/lms/templates/courseware/test-theme-custom.html create mode 100644 common/test/test-theme/lms/templates/dashboard.html diff --git a/common/djangoapps/edxmako/paths.py b/common/djangoapps/edxmako/paths.py index 5a3d00423e..e4ea566be9 100644 --- a/common/djangoapps/edxmako/paths.py +++ b/common/djangoapps/edxmako/paths.py @@ -18,6 +18,15 @@ from openedx.core.djangoapps.theming.helpers import get_template_path_with_theme from . import LOOKUP +class TopLevelTemplateURI(unicode): + """ + A marker class for template URIs used to signal the template lookup infrastructure that the template corresponding + to the URI should be looked up straight in the standard edx-platform location instead of trying to locate an + overridding template in the current theme first. + """ + pass + + class DynamicTemplateLookup(TemplateLookup): """ A specialization of the standard mako `TemplateLookup` class which allows @@ -51,6 +60,28 @@ class DynamicTemplateLookup(TemplateLookup): self._collection.clear() self._uri_cache.clear() + def adjust_uri(self, uri, calling_uri): + """ + This method is called by mako when including a template in another template or when inheriting an existing mako + template. The method adjusts the `uri` to make it relative to the calling template's location. + + This method is overridden to detect when a template from a theme tries to override the same template from a + standard location, for example when the dashboard.html template is overridden in the theme while at the same + time inheriting from the standard LMS dashboard.html template. + + When this self-inheritance is detected, the uri is wrapped in the TopLevelTemplateURI marker class to ensure + that template lookup skips the current theme and looks up the built-in template in standard locations. + """ + # Make requested uri relative to the calling uri. + relative_uri = super(DynamicTemplateLookup, self).adjust_uri(uri, calling_uri) + # Is the calling template (calling_uri) which is including or inheriting current template (uri) + # located inside a theme? + if calling_uri != strip_site_theme_templates_path(calling_uri): + # Is the calling template trying to include/inherit itself? + if calling_uri == get_template_path_with_theme(relative_uri): + return TopLevelTemplateURI(relative_uri) + return relative_uri + def get_template(self, uri): """ Overridden method for locating a template in either the database or the site theme. @@ -68,15 +99,24 @@ class DynamicTemplateLookup(TemplateLookup): # if microsite template is not present or request is not in microsite then # let mako find and serve a template if not template: - try: - # Try to find themed template, i.e. see if current theme overrides the template - template = super(DynamicTemplateLookup, self).get_template(get_template_path_with_theme(uri)) - except TopLevelLookupException: - # strip off the prefix path to theme and look in default template dirs - template = super(DynamicTemplateLookup, self).get_template(strip_site_theme_templates_path(uri)) + if isinstance(uri, TopLevelTemplateURI): + template = self._get_toplevel_template(uri) + else: + try: + # Try to find themed template, i.e. see if current theme overrides the template + template = super(DynamicTemplateLookup, self).get_template(get_template_path_with_theme(uri)) + except TopLevelLookupException: + template = self._get_toplevel_template(uri) return template + def _get_toplevel_template(self, uri): + """ + Lookup a default/toplevel template, ignoring current theme. + """ + # Strip off the prefix path to theme and look in default template dirs. + return super(DynamicTemplateLookup, self).get_template(strip_site_theme_templates_path(uri)) + def clear_lookups(namespace): """ diff --git a/common/test/test-theme/lms/templates/courseware/courses.html b/common/test/test-theme/lms/templates/courseware/courses.html new file mode 100644 index 0000000000..0500a293df --- /dev/null +++ b/common/test/test-theme/lms/templates/courseware/courses.html @@ -0,0 +1,8 @@ +<%page expression_filter="h"/> + +# Include template which does not exist in the theme. +<%include file="/courseware/error-message.html" /> +# Include template which is overriden in the theme. +<%include file="/courseware/info.html" /> +# Include custom template which only exists in the theme. +<%include file="/courseware/test-theme-custom.html" /> diff --git a/common/test/test-theme/lms/templates/courseware/info.html b/common/test/test-theme/lms/templates/courseware/info.html new file mode 100644 index 0000000000..b47b665532 --- /dev/null +++ b/common/test/test-theme/lms/templates/courseware/info.html @@ -0,0 +1,2 @@ +<%page expression_filter="h"/> +

This overrides the courseware/info.html template.

diff --git a/common/test/test-theme/lms/templates/courseware/test-theme-custom.html b/common/test/test-theme/lms/templates/courseware/test-theme-custom.html new file mode 100644 index 0000000000..1d2fcaf4d5 --- /dev/null +++ b/common/test/test-theme/lms/templates/courseware/test-theme-custom.html @@ -0,0 +1,2 @@ +<%page expression_filter="h"/> +

This is a custom template.

diff --git a/common/test/test-theme/lms/templates/dashboard.html b/common/test/test-theme/lms/templates/dashboard.html new file mode 100644 index 0000000000..5de886fdfe --- /dev/null +++ b/common/test/test-theme/lms/templates/dashboard.html @@ -0,0 +1,9 @@ +<%page expression_filter="h"/> +<%! + from openedx.core.djangolib.markup import HTML +%> + +<%inherit file="dashboard.html" /> +<%block name="pagetitle">Overridden Title! +${HTML(parent.body())} +<%block name="bodyextra">Overriden Body Extra! diff --git a/openedx/core/djangoapps/theming/tests/test_theme_style_overrides.py b/openedx/core/djangoapps/theming/tests/test_theme_style_overrides.py index a1a0c8d0b3..865fc28096 100644 --- a/openedx/core/djangoapps/theming/tests/test_theme_style_overrides.py +++ b/openedx/core/djangoapps/theming/tests/test_theme_style_overrides.py @@ -64,6 +64,81 @@ class TestComprehensiveThemeLMS(TestCase): result = staticfiles.finders.find('test-theme/images/logo.png') self.assertEqual(result, settings.TEST_THEME / 'lms/static/images/logo.png') + @with_comprehensive_theme("test-theme") + def test_override_block_in_parent(self): + """ + Test that theme title is used instead of parent title. + """ + self._login() + dashboard_url = reverse('dashboard') + resp = self.client.get(dashboard_url) + self.assertEqual(resp.status_code, 200) + # This string comes from the 'pagetitle' block of the overriding theme. + self.assertContains(resp, "Overridden Title!") + + @with_comprehensive_theme("test-theme") + def test_override_block_in_grandparent(self): + """ + Test that theme title is used instead of parent's parent's title. + """ + self._login() + dashboard_url = reverse('dashboard') + resp = self.client.get(dashboard_url) + self.assertEqual(resp.status_code, 200) + # This string comes from the 'bodyextra' block of the overriding theme. + self.assertContains(resp, "Overriden Body Extra!") + + @with_comprehensive_theme("test-theme") + def test_parent_content_in_self_inherited_template(self): + """ + Test that parent's body is present in self inherited template. + """ + self._login() + dashboard_url = reverse('dashboard') + resp = self.client.get(dashboard_url) + self.assertEqual(resp.status_code, 200) + # This string comes from the default dashboard.html template. + self.assertContains(resp, "Explore courses") + + @with_comprehensive_theme("test-theme") + def test_include_default_template(self): + """ + Test that theme template can include template which is not part of the theme. + """ + self._login() + courses_url = reverse('courses') + resp = self.client.get(courses_url) + self.assertEqual(resp.status_code, 200) + # The courses.html template includes the error-message.html template. + # Verify that the error message is included in the output. + self.assertContains(resp, "this module is temporarily unavailable") + + @with_comprehensive_theme("test-theme") + def test_include_overridden_template(self): + """ + Test that theme template can include template which is overridden in the active theme. + """ + self._login() + courses_url = reverse('courses') + resp = self.client.get(courses_url) + self.assertEqual(resp.status_code, 200) + # The courses.html template includes the info.html file, which is overriden in the theme. + self.assertContains(resp, "This overrides the courseware/info.html template.") + + @with_comprehensive_theme("test-theme") + def test_include_custom_template(self): + """ + Test that theme template can include template which is only present in the theme, but has no standard LMS + equivalent. + """ + self._login() + courses_url = reverse('courses') + resp = self.client.get(courses_url) + self.assertEqual(resp.status_code, 200) + # The courses.html template includes the test-theme.custom.html file. + # Verify its contents are present in the output. + self.assertContains(resp, "This is a custom template.") + @skip_unless_cms class TestComprehensiveThemeCMS(TestCase):