simplify and fix get_visible_document_or_404 (#6710)

escattone · web-flow · commit 91c529333d7c · 2025-06-06T15:29:28.000-07:00
diff --git a/kitsune/wiki/tests/test_utils.py b/kitsune/wiki/tests/test_utils.py
@@ -11,7 +11,7 @@
 from kitsune.dashboards.models import WikiDocumentVisits
 from kitsune.products.tests import ProductFactory, TopicFactory
 from kitsune.sumo.tests import TestCase
-from kitsune.users.tests import UserFactory
+from kitsune.users.tests import GroupFactory, UserFactory
 from kitsune.wiki.tests import (
     ApprovedRevisionFactory,
     DocumentFactory,
@@ -852,3 +852,132 @@ def test_unapproved_revision(self):
         creator = unapproved_doc.revisions.first().creator
         doc = get_visible_document_or_404(creator, locale="en-US", slug="unapproved")
         self.assertEqual(doc, unapproved_doc)
+
+    def test_cross_locale_lookup_with_restricted_visibility(self):
+        g1 = GroupFactory(name="group1")
+
+        g1_user = UserFactory(groups=[g1])
+
+        en_doc = ApprovedRevisionFactory(
+            document__locale="en-US",
+            document__slug="restricted-test-document",
+            document__restrict_to_groups=[g1],
+            document__allow_discussion=False,
+            is_ready_for_localization=True,
+        ).document
+
+        de_doc = TranslatedRevisionFactory(
+            document__locale="de",
+            document__slug="restricted-test-document-de",
+            document__allow_discussion=False,
+            document__parent=en_doc,
+            based_on=en_doc.current_revision,
+        ).document
+
+        fr_doc = TranslatedRevisionFactory(
+            document__locale="fr",
+            document__slug="restricted-test-document-fr",
+            document__allow_discussion=False,
+            document__parent=en_doc,
+            based_on=en_doc.current_revision,
+        ).document
+
+        with self.assertRaises(Http404):
+            get_visible_document_or_404(
+                self.user,
+                locale="en-US",
+                slug="restricted-test-document-de",
+                look_for_translation_via_parent=True,
+            )
+
+        with self.assertRaises(Http404):
+            get_visible_document_or_404(
+                self.user,
+                locale="en-US",
+                slug="restricted-test-document-fr",
+                look_for_translation_via_parent=True,
+            )
+
+        with self.assertRaises(Http404):
+            get_visible_document_or_404(
+                self.user,
+                locale="de",
+                slug="restricted-test-document",
+                look_for_translation_via_parent=True,
+            )
+
+        with self.assertRaises(Http404):
+            get_visible_document_or_404(
+                self.user,
+                locale="de",
+                slug="restricted-test-document-fr",
+                look_for_translation_via_parent=True,
+            )
+
+        doc = get_visible_document_or_404(
+            g1_user,
+            locale="en-US",
+            slug="restricted-test-document-de",
+            look_for_translation_via_parent=True,
+        )
+        self.assertEqual(doc, en_doc)
+
+        doc = get_visible_document_or_404(
+            g1_user,
+            locale="en-US",
+            slug="restricted-test-document-fr",
+            look_for_translation_via_parent=True,
+        )
+        self.assertEqual(doc, en_doc)
+
+        doc = get_visible_document_or_404(
+            g1_user,
+            locale="de",
+            slug="restricted-test-document",
+            look_for_translation_via_parent=True,
+        )
+        self.assertEqual(doc, de_doc)
+
+        doc = get_visible_document_or_404(
+            g1_user,
+            locale="de",
+            slug="restricted-test-document-fr",
+            look_for_translation_via_parent=True,
+        )
+        self.assertEqual(doc, de_doc)
+
+        doc = get_visible_document_or_404(
+            g1_user,
+            locale="fr",
+            slug="restricted-test-document-de",
+            look_for_translation_via_parent=True,
+        )
+        self.assertEqual(doc, fr_doc)
+
+        # All of the kwargs should be respected.
+        with self.assertRaises(Http404):
+            get_visible_document_or_404(
+                g1_user,
+                locale="en-US",
+                slug="restricted-test-document-de",
+                allow_discussion=True,
+                look_for_translation_via_parent=True,
+            )
+
+        with self.assertRaises(Http404):
+            get_visible_document_or_404(
+                g1_user,
+                locale="de",
+                slug="restricted-test-document",
+                allow_discussion=True,
+                look_for_translation_via_parent=True,
+            )
+
+        with self.assertRaises(Http404):
+            get_visible_document_or_404(
+                g1_user,
+                locale="de",
+                slug="restricted-test-document-fr",
+                allow_discussion=True,
+                look_for_translation_via_parent=True,
+            )
diff --git a/kitsune/wiki/utils.py b/kitsune/wiki/utils.py
@@ -183,44 +183,32 @@ def get_visible_document_or_404(
     except Document.DoesNotExist:
         pass
 
-    if not (locale := kwargs.get("locale")):
+    if not (look_for_translation_via_parent and (locale := kwargs.get("locale"))):
         raise Http404
 
-    slug = kwargs.get("slug")
-
-    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")
-    )
+    # Make the same query without the locale.
+    other_docs_qs = Document.objects.visible(
+        user, **{k: v for k, v in kwargs.items() if k != "locale"}
+    ).select_related("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
-
-    # 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:
+        if base_doc.locale == locale:
+            return base_doc
+        elif (base_doc.locale == settings.WIKI_DEFAULT_LANGUAGE) and (
+            translation := base_doc.translated_to(locale, visible_for_user=user)
+        ):
+            return translation
+
+    if not (return_parent_if_no_translation and (locale != settings.WIKI_DEFAULT_LANGUAGE)):
         raise Http404
 
     kwargs["locale"] = settings.WIKI_DEFAULT_LANGUAGE
     try:
-        parent = Document.objects.get_visible(user, **kwargs)
+        return Document.objects.get_visible(user, **kwargs)
     except Document.DoesNotExist:
         raise Http404
 
-    translation = parent.translated_to(locale, visible_for_user=user)
-    if translation:
-        return translation
-    if return_parent_if_no_translation:
-        return parent
-    raise Http404
-
 
 def get_visible_revision_or_404(user, **kwargs):
     return get_object_or_404(Revision.objects.visible(user, **kwargs))
