Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 46 additions & 6 deletions common/djangoapps/edxmako/paths.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,15 @@
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
Expand Down Expand Up @@ -51,6 +60,28 @@ def add_directory(self, directory, prepend=False):
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.
Expand All @@ -68,15 +99,24 @@ def get_template(self, uri):
# 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):
"""
Expand Down
8 changes: 8 additions & 0 deletions common/test/test-theme/lms/templates/courseware/courses.html
Original file line number Diff line number Diff line change
@@ -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" />
2 changes: 2 additions & 0 deletions common/test/test-theme/lms/templates/courseware/info.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
<%page expression_filter="h"/>
<p>This overrides the courseware/info.html template.</p>
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
<%page expression_filter="h"/>
<p>This is a custom template.</p>
9 changes: 9 additions & 0 deletions common/test/test-theme/lms/templates/dashboard.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<%page expression_filter="h"/>
<%!
from openedx.core.djangolib.markup import HTML

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is HTML(…) needed, when we already have "h"?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HTML is the reverse of h. <%page expression_filter="h"/> at the top of the template means all interpolated strings are HTML escaped by default, so you have to use HTML to explicitly disable that behavior.

EdX quality scripts enforce that all mako templates have <%page expression_filter="h"/> enabled, which is why I used it here even though it's technically unnecessary.

%>

<%inherit file="dashboard.html" />
<%block name="pagetitle">Overridden Title!</%block>
${HTML(parent.body())}
<%block name="bodyextra">Overriden Body Extra!</%block>
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,81 @@ def test_logo_image(self):
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!")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should also self.assertContains(resp, "Explore courses") or similar, to check that we're getting the full dashboard with our modifications on top, not just a blank template with the content of the overridden blocks.
Same in the other „dashboard“ test. Or this could be in its own test.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the test_parent_content_in_self_inherited_template test covers this case. They are testing the same theme with one test testing the overwritten fields and one testing the non overwritten fields.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the test_parent_content_in_self_inherited_template covers this case, but this test did not yet exist when @clemente made that comment, I added it later.


@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):
Expand Down