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

[fix bug 1222137] Enable bedrock locale-based templates. #3880

Merged

Conversation

jpetto
Copy link
Contributor

@jpetto jpetto commented Feb 18, 2016

@jpetto
Copy link
Contributor Author

jpetto commented Feb 19, 2016

Hm, tests are all passing locally...

pass

# Render try #2: Look for locale-specific template in app/templates/
locale_tmpl = template.replace('.html', '.' + request.locale + '.html')
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it'd be more robust to use some stdlib tools here:

>>> from os.path import splitext
>>> template = 'los-angles/laziest/dude.html'
>>> '.{}'.format('pt-BR').join(splitext(template))
'los-angles/laziest/dude.pt-BR.html'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

@pmclanahan
Copy link
Contributor

Hmm. This looks good, but tests failing. Looks like an unapplied mock. Likely due to lib.l10n_utils having been imported already. Might try...

from lib import l10n_utils

@patch.object(l10n_utils, 'django_render')
class ...

You'd have to change calls to render to l10n_utils.render, but that'd be okay.

@@ -125,3 +127,53 @@ def test_render_no_locale(self, django_render, get_lang_path):
# Note: no .locale on request
# Should not cause an exception
render(request, '500.html')


@patch.object(l10n_utils, 'django_render')
Copy link
Contributor

Choose a reason for hiding this comment

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

We also have to patch template_is_active.

@patch('lib.l10n_utils.template_is_active', Mock(return_value=True))
@patch('lib.l10n_utils.django_render')

That set of patches works for me.

@jpetto jpetto force-pushed the bug-1222137-locale-based-templates branch from 3cc15ef to 62f0107 Compare February 19, 2016 21:15
@pmclanahan
Copy link
Contributor

Yay! I think we're good here. Let me check one more time.

pmclanahan added a commit that referenced this pull request Feb 19, 2016
[fix bug 1222137] Enable bedrock locale-based templates.
@pmclanahan pmclanahan merged commit 93dd85e into mozilla:master Feb 19, 2016
@jpetto jpetto deleted the bug-1222137-locale-based-templates branch March 4, 2016 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants