✨(backend) list only the first visible parent document for a user
Now that we have a tree structure, we should only include parents of a visible subtree in list results.
This commit is contained in:
committed by
Anthony LC
parent
9a12452c26
commit
48662ceecb
@@ -15,12 +15,14 @@ from django.db import models as db
|
|||||||
from django.db.models import (
|
from django.db.models import (
|
||||||
Count,
|
Count,
|
||||||
Exists,
|
Exists,
|
||||||
|
F,
|
||||||
OuterRef,
|
OuterRef,
|
||||||
Q,
|
Q,
|
||||||
Subquery,
|
Subquery,
|
||||||
Value,
|
Value,
|
||||||
)
|
)
|
||||||
from django.db.models.expressions import RawSQL
|
from django.db.models.expressions import RawSQL
|
||||||
|
from django.db.models.functions import Left, Length
|
||||||
from django.http import Http404
|
from django.http import Http404
|
||||||
|
|
||||||
import rest_framework as drf
|
import rest_framework as drf
|
||||||
@@ -395,6 +397,24 @@ class DocumentViewSet(
|
|||||||
& ~db.Q(link_reach=models.LinkReachChoices.RESTRICTED)
|
& ~db.Q(link_reach=models.LinkReachChoices.RESTRICTED)
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# Among the results, we may have documents that are ancestors/children of each other
|
||||||
|
# In this case we want to keep only the highest ancestor. Let's annotate, each document
|
||||||
|
# with the path of its highest ancestor within results so we can use it to filter
|
||||||
|
shortest_path = Subquery(
|
||||||
|
queryset.filter(path=Left(OuterRef("path"), Length("path")))
|
||||||
|
.order_by("path") # Get the shortest (root) path
|
||||||
|
.values("path")[:1]
|
||||||
|
)
|
||||||
|
queryset = queryset.annotate(root_path=shortest_path)
|
||||||
|
|
||||||
|
# Filter documents based on their shortest path (root path)
|
||||||
|
queryset = queryset.filter(
|
||||||
|
root_path=F(
|
||||||
|
"path"
|
||||||
|
) # Keep only documents who are the annotated highest ancestor
|
||||||
|
)
|
||||||
|
|
||||||
else:
|
else:
|
||||||
queryset = queryset.none()
|
queryset = queryset.none()
|
||||||
|
|
||||||
|
|||||||
@@ -2,6 +2,7 @@
|
|||||||
Tests for Documents API endpoint in impress's core app: list
|
Tests for Documents API endpoint in impress's core app: list
|
||||||
"""
|
"""
|
||||||
|
|
||||||
|
import random
|
||||||
from unittest import mock
|
from unittest import mock
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
@@ -82,7 +83,7 @@ def test_api_documents_list_authenticated_direct(django_assert_num_queries):
|
|||||||
client = APIClient()
|
client = APIClient()
|
||||||
client.force_login(user)
|
client.force_login(user)
|
||||||
|
|
||||||
documents = [
|
document1, document2 = [
|
||||||
access.document
|
access.document
|
||||||
for access in factories.UserDocumentAccessFactory.create_batch(2, user=user)
|
for access in factories.UserDocumentAccessFactory.create_batch(2, user=user)
|
||||||
]
|
]
|
||||||
@@ -92,14 +93,29 @@ def test_api_documents_list_authenticated_direct(django_assert_num_queries):
|
|||||||
for role in models.LinkRoleChoices:
|
for role in models.LinkRoleChoices:
|
||||||
factories.DocumentFactory(link_reach=reach, link_role=role)
|
factories.DocumentFactory(link_reach=reach, link_role=role)
|
||||||
|
|
||||||
expected_ids = {str(document.id) for document in documents}
|
# Children of visible documents should not get listed even with a specific access
|
||||||
|
factories.DocumentFactory(parent=document1)
|
||||||
|
|
||||||
|
child1_with_access = factories.DocumentFactory(parent=document1)
|
||||||
|
factories.UserDocumentAccessFactory(user=user, document=child1_with_access)
|
||||||
|
|
||||||
|
middle_document = factories.DocumentFactory(parent=document2)
|
||||||
|
child2_with_access = factories.DocumentFactory(parent=middle_document)
|
||||||
|
factories.UserDocumentAccessFactory(user=user, document=child2_with_access)
|
||||||
|
|
||||||
|
# Children of hidden documents should get listed when visible by the logged-in user
|
||||||
|
hidden_root = factories.DocumentFactory()
|
||||||
|
child3_with_access = factories.DocumentFactory(parent=hidden_root)
|
||||||
|
factories.UserDocumentAccessFactory(user=user, document=child3_with_access)
|
||||||
|
|
||||||
|
expected_ids = {str(document1.id), str(document2.id), str(child3_with_access.id)}
|
||||||
|
|
||||||
with django_assert_num_queries(3):
|
with django_assert_num_queries(3):
|
||||||
response = client.get("/api/v1.0/documents/")
|
response = client.get("/api/v1.0/documents/")
|
||||||
|
|
||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
results = response.json()["results"]
|
results = response.json()["results"]
|
||||||
assert len(results) == 2
|
assert len(results) == 3
|
||||||
results_id = {result["id"] for result in results}
|
results_id = {result["id"] for result in results}
|
||||||
assert expected_ids == results_id
|
assert expected_ids == results_id
|
||||||
|
|
||||||
@@ -183,12 +199,27 @@ def test_api_documents_list_authenticated_link_reach_public_or_authenticated(
|
|||||||
client = APIClient()
|
client = APIClient()
|
||||||
client.force_login(user)
|
client.force_login(user)
|
||||||
|
|
||||||
documents = [
|
document1, document2 = [
|
||||||
factories.DocumentFactory(link_traces=[user], link_reach=reach)
|
factories.DocumentFactory(link_traces=[user], link_reach=reach)
|
||||||
for reach in models.LinkReachChoices
|
for reach in models.LinkReachChoices
|
||||||
if reach != "restricted"
|
if reach != "restricted"
|
||||||
]
|
]
|
||||||
expected_ids = {str(document.id) for document in documents}
|
factories.DocumentFactory(
|
||||||
|
link_reach=random.choice(["public", "authenticated"]),
|
||||||
|
link_traces=[user],
|
||||||
|
parent=document1,
|
||||||
|
)
|
||||||
|
|
||||||
|
hidden_document = factories.DocumentFactory(
|
||||||
|
link_reach=random.choice(["public", "authenticated"])
|
||||||
|
)
|
||||||
|
visible_child = factories.DocumentFactory(
|
||||||
|
link_traces=[user],
|
||||||
|
link_reach=random.choice(["public", "authenticated"]),
|
||||||
|
parent=hidden_document,
|
||||||
|
)
|
||||||
|
|
||||||
|
expected_ids = {str(document1.id), str(document2.id), str(visible_child.id)}
|
||||||
|
|
||||||
with django_assert_num_queries(3):
|
with django_assert_num_queries(3):
|
||||||
response = client.get(
|
response = client.get(
|
||||||
@@ -197,7 +228,7 @@ def test_api_documents_list_authenticated_link_reach_public_or_authenticated(
|
|||||||
|
|
||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
results = response.json()["results"]
|
results = response.json()["results"]
|
||||||
assert len(results) == 2
|
assert len(results) == 3
|
||||||
results_id = {result["id"] for result in results}
|
results_id = {result["id"] for result in results}
|
||||||
assert expected_ids == results_id
|
assert expected_ids == results_id
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user