(backend) add "tree" action on document API endpoint

We want to display the tree structure to which a document belongs
on the left side panel of its detail view. For this, we need an
endpoint to retrieve the list view of the document's ancestors
opened.

By opened, we mean that when display the document, we also need to
display its siblings. When displaying the parent of the current
document, we also need to display the siblings of the parent...
This commit is contained in:
Samuel Paccoud - DINUM
2025-02-16 17:26:51 +01:00
committed by Manuel Raynaud
parent fcf8b38021
commit 0aabf26694
11 changed files with 1328 additions and 23 deletions

View File

@@ -32,7 +32,8 @@ and this project adheres to
## Added ## Added
- ✨(backend) allow forcing page size within limits - ✨(backend) new "tree" action on document detail endpoint #645
- ✨(backend) allow forcing page size within limits #645
- 💄(frontend) add error pages #643 - 💄(frontend) add error pages #643
- 🔒️ Manage unsafe attachments #663 - 🔒️ Manage unsafe attachments #663
- ✨(frontend) Custom block quote with export #646 - ✨(frontend) Custom block quote with export #646

View File

@@ -128,26 +128,13 @@ class TemplateAccessSerializer(BaseAccessSerializer):
read_only_fields = ["id", "abilities"] read_only_fields = ["id", "abilities"]
class BaseResourceSerializer(serializers.ModelSerializer): class ListDocumentSerializer(serializers.ModelSerializer):
"""Serialize documents."""
abilities = serializers.SerializerMethodField(read_only=True)
accesses = TemplateAccessSerializer(many=True, read_only=True)
def get_abilities(self, document) -> dict:
"""Return abilities of the logged-in user on the instance."""
request = self.context.get("request")
if request:
return document.get_abilities(request.user)
return {}
class ListDocumentSerializer(BaseResourceSerializer):
"""Serialize documents with limited fields for display in lists.""" """Serialize documents with limited fields for display in lists."""
is_favorite = serializers.BooleanField(read_only=True) is_favorite = serializers.BooleanField(read_only=True)
nb_accesses = serializers.IntegerField(read_only=True) nb_accesses = serializers.IntegerField(read_only=True)
user_roles = serializers.SerializerMethodField(read_only=True) user_roles = serializers.SerializerMethodField(read_only=True)
abilities = serializers.SerializerMethodField(read_only=True)
class Meta: class Meta:
model = models.Document model = models.Document
@@ -185,6 +172,18 @@ class ListDocumentSerializer(BaseResourceSerializer):
"user_roles", "user_roles",
] ]
def get_abilities(self, document) -> dict:
"""Return abilities of the logged-in user on the instance."""
request = self.context.get("request")
if request:
paths_links_mapping = self.context.get("paths_links_mapping", None)
return document.get_abilities(
request.user, paths_links_mapping=paths_links_mapping
)
return {}
def get_user_roles(self, document): def get_user_roles(self, document):
""" """
Return roles of the logged-in user for the current document, Return roles of the logged-in user for the current document,
@@ -359,7 +358,7 @@ class ServerCreateDocumentSerializer(serializers.Serializer):
raise NotImplementedError("Update is not supported for this serializer.") raise NotImplementedError("Update is not supported for this serializer.")
class LinkDocumentSerializer(BaseResourceSerializer): class LinkDocumentSerializer(serializers.ModelSerializer):
""" """
Serialize link configuration for documents. Serialize link configuration for documents.
We expose it separately from document in order to simplify and secure access control. We expose it separately from document in order to simplify and secure access control.
@@ -431,9 +430,12 @@ class FileUploadSerializer(serializers.Serializer):
return attrs return attrs
class TemplateSerializer(BaseResourceSerializer): class TemplateSerializer(serializers.ModelSerializer):
"""Serialize templates.""" """Serialize templates."""
abilities = serializers.SerializerMethodField(read_only=True)
accesses = TemplateAccessSerializer(many=True, read_only=True)
class Meta: class Meta:
model = models.Template model = models.Template
fields = [ fields = [
@@ -447,6 +449,13 @@ class TemplateSerializer(BaseResourceSerializer):
] ]
read_only_fields = ["id", "accesses", "abilities"] read_only_fields = ["id", "accesses", "abilities"]
def get_abilities(self, document) -> dict:
"""Return abilities of the logged-in user on the instance."""
request = self.context.get("request")
if request:
return document.get_abilities(request.user)
return {}
# pylint: disable=abstract-method # pylint: disable=abstract-method
class DocumentGenerationSerializer(serializers.Serializer): class DocumentGenerationSerializer(serializers.Serializer):

