Fix: annotate effective content in the filters rather than relying on callers

src/documents/filters.py

@@ -12,7 +12,6 @@ from typing import TYPE_CHECKING
 from typing import Any
 
 from django.contrib.contenttypes.models import ContentType
-from django.core.exceptions import FieldError
 from django.db.models import Case
 from django.db.models import CharField
 from django.db.models import Count
@@ -53,6 +52,7 @@ from documents.models import StoragePath
 from documents.models import Tag
 from documents.permissions import permitted_document_ids
 from documents.permissions import permitted_object_ids
+from documents.versioning import ensure_effective_content
 
 if TYPE_CHECKING:
     from collections.abc import Callable
@@ -182,14 +182,9 @@ class TitleContentFilter(Filter):
             logger.warning(
                 "Deprecated document filter parameter 'title_content' used; use `text` instead.",
             )
-            try:
+            return ensure_effective_content(qs).filter(
-                return qs.filter(
+                Q(title__icontains=value) | Q(effective_content__icontains=value),
-                    Q(title__icontains=value) | Q(effective_content__icontains=value),
+            )
-                )
-            except FieldError:
-                return qs.filter(
-                    Q(title__icontains=value) | Q(content__icontains=value),
-                )
         else:
             return qs
 
@@ -200,14 +195,9 @@ class EffectiveContentFilter(Filter):
         value = value.strip() if isinstance(value, str) else value
         if not value:
             return qs
-        try:
+        return ensure_effective_content(qs).filter(
-            return qs.filter(
+            **{f"effective_content__{self.lookup_expr}": value},
-                **{f"effective_content__{self.lookup_expr}": value},
+        )
-            )
-        except FieldError:
-            return qs.filter(
-                **{f"content__{self.lookup_expr}": value},
-            )
 
 
 @extend_schema_field(serializers.BooleanField)

src/documents/tests/test_api_document_versions.py

@@ -2,14 +2,12 @@ from __future__ import annotations
 
 import datetime
 from typing import TYPE_CHECKING
-from unittest import TestCase
 from unittest import mock
 
 from auditlog.models import LogEntry  # type: ignore[import-untyped]
 from django.contrib.auth.models import Permission
 from django.contrib.auth.models import User
 from django.contrib.contenttypes.models import ContentType
-from django.core.exceptions import FieldError
 from django.core.files.uploadedfile import SimpleUploadedFile
 from django.test import TestCase as DjangoTestCase
 from django.utils import timezone
@@ -22,6 +20,7 @@ from documents.filters import TitleContentFilter
 from documents.models import Document
 from documents.tests.utils import DirectoriesMixin
 from documents.tests.utils import read_streaming_response
+from documents.versioning import annotate_effective_content
 from documents.views import DocumentSelectionMixin
 
 if TYPE_CHECKING:
@@ -892,32 +891,103 @@ class TestDocumentVersioningApi(DirectoriesMixin, APITestCase):
         )
 
 
-class TestVersionAwareFilters(TestCase):
+class TestVersionAwareFilters(DjangoTestCase):
-    def test_title_content_filter_falls_back_to_content(self) -> None:
+    """
-        queryset = mock.Mock()
+    The filters annotate effective_content themselves rather than relying on
-        fallback_queryset = mock.Mock()
+    the caller's queryset carrying it, so they stay version-aware on a plain
-        queryset.filter.side_effect = [FieldError("missing field"), fallback_queryset]
+    Document queryset (e.g. the bulk-edit "select all matching" path).
+    """
 
-        result = TitleContentFilter().filter(queryset, " latest ")
+    def setUp(self) -> None:
+        super().setUp()
+        self.root = Document.objects.create(
+            title="root",
+            checksum="root",
+            mime_type="application/pdf",
+            content="superseded-content",
+        )
+        Document.objects.create(
+            title="version",
+            checksum="version",
+            mime_type="application/pdf",
+            root_document=self.root,
+            version_index=1,
+            content="latest-content",
+        )
+        self.unversioned = Document.objects.create(
+            title="unversioned",
+            checksum="unversioned",
+            mime_type="application/pdf",
+            content="latest-content",
+        )
 
