diff --git a/src/documents/permissions.py b/src/documents/permissions.py index 9d5c9eb68..d1cfa24aa 100644 --- a/src/documents/permissions.py +++ b/src/documents/permissions.py @@ -139,15 +139,25 @@ def get_document_count_filter_for_user(user): if getattr(user, "is_superuser", False): return Q(documents__deleted_at__isnull=True) return Q( - documents__deleted_at__isnull=True, - documents__id__in=get_objects_for_user_owner_aware( - user, - "documents.view_document", - Document, - ).values_list("id", flat=True), + documents__id__in=permitted_document_ids(user), ) +def permitted_document_ids(user): + """ + Return a Subquery of permitted, non-deleted document IDs for the user. + Used to avoid repeated joins to the Document table in count annotations. + """ + if user is None or not getattr(user, "is_authenticated", False): + return Document.objects.none().values_list("id") + qs = get_objects_for_user_owner_aware( + user, + "documents.view_document", + Document, + ).filter(deleted_at__isnull=True) + return qs.values_list("id") + + def get_objects_for_user_owner_aware( user, perms, diff --git a/src/documents/serialisers.py b/src/documents/serialisers.py index 4dc23d740..71daafce0 100644 --- a/src/documents/serialisers.py +++ b/src/documents/serialisers.py @@ -24,6 +24,7 @@ from django.core.validators import RegexValidator from django.core.validators import integer_validator from django.db.models import Count from django.db.models import Q +from django.db.models import Subquery from django.db.models.functions import Lower from django.utils.crypto import get_random_string from django.utils.dateparse import parse_datetime @@ -71,9 +72,9 @@ from documents.models import WorkflowActionEmail from documents.models import WorkflowActionWebhook from documents.models import WorkflowTrigger from documents.parsers import is_mime_type_supported -from documents.permissions import get_document_count_filter_for_user from documents.permissions import get_groups_with_only_permission from documents.permissions import get_objects_for_user_owner_aware +from documents.permissions import permitted_document_ids from documents.permissions import set_permissions_for_object from documents.regex import validate_regex_pattern from documents.templating.filepath import validate_filepath_template_and_render @@ -589,18 +590,41 @@ class TagSerializer(MatchingModelSerializer, OwnedObjectSerializer): if children_map is not None: children = children_map.get(obj.pk, []) else: - filter_q = self.context.get("document_count_filter") request = self.context.get("request") - if filter_q is None: - user = getattr(request, "user", None) if request else None - filter_q = get_document_count_filter_for_user(user) - self.context["document_count_filter"] = filter_q + user = getattr(request, "user", None) if request else None - children = ( - obj.get_children_queryset() - .select_related("owner") - .annotate(document_count=Count("documents", filter=filter_q)) - ) + filter_kind = self.context.get("document_count_filter") + if filter_kind is None: + filter_kind = ( + "superuser" + if user and getattr(user, "is_superuser", False) + else "restricted" + ) + self.context["document_count_filter"] = filter_kind + + queryset = obj.get_children_queryset().select_related("owner") + + if filter_kind == "superuser": + children = queryset.annotate( + document_count=Count( + "documents", + filter=Q(documents__deleted_at__isnull=True), + distinct=True, + ), + ) + else: + permitted_ids = Subquery(permitted_document_ids(user)) + counts = dict( + Tag.documents.through.objects.filter( + document_id__in=permitted_ids, + ) + .values("tag_id") + .annotate(c=Count("document_id")) + .values_list("tag_id", "c"), + ) + children = list(queryset) + for child in children: + child.document_count = counts.get(child.id, 0) view = self.context.get("view") ordering = ( @@ -609,7 +633,11 @@ class TagSerializer(MatchingModelSerializer, OwnedObjectSerializer): else None ) ordering = ordering or (Lower("name"),) - children = children.order_by(*ordering) + if hasattr(children, "order_by"): + children = children.order_by(*ordering) + else: + # children is a list (pre-fetched); apply basic ordering on name + children = sorted(children, key=lambda c: (c.name or "").lower()) serializer = TagSerializer( children, diff --git a/src/documents/views.py b/src/documents/views.py index 88c9c5cf7..9c9434276 100644 --- a/src/documents/views.py +++ b/src/documents/views.py @@ -33,6 +33,7 @@ from django.db.models import IntegerField from django.db.models import Max from django.db.models import Model from django.db.models import Q +from django.db.models import Subquery from django.db.models import Sum from django.db.models import When from django.db.models.functions import Lower @@ -153,6 +154,7 @@ from documents.permissions import ViewDocumentsPermissions from documents.permissions import get_document_count_filter_for_user from documents.permissions import get_objects_for_user_owner_aware from documents.permissions import has_perms_owner_aware +from documents.permissions import permitted_document_ids from documents.permissions import set_permissions_for_object from documents.schema import generate_object_with_permissions_schema from documents.serialisers import AcknowledgeTasksViewSerializer @@ -3007,27 +3009,32 @@ class CustomFieldViewSet(ModelViewSet): queryset = CustomField.objects.all().order_by("-created") def get_queryset(self): - filter = ( - Q(fields__document__deleted_at__isnull=True) - if self.request.user is None or self.request.user.is_superuser - else ( - Q( - fields__document__deleted_at__isnull=True, - fields__document__id__in=get_objects_for_user_owner_aware( - self.request.user, - "documents.view_document", - Document, - ).values_list("id", flat=True), + user = self.request.user + if user is None or user.is_superuser: + return ( + super() + .get_queryset() + .annotate( + document_count=Count( + "fields", + filter=Q(fields__document__deleted_at__isnull=True), + distinct=True, + ), ) ) - ) + + permitted_ids = Subquery(permitted_document_ids(user)) return ( super() .get_queryset() .annotate( document_count=Count( "fields", - filter=filter, + filter=Q( + fields__document__deleted_at__isnull=True, + fields__document_id__in=permitted_ids, + ), + distinct=True, ), ) )