View File

@@ -11,6 +11,35 @@ import botocore
from rest_framework.throttling import BaseThrottle from rest_framework.throttling import BaseThrottle
def nest_tree(flat_list, steplen):
"""
Convert a flat list of serialized documents into a nested tree making advantage
of the`path` field and its step length.
"""
node_dict = {}
roots = []
# Sort the flat list by path to ensure parent nodes are processed first
flat_list.sort(key=lambda x: x["path"])
for node in flat_list:
node["children"] = [] # Initialize children list
node_dict[node["path"]] = node
# Determine parent path
parent_path = node["path"][:-steplen]
if parent_path in node_dict:
node_dict[parent_path]["children"].append(node)
else:
roots.append(node) # Collect root nodes
if len(roots) > 1:
raise ValueError("More than one root element detected.")
return roots[0] if roots else None
def filter_root_paths(paths, skip_sorting=False): def filter_root_paths(paths, skip_sorting=False):
""" """
Filters root paths from a list of paths representing a tree structure. Filters root paths from a list of paths representing a tree structure.

View File

@@ -4,6 +4,7 @@
import logging import logging
import re import re
import uuid import uuid
from collections import defaultdict
from urllib.parse import urlparse from urllib.parse import urlparse
from django.conf import settings from django.conf import settings
@@ -424,10 +425,11 @@ class DocumentViewSet(
] ]
queryset = models.Document.objects.all() queryset = models.Document.objects.all()
serializer_class = serializers.DocumentSerializer serializer_class = serializers.DocumentSerializer
ai_translate_serializer_class = serializers.AITranslateSerializer
children_serializer_class = serializers.ListDocumentSerializer
list_serializer_class = serializers.ListDocumentSerializer list_serializer_class = serializers.ListDocumentSerializer
trashbin_serializer_class = serializers.ListDocumentSerializer trashbin_serializer_class = serializers.ListDocumentSerializer
children_serializer_class = serializers.ListDocumentSerializer tree_serializer_class = serializers.ListDocumentSerializer
ai_translate_serializer_class = serializers.AITranslateSerializer
def annotate_is_favorite(self, queryset): def annotate_is_favorite(self, queryset):
""" """
@@ -523,7 +525,7 @@ class DocumentViewSet(
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 in 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()
@@ -767,6 +769,68 @@ class DocumentViewSet(
queryset = self.annotate_user_roles(queryset) queryset = self.annotate_user_roles(queryset)
return self.get_response_for_queryset(queryset) return self.get_response_for_queryset(queryset)
@drf.decorators.action(
detail=True,
methods=["get"],
ordering=["path"],
)
def tree(self, request, pk, *args, **kwargs):
"""
List ancestors tree above the document.
What we need to display is the tree structure opened for the current document.
"""
try:
current_document = self.queryset.only("depth", "path").get(pk=pk)
except models.Document.DoesNotExist as excpt:
raise drf.exceptions.NotFound from excpt
ancestors = (
(current_document.get_ancestors() | self.queryset.filter(pk=pk))
.filter(ancestors_deleted_at__isnull=True)
.order_by("path")
)
# Get the highest readable ancestor
highest_readable = ancestors.readable_per_se(request.user).only("depth").first()
if highest_readable is None:
raise (
drf.exceptions.PermissionDenied()
if request.user.is_authenticated
else drf.exceptions.NotAuthenticated()
)
ancestors_links_definitions = defaultdict(set)
children_clause = db.Q()
for ancestor in ancestors:
if ancestor.depth < highest_readable.depth:
continue
ancestors_links_definitions[ancestor.link_reach].add(ancestor.link_role)
children_clause |= db.Q(
path__startswith=ancestor.path, depth=ancestor.depth + 1
)
children = self.queryset.filter(children_clause, deleted_at__isnull=True)
queryset = ancestors.filter(depth__gte=highest_readable.depth) | children
queryset = queryset.order_by("path")
queryset = self.annotate_user_roles(queryset)
queryset = self.annotate_is_favorite(queryset)
# Pass ancestors' links definitions to the serializer as a context variable
# in order to allow saving time while computing abilities on the instance
serializer = self.get_serializer(
queryset,
many=True,
context={
"request": request,
"ancestors_links_definitions": ancestors_links_definitions,
},
)
return drf.response.Response(
utils.nest_tree(serializer.data, self.queryset.model.steplen)
)
@drf.decorators.action(detail=True, methods=["get"], url_path="versions") @drf.decorators.action(detail=True, methods=["get"], url_path="versions")
def versions_list(self, request, *args, **kwargs): def versions_list(self, request, *args, **kwargs):
""" """

View File

@@ -29,7 +29,7 @@ from django.utils.translation import gettext_lazy as _
from botocore.exceptions import ClientError from botocore.exceptions import ClientError
from rest_framework.exceptions import ValidationError from rest_framework.exceptions import ValidationError
from timezone_field import TimeZoneField from timezone_field import TimeZoneField
from treebeard.mp_tree import MP_Node from treebeard.mp_tree import MP_Node, MP_NodeManager, MP_NodeQuerySet
logger = getLogger(__name__) logger = getLogger(__name__)
@@ -369,6 +369,51 @@ class BaseAccess(BaseModel):
} }
class DocumentQuerySet(MP_NodeQuerySet):
"""
Custom queryset for the Document model, providing additional methods
to filter documents based on user permissions.
"""
def readable_per_se(self, user):
"""
Filters the queryset to return documents that the given user has
permission to read.
:param user: The user for whom readable documents are to be fetched.
:return: A queryset of documents readable by the user.
"""
if user.is_authenticated:
return self.filter(
models.Q(accesses__user=user)
| models.Q(accesses__team__in=user.teams)
| ~models.Q(link_reach=LinkReachChoices.RESTRICTED)
)
return self.filter(link_reach=LinkReachChoices.PUBLIC)
class DocumentManager(MP_NodeManager):
"""
Custom manager for the Document model, enabling the use of the custom
queryset methods directly from the model manager.
"""
def get_queryset(self):
"""
Overrides the default get_queryset method to return a custom queryset.
:return: An instance of DocumentQuerySet.
"""
return DocumentQuerySet(self.model, using=self._db)
def readable_per_se(self, user):
"""
Filters documents based on user permissions using the custom queryset.
:param user: The user for whom readable documents are to be fetched.
:return: A queryset of documents readable by the user.
"""
return self.get_queryset().readable_per_se(user)
class Document(MP_Node, BaseModel): class Document(MP_Node, BaseModel):
"""Pad document carrying the content.""" """Pad document carrying the content."""
@@ -401,6 +446,8 @@ class Document(MP_Node, BaseModel):
path = models.CharField(max_length=7 * 36, unique=True, db_collation="C") path = models.CharField(max_length=7 * 36, unique=True, db_collation="C")
objects = DocumentManager()
class Meta: class Meta:
db_table = "impress_document" db_table = "impress_document"
ordering = ("path",) ordering = ("path",)
@@ -574,7 +621,11 @@ class Document(MP_Node, BaseModel):
def invalidate_nb_accesses_cache(self): def invalidate_nb_accesses_cache(self):
""" """
Invalidate the cache for number of accesses, including on affected descendants. Invalidate the cache for number of accesses, including on affected descendants.
Args:
path: can optionally be passed as argument (useful when invalidating cache for a
document we just deleted)
""" """
for document in Document.objects.filter(path__startswith=self.path).only("id"): for document in Document.objects.filter(path__startswith=self.path).only("id"):
cache_key = document.get_nb_accesses_cache_key() cache_key = document.get_nb_accesses_cache_key()
cache.delete(cache_key) cache.delete(cache_key)
@@ -682,6 +733,7 @@ class Document(MP_Node, BaseModel):
"restore": is_owner, "restore": is_owner,
"retrieve": can_get, "retrieve": can_get,
"media_auth": can_get, "media_auth": can_get,
"tree": can_get,
"update": can_update, "update": can_update,
"versions_destroy": is_owner_or_admin, "versions_destroy": is_owner_or_admin,
"versions_list": has_access_role, "versions_list": has_access_role,

