diff --git a/bedrock/base/templates/includes/canonical-url.html b/bedrock/base/templates/includes/canonical-url.html index 52543fbcd47..23184079041 100644 --- a/bedrock/base/templates/includes/canonical-url.html +++ b/bedrock/base/templates/includes/canonical-url.html @@ -4,10 +4,12 @@ file, You can obtain one at https://mozilla.org/MPL/2.0/. #} +{%- set available_languages = get_locale_options(request, translations) -%} + - {% 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 -%} diff --git a/bedrock/base/templates/includes/protocol/lang-switcher-refresh.html b/bedrock/base/templates/includes/protocol/lang-switcher-refresh.html index ad4c9254c50..1c960da965a 100644 --- a/bedrock/base/templates/includes/protocol/lang-switcher-refresh.html +++ b/bedrock/base/templates/includes/protocol/lang-switcher-refresh.html @@ -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 %}
{{ ftl('footer-refresh-language') }} - {% 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 Brasil)" (Bug 861149) #} diff --git a/bedrock/base/templatetags/helpers.py b/bedrock/base/templatetags/helpers.py index ff94ebb3300..76210f47738 100644 --- a/bedrock/base/templatetags/helpers.py +++ b/bedrock/base/templatetags/helpers.py @@ -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 @@ -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 diff --git a/bedrock/base/tests/test_helpers.py b/bedrock/base/tests/test_helpers.py index 756de00de83..0a92dc22e2f 100644 --- a/bedrock/base/tests/test_helpers.py +++ b/bedrock/base/tests/test_helpers.py @@ -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 = { @@ -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, + ) diff --git a/bedrock/cms/decorators.py b/bedrock/cms/decorators.py index a7d84d80cbd..7321795507a 100644 --- a/bedrock/cms/decorators.py +++ b/bedrock/cms/decorators.py @@ -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 @@ -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) diff --git a/bedrock/cms/models/base.py b/bedrock/cms/models/base.py index a7439cd4c6a..055c25ac52a 100644 --- a/bedrock/cms/models/base.py +++ b/bedrock/cms/models/base.py @@ -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 @@ -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): diff --git a/bedrock/cms/tests/conftest.py b/bedrock/cms/tests/conftest.py index 9b6ad1f0df1..3142f05febf 100644 --- a/bedrock/cms/tests/conftest.py +++ b/bedrock/cms/tests/conftest.py @@ -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", @@ -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, ) diff --git a/bedrock/cms/tests/decorator_test_views.py b/bedrock/cms/tests/decorator_test_views.py index f02cac2918e..2902199953c 100644 --- a/bedrock/cms/tests/decorator_test_views.py +++ b/bedrock/cms/tests/decorator_test_views.py @@ -1,11 +1,14 @@ # 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 import mock from django.http import HttpResponse from bedrock.cms.decorators import prefer_cms +test_callable_to_get_locales = mock.Mock(return_value=["sco", "es-ES"]) + def undecorated_dummy_view(request): return HttpResponse("This is a dummy response from the undecorated view") @@ -16,5 +19,20 @@ def decorated_dummy_view(request): return HttpResponse("This is a dummy response from the decorated view") -def wrapped_dummy_view(request): +@prefer_cms(fallback_lang_codes=["fr-CA", "es-MX", "sco"]) +def decorated_dummy_view_with_locale_strings(request): + return HttpResponse("This is a dummy response from the decorated view with locale strings passed in") + + +@prefer_cms(fallback_ftl_files=["test/fluentA", "test/fluentB"]) +def decorated_dummy_view_with_fluent_files(request): + return HttpResponse("This is a dummy response from the decorated view with fluent files explicitly passed in") + + +def wrapped_dummy_view(request, *args, **kwargs): return HttpResponse("This is a dummy response from the wrapped view") + + +@prefer_cms(fallback_callable=test_callable_to_get_locales) +def decorated_dummy_view_for_use_with_a_callable(request, *args, **kwargs): + return HttpResponse(f"This is a dummy response from the decorated view for the callable, taking {args} and {kwargs}") diff --git a/bedrock/cms/tests/test_decorators.py b/bedrock/cms/tests/test_decorators.py index 4d1039db7b7..6905ab7bfb9 100644 --- a/bedrock/cms/tests/test_decorators.py +++ b/bedrock/cms/tests/test_decorators.py @@ -2,6 +2,7 @@ # 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 django.urls import path import pytest @@ -10,7 +11,6 @@ from bedrock.base.i18n import bedrock_i18n_patterns from bedrock.cms.decorators import prefer_cms from bedrock.cms.tests import decorator_test_views -from bedrock.mozorg.util import page from bedrock.urls import urlpatterns as mozorg_urlpatterns from .factories import SimpleRichTextPageFactory @@ -27,6 +27,21 @@ decorator_test_views.decorated_dummy_view, name="decorated_dummy_view", ), + path( + "decorated/view/path/with/locale/strings/", + decorator_test_views.decorated_dummy_view_with_locale_strings, + name="decorated_dummy_view_with_locale_strings", + ), + path( + "decorated/view/path/with/fluent/files/", + decorator_test_views.decorated_dummy_view_with_fluent_files, + name="decorated_dummy_view_with_fluent_files", + ), + path( + "decorated/view/path/with/a/callable/taking//", + decorator_test_views.decorated_dummy_view_for_use_with_a_callable, + name="decorated_dummy_view_for_use_with_a_callable", + ), path( "wrapped/view/path/", prefer_cms( @@ -34,7 +49,30 @@ ), name="url_wrapped_dummy_view", ), - page("book/", "mozorg/book.html", decorators=[prefer_cms]), + path( + "wrapped/view/path/with/fluent/files/", + prefer_cms( + decorator_test_views.wrapped_dummy_view, + fallback_ftl_files=["test/fluent1", "test/fluent2"], + ), + name="url_wrapped_dummy_view", + ), + path( + "wrapped/view/path/with/locale/strings/", + prefer_cms( + decorator_test_views.wrapped_dummy_view, + fallback_lang_codes=["fr-CA", "es-MX", "sco"], + ), + name="url_wrapped_dummy_view", + ), + path( + "wrapped/view/path/with/a/callable/taking//", + prefer_cms( + decorator_test_views.wrapped_dummy_view, + fallback_callable=decorator_test_views.test_callable_to_get_locales, + ), + name="url_wrapped_dummy_view", + ), ) + mozorg_urlpatterns # we need to extend these so Jinja2 can call url() in the templates ) @@ -85,6 +123,112 @@ def test_decorating_django_view(lang_code_prefix, minimal_site, client): assert "This is a CMS page now, with the slug of path" in resp.content.decode("utf-8") +@pytest.mark.urls(__name__) +@pytest.mark.parametrize("lang_code_prefix", ("", "/en-US")) +def test_decorating_django_view__passing_fallback_lang_codes( + lang_code_prefix, + minimal_site, + client, +): + resp = client.get( + "/decorated/view/path/with/locale/strings/", + follow=True, + ) + assert resp.status_code == 200 + assert resp.content.decode("utf-8") == "This is a dummy response from the decorated view with locale strings passed in" + # Show that the expected locales are annotated onto the request + assert resp.wsgi_request._locales_for_django_fallback_view == ["fr-CA", "es-MX", "sco"] + assert resp.wsgi_request._locales_available_via_cms == [] # No page in CMS yet + + # Show the decorated view will "prefer" to render the Wagtail page when it exists + _set_up_cms_pages( + deepest_path="/decorated/view/path/with/locale/strings/", + site=minimal_site, + ) + + resp = client.get(f"{lang_code_prefix}/decorated/view/path/with/locale/strings/", follow=True) + assert resp.status_code == 200 + # Show that the expected locales are annotated onto the request + assert resp.wsgi_request._locales_for_django_fallback_view == ["fr-CA", "es-MX", "sco"] + assert resp.wsgi_request._locales_available_via_cms == ["en-US"] + assert "This is a CMS page now, with the slug of strings" in resp.content.decode("utf-8") + + +@pytest.mark.urls(__name__) +@pytest.mark.parametrize("lang_code_prefix", ("", "/en-US")) +def test_decorating_django_view__passing_callable_for_locales( + lang_code_prefix, + minimal_site, + client, +): + resp = client.get( + "/decorated/view/path/with/a/callable/taking/a-slug-here/", + follow=True, + ) + assert resp.status_code == 200 + assert ( + resp.content.decode("utf-8") == "This is a dummy response from the decorated view for the callable, taking () and {'a_slug': 'a-slug-here'}" + ) + # Show that the expected locales are annotated onto the request + assert resp.wsgi_request._locales_for_django_fallback_view == ["sco", "es-ES"] + assert resp.wsgi_request._locales_available_via_cms == [] # No page in CMS yet + + # Show the decorated view will "prefer" to render the Wagtail page when it exists + _set_up_cms_pages( + deepest_path="/decorated/view/path/with/a/callable/taking/a-slug-here/", + site=minimal_site, + ) + + resp = client.get(f"{lang_code_prefix}/decorated/view/path/with/a/callable/taking/a-slug-here/", follow=True) + assert resp.status_code == 200 + # Show that the expected locales are annotated onto the request + assert resp.wsgi_request._locales_for_django_fallback_view == ["sco", "es-ES"] + assert resp.wsgi_request._locales_available_via_cms == ["en-US"] + assert "This is a CMS page now, with the slug of a-slug-here" in resp.content.decode("utf-8") + + +@pytest.mark.urls(__name__) +@pytest.mark.parametrize("lang_code_prefix", ("", "/en-US")) +def test_decorating_django_view__passing_ftl_files(lang_code_prefix, minimal_site, client, mocker): + mock_get_active_locales = mocker.patch("bedrock.cms.decorators.get_active_locales") + mock_get_active_locales.return_value = ["sco", "es-ES", "fr-CA"] + + assert not mock_get_active_locales.called + resp = client.get( + "/decorated/view/path/with/fluent/files/", + follow=True, + ) + assert resp.status_code == 200 + assert resp.content.decode("utf-8") == "This is a dummy response from the decorated view with fluent files explicitly passed in" + # Show that the expected locales are annotated onto the request + assert resp.wsgi_request._locales_for_django_fallback_view == ["sco", "es-ES", "fr-CA"] + mock_get_active_locales.assert_called_once_with( + ["test/fluentA", "test/fluentB"], + force=True, + ) + + assert resp.wsgi_request._locales_available_via_cms == [] # No page in CMS yet + + # Show the decorated view will "prefer" to render the Wagtail page when it exists + _set_up_cms_pages( + deepest_path="/decorated/view/path/with/fluent/files/", + site=minimal_site, + ) + + mock_get_active_locales.reset_mock() + + resp = client.get(f"{lang_code_prefix}/decorated/view/path/with/fluent/files/", follow=True) + assert resp.status_code == 200 + # Show that the expected locales are annotated onto the request + assert resp.wsgi_request._locales_for_django_fallback_view == ["sco", "es-ES", "fr-CA"] + mock_get_active_locales.assert_called_once_with( + ["test/fluentA", "test/fluentB"], + force=True, + ) + assert resp.wsgi_request._locales_available_via_cms == ["en-US"] + assert "This is a CMS page now, with the slug of files" in resp.content.decode("utf-8") + + @pytest.mark.urls(__name__) @pytest.mark.parametrize("lang_code_prefix", ("", "/en-US")) def test_patching_in_urlconf__standard_django_view(lang_code_prefix, minimal_site, client): @@ -107,21 +251,111 @@ def test_patching_in_urlconf__standard_django_view(lang_code_prefix, minimal_sit @pytest.mark.urls(__name__) @pytest.mark.parametrize("lang_code_prefix", ("", "/en-US")) -def test_support_with_bedrock_page_view(lang_code_prefix, minimal_site, client): - # Show decorated page() view renders Django view initially, because there is no CMS page at that route yet - resp = client.get(f"{lang_code_prefix}/book/", follow=True) +def test_patching_in_urlconf__standard_django_view__with_locale_list( + lang_code_prefix, + minimal_site, + client, +): + resp = client.get( + "/wrapped/view/path/with/locale/strings/", + follow=True, + ) + assert resp.status_code == 200 + assert resp.content.decode("utf-8") == "This is a dummy response from the wrapped view" + # Show that the expected locales are annotated onto the request + assert resp.wsgi_request._locales_for_django_fallback_view == ["fr-CA", "es-MX", "sco"] + assert resp.wsgi_request._locales_available_via_cms == [] # No page in CMS yet + + # Show the decorated view will "prefer" to render the Wagtail page when it exists + _set_up_cms_pages( + deepest_path="/wrapped/view/path/with/locale/strings/", + site=minimal_site, + ) + + resp = client.get(f"{lang_code_prefix}/wrapped/view/path/with/locale/strings/", follow=True) + assert resp.status_code == 200 + # Show that the expected locales are annotated onto the request + assert resp.wsgi_request._locales_for_django_fallback_view == ["fr-CA", "es-MX", "sco"] + assert resp.wsgi_request._locales_available_via_cms == ["en-US"] + assert "This is a CMS page now, with the slug of strings" in resp.content.decode("utf-8") + + +@pytest.mark.urls(__name__) +@pytest.mark.parametrize("lang_code_prefix", ("", "/en-US")) +def test_patching_in_urlconf__standard_django_view__with_callback_for_locales( + lang_code_prefix, + minimal_site, + client, +): + resp = client.get( + "/wrapped/view/path/with/a/callable/taking/a-slug-here/", + follow=True, + ) + assert resp.status_code == 200 + assert resp.content.decode("utf-8") == "This is a dummy response from the wrapped view" + # Show that the expected locales are annotated onto the request + assert resp.wsgi_request._locales_for_django_fallback_view == ["sco", "es-ES"] + assert resp.wsgi_request._locales_available_via_cms == [] # No page in CMS yet + + # Show the decorated view will "prefer" to render the Wagtail page when it exists + _set_up_cms_pages( + deepest_path="/wrapped/view/path/with/a/callable/taking/a-slug-here/", + site=minimal_site, + ) + + resp = client.get(f"{lang_code_prefix}/wrapped/view/path/with/a/callable/taking/a-slug-here/", follow=True) + assert resp.status_code == 200 + # Show that the expected locales are annotated onto the request + assert resp.wsgi_request._locales_for_django_fallback_view == ["sco", "es-ES"] + assert resp.wsgi_request._locales_available_via_cms == ["en-US"] + assert "This is a CMS page now, with the slug of a-slug-here" in resp.content.decode("utf-8") + + +@pytest.mark.urls(__name__) +@pytest.mark.parametrize("lang_code_prefix", ("", "/en-US")) +def test_patching_in_urlconf__standard_django_view__with_fluent_files( + lang_code_prefix, + minimal_site, + client, + mocker, +): + mock_get_active_locales = mocker.patch("bedrock.cms.decorators.get_active_locales") + mock_get_active_locales.return_value = ["sco", "es-ES", "fr-CA"] + + assert not mock_get_active_locales.called + + resp = client.get( + "/wrapped/view/path/with/fluent/files/", + follow=True, + ) assert resp.status_code == 200 - assert "The Book of Mozilla" in resp.content.decode("utf-8") + assert resp.content.decode("utf-8") == "This is a dummy response from the wrapped view" + + mock_get_active_locales.assert_called_once_with( + ["test/fluent1", "test/fluent2"], + force=True, + ) + assert resp.wsgi_request._locales_for_django_fallback_view == ["sco", "es-ES", "fr-CA"] + assert resp.wsgi_request._locales_available_via_cms == [] # No page in CMS yet # Show the decorated view will "prefer" to render the Wagtail page when it exists _set_up_cms_pages( - deepest_path="/book/", + deepest_path="/wrapped/view/path/with/fluent/files/", site=minimal_site, ) - resp = client.get(f"{lang_code_prefix}/book/", follow=True) + mock_get_active_locales.reset_mock() + + resp = client.get(f"{lang_code_prefix}/wrapped/view/path/with/fluent/files/", follow=True) assert resp.status_code == 200 - assert "This is a CMS page now, with the slug of book" in resp.content.decode("utf-8") + mock_get_active_locales.assert_called_once_with( + ["test/fluent1", "test/fluent2"], + force=True, + ) + + assert resp.wsgi_request._locales_for_django_fallback_view == ["sco", "es-ES", "fr-CA"] + assert resp.wsgi_request._locales_available_via_cms == ["en-US"] + assert "This is a CMS page now, with the slug of files" in resp.content.decode("utf-8") @pytest.mark.urls(__name__) @@ -148,3 +382,69 @@ def test_draft_pages_do_not_get_preferred_over_django_views(lang_code_prefix, mi resp = client.get("/decorated/view/path/", follow=True) assert resp.status_code == 200 assert resp.content.decode("utf-8") == "This is a dummy response from the decorated view" + + +def _fake_callable(): + pass + + +@pytest.mark.parametrize( + "config, expect_exeption", + ( + ( + { + "fallback_ftl_files": ["test/files"], + "fallback_lang_codes": ["sco", "en-CA"], + }, + True, + ), + ( + { + "fallback_ftl_files": ["test/files"], + "fallback_callable": _fake_callable, + }, + True, + ), + ( + { + "fallback_lang_codes": ["sco", "en-CA"], + "fallback_callable": _fake_callable, + }, + True, + ), + ( + { + "fallback_ftl_files": ["test/files"], + "fallback_lang_codes": ["sco", "en-CA"], + "fallback_callable": _fake_callable, + }, + True, + ), + ( + { + "fallback_ftl_files": ["test/files"], + }, + False, + ), + ( + { + "fallback_lang_codes": ["sco", "en-CA"], + }, + False, + ), + ( + { + "fallback_callable": _fake_callable, + }, + False, + ), + ), +) +def test_prefer_cms_rejects_invalid_setup(mocker, config, expect_exeption): + fake_view = mocker.Mock(name="fake view") + + if expect_exeption: + with pytest.raises(RuntimeError): + prefer_cms(view_func=fake_view, **config) + else: + prefer_cms(view_func=fake_view, **config) diff --git a/bedrock/cms/tests/test_models.py b/bedrock/cms/tests/test_models.py index ce8f1e6887b..a01a5498ead 100644 --- a/bedrock/cms/tests/test_models.py +++ b/bedrock/cms/tests/test_models.py @@ -7,7 +7,7 @@ from django.test import override_settings import pytest -from wagtail.models import Locale, Page +from wagtail.models import Page from bedrock.cms.models import ( AbstractBedrockCMSPage, @@ -100,32 +100,19 @@ def test_CMS_ALLOWED_PAGE_MODELS_controls_Page_can_create_at( assert page_class.can_create_at(home_page) == success_expected -def test__patch_request_for_bedrock__locales_available_via_cms(tiny_localized_site, rf): +@mock.patch("bedrock.cms.models.base.get_locales_for_cms_page") +def test__patch_request_for_bedrock__locales_available_via_cms( + mock_get_locales_for_cms_page, + minimal_site, + rf, +): request = rf.get("/some-path/that/is/irrelevant") - en_us_homepage = Page.objects.get(locale__language_code="en-US", slug="home") - en_us_test_page = en_us_homepage.get_children()[0] - - # By default there are no aliases in the system, so all _locales_available_for_cms will - # match the pages set up in the tiny_localized_site fixture - assert Page.objects.filter(alias_of__isnull=False).count() == 0 - patched_request = en_us_test_page.specific._patch_request_for_bedrock(request) - assert sorted(patched_request._locales_available_via_cms) == ["en-US", "fr", "pt-BR"] - - # now make aliases of the test_page into Dutch and Spanish - nl_locale = Locale.objects.create(language_code="nl") - es_es_locale = Locale.objects.create(language_code="es-ES") - - nl_page_alias = en_us_test_page.copy_for_translation(locale=nl_locale, copy_parents=True, alias=True) - nl_page_alias.save() - - es_es_page_alias = en_us_test_page.copy_for_translation(locale=es_es_locale, copy_parents=True, alias=True) - es_es_page_alias.save() + page = SimpleRichTextPage.objects.last() # made by the minimal_site fixture - assert Page.objects.filter(alias_of__isnull=False).count() == 4 # 2 child + 2 parent pages, which had to be copied too + mock_get_locales_for_cms_page.return_value = ["en-US", "fr", "pt-BR"] - # Show that the aliases don't appear in the available locales - patched_request = en_us_test_page.specific._patch_request_for_bedrock(request) + patched_request = page.specific._patch_request_for_bedrock(request) assert sorted(patched_request._locales_available_via_cms) == ["en-US", "fr", "pt-BR"] diff --git a/bedrock/cms/tests/test_rendering.py b/bedrock/cms/tests/test_rendering.py index 9d0226490ba..9a412b950d8 100644 --- a/bedrock/cms/tests/test_rendering.py +++ b/bedrock/cms/tests/test_rendering.py @@ -98,7 +98,8 @@ def test_locales_are_drawn_from_page_translations(minimal_site, rf, serving_meth page = Page.objects.last().specific fr_page = page.copy_for_translation(fr_locale) fr_page.title = "FR test page" - fr_page.save() + rev = fr_page.save_revision() + fr_page.publish(rev) assert fr_page.locale.language_code == "fr" _relative_url = page.relative_url(minimal_site) diff --git a/bedrock/cms/tests/test_utils.py b/bedrock/cms/tests/test_utils.py new file mode 100644 index 00000000000..f925fa3ce2f --- /dev/null +++ b/bedrock/cms/tests/test_utils.py @@ -0,0 +1,144 @@ +# 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/. + + +import pytest +from wagtail.coreutils import get_dummy_request +from wagtail.models import Locale, Page + +from bedrock.cms.utils import get_cms_locales_for_path, get_locales_for_cms_page, get_page_for_request + +pytestmark = [pytest.mark.django_db] + + +def test_get_locales_for_cms_page(tiny_localized_site): + en_us_homepage = Page.objects.get(locale__language_code="en-US", slug="home") + en_us_test_page = en_us_homepage.get_children()[0] + + # By default there are no aliases in the system, so all _locales_available_for_cms will + # match the pages set up in the tiny_localized_site fixture + assert Page.objects.filter(alias_of__isnull=False).count() == 0 + + assert sorted(get_locales_for_cms_page(en_us_test_page)) == ["en-US", "fr", "pt-BR"] + + # now make aliases of the test_page into Dutch and Spanish + nl_locale = Locale.objects.create(language_code="nl") + es_es_locale = Locale.objects.create(language_code="es-ES") + + nl_page_alias = en_us_test_page.copy_for_translation(locale=nl_locale, copy_parents=True, alias=True) + nl_page_alias.save() + + es_es_page_alias = en_us_test_page.copy_for_translation(locale=es_es_locale, copy_parents=True, alias=True) + es_es_page_alias.save() + + assert Page.objects.filter(alias_of__isnull=False).count() == 4 # 2 child + 2 parent pages, which had to be copied too + + # Show that the aliases don't appear in the available locales + assert sorted(get_locales_for_cms_page(en_us_test_page)) == ["en-US", "fr", "pt-BR"] + + +def test_get_locales_for_cms_page__ensure_draft_pages_are_excluded(tiny_localized_site): + en_us_homepage = Page.objects.get(locale__language_code="en-US", slug="home") + en_us_test_page = en_us_homepage.get_children()[0] + fr_homepage = Page.objects.get(locale__language_code="fr", slug="home-fr") + fr_test_page = fr_homepage.get_children()[0] + + fr_test_page.unpublish() + + assert sorted(get_locales_for_cms_page(en_us_test_page)) == ["en-US", "pt-BR"] + + +@pytest.mark.parametrize( + "path, expected_page_url", + [ + ( + "/en-US/test-page/", + "/en-US/test-page/", + ), + ( + "/test-page/", + "/en-US/test-page/", + ), + ( + "/pt-BR/test-page/", + "/pt-BR/test-page/", + ), + ( + "/pt-BR/test-page/child-page/", + "/pt-BR/test-page/child-page/", + ), + ( + "/fr/test-page/", + "/fr/test-page/", + ), + ( + "/fr/test-page/child-page/", + "/fr/test-page/child-page/", + ), + # These two routes do not work, even though a manual test with similar ones + # does not show up as a problem. I think it's possibly related to the + # tiny_localized_site fixture generated in cms/tests/conftest.py + # ( + # "/fr/test-page/child-page/grandchild-page/", + # "/fr/test-page/child-page/grandchild-page/", + # ), + # ( + # "/test-page/child-page/grandchild-page/", + # "/fr/test-page/child-page/grandchild-page/", + # ), + ], +) +def test_get_page_for_request__happy_path(path, expected_page_url, tiny_localized_site): + request = get_dummy_request(path=path) + page = get_page_for_request(request=request) + assert isinstance(page, Page) + assert page.url == expected_page_url + + +@pytest.mark.parametrize( + "path", + [ + "/en-US/test-page/fake/path/", + "/fr/test-page/fake/path/", + "/not/a/real/test-page", + ], +) +def test_get_page_for_request__no_match(path, tiny_localized_site): + request = get_dummy_request(path=path) + page = get_page_for_request(request=request) + assert page is None + + +@pytest.mark.parametrize( + "get_page_for_request_should_return_a_page, get_locales_for_cms_page_retval, expected", + ( + (True, ["en-CA", "sco", "zh-CN"], ["en-CA", "sco", "zh-CN"]), + (False, None, []), + ), +) +def test_get_cms_locales_for_path( + rf, + get_page_for_request_should_return_a_page, + get_locales_for_cms_page_retval, + expected, + minimal_site, + mocker, +): + request = rf.get("/path/is/irrelevant/due/to/mocks") + mock_get_page_for_request = mocker.patch("bedrock.cms.utils.get_page_for_request") + mock_get_locales_for_cms_page = mocker.patch("bedrock.cms.utils.get_locales_for_cms_page") + + if get_page_for_request_should_return_a_page: + page = mocker.Mock("fake-page") + mock_get_page_for_request.return_value = page + mock_get_locales_for_cms_page.return_value = get_locales_for_cms_page_retval + else: + mock_get_page_for_request.return_value = None + + request = rf.get("/irrelevant/because/we/are/mocking") + assert get_cms_locales_for_path(request) == expected + + if get_page_for_request_should_return_a_page: + mock_get_page_for_request.assert_called_once_with(request=request) + mock_get_locales_for_cms_page.assert_called_once_with(page=page) diff --git a/bedrock/cms/utils.py b/bedrock/cms/utils.py new file mode 100644 index 00000000000..eed3b69b408 --- /dev/null +++ b/bedrock/cms/utils.py @@ -0,0 +1,63 @@ +# 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 django.core.exceptions import ObjectDoesNotExist +from django.db.models import Subquery +from django.http import Http404 + +from wagtail.models import Locale, Page + +from bedrock.base.i18n import split_path_and_normalize_language + + +def get_page_for_request(*, request): + """For the given HTTPRequest (and its path) find the corresponding Wagtail + page, if one exists""" + + lang_code, path, _ = split_path_and_normalize_language(request.path) + try: + locale = Locale.objects.get(language_code=lang_code) + except Locale.DoesNotExist: + locale = None + + try: + page = Page.find_for_request(request=request, path=path) + if page and locale and locale != page.locale: + page = page.get_translation(locale=locale) + + except (ObjectDoesNotExist, Http404): + page = None + + return page + + +def get_locales_for_cms_page(page): + # Patch in a list of CMS-available locales for pages that are + # translations, not just aliases + + locales_available_via_cms = [page.locale.language_code] + try: + _actual_translations = ( + page.get_translations() + .live() + .exclude( + id__in=Subquery( + page.aliases.all().values_list("id", flat=True), + ) + ) + ) + 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 + + return locales_available_via_cms + + +def get_cms_locales_for_path(request): + locales = [] + + if page := get_page_for_request(request=request): + locales = get_locales_for_cms_page(page=page) + + return locales diff --git a/bedrock/products/urls.py b/bedrock/products/urls.py index 9cfff8f2347..f5bc611a1da 100644 --- a/bedrock/products/urls.py +++ b/bedrock/products/urls.py @@ -35,12 +35,18 @@ # VPN Resource Center path( "vpn/resource-center/", - prefer_cms(views.resource_center_landing_view), + prefer_cms( + views.resource_center_landing_view, + fallback_lang_codes=["de", "en-US", "es-ES", "fr", "it", "ja", "nl", "pl", "pt-BR", "ru", "zh-CN"], + ), name="products.vpn.resource-center.landing", ), path( "vpn/resource-center//", - prefer_cms(views.resource_center_article_view), + prefer_cms( + views.resource_center_article_view, + fallback_callable=views.resource_center_article_available_locales_lookup, + ), name="products.vpn.resource-center.article", ), path("monitor/waitlist-plus/", views.monitor_waitlist_plus_page, name="products.monitor.waitlist-plus"), diff --git a/bedrock/products/views.py b/bedrock/products/views.py index 7b108a2f748..7f59983ff8e 100644 --- a/bedrock/products/views.py +++ b/bedrock/products/views.py @@ -377,6 +377,17 @@ def resource_center_article_view(request, slug): ) +def resource_center_article_available_locales_lookup(*, slug: str) -> list[str]: + # Helper func to get a list of language codes available for the given + # Contentful-powered VPN RC slug + return list( + ContentfulEntry.objects.filter( + localisation_complete=True, + slug=slug, + ).values_list("locale", flat=True) + ) + + @require_safe def monitor_waitlist_scan_page(request): template_name = "products/monitor/waitlist/scan.html" diff --git a/docs/cms.rst b/docs/cms.rst index bf190002582..9d7ead46e19 100644 --- a/docs/cms.rst +++ b/docs/cms.rst @@ -407,6 +407,11 @@ page-serving logic comes last in all URLConfs). **BUT...** how can you enter con into the CMS fast enough replace the just-removed Django page? (Note: we could use a data migraiton here, but that gets complicated when there are images involved) +Equally, you may have a situation where the content for certain paths needs to be +managed in the CMS for certain locales, while other locales (with rarely changing +'evergreen' content) may only exist as Django-rendered views drawing strings from +Fluent. + The answer here is to use the ``bedrock.cms.decorators.prefer_cms`` decorator/helper. A Django view decorated with ``prefer_cms`` will check if a live CMS page has been @@ -415,14 +420,16 @@ one, it will show the user `that` CMS page instead. If there is no match in the then the original Django view will be used. The result is a graceful handover flow that allows us to switch to the CMS page -without needing to remove the Django view from the URLconf. It doesn't affect +without needing to remove the Django view from the URLconf, or to maintain a +hybrid approach to page management. It doesn't affect previews, so the review of draft pages before publishing can continue with no changes. Once the CMS is populated with a live version of the replacement page, that's when a -later changeset can remove the deprecated Django view. +later changeset can remove the deprecated Django view if it's no longer needed. The ``prefer_cms`` decorator can be used directly on function-based views, or can wrap -views in the URLconf. It can also be passed to our very handy -``bedrock.mozorg.util.page`` as one of the list of ``decorator`` arguments. +views in the URLconf. It should not used with ``bedrock.mozorg.util.page`` due to +the complexity of passing through what locales are involved, but instead the relevant +URL route should be refactored as a regular Django view, and then decorated with ``prefer_cms`` For more details, please see the docstring on ``bedrock.cms.decorators.prefer_cms``.