Merge branch 'beta' into beta-sec-autocomplete-perms

Mirrors the existing short-circuit already present in its sibling class,
ObjectOwnedPermissionsFilter, which this one lacked.

Benchmarked as a superuser against a 100k-document corpus: no measurable
difference. django-guardian's own get_objects_for_user() (called via
super().filter_queryset()) already returns the queryset unrestricted for
superusers before this change, so the guardian permission tables were
never actually queried for superusers either way. This change only skips
building a redundant 3-way OR of conditions that are all subsets of the
already-unrestricted queryset -- worth keeping for clarity/consistency
with the sibling filter, not as a performance claim.

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

src/documents/conditionals.py

@@ -1,9 +1,9 @@
 from datetime import UTC
 from datetime import datetime
-from typing import Any
 
 from django.conf import settings
 from django.core.cache import cache
+from rest_framework.request import Request
 
 from documents.caching import CACHE_5_MINUTES
 from documents.caching import CACHE_50_MINUTES
@@ -117,7 +117,7 @@ def preview_last_modified(request, pk: int) -> datetime | None:
     return doc.modified
 
 
-def thumbnail_etag(request: Any, pk: int) -> str | None:
+def thumbnail_etag(request: Request, pk: int) -> str | None:
     """
     Thumbnails are version-dependent, so use the effective document checksum as
     the ETag to invalidate cache when the latest version changes.
@@ -128,7 +128,7 @@ def thumbnail_etag(request: Any, pk: int) -> str | None:
     return doc.checksum
 
 
-def thumbnail_last_modified(request: Any, pk: int) -> datetime | None:
+def thumbnail_last_modified(request: Request, pk: int) -> datetime | None:
     """
     Returns the filesystem last modified either from cache or from filesystem.
     Cache should be (slightly?) faster than filesystem

src/documents/consumer.py

