Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make custom themes work on microsites. #9282

Closed
wants to merge 1 commit into from
Closed
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
19 changes: 9 additions & 10 deletions common/djangoapps/edxmako/shortcuts.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,22 +81,21 @@ def open_source_footer_context_processor(request):
"""
Checks the site name to determine whether to use the edX.org footer or the Open Source Footer.
"""
return dict(
[
("IS_EDX_DOMAIN", settings.FEATURES.get('IS_EDX_DOMAIN', False))
]
)
return {'IS_EDX_DOMAIN': settings.FEATURES.get('IS_EDX_DOMAIN', False)}


def microsite_footer_context_processor(request):
"""
Checks the site name to determine whether to use the edX.org footer or the Open Source Footer.
"""
return dict(
[
("IS_REQUEST_IN_MICROSITE", microsite.is_request_in_microsite())
]
)
return {'IS_REQUEST_IN_MICROSITE': microsite.is_request_in_microsite()}


def custom_theme_context_processor(request): # pylint: disable=unused-argument
"""
Sets a boolean context variable to indicate whether the site uses a custom theme.
"""
return {'THEME_ENABLED': settings.FEATURES.get('USE_CUSTOM_THEME', False)}


def render_to_string(template_name, dictionary, context=None, namespace='main'):
Expand Down
11 changes: 10 additions & 1 deletion common/djangoapps/edxmako/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@
from edxmako.shortcuts import (
marketing_link,
render_to_string,
open_source_footer_context_processor
open_source_footer_context_processor,
custom_theme_context_processor
)
from student.tests.factories import UserFactory
from util.testing import UrlResetMixin
Expand Down Expand Up @@ -50,6 +51,14 @@ def test_edx_footer(self, expected_result, _):
result = open_source_footer_context_processor({})
self.assertEquals(expected_result, result.get('IS_EDX_DOMAIN'))

@ddt.data(True, False)
def test_custom_theme(self, expected_result):
with patch.dict('django.conf.settings.FEATURES', {
'USE_CUSTOM_THEME': expected_result
}):
result = custom_theme_context_processor({})
self.assertEquals(expected_result, result.get('THEME_ENABLED'))


