fix: Prefer titles matching request language (#7144)

* prefer titles matching request language
* add comments on use of annotate
* fix wayward imports
* Add changelog entry

Co-authored-by: Vinit Kumar <mail@vinitkumar.me>
Co-authored-by: Mark Walker <theshow@gmail.com>

CHANGELOG.rst

@@ -15,6 +15,7 @@ unreleased
 * Unlocalize page and node ids when rendering the page tree in the admin (#7175)
 * Fixed permission denied error after page create (#6866)
 * Improved performance when using CMS_RAW_ID_USERS=True on a Postgres DB with many users
+* Fix #3548 by reflecting the request language when resolving identical page slugs 
 * Deprecate usage of legacy SEND_BROKEN_LINK_EMAILS setting (removed since Django 1.8)
 * Add deprecation warning to ``cms.utils.i18n.get_current_language()`` (#6720)
 

cms/test_utils/testcases.py

@@ -15,9 +15,9 @@
 from django.test import testcases
 from django.test.client import RequestFactory
 from django.urls import reverse
+from django.utils import translation
 from django.utils.http import urlencode
 from django.utils.timezone import now
-from django.utils.translation import activate
 
 from cms.api import create_page
 from cms.constants import PUBLISHER_STATE_DEFAULT, PUBLISHER_STATE_DIRTY, PUBLISHER_STATE_PENDING
@@ -101,7 +101,7 @@ class BaseCMSTestCase:
     def _fixture_setup(self):
         super()._fixture_setup()
         self.create_fixtures()
-        activate("en")
+        translation.activate("en")
 
     def create_fixtures(self):
         pass
@@ -398,10 +398,7 @@ def get_request(self, path=None, language=None, post_data=None, enforce_csrf_che
             path = self.get_pages_root()
 
         if not language:
-            if settings.USE_I18N:
-                language = settings.LANGUAGES[0][0]
-            else:
-                language = settings.LANGUAGE_CODE
+            language = translation.get_language()
 
         if post_data:
             request = factory.post(path, post_data)

cms/tests/test_page.py

@@ -3,6 +3,7 @@
 import os.path
 from unittest import skipIf
 
+import mock
 from django.conf import settings
 from django.contrib.auth import get_user_model
 from django.contrib.sites.models import Site
@@ -11,7 +12,7 @@
 from django.http import HttpResponse, HttpResponseNotFound
 from django.urls import reverse
 from django.utils.timezone import now as tz_now
-from django.utils.translation import override as force_language
+from django.utils.translation import activate, override as force_language
 
 from cms import constants
 from cms.api import add_plugin, create_page, create_title, publish_page
@@ -678,6 +679,35 @@ def test_get_page_without_final_slash(self):
         self.assertIsNotNone(found_page)
         self.assertFalse(found_page.publisher_is_draft)
 
+    def test_language_homograph(self):
+        # a page about boots you can wear
+        wearable_boot = create_page("boot", "nav_playground.html", "en", slug="boot",
+                           published=True)
+        create_title('de', 'stiefel', wearable_boot, slug='stiefel')
+        wearable_boot.publish('de')
+        # a page about boats that float on water
+        floating_boat = create_page("boat", "nav_playground.html", "en", slug="boat",
+                           published=True)
+        create_title('de', 'boot', floating_boat, slug='boot')
+        floating_boat.publish('de')
+
+        activate('en')
+        request = self.get_request('/en/boot/')
+        page = get_page_from_request(request)
+        self.assertEqual(page.pk, wearable_boot.publisher_public_id)
+
+        activate('de')
+        request = self.get_request('/de/boot/', language='de')
+        page = get_page_from_request(request)
+        self.assertEqual(page.pk, floating_boat.publisher_public_id)
+
+    def test_no_request_language_code(self):
+        request = self.get_request('/any-page/')
+        del request.LANGUAGE_CODE
+        with mock.patch('cms.utils.page.get_page_from_path') as mock_page_from_path:
+            get_page_from_request(request)
+        self.assertIsNone(mock_page_from_path.call_args.kwargs['language_code'])
+
     def test_get_page_from_request_without_cache_when_has_use_path_argument(self):
         request = self.get_request('/test')
         request._current_page_cache = True

cms/utils/page.py

@@ -1,6 +1,7 @@
 import re
 
-from django.db.models import Q
+from django.db.models import ExpressionWrapper, Q
+from django.db.models.fields import BooleanField
 from django.urls import reverse
 from django.utils import timezone
 from django.utils.encoding import force_str
@@ -75,10 +76,15 @@ def get_page_queryset(site, draft=True, published=False):
     return Page.objects.public().on_site(site)
 
 
-def get_page_from_path(site, path, preview=False, draft=False):
+def get_page_from_path(site, path, preview=False, draft=False, language_code=None):
     """
     Resolves a url path to a single page object.
     Returns None if page does not exist
+
+    Preferres page results based on language code in this order:
+    - Matches sublanguage (e.g. de-at)
+    - Matches language (e.g. de)
+    - All other pages matching the path
     """
     from cms.models import Title
 
@@ -91,6 +97,22 @@ def get_page_from_path(site, path, preview=False, draft=False):
         titles = titles.filter(publisher_is_draft=False)
     else:
         titles = titles.filter(published=True, publisher_is_draft=False)
+    if language_code is not None:
+        # since a language code is provided, prefer pages that match that language
+        sublanguage = language_code
+        language = sublanguage.split("-", maxsplit=1)[0]
+        # use `annotate()` to add two new fields to our queryset in order to prefer the request language code
+        # `matches_sublanguage` will be true when the full code (both language and sublanguage) match
+        # `matches_language` will be true for all pages matching the top-level language code
+        # https://docs.djangoproject.com/en/stable/ref/models/querysets/#django.db.models.query.QuerySet.annotate
+        titles = titles.annotate(
+            matches_sublanguage=ExpressionWrapper(
+                Q(language=sublanguage), output_field=BooleanField()
+            ),
+            matches_language=ExpressionWrapper(
+                Q(language=language), output_field=BooleanField()
+            ),
+        ).order_by("-matches_sublanguage", "-matches_language")
     titles = titles.filter(path=(path or ''))
 
     for title in titles.iterator():
@@ -162,10 +184,15 @@ def get_page_from_request(request, use_path=None, clean_path=None):
             path = path[:-1]
 
     site = get_current_site()
-    page = get_page_from_path(site, path, preview, draft)
+    request_language_code = getattr(request, "LANGUAGE_CODE", None)
+    page = get_page_from_path(
+        site, path, preview, draft, language_code=request_language_code
+    )
 
     if draft and page and not user_can_view_page_draft(request.user, page):
-        page = get_page_from_path(site, path, preview, draft=False)
+        page = get_page_from_path(
+            site, path, preview, draft=False, language_code=request_language_code
+        )
 
     # For public pages, check if any parent is hidden due to published dates
     # In this case the selected page is not reachable