Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 262 tests #277

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 18 additions & 7 deletions apps/core/middleware.py
Original file line number Diff line number Diff line change
@@ -1,18 +1,29 @@
from django.http import Http404
from django.conf import settings
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.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)
Comment on lines +15 to +28
Copy link
Contributor

Choose a reason for hiding this comment

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

If you call the resolver before get_response you will always have None, so this code defaults to the else statement every single time. I ran the tests with a print statement in the else section and it printed every time.

I also tried to set my language to a language that had a matching homepage and it did not properly set my preferred language until I put the response = self.get_response(request) at the top.

I see that you removed the 404 response when a language does not exist, was there a reason for that?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure why the tests do not fail on the language preference, but at least it was not working in the browser for me

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that the tests that correctly pass when a user asks for "/" and is redirected to the preferred language (browser or cookie) because of LocaleMiddleware

Copy link
Contributor

Choose a reason for hiding this comment

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

So, I guess we need to take a step back and actually think about what the requirements are

return response
119 changes: 119 additions & 0 deletions apps/core/tests/test_middleware.py
Original file line number Diff line number Diff line change
@@ -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 = "<h1>Inloggen in Wagtail</h1>"
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 = '<h1 id="site-name"><a href="/django-admin/">Django-beheer</a></h1>'
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'<urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9" '
b'xmlns:xhtml="http://www.w3.org/1999/xhtml">'
)
self.assertIn(expected, response.content)