Support cross-locale slug lookup in language switcher (#6634)

smithellis · web-flow · commit 41ac896f8deb · 2025-06-04T14:55:59.000-04:00
The language switcher previously failed when switching between locales
where document slugs differ. For example, when switching from German
"add-on-badges-abzeichen" back to English "add-on-badges", users would
see a 404 error.

This commit enhances the get_visible_document_or_404 function to:
- Find correct documents across locales with different slugs
- Look up documents by finding related translations in any locale
- Maintain backward compatibility with existing behavior

Added comprehensive tests to verify the new functionality works with:
- Direct locale/slug matches
- Cross-locale lookups (any locale to any locale)
- Document fallbacks when translations don't exist
diff --git a/kitsune/wiki/tests/test_utils.py b/kitsune/wiki/tests/test_utils.py
@@ -25,11 +25,13 @@
     generate_short_url,
     get_featured_articles,
     get_kb_visited,
+    get_visible_document_or_404,
     has_visited_kb,
     num_active_contributors,
     remove_expired_from_kb_visited,
     update_kb_visited,
 )
+from django.http import Http404
 
 
 class ActiveContributorsTestCase(TestCase):
@@ -700,3 +702,135 @@ def test_with_product_and_topic_2(self):
             },
         )
         self.assertFalse(self.session.modified)
