♻️(backend) refactor list view to allow filtering other views

the "filter_queryset" method is called in the middle of the
"get_object" method. We use the "get_object" in actions like
"children", "tree", etc. which start by calling "get_object"
but return lists of documents.

We would like to apply filters to these views but the it didn't
work because the "get_object" method was also impacted by the
filters...

In a future PR, we should take control of the "get_object" method
and decouple all this. We need a quick solution to allow releasing
the hierchical documents feature in the frontend.
This commit is contained in:
Samuel Paccoud - DINUM
2025-02-17 10:19:06 +01:00
committed by Manuel Raynaud
parent 0aabf26694
commit 56aa69f56a
7 changed files with 109 additions and 62 deletions

View File

@@ -11,6 +11,8 @@ and this project adheres to
## Fixed ## Fixed
- 🐛(frontend) remove scroll listener table content #688 - 🐛(frontend) remove scroll listener table content #688
- 🔒️(back) restrict access to favorite_list endpoint #690 - 🔒️(back) restrict access to favorite_list endpoint #690
- 🐛(backend) refactor to fix filtering on children
and descendants views #695
## [2.4.0] - 2025-03-06 ## [2.4.0] - 2025-03-06

View File

@@ -12,15 +12,26 @@ class DocumentFilter(django_filters.FilterSet):
Custom filter for filtering documents. Custom filter for filtering documents.
""" """
title = django_filters.CharFilter(
field_name="title", lookup_expr="icontains", label=_("Title")
)
class Meta:
model = models.Document
fields = ["title"]
class ListDocumentFilter(DocumentFilter):
"""
Custom filter for filtering documents.
"""
is_creator_me = django_filters.BooleanFilter( is_creator_me = django_filters.BooleanFilter(
method="filter_is_creator_me", label=_("Creator is me") method="filter_is_creator_me", label=_("Creator is me")
) )
is_favorite = django_filters.BooleanFilter( is_favorite = django_filters.BooleanFilter(
method="filter_is_favorite", label=_("Favorite") method="filter_is_favorite", label=_("Favorite")
) )
title = django_filters.CharFilter(
field_name="title", lookup_expr="icontains", label=_("Title")
)
class Meta: class Meta:
model = models.Document model = models.Document

View File

@@ -177,10 +177,7 @@ class ListDocumentSerializer(serializers.ModelSerializer):
request = self.context.get("request") request = self.context.get("request")
if request: if request:
paths_links_mapping = self.context.get("paths_links_mapping", None) return document.get_abilities(request.user)
return document.get_abilities(
request.user, paths_links_mapping=paths_links_mapping
)
return {} return {}

View File

