From dbf0c00cb96ef27e49abfc0b54b51c6f65c95c4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A6var=20=C3=96fj=C3=B6r=C3=B0=20Magn=C3=BAsson?= Date: Fri, 11 Nov 2022 10:05:01 +0000 Subject: [PATCH 1/4] Fixes #262. Move the homepage lookup after response handling so that we have access to the url resolver. Then check if the requested path belongs to i18n_patterns. --- apps/core/middleware.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/apps/core/middleware.py b/apps/core/middleware.py index a91c6096..dc41369c 100644 --- a/apps/core/middleware.py +++ b/apps/core/middleware.py @@ -1,18 +1,25 @@ from django.http import Http404 +from django.urls import LocalePrefixPattern from django.utils.translation import activate, get_language from apps.core.models import HomePage +pattern = LocalePrefixPattern() + class ValidateLocaleMiddleware: def __init__(self, get_response): self.get_response = get_response def __call__(self, request): - try: - HomePage.objects.get(locale__language_code=get_language(), live=True) - except HomePage.DoesNotExist: - activate("en-latest") - raise Http404() response = self.get_response(request) + if request.resolver_match: + # Check if the path is in the i18n_patterns + if pattern.match(request.resolver_match.route): + try: + HomePage.objects.get(locale__language_code=get_language(), live=True) + except HomePage.DoesNotExist: + # Activate English so that we have a site menu + activate("en-latest") + raise Http404() return response From 63ccc56ec43e5dbf3fbc748d4e6212df0f54a019 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A6var=20=C3=96fj=C3=B6r=C3=B0=20Magn=C3=BAsson?= Date: Fri, 11 Nov 2022 10:14:23 +0000 Subject: [PATCH 2/4] Lint python --- apps/core/middleware.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/apps/core/middleware.py b/apps/core/middleware.py index dc41369c..c4a4b966 100644 --- a/apps/core/middleware.py +++ b/apps/core/middleware.py @@ -14,10 +14,11 @@ def __init__(self, get_response): def __call__(self, request): response = self.get_response(request) if request.resolver_match: - # Check if the path is in the i18n_patterns + # Check if the path is in the i18n_patterns if pattern.match(request.resolver_match.route): try: - HomePage.objects.get(locale__language_code=get_language(), live=True) + HomePage.objects.get( + locale__language_code=get_language(), live=True) except HomePage.DoesNotExist: # Activate English so that we have a site menu activate("en-latest") From 1b29d5834e2c6bf790ee63e84e32deaf22152f8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A6var=20=C3=96fj=C3=B6r=C3=B0=20Magn=C3=BAsson?= Date: Fri, 11 Nov 2022 10:29:33 +0000 Subject: [PATCH 3/4] Lint python --- apps/core/middleware.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/core/middleware.py b/apps/core/middleware.py index c4a4b966..8bc409e2 100644 --- a/apps/core/middleware.py +++ b/apps/core/middleware.py @@ -18,7 +18,8 @@ def __call__(self, request): if pattern.match(request.resolver_match.route): try: HomePage.objects.get( - locale__language_code=get_language(), live=True) + locale__language_code=get_language(), live=True + ) except HomePage.DoesNotExist: # Activate English so that we have a site menu activate("en-latest") From 335e2b4db0067286cc8e2610aa687e0289c5d4b3 Mon Sep 17 00:00:00 2001 From: Coen van der Kamp Date: Fri, 18 Nov 2022 23:39:26 +0100 Subject: [PATCH 4/4] Fix accept header scenario, add tests --- apps/core/middleware.py | 28 +++---- apps/core/tests/test_middleware.py | 119 +++++++++++++++++++++++++++++ 2 files changed, 134 insertions(+), 13 deletions(-) create mode 100644 apps/core/tests/test_middleware.py diff --git a/apps/core/middleware.py b/apps/core/middleware.py index 8bc409e2..3931959d 100644 --- a/apps/core/middleware.py +++ b/apps/core/middleware.py @@ -1,4 +1,4 @@ -from django.http import Http404 +from django.conf import settings from django.urls import LocalePrefixPattern from django.utils.translation import activate, get_language @@ -12,16 +12,18 @@ def __init__(self, get_response): self.get_response = get_response def __call__(self, request): - response = self.get_response(request) - if request.resolver_match: - # Check if the path is in the i18n_patterns - if pattern.match(request.resolver_match.route): - try: - HomePage.objects.get( - locale__language_code=get_language(), live=True - ) - except HomePage.DoesNotExist: - # Activate English so that we have a site menu - activate("en-latest") - raise Http404() + if ( + request.path == "/" + or request.resolver_match + and pattern.match(request.resolver_match.route) + ): + try: + HomePage.objects.get(locale__language_code=get_language(), live=True) + response = self.get_response(request) + except HomePage.DoesNotExist: + # The requested language is not available, use the default + activate(settings.LANGUAGE_CODE) + response = self.get_response(request) + else: + response = self.get_response(request) return response diff --git a/apps/core/tests/test_middleware.py b/apps/core/tests/test_middleware.py new file mode 100644 index 00000000..23dbc7bb --- /dev/null +++ b/apps/core/tests/test_middleware.py @@ -0,0 +1,119 @@ +from http import HTTPStatus + +from django.conf import settings +from django.test import TestCase +from django.urls import reverse + +from apps.core.factories import HomePageFactory, LocaleFactory + + +class TestValidateLocaleMiddleware(TestCase): + """ + LocaleMiddleware tries to determine the user’s language preference by following + this algorithm: + + 1. First, it looks for the language prefix in the requested URL. + 2. Failing that, it looks for a cookie, named `django_language`. + 3. Failing that, it looks at the Accept-Language HTTP header. This header is sent by + your browser and tells the server which language(s) you prefer, in order by + priority. Django tries each language in the header until it finds one with + available translations. + 4. Failing that, it uses the global LANGUAGE_CODE setting. + + Wagtail Guide has all languages in LANGUAGES setting, therefore LocaleMiddleware + will redirect to any language prefix URL. Unfortunately, Wagtail might not have + the corresponding homepage for that language published. If Wagtail has no + homepage available for the requested language, it raises a 404. + + This behaviour is undesirable. As step 3 of the algoritm will end up in a 404. + To resolve this issue, we introduce our ValidateLocaleMiddleware. + + ValidateLocaleMiddleware checks for a published homepage for the requested language. + If it does not exist, it falls back to English. The default, and always published + language/homepage. + + The ValidateLocaleMiddleware kicks in on `/` and any i18n pattern. + Other URLs are ignored, and have the default LocaleMiddleware behaviour. + """ + + def setUp(self): + self.en = LocaleFactory(language_code="en-latest") + self.home_en = HomePageFactory(locale=self.en) + + def test_middleware_settings(self): + self.assertIn("django.middleware.locale.LocaleMiddleware", settings.MIDDLEWARE) + self.assertIn( + "apps.core.middleware.ValidateLocaleMiddleware", settings.MIDDLEWARE + ) + self.assertGreater( + settings.MIDDLEWARE.index("apps.core.middleware.ValidateLocaleMiddleware"), + settings.MIDDLEWARE.index("django.middleware.locale.LocaleMiddleware"), + ) + + def test_request_root_redirects_to_language_code(self): + self.assertEqual(settings.LANGUAGE_CODE, "en-latest") + response = self.client.get("/") + self.assertEqual(response.status_code, HTTPStatus.FOUND) + self.assertEqual(response.url, "/en-latest/") + self.assertEqual(self.client.get("/en-latest/").status_code, HTTPStatus.OK) + + def test_request_root_with_accept_language_header(self): + # To English, if German doesn't exist + response = self.client.get("/", HTTP_ACCEPT_LANGUAGE="de") + self.assertEqual(response.status_code, HTTPStatus.FOUND) + self.assertEqual(response.url, "/en-latest/") + self.assertEqual(self.client.get("/en-latest/").status_code, HTTPStatus.OK) + # To German, if German exists + de = LocaleFactory(language_code="de-latest") + self.home_de = self.home_en.copy_for_translation(locale=de) + self.home_de.save_revision().publish() + response = self.client.get("/", HTTP_ACCEPT_LANGUAGE="de") + self.assertEqual(response.status_code, HTTPStatus.FOUND) + self.assertEqual(response.url, "/de-latest/") + self.assertEqual(self.client.get("/de-latest/").status_code, HTTPStatus.OK) + + def test_request_root_with_cookie(self): + self.assertEqual(settings.LANGUAGE_COOKIE_NAME, "django_language") + de = LocaleFactory(language_code="de-latest") + self.home_de = self.home_en.copy_for_translation(locale=de) + self.home_de.save_revision().publish() + # The HTTP_ACCEPT_LANGUAGE is ignored, the cookie takes precedence + self.client.cookies["django_language"] = "en-latest" + response = self.client.get("/", HTTP_ACCEPT_LANGUAGE="de") + self.assertEqual(response.status_code, HTTPStatus.FOUND) + self.assertEqual(response.url, "/en-latest/") + self.assertEqual(self.client.get("/en-latest/").status_code, HTTPStatus.OK) + + def test_request_specific_url(self): + de = LocaleFactory(language_code="de-latest") + self.home_de = self.home_en.copy_for_translation(locale=de) + self.home_de.save_revision().publish() + # The HTTP_ACCEPT_LANGUAGE is ignored + self.client.cookies["django_language"] = "de" + # The HTTP_ACCEPT_LANGUAGE is ignored + response = self.client.get("/en-latest/", HTTP_ACCEPT_LANGUAGE="de") + self.assertEqual(response.status_code, HTTPStatus.OK) + + def test_wagtail_admin_respects_accept_language(self): + # Non i18n_patterns respect HTTP_ACCEPT_LANGUAGE + url = reverse("wagtailadmin_login") + response = self.client.get(url, HTTP_ACCEPT_LANGUAGE="nl") + expected = "

Inloggen in Wagtail

" + self.assertInHTML(expected, str(response.content)) + + def test_django_admin_respects_accept_language(self): + # Non i18n_patterns respect HTTP_ACCEPT_LANGUAGE + url = reverse("admin:login") + response = self.client.get(url, HTTP_ACCEPT_LANGUAGE="nl") + expected = '

Django-beheer

' + self.assertInHTML(expected, str(response.content)) + + def test_sitemap_xml(self): + # Non i18n_patterns respect HTTP_ACCEPT_LANGUAGE + url = "/sitemap.xml" + response = self.client.get(url, HTTP_ACCEPT_LANGUAGE="nl") + expected = ( + b'' + ) + self.assertIn(expected, response.content)