+
+
+class GetVisibleDocumentTests(TestCase):
+    def setUp(self):
+        self.user = UserFactory()
+
+        self.en_doc = ApprovedRevisionFactory(
+            document__locale="en-US",
+            document__slug="test-document",
+            is_ready_for_localization=True,
+        ).document
+
+        self.de_rev = TranslatedRevisionFactory(
+            document__locale="de",
+            document__slug="test-document-de",
+            document__parent=self.en_doc,
+            based_on=self.en_doc.current_revision,
+        )
+        self.de_doc = self.de_rev.document
+
+        self.fr_rev = TranslatedRevisionFactory(
+            document__locale="fr",
+            document__slug="test-document-fr",
+            document__parent=self.en_doc,
+            based_on=self.en_doc.current_revision,
+        )
+        self.fr_doc = self.fr_rev.document
+
+    def test_get_document_by_locale_and_slug_direct_match(self):
+        """Test getting a document when there's a direct match for locale and slug."""
+        doc = get_visible_document_or_404(self.user, locale="en-US", slug="test-document")
+        self.assertEqual(doc, self.en_doc)
+
+        doc = get_visible_document_or_404(self.user, locale="de", slug="test-document-de")
+        self.assertEqual(doc, self.de_doc)
+
+        doc = get_visible_document_or_404(self.user, locale="fr", slug="test-document-fr")
+        self.assertEqual(doc, self.fr_doc)
+
+    def test_get_document_via_translation_for_nondefault_locale(self):
+        """Test getting a document via its parent when no direct match in the requested locale."""
+        with self.assertRaises(Http404):
+            # This should raise a 404 because we're explicitly
+            # setting look_for_translation_via_parent=False
+            get_visible_document_or_404(
+                self.user,
+                locale="en-US",
+                slug="test-document-de",
+                look_for_translation_via_parent=False,
+            )
+
+        # This should also raise a 404 because cross-locale lookups should only work when
+        # look_for_translation_via_parent is True
+        with self.assertRaises(Http404):
+            get_visible_document_or_404(self.user, locale="en-US", slug="test-document-de")
+
+    def test_cross_locale_lookup(self):
+        """Test the new cross-locale lookup functionality for language switcher."""
+        doc = get_visible_document_or_404(
+            self.user,
+            locale="en-US",
+            slug="test-document-de",
+            look_for_translation_via_parent=True,
+        )
+        self.assertEqual(doc, self.en_doc)
+
+        doc = get_visible_document_or_404(
+            self.user,
+            locale="en-US",
+            slug="test-document-fr",
+            look_for_translation_via_parent=True,
+        )
+        self.assertEqual(doc, self.en_doc)
+
+        doc = get_visible_document_or_404(
+            self.user, locale="de", slug="test-document", look_for_translation_via_parent=True
+        )
+        self.assertEqual(doc, self.de_doc)
+
+        doc = get_visible_document_or_404(
+            self.user, locale="de", slug="test-document-fr", look_for_translation_via_parent=True
+        )
+        self.assertEqual(doc, self.de_doc)
+
+    def test_fallback_to_parent(self):
+        """Test falling back to parent document when translation doesn't exist."""
+        no_es_doc = ApprovedRevisionFactory(
+            document__locale="en-US", document__slug="no-spanish"
+        ).document
+
+        with self.assertRaises(Http404):
+            get_visible_document_or_404(
+                self.user, locale="es", slug="no-spanish", look_for_translation_via_parent=True
+            )
+
+        doc = get_visible_document_or_404(
+            self.user,
+            locale="es",
+            slug="no-spanish",
+            look_for_translation_via_parent=True,
+            return_parent_if_no_translation=True,
+        )
+        self.assertEqual(doc, no_es_doc)
+
+    def test_nonexistent_document(self):
+        """Test that we get a 404 for documents that don't exist in any locale."""
+        with self.assertRaises(Http404):
+            get_visible_document_or_404(
+                self.user,
+                locale="en-US",
+                slug="doesnt-exist",
+                look_for_translation_via_parent=True,
+            )
+
+        with self.assertRaises(Http404):
+            get_visible_document_or_404(
+                self.user, locale="de", slug="doesnt-exist", look_for_translation_via_parent=True
+            )
+
+    def test_unapproved_revision(self):
+        """Test that users can't see documents without approved revisions."""
+        unapproved_doc = RevisionFactory(
+            document__locale="en-US", document__slug="unapproved", is_approved=False
+        ).document
+
+        random_user = UserFactory()
+        with self.assertRaises(Http404):
+            get_visible_document_or_404(random_user, locale="en-US", slug="unapproved")
+
+        creator = unapproved_doc.revisions.first().creator
+        doc = get_visible_document_or_404(creator, locale="en-US", slug="unapproved")
+        self.assertEqual(doc, unapproved_doc)
diff --git a/kitsune/wiki/utils.py b/kitsune/wiki/utils.py
@@ -177,34 +177,48 @@ def get_visible_document_or_404(
     """
     Get the document specified by the keyword arguments and visible to the given user, or 404.
     """
+
     try:
         return Document.objects.get_visible(user, **kwargs)
     except Document.DoesNotExist:
         pass
 
-    if (
-        not look_for_translation_via_parent
-        or not (locale := kwargs.get("locale"))
-        or (locale == settings.WIKI_DEFAULT_LANGUAGE)
-    ):
-        # We either don't want to try to find the translation via its parent, or it doesn't
-        # make sense, because we're not making a locale-specific request or the locale we're
-        # requesting is already the default locale.
+    if not (locale := kwargs.get("locale")):
         raise Http404
 
-    # We couldn't find a visible translation in the requested non-default locale, so let's
-    # see if we can find a visible translation via its parent.
-    kwargs.update(locale=settings.WIKI_DEFAULT_LANGUAGE)
-    parent = get_object_or_404(Document.objects.visible(user, **kwargs))
+    slug = kwargs.get("slug")
 
-    # If there's a visible translation of the parent for the requested locale, return it.
-    if translation := parent.translated_to(locale, visible_for_user=user):
-        return translation
+    if not (slug and look_for_translation_via_parent):
+        raise Http404
+
+    other_docs_qs = (
+        Document.objects.filter(slug=slug).exclude(locale=locale).select_related("parent")
+    )
 
-    # Otherwise, we're left with the parent.
+    for doc in other_docs_qs:
+        base_doc = doc.parent or doc
+        if base_doc.locale == settings.WIKI_DEFAULT_LANGUAGE:
+            if base_doc.locale == locale:
+                return base_doc
+            translation = base_doc.translated_to(locale, visible_for_user=user)
+            if translation:
+                return translation
+        else:
+            translation = base_doc.translated_to(locale, visible_for_user=user)
+            if translation:
+                return translation
+
+    # Don't try final fallback if not looking for translations or in default language
+    if not look_for_translation_via_parent or locale == settings.WIKI_DEFAULT_LANGUAGE:
+        raise Http404
+
+    kwargs["locale"] = settings.WIKI_DEFAULT_LANGUAGE
+    parent = get_object_or_404(Document.objects.visible(user, **kwargs))
+    translation = parent.translated_to(locale, visible_for_user=user)
+    if translation:
+        return translation
     if return_parent_if_no_translation:
         return parent
-
     raise Http404
 
 
