Skip to content

Commit

Permalink
Provide a hybrid list of locales drawn from CMS-backed and Django-bac…
Browse files Browse the repository at this point in the history
…ked sources (#15475)

* Refactor the logic for enumerating available locales for a CMS page

...so that we can use it independently of AbstractBedrockCMSPage._patch_request_for_bedrock()
in the enhancement to follow

* Ensure get_locales_for_cms_page ignores draft pages

* Set correct lang-picker options when different locales for a given path may be served by the CMS or the Django fallback view

* Expand prefer_cms to take args that tell us what locales are available in the Django fallback version of the page
* In prefer_cms annotate the request with lists of locales available in a CMS-backed version of the page and a Django-backed version
* Add a new jinja helper to select the most appropriate list of locales to pass to the lang picker UI element

* Add tests; drop support for using prefer_cms with the page() decorator

Making it support a list of automatically-detected Fluent files would make
the page() helper even busier, with little value: if we need to decorate a
page() view with prefer_cms, we can refactor it to be a regular Django view
and then decorate in the URLConf or on the view itself

(Had to commit all in one go, alas, due to pre-commit constraints)

* Add support for a dynamic callable of locales for a page

This is to satisfy situations like the current VPN RC where overall it's available in 10 locales,
but it's not consistently translated into all 10, so we can't claim that all pages are available
in all of those locales in the picker. This approach makes the locale derivation pluggable

* Use the new dynamic/callable approach to get locales with the _old_ VPN RC article page

(The version with data exported from Contentful, ahead of us moving it all to Wagtail)

* Update other parts of the codebase that may need to use the new lang-switcher code to cope with alt/fallback sources of locale info

* Rename helper func for getting locales to be more generic

* Update documentation for prefer_cms

* Fix regression due to a bad rebase

* Remove deprecated approach to passing in locale info

* Expand test coverage, fix comments/feedback

* Update import to be absolute not relative

* Change DB-hitting list comprehension to be a subquery, for better performance
  • Loading branch information
stevejalim authored Nov 18, 2024
1 parent 30d8722 commit 42ea7e3
Show file tree
Hide file tree
Showing 17 changed files with 791 additions and 86 deletions.
6 changes: 4 additions & 2 deletions bedrock/base/templates/includes/canonical-url.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@
file, You can obtain one at https://mozilla.org/MPL/2.0/.
#}

{%- set available_languages = get_locale_options(request, translations) -%}

<link rel="canonical" href="{{ settings.CANONICAL_URL + '/' + LANG + canonical_path }}">
<link rel="alternate" hreflang="x-default" href="{{ settings.CANONICAL_URL }}{{ canonical_path }}">
{% if translations -%}
{%- for code, label in translations|dictsort -%}
{% if available_languages -%}
{%- for code, label in available_languages|dictsort -%}
{%- set alt_url = alternate_url(canonical_path, code) -%}
{%- if alt_url -%}
{%- set loop_canonical_path = alt_url -%}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@
file, You can obtain one at https://mozilla.org/MPL/2.0/.
#}

{% if translations|length > 1 %}
{%- set available_languages = get_locale_options(request, translations) -%}

{% if available_languages|length > 1 %}
<form id="lang_form" class="moz24-c-language-switcher" method="get" action="#">
<label for="page-language-select">{{ ftl('footer-refresh-language') }}</label>
<a class="mzp-c-language-switcher-link" href="{{ url('mozorg.locales') }}">{{ ftl('footer-refresh-language') }}</a>
<select id="page-language-select" class="mzp-js-language-switcher-select" name="lang" dir="ltr" data-testid="footer-language-select">
{% for code, label in translations|dictsort -%}
{% for code, label in available_languages|dictsort -%}
{# Don't escape the label as it may include entity references
# like "Português (do&nbsp;Brasil)" (Bug 861149) #}
<option lang="{{ code }}" value="{{ code }}"{{ code|ifeq(LANG, " selected") }}>{{ label|safe|replace('English (US)', 'English') }}</option>
Expand Down
6 changes: 4 additions & 2 deletions bedrock/base/templates/includes/protocol/lang-switcher.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@
file, You can obtain one at https://mozilla.org/MPL/2.0/.
#}

{% if translations|length > 1 %}
{%- set available_languages = get_locale_options(request, translations) -%}

{% if available_languages|length > 1 %}
<form id="lang_form" class="mzp-c-language-switcher" method="get" action="#">
<a class="mzp-c-language-switcher-link" href="{{ url('mozorg.locales') }}">{{ ftl('footer-language') }}</a>
<label for="page-language-select">{{ ftl('footer-language') }}</label>
<select id="page-language-select" class="mzp-js-language-switcher-select" name="lang" dir="ltr" data-testid="footer-language-select">
{% for code, label in translations|dictsort -%}
{% for code, label in available_languages|dictsort -%}
{# Don't escape the label as it may include entity references
# like "Português (do&nbsp;Brasil)" (Bug 861149) #}
<option lang="{{ code }}" value="{{ code }}"{{ code|ifeq(LANG, " selected") }}>{{ label|safe|replace('English (US)', 'English') }}</option>
Expand Down
20 changes: 20 additions & 0 deletions bedrock/base/templatetags/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

from bedrock.base import waffle
from bedrock.utils import expand_locale_groups
from lib.l10n_utils import get_translations_native_names

from ..urlresolvers import reverse

Expand Down Expand Up @@ -153,3 +154,22 @@ def alternate_url(path, locale):
return alt_paths[path][locale]

return None


@library.global_function
def get_locale_options(request, translations):
# For purely Django-rendered pages, or purely CMS-backed pages, we can just
# rely on the `translations` var in the render context to know what locales
# are viable for the page being rendered. Great! \o/
available_locales = translations

# However, if a URL route is decorated with bedrock.cms.decorators.prefer_cms
# that means that a page could come from the CMS or from Django depending on
# the locale being requested. In this situation _locales_available_via_cms
# and _locales_for_django_fallback_view are annotated onto the request.
# We need to use these to create a more accurate view of what locales are
# available
if hasattr(request, "_locales_available_via_cms") and hasattr(request, "_locales_for_django_fallback_view"):
available_locales = get_translations_native_names(sorted(set(request._locales_available_via_cms + request._locales_for_django_fallback_view)))

return available_locales
36 changes: 35 additions & 1 deletion bedrock/base/tests/test_helpers.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
# This Source Code Form is subject to the terms of the Mozilla Public
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, You can obtain one at https://mozilla.org/MPL/2.0/.

from unittest.mock import patch

from django.test import TestCase, override_settings

import pytest
from django_jinja.backend import Jinja2

from bedrock.base.templatetags import helpers
from lib.l10n_utils import get_translations_native_names

jinja_env = Jinja2.get_default()
SEND_TO_DEVICE_MESSAGE_SETS = {
Expand Down Expand Up @@ -69,3 +70,36 @@ def test_switch():

assert ret is waffle.switch.return_value
waffle.switch.assert_called_with("dude")


@pytest.mark.parametrize(
"translations_locales, cms_locales, django_locales, expected",
(
(
["en-US", "fr", "sco"],
["de", "pt-BR"],
["ja-JP", "zh-CN"],
["de", "pt-BR", "ja-JP", "zh-CN"],
),
(
["en-US", "fr", "sco"],
[],
[],
["en-US", "fr", "sco"],
),
),
)
def test_get_locale_options(rf, translations_locales, cms_locales, django_locales, expected):
native_translations = get_translations_native_names(translations_locales)
native_expected = get_translations_native_names(expected)

request = rf.get("/dummy/path/")

if cms_locales and django_locales:
request._locales_available_via_cms = cms_locales
request._locales_for_django_fallback_view = django_locales

assert native_expected == helpers.get_locale_options(
request=request,
translations=native_translations,
)
164 changes: 134 additions & 30 deletions bedrock/cms/decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,54 @@
from wagtail.views import serve as wagtail_serve

from bedrock.base.i18n import remove_lang_prefix
from lib.l10n_utils.fluent import get_active_locales

from .utils import get_cms_locales_for_path

HTTP_200_OK = 200


def prefer_cms(view_func):
def prefer_cms(
view_func=None,
fallback_ftl_files=None,
fallback_lang_codes=None,
fallback_callable=None,
):
"""
A decorator that helps us migrate from pure Django-based views
to CMS views.
to CMS views, or support having _some_ locales served from the CMS and
other / evergreen content in other locales coming from Django views .
It will try to see if `wagtail.views.serve` can find a live CMS page for the
URL path that matches the current Django view's path, and if so, will
return that. If not, it will let the regular Django view run.
Args:
view_func - the function to wrap
fallback_ftl_files (optional) - a list of the names of the Fluent files used by
the Django view that's being wrapped. It's a little repetitive, but
from those we can work out what locales the page is availble in
across the CMS and Django views
fallback_lang_codes (optional) - a list of strings of language codes that
show what locales are available for the Django view being wrapped.
This is useful if, for some reason, the Fluent files are not a reliable
way to determine the available locales for a page
(e.g. the Fluent files cover 20 locales for some strings which appear
on the page, but the main localized content is only in two languages,
because the contnet doesn't come from Fluent - such as Legal Docs,
which comes from a git repo). This works best when all the pages
for the decorated route are available in all the specified locales --
if not, some of the footer language-selector options will 404.
fallback_callable (optional) - a method or function that takes the same
arguments as the URL path (if any) in order to return a list of appropriate
locale language codes. This is intended for use if we can't reliably
pass either fluent files or specific lang codes (e.g. the view being
decorated is not consistently translated via whatever non-Fluent method
is being used)
Note that setting both fallback_lang_codes and fallback_ftl_files will cause
an exception to be raised - only one should be set, not both.
Workflow:
1. This decorator is added to the target view that will be replaced with CMS content
Expand All @@ -33,42 +68,111 @@ def prefer_cms(view_func):
Example for a function-based view:
@prefer_cms
@prefer_cms(fallback_ftl_files=[...]) # or fallback_lang_codes or fallback_callable
def some_path(request):
...
Or, in a URLconf for a regular Django view:
...
path("some/path/", prefer_cms(views.some_view)),
path("some/path/", prefer_cms(views.some_view, fallback_ftl_files=[...])),
# or
path("some/path/", prefer_cms(views.some_view, fallback_lang_codes=["fr", "pt-BR",])),
...
Or, in a URLconf with Bedrock's page() helper:
page(
"about/leadership/",
"mozorg/about/leadership/index.html",
decorators=[prefer_cms],
)
# or
path("some/path/", prefer_cms(views.some_view, fallback_callable=path.to.callable)),
...
IMPORTANT: there's no support for bedrock.mozorg.util.page(), deliberately.
Making prefer_cms work with our page() helper would have made that function
more complex. Given that it's straightforward to convert a page()-based view to
a dedicated TemplateView, which _can_ be decorated with prefer_cms at the
URLConf level, that is the recommended approach if you are migrating a page()
based view to the CMS, or need to have hybrid behaviour.
"""

@wraps(view_func)
def wrapped_view(request, *args, **kwargs):
path = remove_lang_prefix(request.path_info)
try:
# Does Wagtail have a route that matches this? If so, show that page
wagtail_response = wagtail_serve(request, path)
if wagtail_response.status_code == HTTP_200_OK:
return wagtail_response
except Http404:
pass

# If not, call the original view function, but remember to
# un-mark the request as being for a CMS page. This is so to avoid
# lib.l10n_utils.render() incorrectly looking for available translations
# based on CMS data - we're falling back to non-CMS pages, so we want
# the Fluent translations that are normal for a Django-rendered page
request.is_cms_page = False
return view_func(request, *args, **kwargs)

return wrapped_view
if len([x for x in [fallback_ftl_files, fallback_lang_codes, fallback_callable] if x]) > 1:
raise RuntimeError(
"The prefer_cms decorator can be configured with only one of fallback_ftl_files or fallback_lang_codes or fallback_callable."
)

fallback_ftl_files = fallback_ftl_files or []
fallback_lang_codes = fallback_lang_codes or []

def _get_django_locales_available(
*,
fallback_ftl_files,
fallback_lang_codes,
fallback_callable,
kwargs,
):
# Prefer explicit callable to get lang codes
if fallback_callable:
return fallback_callable(**kwargs)

# Use explicit list of lang codes over fluent files
if fallback_lang_codes:
return fallback_lang_codes

_ftl_files = kwargs.get("ftl_files", fallback_ftl_files)
return get_active_locales(_ftl_files, force=True)

def decorator(func):
@wraps(func)
def wrapped_view(request, *args, **kwargs):
path = remove_lang_prefix(request.path_info)

# Annotate the request with the Django/fallback locales, as we'll
# need them for the language picket in the footer when rendering
# the Wagtail response IF there is a Wagtail match

request._locales_for_django_fallback_view = _get_django_locales_available(
fallback_ftl_files=fallback_ftl_files,
fallback_lang_codes=fallback_lang_codes,
fallback_callable=fallback_callable,
kwargs=kwargs,
)

try:
# Does Wagtail have a route that matches this? If so, show that page
wagtail_response = wagtail_serve(request, path)
if wagtail_response.status_code == HTTP_200_OK:
return wagtail_response
except Http404:
pass

# If the page does not exist in Wagtail, call the original view function and...
#
# 1) Un-mark this request as being for a CMS page (which happened
# via wagtail_serve()) to avoid lib.l10n_utils.render() incorrectly
# looking for available translations based on CMS data, rather than
# Fluent files
request.is_cms_page = False

# 2) Make extra sure this request is still annotated with any CMS-backed
# locale versions that are available, so that we can populate the
# language picker appropriately. (The annotation also happened via
# wagtail_serve() thanks to AbstractBedrockCMSPage._patch_request_for_bedrock
request._locales_available_via_cms = getattr(
request,
"_locales_available_via_cms",
get_cms_locales_for_path(request),
)
return func(request, *args, **kwargs)

return wrapped_view

# If view_func is None, the decorator was called with parameters
if view_func is None:
return decorator
else:
# Otherwise, apply the decorator directly to view_func
return decorator(view_func)
11 changes: 3 additions & 8 deletions bedrock/cms/models/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from wagtail.models import Page as WagtailBasePage
from wagtail_localize.fields import SynchronizedField

from bedrock.cms.utils import get_locales_for_cms_page
from lib import l10n_utils


Expand Down Expand Up @@ -57,14 +58,8 @@ def _patch_request_for_bedrock(self, request):
# Quick annotation to help us track the origin of the page
request.is_cms_page = True

# Patch in a list of CMS-available locales for pages that are translations, not just aliases
request._locales_available_via_cms = [self.locale.language_code]
try:
_actual_translations = self.get_translations().exclude(id__in=[x.id for x in self.aliases.all()])
request._locales_available_via_cms += [x.locale.language_code for x in _actual_translations]
except ValueError:
# when there's no draft and no potential for aliases, etc, the above lookup will fail
pass
# Patch in a list of available locales for pages that are translations, not just aliases
request._locales_available_via_cms = get_locales_for_cms_page(self)
return request

def _render_with_fluent_string_support(self, request, *args, **kwargs):
Expand Down
11 changes: 10 additions & 1 deletion bedrock/cms/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,11 @@ def tiny_localized_site():
fr_root_page = en_us_root_page.copy_for_translation(fr_locale)
pt_br_root_page = en_us_root_page.copy_for_translation(pt_br_locale)

en_us_homepage = SimpleRichTextPageFactory(title="Test Page", slug="test-page", parent=en_us_root_page)
en_us_homepage = SimpleRichTextPageFactory(
title="Test Page",
slug="test-page",
parent=en_us_root_page,
)

en_us_child = SimpleRichTextPageFactory(
title="Child",
Expand All @@ -97,9 +101,14 @@ def tiny_localized_site():
rev = fr_child.save_revision()
fr_child.publish(rev)

# WARNING: there may be a bug with the page tree here
# fr_grandchild cannot be found with Page.find_for_request
# when all the others can. TODO: debug this, but manually
# it works
fr_grandchild = SimpleRichTextPageFactory(
title="Petit-enfant",
slug="grandchild-page",
locale=fr_locale,
parent=fr_child,
)

Expand Down
Loading

0 comments on commit 42ea7e3

Please sign in to comment.