From 42ea7e3a76976c675435c934b4cae942fc273559 Mon Sep 17 00:00:00 2001 From: Steve Jalim Date: Mon, 18 Nov 2024 09:22:37 +0000 Subject: [PATCH] Provide a hybrid list of locales drawn from CMS-backed and Django-backed 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 --- .../templates/includes/canonical-url.html | 6 +- .../protocol/lang-switcher-refresh.html | 6 +- .../includes/protocol/lang-switcher.html | 6 +- bedrock/base/templatetags/helpers.py | 20 ++ bedrock/base/tests/test_helpers.py | 36 +- bedrock/cms/decorators.py | 164 +++++++-- bedrock/cms/models/base.py | 11 +- bedrock/cms/tests/conftest.py | 11 +- bedrock/cms/tests/decorator_test_views.py | 20 +- bedrock/cms/tests/test_decorators.py | 318 +++++++++++++++++- bedrock/cms/tests/test_models.py | 33 +- bedrock/cms/tests/test_rendering.py | 3 +- bedrock/cms/tests/test_utils.py | 144 ++++++++ bedrock/cms/utils.py | 63 ++++ bedrock/products/urls.py | 10 +- bedrock/products/views.py | 11 + docs/cms.rst | 15 +- 17 files changed, 791 insertions(+), 86 deletions(-) create mode 100644 bedrock/cms/tests/test_utils.py create mode 100644 bedrock/cms/utils.py 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``.