@@ -31,7 +31,7 @@ from core.services.ai_services import AIService
from core.services.collaboration_services import CollaborationService from core.services.collaboration_services import CollaborationService
from . import permissions, serializers, utils from . import permissions, serializers, utils
from .filters import DocumentFilter from .filters import DocumentFilter, ListDocumentFilter
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
@@ -316,7 +316,6 @@ class DocumentViewSet(
SerializerPerActionMixin, SerializerPerActionMixin,
drf.mixins.CreateModelMixin, drf.mixins.CreateModelMixin,
drf.mixins.DestroyModelMixin, drf.mixins.DestroyModelMixin,
drf.mixins.ListModelMixin,
drf.mixins.UpdateModelMixin, drf.mixins.UpdateModelMixin,
viewsets.GenericViewSet, viewsets.GenericViewSet,
): ):
@@ -414,8 +413,6 @@ class DocumentViewSet(
- Implements soft delete logic to retain document tree structures. - Implements soft delete logic to retain document tree structures.
""" """
filter_backends = [drf_filters.DjangoFilterBackend]
filterset_class = DocumentFilter
metadata_class = DocumentMetadata metadata_class = DocumentMetadata
ordering = ["-updated_at"] ordering = ["-updated_at"]
ordering_fields = ["created_at", "updated_at", "title"] ordering_fields = ["created_at", "updated_at", "title"]
@@ -502,11 +499,42 @@ class DocumentViewSet(
) )
def filter_queryset(self, queryset): def filter_queryset(self, queryset):
"""Apply annotations and filters sequentially.""" """Override to apply annotations to generic views."""
filterset = DocumentFilter( queryset = super().filter_queryset(queryset)
queryset = self.annotate_is_favorite(queryset)
queryset = self.annotate_user_roles(queryset)
return queryset
def get_response_for_queryset(self, queryset):
"""Return paginated response for the queryset if requested."""
page = self.paginate_queryset(queryset)
if page is not None:
serializer = self.get_serializer(page, many=True)
return self.get_paginated_response(serializer.data)
serializer = self.get_serializer(queryset, many=True)
return drf.response.Response(serializer.data)
def list(self, request, *args, **kwargs):
"""
Returns a DRF response containing the filtered, annotated and ordered document list.
This method applies filtering based on request parameters using `ListDocumentFilter`.
It performs early filtering on model fields, annotates user roles, and removes
descendant documents to keep only the highest ancestors readable by the current user.
Additional annotations (e.g., `is_highest_ancestor_for_user`, favorite status) are
applied before ordering and returning the response.
"""
queryset = (
self.get_queryset()
) # Not calling filter_queryset. We do our own cooking.
filterset = ListDocumentFilter(
self.request.GET, queryset=queryset, request=self.request self.request.GET, queryset=queryset, request=self.request
) )
filterset.is_valid() if not filterset.is_valid():
raise drf.exceptions.ValidationError(filterset.errors)
filter_data = filterset.form.cleaned_data filter_data = filterset.form.cleaned_data
# Filter as early as possible on fields that are available on the model # Filter as early as possible on fields that are available on the model
@@ -515,22 +543,19 @@ class DocumentViewSet(
queryset = self.annotate_user_roles(queryset) queryset = self.annotate_user_roles(queryset)
if self.action == "list": # Among the results, we may have documents that are ancestors/descendants
# Among the results, we may have documents that are ancestors/descendants # of each other. In this case we want to keep only the highest ancestors.
# of each other. In this case we want to keep only the highest ancestors. root_paths = utils.filter_root_paths(
root_paths = utils.filter_root_paths( queryset.order_by("path").values_list("path", flat=True),
queryset.order_by("path").values_list("path", flat=True), skip_sorting=True,
skip_sorting=True, )
) queryset = queryset.filter(path__in=root_paths)
queryset = queryset.filter(path__in=root_paths)
# Annotate the queryset with an attribute marking instances as highest ancestor # Annotate the queryset with an attribute marking instances as highest ancestor
# in order to save some time while computing abilities on the instance # in order to save some time while computing abilities on the instance
queryset = queryset.annotate( queryset = queryset.annotate(
is_highest_ancestor_for_user=db.Value( is_highest_ancestor_for_user=db.Value(True, output_field=db.BooleanField())
True, output_field=db.BooleanField() )
)
)
# Annotate favorite status and filter if applicable as late as possible # Annotate favorite status and filter if applicable as late as possible
queryset = self.annotate_is_favorite(queryset) queryset = self.annotate_is_favorite(queryset)
@@ -539,18 +564,11 @@ class DocumentViewSet(
) )
# Apply ordering only now that everyting is filtered and annotated # Apply ordering only now that everyting is filtered and annotated
return filters.OrderingFilter().filter_queryset(self.request, queryset, self) queryset = filters.OrderingFilter().filter_queryset(
self.request, queryset, self
)
def get_response_for_queryset(self, queryset): return self.get_response_for_queryset(queryset)
"""Return paginated response for the queryset if requested."""
page = self.paginate_queryset(queryset)
if page is not None:
serializer = self.get_serializer(page, many=True)
result = self.get_paginated_response(serializer.data)
return result
serializer = self.get_serializer(queryset, many=True)
return drf.response.Response(serializer.data)
def retrieve(self, request, *args, **kwargs): def retrieve(self, request, *args, **kwargs):
""" """
@@ -604,7 +622,7 @@ class DocumentViewSet(
user=user user=user
).values_list("document_id", flat=True) ).values_list("document_id", flat=True)
queryset = self.get_queryset() queryset = self.filter_queryset(self.get_queryset())
queryset = queryset.filter(id__in=favorite_documents_ids) queryset = queryset.filter(id__in=favorite_documents_ids)
return self.get_response_for_queryset(queryset) return self.get_response_for_queryset(queryset)
@@ -731,7 +749,6 @@ class DocumentViewSet(
detail=True, detail=True,
methods=["get", "post"], methods=["get", "post"],
ordering=["path"], ordering=["path"],
url_path="children",
) )
def children(self, request, *args, **kwargs): def children(self, request, *args, **kwargs):
"""Handle listing and creating children of a document""" """Handle listing and creating children of a document"""
@@ -763,10 +780,35 @@ class DocumentViewSet(
) )
# GET: List children # GET: List children
queryset = document.get_children().filter(deleted_at__isnull=True) queryset = document.get_children().filter(ancestors_deleted_at__isnull=True)
queryset = self.filter_queryset(queryset) queryset = self.filter_queryset(queryset)
queryset = self.annotate_is_favorite(queryset)
queryset = self.annotate_user_roles(queryset) filterset = DocumentFilter(request.GET, queryset=queryset)
if not filterset.is_valid():
raise drf.exceptions.ValidationError(filterset.errors)
queryset = filterset.qs
return self.get_response_for_queryset(queryset)
@drf.decorators.action(
detail=True,
methods=["get"],
ordering=["path"],
)
def descendants(self, request, *args, **kwargs):
"""Handle listing descendants of a document"""
document = self.get_object()
queryset = document.get_descendants().filter(ancestors_deleted_at__isnull=True)
queryset = self.filter_queryset(queryset)
filterset = DocumentFilter(request.GET, queryset=queryset)
if not filterset.is_valid():
raise drf.exceptions.ValidationError(filterset.errors)
queryset = filterset.qs
return self.get_response_for_queryset(queryset) return self.get_response_for_queryset(queryset)
@drf.decorators.action( @drf.decorators.action(

View File

@@ -809,17 +809,18 @@ class Document(MP_Node, BaseModel):
Soft delete the document, marking the deletion on descendants. Soft delete the document, marking the deletion on descendants.
We still keep the .delete() method untouched for programmatic purposes. We still keep the .delete() method untouched for programmatic purposes.
""" """
if self.deleted_at or self.ancestors_deleted_at: if (
self._meta.model.objects.filter(
models.Q(deleted_at__isnull=False)
| models.Q(ancestors_deleted_at__isnull=False),
pk=self.pk,
).exists()
or self.get_ancestors().filter(deleted_at__isnull=False).exists()
):
raise RuntimeError( raise RuntimeError(
"This document is already deleted or has deleted ancestors." "This document is already deleted or has deleted ancestors."
) )
# Check if any ancestors are deleted
if self.get_ancestors().filter(deleted_at__isnull=False).exists():
raise RuntimeError(
"Cannot delete this document because one or more ancestors are already deleted."
)
self.ancestors_deleted_at = self.deleted_at = timezone.now() self.ancestors_deleted_at = self.deleted_at = timezone.now()
self.save() self.save()
@@ -836,14 +837,8 @@ class Document(MP_Node, BaseModel):
raise ValidationError({"deleted_at": [_("This document is not deleted.")]}) raise ValidationError({"deleted_at": [_("This document is not deleted.")]})
if self.deleted_at < get_trashbin_cutoff(): if self.deleted_at < get_trashbin_cutoff():
raise ValidationError( raise RuntimeError(
{ "This document was permanently deleted and cannot be restored."
"deleted_at": [
_(
"This document was permanently deleted and cannot be restored."
)
]
}
) )
# save the current deleted_at value to exclude it from the descendants update # save the current deleted_at value to exclude it from the descendants update

View File

@@ -1,5 +1,5 @@
""" """
Tests for Documents API endpoint in impress's core app: create Tests for Documents API endpoint in impress's core app: children create
""" """
from uuid import uuid4 from uuid import uuid4

View File

@@ -1,5 +1,5 @@
""" """
Tests for Documents API endpoint in impress's core app: retrieve Tests for Documents API endpoint in impress's core app: children list
""" """
import random import random