View File

@@ -15,7 +15,7 @@ pytestmark = pytest.mark.django_db
def test_api_documents_children_list_anonymous_public_standalone(): def test_api_documents_children_list_anonymous_public_standalone():
"""Anonymous users should be allowed to retrieve the children of a public documents.""" """Anonymous users should be allowed to retrieve the children of a public document."""
document = factories.DocumentFactory(link_reach="public") document = factories.DocumentFactory(link_reach="public")
child1, child2 = factories.DocumentFactory.create_batch(2, parent=document) child1, child2 = factories.DocumentFactory.create_batch(2, parent=document)
factories.UserDocumentAccessFactory(document=child1) factories.UserDocumentAccessFactory(document=child1)

View File

@@ -44,6 +44,7 @@ def test_api_documents_retrieve_anonymous_public_standalone():
"partial_update": document.link_role == "editor", "partial_update": document.link_role == "editor",
"restore": False, "restore": False,
"retrieve": True, "retrieve": True,
"tree": True,
"update": document.link_role == "editor", "update": document.link_role == "editor",
"versions_destroy": False, "versions_destroy": False,
"versions_list": False, "versions_list": False,
@@ -100,6 +101,7 @@ def test_api_documents_retrieve_anonymous_public_parent():
"partial_update": grand_parent.link_role == "editor", "partial_update": grand_parent.link_role == "editor",
"restore": False, "restore": False,
"retrieve": True, "retrieve": True,
"tree": True,
"update": grand_parent.link_role == "editor", "update": grand_parent.link_role == "editor",
"versions_destroy": False, "versions_destroy": False,
"versions_list": False, "versions_list": False,
@@ -189,6 +191,7 @@ def test_api_documents_retrieve_authenticated_unrelated_public_or_authenticated(
"partial_update": document.link_role == "editor", "partial_update": document.link_role == "editor",
"restore": False, "restore": False,
"retrieve": True, "retrieve": True,
"tree": True,
"update": document.link_role == "editor", "update": document.link_role == "editor",
"versions_destroy": False, "versions_destroy": False,
"versions_list": False, "versions_list": False,
@@ -252,6 +255,7 @@ def test_api_documents_retrieve_authenticated_public_or_authenticated_parent(rea
"partial_update": grand_parent.link_role == "editor", "partial_update": grand_parent.link_role == "editor",
"restore": False, "restore": False,
"retrieve": True, "retrieve": True,
"tree": True,
"update": grand_parent.link_role == "editor", "update": grand_parent.link_role == "editor",
"versions_destroy": False, "versions_destroy": False,
"versions_list": False, "versions_list": False,
@@ -424,6 +428,7 @@ def test_api_documents_retrieve_authenticated_related_parent():
"partial_update": access.role != "reader", "partial_update": access.role != "reader",
"restore": access.role == "owner", "restore": access.role == "owner",
"retrieve": True, "retrieve": True,
"tree": True,
"update": access.role != "reader", "update": access.role != "reader",
"versions_destroy": access.role in ["administrator", "owner"], "versions_destroy": access.role in ["administrator", "owner"],
"versions_list": True, "versions_list": True,

View File

@@ -87,6 +87,7 @@ def test_api_documents_trashbin_format():
"partial_update": True, "partial_update": True,
"restore": True, "restore": True,
"retrieve": True, "retrieve": True,
"tree": True,
"update": True, "update": True,
"versions_destroy": True, "versions_destroy": True,
"versions_list": True, "versions_list": True,

File diff suppressed because it is too large Load Diff

View File

@@ -0,0 +1,107 @@
"""Unit tests for the nest_tree utility function."""
import pytest
from core.api.utils import nest_tree
def test_api_utils_nest_tree_empty_list():
"""Test that an empty list returns an empty nested structure."""
# pylint: disable=use-implicit-booleaness-not-comparison
assert nest_tree([], 4) is None
def test_api_utils_nest_tree_single_document():
"""Test that a single document is returned as the only root element."""
documents = [{"id": "1", "path": "0001"}]
expected = {"id": "1", "path": "0001", "children": []}
assert nest_tree(documents, 4) == expected
def test_api_utils_nest_tree_multiple_root_documents():
"""Test that multiple root-level documents are correctly added to the root."""
documents = [
{"id": "1", "path": "0001"},
{"id": "2", "path": "0002"},
]
with pytest.raises(
ValueError,
match="More than one root element detected.",
):
nest_tree(documents, 4)
def test_api_utils_nest_tree_nested_structure():
"""Test that documents are correctly nested based on path levels."""
documents = [
{"id": "1", "path": "0001"},
{"id": "2", "path": "00010001"},
{"id": "3", "path": "000100010001"},
{"id": "4", "path": "00010002"},
]
expected = {
"id": "1",
"path": "0001",
"children": [
{
"id": "2",
"path": "00010001",
"children": [{"id": "3", "path": "000100010001", "children": []}],
},
{"id": "4", "path": "00010002", "children": []},
],
}
assert nest_tree(documents, 4) == expected
def test_api_utils_nest_tree_siblings_at_same_path():
"""
Test that sibling documents with the same path are correctly grouped under the same parent.
"""
documents = [
{"id": "1", "path": "0001"},
{"id": "2", "path": "00010001"},
{"id": "3", "path": "00010002"},
]
expected = {
"id": "1",
"path": "0001",
"children": [
{"id": "2", "path": "00010001", "children": []},
{"id": "3", "path": "00010002", "children": []},
],
}
assert nest_tree(documents, 4) == expected
def test_api_utils_nest_tree_decreasing_path_resets_parent():
"""Test that a document at a lower path resets the parent assignment correctly."""
documents = [
{"id": "1", "path": "0001"},
{"id": "6", "path": "00010001"},
{"id": "2", "path": "00010002"}, # unordered
{"id": "5", "path": "000100010001"},
{"id": "3", "path": "000100010002"},
{"id": "4", "path": "00010003"},
]
expected = {
"id": "1",
"path": "0001",
"children": [
{
"id": "6",
"path": "00010001",
"children": [
{"id": "5", "path": "000100010001", "children": []},
{"id": "3", "path": "000100010002", "children": []},
],
},
{
"id": "2",
"path": "00010002",
"children": [],
},
{"id": "4", "path": "00010003", "children": []},
],
}
assert nest_tree(documents, 4) == expected

View File

@@ -166,6 +166,7 @@ def test_models_documents_get_abilities_forbidden(
"partial_update": False, "partial_update": False,
"restore": False, "restore": False,
"retrieve": False, "retrieve": False,
"tree": False,
"update": False, "update": False,
"versions_destroy": False, "versions_destroy": False,
"versions_list": False, "versions_list": False,
@@ -217,6 +218,7 @@ def test_models_documents_get_abilities_reader(
"partial_update": False, "partial_update": False,
"restore": False, "restore": False,
"retrieve": True, "retrieve": True,
"tree": True,
"update": False, "update": False,
"versions_destroy": False, "versions_destroy": False,
"versions_list": False, "versions_list": False,
@@ -265,6 +267,7 @@ def test_models_documents_get_abilities_editor(
"partial_update": True, "partial_update": True,
"restore": False, "restore": False,
"retrieve": True, "retrieve": True,
"tree": True,
"update": True, "update": True,
"versions_destroy": False, "versions_destroy": False,
"versions_list": False, "versions_list": False,
@@ -303,6 +306,7 @@ def test_models_documents_get_abilities_owner(django_assert_num_queries):
"partial_update": True, "partial_update": True,
"restore": True, "restore": True,
"retrieve": True, "retrieve": True,
"tree": True,
"update": True, "update": True,
"versions_destroy": True, "versions_destroy": True,
"versions_list": True, "versions_list": True,
@@ -342,6 +346,7 @@ def test_models_documents_get_abilities_administrator(django_assert_num_queries)
"partial_update": True, "partial_update": True,
"restore": False, "restore": False,
"retrieve": True, "retrieve": True,
"tree": True,
"update": True, "update": True,
"versions_destroy": True, "versions_destroy": True,
"versions_list": True, "versions_list": True,
@@ -380,6 +385,7 @@ def test_models_documents_get_abilities_editor_user(django_assert_num_queries):
"partial_update": True, "partial_update": True,
"restore": False, "restore": False,
"retrieve": True, "retrieve": True,
"tree": True,
"update": True, "update": True,
"versions_destroy": False, "versions_destroy": False,
"versions_list": True, "versions_list": True,
@@ -425,6 +431,7 @@ def test_models_documents_get_abilities_reader_user(
"partial_update": access_from_link, "partial_update": access_from_link,
"restore": False, "restore": False,
"retrieve": True, "retrieve": True,
"tree": True,
"update": access_from_link, "update": access_from_link,
"versions_destroy": False, "versions_destroy": False,
"versions_list": True, "versions_list": True,
@@ -468,6 +475,7 @@ def test_models_documents_get_abilities_preset_role(django_assert_num_queries):
"partial_update": False, "partial_update": False,
"restore": False, "restore": False,
"retrieve": True, "retrieve": True,
"tree": True,
"update": False, "update": False,
"versions_destroy": False, "versions_destroy": False,
"versions_list": True, "versions_list": True,