@@ -621,6 +621,11 @@ class ConsumerPlugin(
                             else:
                                 original_document.save()
 
+                            # Adding a version changes the effective document, so update root modified
+                            Document.objects.filter(pk=root_doc.pk).update(
+                                modified=timezone.now(),
+                            )
+
                             # Create a log entry for the version addition, if enabled
                             if settings.AUDIT_LOG_ENABLED:
                                 from auditlog.models import (  # type: ignore[import-untyped]

src/documents/filters.py

@@ -1024,6 +1024,8 @@ class ObjectOwnedOrGrantedPermissionsFilter(ObjectPermissionsFilter):
     """
 
     def filter_queryset(self, request, queryset, view):
+        if request.user.is_superuser:
+            return queryset
         objects_with_perms = super().filter_queryset(request, queryset, view)
         objects_owned = queryset.filter(owner=request.user)
         objects_unowned = queryset.filter(owner__isnull=True)

src/documents/migrations/0022_add_perf_indexes.py

@@ -0,0 +1,73 @@
+# Generated by Django 5.2.16 on 2026-07-19 20:33
+
+import django.core.validators
+from django.conf import settings
+from django.db import migrations
+from django.db import models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("documents", "0021_widen_workflow_integer_fields"),
+        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="document",
+            name="checksum",
+            field=models.CharField(
+                db_index=True,
+                editable=False,
+                help_text="The checksum of the original document.",
+                max_length=64,
+                verbose_name="checksum",
+            ),
+        ),
+        migrations.AlterField(
+            model_name="document",
+            name="page_count",
+            field=models.PositiveIntegerField(
+                db_index=True,
+                help_text="The number of pages of the document.",
+                null=True,
+                validators=[django.core.validators.MinValueValidator(1)],
+                verbose_name="page count",
+            ),
+        ),
+        migrations.AddIndex(
+            model_name="customfieldinstance",
+            index=models.Index(
+                fields=["field", "value_date"],
+                name="documents_c_field_i_30b51d_idx",
+            ),
+        ),
+        migrations.AddIndex(
+            model_name="customfieldinstance",
+            index=models.Index(
+                fields=["field", "value_int"],
+                name="documents_c_field_i_d25ab0_idx",
+            ),
+        ),
+        migrations.AddIndex(
+            model_name="customfieldinstance",
+            index=models.Index(
+                fields=["field", "value_float"],
+                name="documents_c_field_i_c35174_idx",
+            ),
+        ),
+        migrations.AddIndex(
+            model_name="customfieldinstance",
+            index=models.Index(
+                fields=["field", "value_monetary_amount"],
+                name="documents_c_field_i_53ce4e_idx",
+            ),
+        ),
+        migrations.AddIndex(
+            model_name="document",
+            index=models.Index(
+                fields=["owner", "created"],
+                name="documents_d_owner_i_ec6ab3_idx",
+            ),
+        ),
+    ]

src/documents/models.py

@@ -217,6 +217,7 @@ class Document(SoftDeleteModel, ModelWithOwner):  # type: ignore[django-manager-
         _("checksum"),
         max_length=64,
         editable=False,
+        db_index=True,
         help_text=_("The checksum of the original document."),
     )
 
@@ -234,7 +235,7 @@ class Document(SoftDeleteModel, ModelWithOwner):  # type: ignore[django-manager-
         blank=False,
         null=True,
         unique=False,
-        db_index=False,
+        db_index=True,
         validators=[MinValueValidator(1)],
         help_text=_(
             "The number of pages of the document.",
@@ -338,6 +339,9 @@ class Document(SoftDeleteModel, ModelWithOwner):  # type: ignore[django-manager-
         ordering = ("-created",)
         verbose_name = _("document")
         verbose_name_plural = _("documents")
+        indexes = [
+            models.Index(fields=["owner", "created"]),
+        ]
         constraints = [
             models.UniqueConstraint(
                 fields=["root_document", "version_index"],
@@ -1190,6 +1194,12 @@ class CustomFieldInstance(SoftDeleteModel):
         ordering = ("created",)
         verbose_name = _("custom field instance")
         verbose_name_plural = _("custom field instances")
+        indexes = [
+            models.Index(fields=["field", "value_date"]),
+            models.Index(fields=["field", "value_int"]),
+            models.Index(fields=["field", "value_float"]),
+            models.Index(fields=["field", "value_monetary_amount"]),
+        ]
         constraints = [
             models.UniqueConstraint(
                 fields=["document", "field"],

src/documents/search/_backend.py

@@ -11,6 +11,7 @@ from enum import StrEnum
 from itertools import islice
 from typing import TYPE_CHECKING
 from typing import Final
+from typing import NamedTuple
 from typing import Self
 from typing import TypedDict
 from typing import TypeVar
@@ -21,6 +22,7 @@ import tantivy
 from django.conf import settings
 from django.contrib.contenttypes.models import ContentType
 from django.utils.timezone import get_current_timezone
+from guardian.shortcuts import get_groups_with_perms
 from guardian.shortcuts import get_users_with_perms
 
 from documents.search._query import build_permission_filter
@@ -45,6 +47,8 @@ if TYPE_CHECKING:
     from pathlib import Path
 
     from django.contrib.auth.models import AbstractUser
+    from django.contrib.auth.models import Group
+    from django.contrib.auth.models import User
     from django.db.models import QuerySet
 
     from documents.models import Document
@@ -59,6 +63,18 @@ _LOCK_BACKOFF_CAP: Final[float] = 10.0  # seconds
 T = TypeVar("T")
 
 
+class ViewerGrant(NamedTuple):
+    """Direct user and group view grants for a single document.
+
+    Named fields (rather than a bare 2-tuple) so ``viewer_ids`` and
+    ``viewer_group_ids`` can't be silently transposed at a call site — both
+    are ``list[int]``, so a positional swap would type-check cleanly.
+    """
+
+    viewer_ids: list[int]
+    viewer_group_ids: list[int]
+
+
 class SearchMode(StrEnum):
     QUERY = "query"
     TEXT = "text"
@@ -367,7 +383,7 @@ class TantivyBackend:
     ) -> tantivy.Query:
         """Wrap a query with a permission filter if the user is not a superuser."""
         if user is not None:
-            permission_filter = build_permission_filter(self._schema, user)
+            permission_filter = self._build_permission_filter(user)
             return tantivy.Query.boolean_query(
                 [
                     (tantivy.Occur.Must, query),
@@ -376,11 +392,21 @@ class TantivyBackend:
             )
         return query
 
+    def _build_permission_filter(self, user: AbstractUser) -> tantivy.Query:
+        """Build a filter using the user's current group memberships."""
+        group_ids = user.groups.values_list("pk", flat=True)
+        return build_permission_filter(
+            self._schema,
+            user,
+            viewer_group_ids=group_ids,
+        )
+
     def _build_tantivy_doc(
         self,
         document: Document,
         effective_content: str | None = None,
         viewer_ids: list[int] | None = None,
+        viewer_group_ids: list[int] | None = None,
     ) -> tantivy.Document:
         """Build a tantivy Document from a Django Document instance.
 
@@ -497,10 +523,26 @@ class TantivyBackend:
             users_with_perms = get_users_with_perms(
                 document,
                 only_with_perms_in=["view_document"],
+                with_group_users=False,
+            )
+            viewer_ids = list(
+                cast("QuerySet[User]", users_with_perms).values_list("id", flat=True),
             )
-            viewer_ids = [int(u.id) for u in users_with_perms]
         for viewer_id in viewer_ids:
             doc.add_unsigned("viewer_id", viewer_id)
+        if viewer_group_ids is None:
+            groups_with_perms = get_groups_with_perms(
+                document,
+                only_with_perms_in=["view_document"],
+            )
+            viewer_group_ids = list(
+                cast("QuerySet[Group]", groups_with_perms).values_list(
+                    "id",
+                    flat=True,
+                ),
+            )
+        for viewer_group_id in viewer_group_ids:
+            doc.add_unsigned("viewer_group_id", viewer_group_id)
 
         # Autocomplete words
         text_sources = [document.title, content]
@@ -813,7 +855,7 @@ class TantivyBackend:
         # Intersect with permission filter so autocomplete words from
         # invisible documents don't leak to other users.
         if user is not None and not user.is_superuser:
-            permission_query = build_permission_filter(self._schema, user)
+            permission_query = self._build_permission_filter(user)
 
         matches = searcher.terms_with_prefix(
             "autocomplete_word",
@@ -915,7 +957,7 @@ class TantivyBackend:
     def rebuild(
         self,
         documents: QuerySet[Document],
-        iter_wrapper: IterWrapper[tuple[Document, list[int]]] = identity,
+        iter_wrapper: IterWrapper[tuple[Document, ViewerGrant]] = identity,
         writer_heap_bytes: int = 512_000_000,
     ) -> None:
         """
@@ -928,8 +970,8 @@ class TantivyBackend:
             documents: QuerySet of Document instances to index
             iter_wrapper: Optional wrapper function for progress tracking
                 (e.g., progress bar). Wraps an iterable of
-                ``(document, viewer_ids)`` pairs and should yield each pair
+                ``(document, (viewer_ids, viewer_group_ids))`` pairs and should yield
-                unchanged, advancing one step per document.
+                each unchanged, advancing one step per document.
             writer_heap_bytes: Tantivy writer memory budget (split across the
                 writer's threads). Larger values buffer more docs in RAM before
                 flushing a segment, deferring merge work; they do not avoid it.
@@ -953,11 +995,14 @@ class TantivyBackend:
         documents_stream = _DocumentViewerStream(documents, chunk_size=1000)
         try:
             writer = new_index.writer(heap_size=writer_heap_bytes)
-            for document, viewer_ids in iter_wrapper(documents_stream):
+            for document, (viewer_ids, viewer_group_ids) in iter_wrapper(
+                documents_stream,
+            ):
                 doc = self._build_tantivy_doc(
                     document,
                     document.get_effective_content(),
                     viewer_ids=viewer_ids,
+                    viewer_group_ids=viewer_group_ids,
                 )
                 writer.add_document(doc)
             writer.commit()
@@ -978,19 +1023,26 @@ def chunked(iterable, size):
         yield chunk
 
 
-class _DocumentViewerStream:
+_EMPTY_VIEWER_GRANT: Final[ViewerGrant] = ViewerGrant(
-    """Yield (document, viewer_ids) pairs while batch-loading viewer ids.
+    viewer_ids=[],
+    viewer_group_ids=[],
+)
 
-    Viewer permissions are fetched one SQL query per chunk (see
+
-    ``_bulk_get_viewer_ids``), but documents are yielded individually so a
+class _DocumentViewerStream:
+    """Yield document permission data while batch-loading grants.
+
+    Viewer permissions are fetched in batches (see
+    ``_bulk_get_viewer_permissions``), but documents are yielded individually so a
     progress bar wrapped around this stream advances per document rather than
     jumping a whole chunk at a time. ``__len__`` lets the progress helper still
     discover the total (it inspects ``QuerySet``/``Sized``).
 
-    The viewer ids travel with each document in the yielded pair rather than
+    The viewer and group ids travel with each document in the yielded pair
-    through a separate mutable attribute, so the pairing survives regardless
+    rather than through a separate mutable attribute, so the pairing survives
-    of how ``iter_wrapper`` consumes the stream (buffering, batching, etc.) —
+    regardless of how ``iter_wrapper`` consumes the stream (buffering,
-    there is no reliance on the caller advancing this generator in lock-step.
+    batching, etc.) — there is no reliance on the caller advancing this
+    generator in lock-step.
     """
 
     def __init__(self, documents: QuerySet[Document], *, chunk_size: int) -> None:
@@ -1000,25 +1052,25 @@ class _DocumentViewerStream:
     def __len__(self) -> int:
         return self._documents.count()
 
-    def __iter__(self) -> Iterator[tuple[Document, list[int]]]:
+    def __iter__(self) -> Iterator[tuple[Document, ViewerGrant]]:
         # iterator(chunk_size=…) streams from a server-side cursor instead of
         # materialising the whole queryset in memory; since Django 4.1 it still
         # honours prefetch_related, running the prefetches one batch at a time.
         documents = self._documents.iterator(chunk_size=self._chunk_size)
         for chunk in chunked(documents, self._chunk_size):
-            viewer_ids_by_pk = _bulk_get_viewer_ids([doc.pk for doc in chunk])
+            grants_by_pk = _bulk_get_viewer_permissions([doc.pk for doc in chunk])
             for doc in chunk:
-                yield doc, viewer_ids_by_pk.get(doc.pk, [])
+                yield doc, grants_by_pk.get(doc.pk, _EMPTY_VIEWER_GRANT)
 
 
-def _bulk_get_viewer_ids(doc_pks: Sequence[int]) -> dict[int, list[int]]:
+def _bulk_get_viewer_permissions(
-    """Fetch all view_document permissions for a batch of documents in one query.
+    doc_pks: Sequence[int],
+) -> dict[int, ViewerGrant]:
+    """Fetch direct user and group view grants for a batch of documents, keyed by pk.
 
-    Mirrors get_users_with_perms(doc, only_with_perms_in=["view_document"])
+    Group grants remain group IDs in the index so permission checks use the
-    (with_group_users defaults to True there): a user counts as a viewer if
+    requesting user's current memberships. Expanding groups to user IDs here
-    they hold the permission directly, OR via membership in a group that
+    would leave stale access behind after a user is removed from a group.
-    holds the permission. Missing the group case would silently drop search
-    access for group-only viewers.
     """
     from collections import defaultdict
 
@@ -1032,6 +1084,7 @@ def _bulk_get_viewer_ids(doc_pks: Sequence[int]) -> dict[int, list[int]]:
     str_pks = [str(pk) for pk in doc_pks]
 
     viewer_map: dict[int, set[int]] = defaultdict(set)
+    viewer_group_map: dict[int, set[int]] = defaultdict(set)
 
     # Fold the permission lookup into the query via a join on codename instead
     # of a separate Permission.objects.get(), which would otherwise run once per
@@ -1045,22 +1098,22 @@ def _bulk_get_viewer_ids(doc_pks: Sequence[int]) -> dict[int, list[int]]:
     for object_pk, user_id in user_qs:
         viewer_map[int(object_pk)].add(user_id)
 
-    # User.groups has related_query_name="user", so group__user__id joins
-    # through the group membership m2m to the member users' ids in the same
-    # single-query fashion as user_qs above (values_list compiles to one SQL
-    # JOIN; no per-row Python-side lookups follow, so select_related /
-    # prefetch_related do not apply here).
     group_qs = GroupObjectPermission.objects.filter(
         content_type=ct,
         permission__content_type=ct,
         permission__codename="view_document",
         object_pk__in=str_pks,
-    ).values_list("object_pk", "group__user__id")
+    ).values_list("object_pk", "group_id")
-    for object_pk, user_id in group_qs:
+    for object_pk, group_id in group_qs:
-        if user_id is not None:
+        viewer_group_map[int(object_pk)].add(group_id)
-            viewer_map[int(object_pk)].add(user_id)
 
-    return {object_pk: list(user_ids) for object_pk, user_ids in viewer_map.items()}
+    return {
+        object_pk: ViewerGrant(
+            viewer_ids=list(viewer_map.get(object_pk, ())),
+            viewer_group_ids=list(viewer_group_map.get(object_pk, ())),
+        )
+        for object_pk in viewer_map.keys() | viewer_group_map.keys()
+    }
 
 
 # Module-level singleton with proper thread safety

src/documents/search/_query.py

@@ -20,6 +20,7 @@ from documents.search._translate import SearchQueryError
 from documents.search._translate import translate_query
 
 if TYPE_CHECKING:
+    from collections.abc import Iterable
     from datetime import tzinfo
 
     from django.contrib.auth.base_user import AbstractBaseUser
@@ -114,6 +115,7 @@ def normalize_query(query: str) -> str:
 def build_permission_filter(
     schema: tantivy.Schema,
     user: AbstractBaseUser,
+    viewer_group_ids: Iterable[int] = (),
 ) -> tantivy.Query:
     """
     Build a query filter for user document permissions.
@@ -123,10 +125,12 @@ def build_permission_filter(
     - Public documents (no owner) are visible to all users
     - Private documents are visible to their owner
     - Documents explicitly shared with the user are visible
+    - Documents shared with one of the user's current groups are visible
 
     Args:
         schema: Tantivy schema for field validation
         user: User to check permissions for
+        viewer_group_ids: Current group memberships for the user
 
     Returns:
         Tantivy query that filters results to visible documents
@@ -140,7 +144,13 @@ def build_permission_filter(
     )
     owned = tantivy.Query.term_query(schema, "owner_id", user.pk)
     shared = tantivy.Query.term_query(schema, "viewer_id", user.pk)
-    return tantivy.Query.disjunction_max_query([no_owner, owned, shared])
+    group_shared = [
+        tantivy.Query.term_query(schema, "viewer_group_id", group_id)
+        for group_id in viewer_group_ids
+    ]
+    return tantivy.Query.disjunction_max_query(
+        [no_owner, owned, shared, *group_shared],
+    )
 
 
 DEFAULT_SEARCH_FIELDS = [

src/documents/search/_schema.py

@@ -102,6 +102,7 @@ def build_schema() -> tantivy.Schema:
         "tag_id",
         "owner_id",
         "viewer_id",
+        "viewer_group_id",
     ):
         sb.add_unsigned_field(field, stored=False, indexed=True, fast=True)
 

src/documents/tests/test_api_bulk_edit.py

@@ -1419,6 +1419,30 @@ class TestBulkEditAPI(DirectoriesMixin, APITestCase):
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         m.assert_called_once()
 
+    @mock.patch("documents.views.bulk_edit.merge")
+    def test_merge_and_delete_requires_change_permission(self, m) -> None:
+        self.setup_mock(m, "merge")
+        user = User.objects.create_user(username="no-change")
+        user.user_permissions.add(
+            Permission.objects.get(codename="add_document"),
+            Permission.objects.get(codename="delete_document"),
+        )
+        self.client.force_authenticate(user=user)
+
+        response = self.client.post(
+            "/api/documents/merge/",
+            json.dumps(
+                {
+                    "documents": [self.doc2.id, self.doc3.id],
+                    "delete_originals": True,
+                },
+            ),
+            content_type="application/json",
+        )
+
+        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+        m.assert_not_called()
+
     @mock.patch("documents.views.bulk_edit.merge")
     def test_merge_invalid_parameters(self, m) -> None:
         self.setup_mock(m, "merge")
@@ -1668,6 +1692,74 @@ class TestBulkEditAPI(DirectoriesMixin, APITestCase):
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         m.assert_called_once()
 
+    @mock.patch("documents.views.bulk_edit.edit_pdf")
+    def test_edit_pdf_update_requires_change_permission(self, m) -> None:
+        self.setup_mock(m, "edit_pdf")
+        user = User.objects.create_user(username="no-change")
+        self.client.force_authenticate(user=user)
+
+        response = self.client.post(
+            "/api/documents/edit_pdf/",
+            json.dumps(
+                {
+                    "documents": [self.doc2.id],
+                    "operations": [{"page": 1}],
+                    "update_document": True,
+                },
+            ),
+            content_type="application/json",
+        )
+
+        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+        m.assert_not_called()
+
+    @mock.patch("documents.views.bulk_edit.remove_password")
+    @mock.patch("documents.views.bulk_edit.edit_pdf")
+    def test_delete_original_requires_delete_permission(
+        self,
+        edit_pdf_mock,
+        remove_password_mock,
+    ) -> None:
+        self.setup_mock(edit_pdf_mock, "edit_pdf")
+        self.setup_mock(remove_password_mock, "remove_password")
+        user = User.objects.create_user(username="no-delete")
+        user.user_permissions.add(
+            Permission.objects.get(codename="add_document"),
+            Permission.objects.get(codename="change_document"),
+        )
+        self.client.force_authenticate(user=user)
+
+        cases = [
+            (
+                "/api/documents/edit_pdf/",
+                {
+                    "documents": [self.doc2.id],
+                    "operations": [{"page": 1}],
+                    "delete_original": True,
+                },
+                edit_pdf_mock,
+            ),
+            (
+                "/api/documents/remove_password/",
+                {
+                    "documents": [self.doc2.id],
+                    "password": "secret",
+                    "delete_original": True,
+                },
+                remove_password_mock,
+            ),
+        ]
+        for endpoint, payload, operation_mock in cases:
+            with self.subTest(endpoint=endpoint):
+                response = self.client.post(
+                    endpoint,
+                    json.dumps(payload),
+                    content_type="application/json",
+                )
+
+                self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+                operation_mock.assert_not_called()
+
     @mock.patch("documents.views.bulk_edit.remove_password")
     def test_remove_password(self, m) -> None:
         self.setup_mock(m, "remove_password")

src/documents/tests/test_api_document_versions.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import datetime
 from typing import TYPE_CHECKING
 from unittest import TestCase
 from unittest import mock
@@ -10,6 +11,7 @@ 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.utils import timezone
 from rest_framework import status
 from rest_framework.test import APITestCase
 
@@ -137,6 +139,8 @@ class TestDocumentVersioningApi(DirectoriesMixin, APITestCase):
             root_document=root,
             content="v2-content",
         )
+        original_modified = timezone.now() - datetime.timedelta(days=1)
+        Document.objects.filter(pk=root.pk).update(modified=original_modified)
 
         with mock.patch("documents.search.get_backend"):
             resp = self.client.delete(f"/api/documents/{root.id}/versions/{v2.id}/")
@@ -146,6 +150,7 @@ class TestDocumentVersioningApi(DirectoriesMixin, APITestCase):
         self.assertEqual(resp.data["current_version_id"], v1.id)
         root.refresh_from_db()
         self.assertEqual(root.content, "root-content")
+        self.assertGreater(root.modified, original_modified)
 
         with mock.patch("documents.search.get_backend"):
             resp = self.client.delete(f"/api/documents/{root.id}/versions/{v1.id}/")
@@ -326,6 +331,8 @@ class TestDocumentVersioningApi(DirectoriesMixin, APITestCase):
             root_document=root,
             version_label="old",
         )
+        original_modified = timezone.now() - datetime.timedelta(days=1)
+        Document.objects.filter(pk=root.pk).update(modified=original_modified)
 
         resp = self.client.patch(
             f"/api/documents/{root.id}/versions/{version.id}/",
@@ -339,6 +346,8 @@ class TestDocumentVersioningApi(DirectoriesMixin, APITestCase):
         self.assertEqual(resp.data["version_label"], "Label 1")
         self.assertEqual(resp.data["id"], version.id)
         self.assertFalse(resp.data["is_root"])
+        root.refresh_from_db()
+        self.assertGreater(root.modified, original_modified)
 
     def test_update_version_label_clears_on_blank(self) -> None:
         root = Document.objects.create(
@@ -660,6 +669,26 @@ class TestDocumentVersioningApi(DirectoriesMixin, APITestCase):
 
         self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)
 
+    def test_update_version_requires_global_change_permission(self) -> None:
+        user = User.objects.create_user(username="add-only")
+        user.user_permissions.add(Permission.objects.get(codename="add_document"))
+        root = Document.objects.create(
+            title="root",
+            checksum="root",
+            mime_type="application/pdf",
+        )
+        self.client.force_authenticate(user=user)
+
+        with mock.patch("documents.views.consume_file") as consume_mock:
+            resp = self.client.post(
+                f"/api/documents/{root.id}/update_version/",
+                {"document": self._make_pdf_upload()},
+                format="multipart",
+            )
+
+        self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)
+        consume_mock.apply_async.assert_not_called()
+
     def test_update_version_returns_404_for_missing_document(self) -> None:
         resp = self.client.post(
             "/api/documents/9999/update_version/",

src/documents/tests/test_api_permissions.py

@@ -131,6 +131,10 @@ class TestApiAuth(DirectoriesMixin, APITestCase):
             self.client.get("/api/saved_views/").status_code,
             status.HTTP_403_FORBIDDEN,
         )
+        self.assertEqual(
+            self.client.get("/api/search/autocomplete/?term=test").status_code,
+            status.HTTP_403_FORBIDDEN,
+        )
 
     def test_api_sufficient_permissions(self) -> None:
         user = User.objects.create_user(username="test")

src/documents/tests/test_api_search.py

@@ -832,6 +832,7 @@ class TestDocumentSearchApi(DirectoriesMixin, APITestCase):
         """
         u1 = User.objects.create_user("user1")
         u2 = User.objects.create_user("user2")
+        u1.user_permissions.add(Permission.objects.get(codename="view_document"))
 
         self.client.force_authenticate(user=u1)
 
@@ -878,6 +879,30 @@ class TestDocumentSearchApi(DirectoriesMixin, APITestCase):
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         self.assertEqual(response.data, ["applebaum", "apples", "appletini"])
 
+    def test_search_autocomplete_group_revocation_is_immediate(self) -> None:
+        user = User.objects.create_user("group-user")
+        owner = User.objects.create_user("document-owner")
+        group = Group.objects.create(name="temporary-viewers")
+        user.user_permissions.add(Permission.objects.get(codename="view_document"))
+        user.groups.add(group)
+
+        document = Document.objects.create(
+            title="private",
+            content="canarysecretautocomplete",
+            checksum="group-revocation",
+            owner=owner,
+        )
+        assign_perm("view_document", group, document)
+        get_backend().add_or_update(document)
+        self.client.force_authenticate(user=user)
+
+        response = self.client.get("/api/search/autocomplete/?term=canarysecret")
+        self.assertEqual(response.data, ["canarysecretautocomplete"])
+
+        user.groups.remove(group)
+        response = self.client.get("/api/search/autocomplete/?term=canarysecret")
+        self.assertEqual(response.data, [])
+
     def test_search_autocomplete_field_name_match(self) -> None:
         """
         GIVEN:

src/documents/tests/test_consumer.py

@@ -767,6 +767,8 @@ class TestConsumer(
         root_doc.archive_serial_number = 42
         root_doc.save()
 
+        original_modified = timezone.now() - datetime.timedelta(days=1)
+        Document.objects.filter(pk=root_doc.pk).update(modified=original_modified)
         actor = User.objects.create_user(
             username="actor",
             email="actor@example.com",
@@ -818,6 +820,8 @@ class TestConsumer(
         self.assertIsNone(version.archive_serial_number)
         self.assertEqual(version.original_filename, version_file.name)
         self.assertTrue(bool(version.content))
+        root_doc.refresh_from_db()
+        self.assertGreater(root_doc.modified, original_modified)
 
     @override_settings(AUDIT_LOG_ENABLED=True)
     @mock.patch("documents.consumer.load_classifier")

src/documents/tests/test_version_conditionals.py

@@ -1,8 +1,12 @@
 from datetime import timedelta
 from types import SimpleNamespace
+from typing import TYPE_CHECKING
+from typing import cast
 from unittest import mock
 
+from django.db import connection
 from django.test import TestCase
+from django.test.utils import CaptureQueriesContext
 from django.utils import timezone
 
 from documents.conditionals import metadata_etag
@@ -13,6 +17,9 @@ from documents.models import Document
 from documents.tests.utils import DirectoriesMixin
 from documents.versioning import resolve_effective_document_by_pk
 
+if TYPE_CHECKING:
+    from rest_framework.request import Request
+
 
 class TestConditionals(DirectoriesMixin, TestCase):
     def test_metadata_etag_uses_latest_version_for_root_request(self) -> None:
@@ -29,14 +36,18 @@ class TestConditionals(DirectoriesMixin, TestCase):
             mime_type="application/pdf",
             root_document=root,
         )
-        request = SimpleNamespace(query_params={})
+        request = cast("Request", SimpleNamespace(query_params={}))
 
         self.assertEqual(
             metadata_etag(request, root.id),
             f"{latest.checksum}:{latest.modified.isoformat()}",
         )
-        self.assertEqual(preview_etag(request, root.id), latest.archive_checksum)
+        # preview_etag/thumbnail_etag resolve the same (pk, request) and should
-        self.assertEqual(thumbnail_etag(request, root.id), latest.checksum)
+        # reuse metadata_etag's cached resolution instead of re-querying.
+        with CaptureQueriesContext(connection) as ctx:
+            self.assertEqual(preview_etag(request, root.id), latest.archive_checksum)
+            self.assertEqual(thumbnail_etag(request, root.id), latest.checksum)
+        self.assertEqual(len(ctx.captured_queries), 0)
 
     def test_metadata_etag_changes_when_document_modified_changes(self) -> None:
         doc = Document.objects.create(
@@ -44,15 +55,20 @@ class TestConditionals(DirectoriesMixin, TestCase):
             checksum="same-checksum",
             mime_type="application/pdf",
         )
-        request = SimpleNamespace(query_params={})
+        # Each call simulates a separate incoming HTTP request, so it gets its
-
+        # own request object -- the per-request resolution cache must not leak
-        original_etag = metadata_etag(request, doc.id)
+        # a stale result across genuinely different requests.
+        original_etag = metadata_etag(
+            cast("Request", SimpleNamespace(query_params={})),
+            doc.id,
+        )
         new_modified = timezone.now() + timedelta(seconds=5)
         Document.objects.filter(id=doc.id).update(modified=new_modified)
 
-        self.assertNotEqual(metadata_etag(request, doc.id), original_etag)
+        second_request = cast("Request", SimpleNamespace(query_params={}))
+        self.assertNotEqual(metadata_etag(second_request, doc.id), original_etag)
         self.assertEqual(
-            metadata_etag(request, doc.id),
+            metadata_etag(second_request, doc.id),
             f"{doc.checksum}:{new_modified.isoformat()}",
         )
 
@@ -76,9 +92,13 @@ class TestConditionals(DirectoriesMixin, TestCase):
             root_document=other_root,
         )
 
-        invalid_request = SimpleNamespace(query_params={"version": "not-a-number"})
+        invalid_request = cast(
-        unrelated_request = SimpleNamespace(
+            "Request",
-            query_params={"version": str(other_version.id)},
+            SimpleNamespace(query_params={"version": "not-a-number"}),
+        )
+        unrelated_request = cast(
+            "Request",
+            SimpleNamespace(query_params={"version": str(other_version.id)}),
         )
 
         self.assertIsNone(
@@ -105,7 +125,7 @@ class TestConditionals(DirectoriesMixin, TestCase):
         latest.thumbnail_path.parent.mkdir(parents=True, exist_ok=True)
         latest.thumbnail_path.write_bytes(b"thumb")
 
-        request = SimpleNamespace(query_params={})
+        request = cast("Request", SimpleNamespace(query_params={}))
         with mock.patch(
             "documents.conditionals.get_thumbnail_modified_key",
             return_value="thumb-modified-key",

src/documents/tests/test_workflows.py

@@ -2938,6 +2938,69 @@ class TestWorkflows(
         self.assertFalse(doc.tags.filter(pk=self.t1.pk).exists())
         self.assertTrue(doc.tags.filter(pk=self.t2.pk).exists())
 
+    def test_document_updated_workflow_assignment_storage_path_persists_with_tag_assignment(
+        self,
+    ) -> None:
+        """
+        GIVEN:
+            - A document updated workflow filtered on a tag
+            - One assignment action assigns a storage path, a second (later-ordered)
+              assignment action adds a tag
+        WHEN:
+            - The document is updated and the workflow is triggered
+        THEN:
+            - Both the tag and the storage path are persisted
+        """
+        trigger = WorkflowTrigger.objects.create(
+            type=WorkflowTrigger.WorkflowTriggerType.DOCUMENT_UPDATED,
+        )
+        trigger.filter_has_tags.add(self.t1)
+        assign_storage_path = WorkflowAction.objects.create(
+            type=WorkflowAction.WorkflowActionType.ASSIGNMENT,
+            assign_storage_path=self.sp,
+            order=0,
+        )
+        assign_tag = WorkflowAction.objects.create(
+            type=WorkflowAction.WorkflowActionType.ASSIGNMENT,
+            order=1,
+        )
+        assign_tag.assign_tags.add(self.t2)
+        assign_tag.save()
+
+        workflow = Workflow.objects.create(
+            name="Workflow assign storage path then tag",
+            order=0,
+        )
+        workflow.triggers.add(trigger)
+        workflow.actions.add(assign_storage_path, assign_tag)
+        workflow.save()
+
+        doc = Document.objects.create(
+            title="sample test",
+            mime_type="application/pdf",
+            checksum="assign-tag-and-storage-path",
+            original_filename="sample.pdf",
+        )
+        generated = generate_unique_filename(doc)
+        destination = (settings.ORIGINALS_DIR / generated).resolve()
+        create_source_path_directory(destination)
+        shutil.copy(self.SAMPLE_DIR / "simple.pdf", destination)
+        Document.objects.filter(pk=doc.pk).update(filename=generated.as_posix())
+        doc.refresh_from_db()
+        doc.tags.set([self.t1])
+
+        superuser = User.objects.create_superuser("superuser")
+        self.client.force_authenticate(user=superuser)
+        self.client.patch(
+            f"/api/documents/{doc.id}/",
+            {"title": "user update to trigger workflow"},
+            format="json",
+        )
+
+        doc.refresh_from_db()
+        self.assertEqual(doc.storage_path, self.sp)
+        self.assertTrue(doc.tags.filter(pk=self.t2.pk).exists())
+
     def test_removal_action_document_updated_removeall(self) -> None:
         """
         GIVEN:

src/documents/versioning.py

@@ -8,7 +8,7 @@ from typing import Any
 from documents.models import Document
 
 if TYPE_CHECKING:
-    from django.http import HttpRequest
+    from rest_framework.request import Request
 
 
 class VersionResolutionError(StrEnum):
@@ -26,7 +26,7 @@ def _document_manager(*, include_deleted: bool) -> Any:
     return Document.global_objects if include_deleted else Document.objects
 
 
-def get_request_version_param(request: HttpRequest) -> str | None:
+def get_request_version_param(request: Request) -> str | None:
     if hasattr(request, "query_params"):
         return request.query_params.get("version")
     return None
@@ -57,7 +57,7 @@ def get_latest_version_for_root(
 
 def resolve_requested_version_for_root(
     root_doc: Document,
-    request: Any,
+    request: Request,
     *,
     include_deleted: bool = False,
 ) -> VersionResolution:
@@ -86,7 +86,7 @@ def resolve_requested_version_for_root(
 
 def resolve_effective_document(
     request_doc: Document,
-    request: Any,
+    request: Request,
     *,
     include_deleted: bool = False,
 ) -> VersionResolution:
@@ -107,18 +107,41 @@ def resolve_effective_document(
     return VersionResolution(document=request_doc)
 
 
+_EFFECTIVE_DOCUMENT_CACHE_ATTR = "_effective_document_resolution_cache"
+
+
 def resolve_effective_document_by_pk(
     pk: int,
-    request: Any,
+    request: Request,
     *,
     include_deleted: bool = False,
 ) -> VersionResolution:
+    # Django's `condition()` decorator (used for ETag/Last-Modified) invokes the
+    # etag_func and last_modified_func separately, and the view itself may resolve
+    # again -- all against the same request. Cache per-request so a single thumb/
+    # metadata/preview request doesn't redo this resolution multiple times.
+    cache = getattr(request, _EFFECTIVE_DOCUMENT_CACHE_ATTR, None)
+    if cache is None:
+        cache = {}
+        setattr(request, _EFFECTIVE_DOCUMENT_CACHE_ATTR, cache)
+
+    key = (pk, include_deleted)
+    if key in cache:
+        return cache[key]
+
     manager = _document_manager(include_deleted=include_deleted)
     request_doc = manager.only("id", "root_document_id").filter(pk=pk).first()
     if request_doc is None:
-        return VersionResolution(document=None, error=VersionResolutionError.NOT_FOUND)
+        resolution = VersionResolution(
-    return resolve_effective_document(
+            document=None,
-        request_doc,
+            error=VersionResolutionError.NOT_FOUND,
-        request,
+        )
-        include_deleted=include_deleted,
+    else:
-    )
+        resolution = resolve_effective_document(
+            request_doc,
+            request,
+            include_deleted=include_deleted,
+        )
+
+    cache[key] = resolution
+    return resolution

src/documents/views.py

@@ -1040,12 +1040,23 @@ class DocumentViewSet(
             .order_by("-id")
             .values("content")[:1],
         )
+        # A correlated subquery avoids the LEFT JOIN + Count() this used to
+        # be, which forced a GROUP BY aggregate over every matching document
+        # before the query could even be sorted or limited.
+        note_count = Subquery(
+            Note.objects.filter(document=OuterRef("pk"))
+            .order_by()
+            .values("document")
+            .annotate(count=Count("pk"))
+            .values("count"),
+            output_field=IntegerField(),
+        )
         return (
             Document.objects.filter(root_document__isnull=True)
             .distinct()
             .order_by("-created", "-id")
             .annotate(effective_content=Coalesce(latest_version_content, F("content")))
-            .annotate(num_notes=Count("notes"))
+            .annotate(num_notes=Coalesce(note_count, 0))
             .select_related("correspondent", "storage_path", "document_type", "owner")
             .prefetch_related(
                 Prefetch(
@@ -1936,10 +1947,13 @@ class DocumentViewSet(
                 "root_document",
             ).get(pk=pk)
             root_doc = get_root_document(request_doc)
-            if request.user is not None and not has_perms_owner_aware(
+            if request.user is not None and (
-                request.user,
+                not request.user.has_perm("documents.change_document")
-                "change_document",
+                or not has_perms_owner_aware(
-                root_doc,
+                    request.user,
+                    "change_document",
+                    root_doc,
+                )
             ):
                 return HttpResponseForbidden("Insufficient permissions")
         except Document.DoesNotExist:
@@ -2057,6 +2071,8 @@ class DocumentViewSet(
         _backend.remove(version_doc.pk)
         version_doc_id = version_doc.id
         version_doc.delete()
+        root_doc.modified = timezone.now()
+        Document.objects.filter(pk=root_doc.pk).update(modified=root_doc.modified)
         _backend.add_or_update(root_doc)
         if settings.AUDIT_LOG_ENABLED:
             actor = (
@@ -2136,6 +2152,8 @@ class DocumentViewSet(
         old_label = version_doc.version_label
         version_doc.version_label = serializer.validated_data["version_label"]
         version_doc.save(update_fields=["version_label"])
+        root_doc.modified = timezone.now()
+        Document.objects.filter(pk=root_doc.pk).update(modified=root_doc.modified)
 
         if settings.AUDIT_LOG_ENABLED and old_label != version_doc.version_label:
             actor = (
@@ -2741,7 +2759,7 @@ class DocumentOperationPermissionMixin(PassUserMixin, DocumentSelectionMixin):
             )
             or (method == bulk_edit.edit_pdf and parameters.get("update_document"))
         ):
-            has_perms = user_is_owner_of_all_documents
+            has_perms = has_perms and user_is_owner_of_all_documents
 
         # check global add permissions for methods that create documents
         if (
@@ -2766,6 +2784,11 @@ class DocumentOperationPermissionMixin(PassUserMixin, DocumentSelectionMixin):
                     method in [bulk_edit.merge, bulk_edit.split]
                     and parameters.get("delete_originals")
                 )
+                or (
+                    method in [bulk_edit.edit_pdf, bulk_edit.remove_password]
+                    and parameters.get("delete_original")
+                    and not parameters.get("update_document")
+                )
             )
             and not user.has_perm("documents.delete_document")
         ):
@@ -3366,7 +3389,7 @@ class SelectionDataView(GenericAPIView[Any]):
     ),
 )
 class SearchAutoCompleteView(GenericAPIView[Any]):
-    permission_classes = (IsAuthenticated,)
+    permission_classes = (IsAuthenticated, ViewDocumentsPermissions)
 
     def get(self, request, format=None):
         user = self.request.user if hasattr(self.request, "user") else None

src/documents/workflows/mutations.py

@@ -24,7 +24,11 @@ def apply_assignment_to_document(
     action: WorkflowAction, annotated with 'has_assign_*' boolean fields
     """
     if action.has_assign_tags:
-        document.add_nested_tags(action.assign_tags.all())
+        # Apply to a freshly-fetched instance rather than the shared `document`.
+        # Document.tags.add() fires an m2m_changed signal that ultimately calls
+        # instance.refresh_from_db(), which would discard any other unsaved
+        # assignment fields (e.g. storage_path) already staged on `document`.
+        Document.objects.get(pk=document.pk).add_nested_tags(action.assign_tags.all())
 
     if action.assign_correspondent:
         document.correspondent = action.assign_correspondent