class AddLookupTests(TestCase):
"""
Expand Down
5 changes: 1 addition & 4 deletions common/djangoapps/microsite_configuration/microsite.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,7 @@ def get_template_path(relative_path):
search_path = os.path.join(microsite_template_path, relative_path)

if os.path.isfile(search_path):
path = '/{0}/templates/{1}'.format(
get_value('microsite_name'),
relative_path
)
path = os.path.join(get_value('microsite_name'), 'templates', relative_path)
return path

return relative_path
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,13 @@ def microsite_css_overrides_file():
return "<link href='{}' rel='stylesheet' type='text/css'>".format(static(file_path))
else:
return ""


@register.filter(name="microsite_template_path")
def microsite_template_path(relative_path):
"""
Django filter that resolves relative path to a template. The resolved path can either
be in a microsite directory (as an override) or will just return what is passed in.
{% include "some template"|microsite_template_path %}
"""
return microsite.get_template_path(relative_path)
12 changes: 12 additions & 0 deletions common/djangoapps/microsite_configuration/tests/test_microsites.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
"""
Tests microsite_configuration templatetags and helper functions.
"""
from mock import patch
from django.test import TestCase
from django.conf import settings
from microsite_configuration.templatetags import microsite
Expand Down Expand Up @@ -32,3 +33,14 @@ def test_breadcrumb_tag(self):
expected = u'my | less specific | Page | edX'
title = microsite.page_title_breadcrumbs_tag(None, *crumbs)
self.assertEqual(expected, title)

def test_microsite_template_path(self):
relative_path = 'some_template.html'
resolved_path = 'resolved/path/to/some_template.html'
with patch(
'microsite_configuration.microsite.get_template_path',
return_value=resolved_path
) as mock:
result = microsite.microsite_template_path(relative_path)
mock.assert_called_once_with(relative_path)
self.assertEqual(result, resolved_path)
3 changes: 3 additions & 0 deletions lms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -539,6 +539,9 @@

# Allows the open edX footer to be leveraged in Django Templates.
'edxmako.shortcuts.microsite_footer_context_processor',

# Allows custom theme overrides to be used in Django Templates.
'edxmako.shortcuts.custom_theme_context_processor',
)

# use the ratelimit backend to prevent brute force attacks
Expand Down
17 changes: 7 additions & 10 deletions lms/templates/main.html
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,12 @@
<%block name="headextra"/>

<%
if theme_enabled() and not is_microsite():
header_extra_file = 'theme-head-extra.html'
header_file = 'theme-header.html'
google_analytics_file = 'theme-google-analytics.html'

style_overrides_file = None
style_overrides_file = microsite.get_value('css_overrides_file')

if theme_enabled():
header_extra_file = microsite.get_template_path('theme-head-extra.html')
header_file = microsite.get_template_path('theme-header.html')
google_analytics_file = microsite.get_template_path('theme-google-analytics.html')
Copy link
Contributor

Choose a reason for hiding this comment

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

In #8271, we're planning to change the names of these files. You can just use header-extra.html, and google_analytics.html in your theme directory, and the template system should include them. As a result, you can roll the contents of theme-head-extra.html into header-extra.html.

else:
header_extra_file = None

Expand All @@ -102,8 +101,6 @@
header_file = microsite.get_template_path('navigation.html')

google_analytics_file = microsite.get_template_path('google_analytics.html')

style_overrides_file = microsite.get_value('css_overrides_file')
%>

% if header_extra_file:
Expand Down Expand Up @@ -142,8 +139,8 @@
% if not disable_footer:
<%block name="footer">
## Can be overridden by child templates wanting to hide the footer.
% if theme_enabled() and not is_microsite():
<%include file="theme-footer.html" />
% if theme_enabled():
<%include file="${microsite.get_template_path('theme-footer.html')}" />
% elif settings.FEATURES.get('IS_EDX_DOMAIN', False) and not is_microsite():
<%include file="footer-edx-v3.html" />
% else:
Expand Down
18 changes: 12 additions & 6 deletions lms/templates/main_django.html
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@
{% block headextra %}{% endblock %}
{% render_block "css" %}

{% if THEME_ENABLED %}
{% include "theme-head-extra.html"|microsite_template_path %}
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like an awkward usage pattern. Django's template lookup system is designed to handle the sort of directory lookup that the microsite.get_template_path() function does: just insert the directory at the front of the list of directories, and the template lookup system will find it first. For that matter, Mako's template lookup system is designed that way, too -- @nedbat and I are refactoring how this works in #8271. Could you take a look at what we're doing in that PR, and see if you can do the same sort of thing here? (Or we could just wait until it gets reviewed and merged, and then merge this in afterward.)

Also, you're doing a conditional include here. I think it would be better to do some kind of optional include: have the template system include the template file if it can find it, and if it can't find it, don't include anything. I implemented that in Mako in #8271, and I'm sure it would be possible to implement that in Django templates as well with a custom template tag.

{% endif %}

{% microsite_css_overrides_file %}

<meta name="path_prefix" content="{{EDX_ROOT_URL}}">
Expand All @@ -30,22 +34,24 @@
<div class="window-wrap" dir="${static.dir_rtl()}">
<a class="nav-skip" href="{% block nav_skip %}#content{% endblock %}">{% trans "Skip to main content" %}</a>
{% with course=request.course %}
{% if IS_EDX_DOMAIN %}
{% if THEME_ENABLED %}
{% include "theme-header.html"|microsite_template_path %}
{% elif IS_EDX_DOMAIN and not IS_REQUEST_IN_MICROSITE %}
{% include "navigation-edx.html" %}
{% else %}
{% include "navigation.html" %}
{% include "navigation.html"|microsite_template_path %}
{% endif %}
{% endwith %}
<div class="content-wrapper" id="content">
{% block body %}{% endblock %}
{% block bodyextra %}{% endblock %}
</div>
{% if IS_REQUEST_IN_MICROSITE %}
{# For now we don't support overriden Django templates in microsites. Leave footer blank for now which is better than saying Edx.#}
{% elif IS_EDX_DOMAIN %}
{% if THEME_ENABLED %}
{% include "theme-footer.html"|microsite_template_path %}
{% elif IS_EDX_DOMAIN and not IS_REQUEST_IN_MICROSITE %}
{% include "footer-edx-v3.html" %}
{% else %}
{% include "footer.html" %}
{% include "footer.html"|microsite_template_path %}
{% endif %}

</div>
Expand Down