-        self.assertIs(result, fallback_queryset)
+    def test_title_content_filter_matches_latest_version_content(self) -> None:
-        self.assertEqual(queryset.filter.call_count, 2)
+        result = TitleContentFilter().filter(
-
+            Document.objects.filter(root_document__isnull=True),
-    def test_effective_content_filter_falls_back_to_content_lookup(self) -> None:
-        queryset = mock.Mock()
-        fallback_queryset = mock.Mock()
-        queryset.filter.side_effect = [FieldError("missing field"), fallback_queryset]
-
-        result = EffectiveContentFilter(lookup_expr="icontains").filter(
-            queryset,
             " latest ",
         )
 
-        self.assertIs(result, fallback_queryset)
+        self.assertCountEqual(
-        first_kwargs = queryset.filter.call_args_list[0].kwargs
+            [doc.id for doc in result],
-        second_kwargs = queryset.filter.call_args_list[1].kwargs
+            [self.root.id, self.unversioned.id],
-        self.assertEqual(first_kwargs, {"effective_content__icontains": "latest"})
+        )
-        self.assertEqual(second_kwargs, {"content__icontains": "latest"})
+
+    def test_effective_content_filter_matches_latest_version_content(self) -> None:
+        result = EffectiveContentFilter(lookup_expr="icontains").filter(
+            Document.objects.filter(root_document__isnull=True),
+            " latest ",
+        )
+
+        self.assertCountEqual(
+            [doc.id for doc in result],
+            [self.root.id, self.unversioned.id],
+        )
+
+    def test_effective_content_filter_ignores_superseded_content(self) -> None:
+        result = EffectiveContentFilter(lookup_expr="icontains").filter(
+            Document.objects.filter(root_document__isnull=True),
+            "superseded",
+        )
+
+        self.assertEqual(list(result), [])
+
+    def test_filters_reuse_an_existing_annotation(self) -> None:
+        """
+        Annotating twice under the same alias is an error, so an already
+        annotated queryset (the search path) has to be left alone.
+        """
+        annotated = annotate_effective_content(
+            Document.objects.filter(root_document__isnull=True),
+        )
+
+        result = EffectiveContentFilter(lookup_expr="icontains").filter(
+            annotated,
+            "latest",
+        )
+
+        self.assertCountEqual(
+            [doc.id for doc in result],
+            [self.root.id, self.unversioned.id],
+        )
+
+    def test_bulk_selection_does_not_match_superseded_content(self) -> None:
+        """
+        Bulk edit's "select all matching" builds its own queryset, so before
+        the filters annotated for themselves it matched the root document's
+        superseded content -- selecting documents the list view, filtered by
+        the same term, does not show.
+        """
+        user = User.objects.create_superuser(username="bulk_selection")
+
+        selected = DocumentSelectionMixin()._resolve_document_ids(
+            user=user,
+            validated_data={
+                "all": True,
+                "filters": {"content__icontains": "superseded"},
+            },
+        )
+
+        self.assertEqual(selected, [])
 
     def test_effective_content_filter_returns_input_for_empty_values(self) -> None:
         queryset = mock.Mock()

src/documents/tests/test_api_search.py

@@ -1947,6 +1947,29 @@ class TestDocumentSearchApi(DirectoriesMixin, APITestCase):
         self.assertEqual(len(response.data["documents"]), 1)
         self.assertEqual(response.data["documents"][0]["id"], title_match.id)
 
+    def test_global_search_returns_latest_version_content(self) -> None:
+        root = Document.objects.create(
+            title="bank statement",
+            content="superseded content",
+            checksum="GSV1",
+            pk=23,
+        )
+        Document.objects.create(
+            title="bank statement v2",
+            content="latest content",
+            checksum="GSV2",
+            pk=24,
+            root_document=root,
+            version_index=1,
+        )
+
+        self.client.force_authenticate(self.user)
+
+        response = self.client.get("/api/search/?query=bank&db_only=true")
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        returned = {doc["id"]: doc["content"] for doc in response.data["documents"]}
+        self.assertEqual(returned.get(root.id), "latest content")
+
     def test_global_search_filters_owned_mail_objects(self) -> None:
         user1 = User.objects.create_user("mail-search-user")
         user2 = User.objects.create_user("other-mail-search-user")

src/documents/versioning.py

@@ -43,6 +43,21 @@ def annotate_effective_content(documents: QuerySet[Document]) -> QuerySet[Docume
     )
 
 
+def ensure_effective_content(documents: QuerySet[Document]) -> QuerySet[Document]:
+    """
+    Annotates effective_content unless the queryset already carries it.
+
+    Lets a filter depend on effective_content without having to assume its
+    caller annotated one -- annotating twice under the same alias is an error,
+    and silently matching on the root document's own content instead is worse,
+    because the same filter then selects different documents depending on which
+    queryset it was handed.
+    """
+    if "effective_content" in documents.query.annotations:
+        return documents
+    return annotate_effective_content(documents)
+
+
 def sort_versions_newest_first(documents: list[Document]) -> list[Document]:
     """
     Same sorting as versions_newest_first()

src/documents/views.py

@@ -232,6 +232,7 @@ from documents.tasks import train_classifier
 from documents.tasks import update_document_parent_tags
 from documents.utils import get_boolean
 from documents.versioning import VersionResolutionError
+from documents.versioning import annotate_effective_content
 from documents.versioning import get_latest_version_for_root
 from documents.versioning import get_request_version_param
 from documents.versioning import get_root_document
@@ -3632,8 +3633,13 @@ class GlobalSearchView(PassUserMixin):
         OBJECT_LIMIT = 3
         docs = []
         if request.user.has_perm("documents.view_document"):
-            all_docs = Document.objects.filter(
+            # Never more than OBJECT_LIMIT rows come back here, so annotating
-                id__in=permitted_document_ids(request.user),
+            # is cheap -- and without it these results show the root
+            # document's superseded content.
+            all_docs = annotate_effective_content(
+                Document.objects.filter(
+                    id__in=permitted_document_ids(request.user),
+                ),
             )
             if db_only:
                 docs = all_docs.filter(title__icontains=query)[:OBJECT_LIMIT]