From 94ae59af7a5c5dcd4f79d8826fee7f302bfd9dc4 Mon Sep 17 00:00:00 2001 From: Manuel Raynaud Date: Thu, 28 May 2026 17:10:22 +0200 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=94=A5(backend)=20remove=20masking=20?= =?UTF-8?q?document=20code?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The mask feature has never been implemented by the frontend application and it is not what we want to do. We want to implement a feature to leave a document. --- src/backend/core/api/filters.py | 22 -- src/backend/core/api/viewsets.py | 48 +-- src/backend/core/factories.py | 9 - .../0032_remove_linktrace_is_masked.py | 16 + src/backend/core/models.py | 2 - .../test_api_documents_list_filters.py | 78 ---- .../documents/test_api_documents_mask.py | 353 ------------------ .../documents/test_api_documents_retrieve.py | 5 - .../documents/test_api_documents_trashbin.py | 1 - .../core/tests/test_models_documents.py | 11 - .../tests/test_models_user_reconciliation.py | 7 +- .../tests/test_services_search_indexers.py | 8 +- 12 files changed, 28 insertions(+), 532 deletions(-) create mode 100644 src/backend/core/migrations/0032_remove_linktrace_is_masked.py delete mode 100644 src/backend/core/tests/documents/test_api_documents_mask.py diff --git a/src/backend/core/api/filters.py b/src/backend/core/api/filters.py index e796834c9d..ae67f063bf 100644 --- a/src/backend/core/api/filters.py +++ b/src/backend/core/api/filters.py @@ -64,9 +64,6 @@ class ListDocumentFilter(DocumentFilter): is_creator_me = django_filters.BooleanFilter( method="filter_is_creator_me", label=_("Creator is me") ) - is_masked = django_filters.BooleanFilter( - method="filter_is_masked", label=_("Masked") - ) is_favorite = django_filters.BooleanFilter( method="filter_is_favorite", label=_("Favorite") ) @@ -114,25 +111,6 @@ def filter_is_favorite(self, queryset, name, value): return queryset.filter(is_favorite=bool(value)) - # pylint: disable=unused-argument - def filter_is_masked(self, queryset, name, value): - """ - Filter documents based on whether they are masked by the current user. - - Example: - - /api/v1.0/documents/?is_masked=true - → Filters documents marked as masked by the logged-in user - - /api/v1.0/documents/?is_masked=false - → Filters documents not marked as masked by the logged-in user - """ - user = self.request.user - - if not user.is_authenticated: - return queryset - - queryset_method = queryset.filter if bool(value) else queryset.exclude - return queryset_method(link_traces__user=user, link_traces__is_masked=True) - class UserSearchFilter(django_filters.FilterSet): """ diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index 43e0e1f7d0..f180cf8dba 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -687,8 +687,9 @@ def list(self, request, *args, **kwargs): # Annotate favorite status and filter if applicable as late as possible queryset = queryset.annotate_is_favorite(user) - for field in ["is_favorite", "is_masked"]: - queryset = filterset.filters[field].filter(queryset, filter_data[field]) + queryset = filterset.filters["is_favorite"].filter( + queryset, filter_data["is_favorite"] + ) # Apply ordering only now that everything is filtered and annotated queryset = filters.OrderingFilter().filter_queryset( @@ -1177,8 +1178,9 @@ def all(self, request, *args, **kwargs): # Annotate favorite status and filter if applicable as late as possible queryset = queryset.annotate_is_favorite(user) - for field in ["is_favorite", "is_masked"]: - queryset = filterset.filters[field].filter(queryset, filter_data[field]) + queryset = filterset.filters["is_favorite"].filter( + queryset, filter_data["is_favorite"] + ) # Apply ordering only now that everything is filtered and annotated queryset = filters.OrderingFilter().filter_queryset( @@ -1770,44 +1772,6 @@ def favorite(self, request, *args, **kwargs): status=drf.status.HTTP_200_OK, ) - @drf.decorators.action(detail=True, methods=["post", "delete"], url_path="mask") - def mask(self, request, *args, **kwargs): - """Mask or unmask the document for the logged-in user based on the HTTP method.""" - # Check permissions first - document = self.get_object() - user = request.user - - try: - link_trace = models.LinkTrace.objects.get(document=document, user=user) - except models.LinkTrace.DoesNotExist: - return drf.response.Response( - {"detail": "User never accessed this document before."}, - status=status.HTTP_400_BAD_REQUEST, - ) - - if request.method == "POST": - if link_trace.is_masked: - return drf.response.Response( - {"detail": "Document was already masked"}, - status=drf.status.HTTP_200_OK, - ) - link_trace.is_masked = True - link_trace.save(update_fields=["is_masked"]) - return drf.response.Response( - {"detail": "Document was masked"}, - status=drf.status.HTTP_201_CREATED, - ) - - # Handle DELETE method to unmask document - if not link_trace.is_masked: - return drf.response.Response( - {"detail": "Document was already not masked"}, - status=drf.status.HTTP_200_OK, - ) - link_trace.is_masked = False - link_trace.save(update_fields=["is_masked"]) - return drf.response.Response(status=drf.status.HTTP_204_NO_CONTENT) - @drf.decorators.action(detail=True, methods=["post"], url_path="attachment-upload") def attachment_upload(self, request, *args, **kwargs): """Upload a file related to a given document""" diff --git a/src/backend/core/factories.py b/src/backend/core/factories.py index 91bdaeaf6c..bf7ee04287 100644 --- a/src/backend/core/factories.py +++ b/src/backend/core/factories.py @@ -150,15 +150,6 @@ def favorited_by(self, create, extracted, **kwargs): for item in extracted: models.DocumentFavorite.objects.create(document=self, user=item) - @factory.post_generation - def masked_by(self, create, extracted, **kwargs): - """Mark document as masked by a list of users.""" - if create and extracted: - for item in extracted: - models.LinkTrace.objects.update_or_create( - document=self, user=item, defaults={"is_masked": True} - ) - class UserDocumentAccessFactory(factory.django.DjangoModelFactory): """Create fake document user accesses for testing.""" diff --git a/src/backend/core/migrations/0032_remove_linktrace_is_masked.py b/src/backend/core/migrations/0032_remove_linktrace_is_masked.py new file mode 100644 index 0000000000..9996f8c728 --- /dev/null +++ b/src/backend/core/migrations/0032_remove_linktrace_is_masked.py @@ -0,0 +1,16 @@ +# Generated by Django 5.2.14 on 2026-05-28 14:58 + +from django.db import migrations + + +class Migration(migrations.Migration): + dependencies = [ + ("core", "0031_clean_onboarding_accesses"), + ] + + operations = [ + migrations.RemoveField( + model_name="linktrace", + name="is_masked", + ), + ] diff --git a/src/backend/core/models.py b/src/backend/core/models.py index 36fd07feca..9b4b8a46e3 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -1312,7 +1312,6 @@ def get_abilities(self, user): "favorite": can_get and user.is_authenticated, "link_configuration": is_owner_or_admin, "invite_owner": is_owner and not is_deleted, - "mask": can_get and user.is_authenticated, "move": is_owner_or_admin and not is_deleted, "partial_update": can_update, "restore": is_owner, @@ -1481,7 +1480,6 @@ class LinkTrace(BaseModel): related_name="link_traces", ) user = models.ForeignKey(User, on_delete=models.CASCADE, related_name="link_traces") - is_masked = models.BooleanField(default=False) class Meta: db_table = "impress_link_trace" diff --git a/src/backend/core/tests/documents/test_api_documents_list_filters.py b/src/backend/core/tests/documents/test_api_documents_list_filters.py index 637109731a..037119ec1a 100644 --- a/src/backend/core/tests/documents/test_api_documents_list_filters.py +++ b/src/backend/core/tests/documents/test_api_documents_list_filters.py @@ -321,84 +321,6 @@ def test_api_documents_list_filter_is_favorite_invalid(): assert len(results) == 5 -# Filters: is_masked - - -def test_api_documents_list_filter_is_masked_true(): - """ - Authenticated users should be able to filter documents they marked as masked. - """ - user = factories.UserFactory() - client = APIClient() - client.force_login(user) - - factories.DocumentFactory.create_batch(2, users=[user]) - masked_documents = factories.DocumentFactory.create_batch( - 3, users=[user], masked_by=[user] - ) - unmasked_documents = factories.DocumentFactory.create_batch(2, users=[user]) - for document in unmasked_documents: - models.LinkTrace.objects.create(document=document, user=user, is_masked=False) - - response = client.get("/api/v1.0/documents/?is_masked=true") - - assert response.status_code == 200 - results = response.json()["results"] - assert len(results) == 3 - - # Ensure all results are marked as masked by the current user - masked_documents_ids = [str(doc.id) for doc in masked_documents] - for result in results: - assert result["id"] in masked_documents_ids - - -def test_api_documents_list_filter_is_masked_false(): - """ - Authenticated users should be able to filter documents they didn't mark as masked. - """ - user = factories.UserFactory() - client = APIClient() - client.force_login(user) - - factories.DocumentFactory.create_batch(2, users=[user]) - masked_documents = factories.DocumentFactory.create_batch( - 3, users=[user], masked_by=[user] - ) - unmasked_documents = factories.DocumentFactory.create_batch(2, users=[user]) - for document in unmasked_documents: - models.LinkTrace.objects.create(document=document, user=user, is_masked=False) - - response = client.get("/api/v1.0/documents/?is_masked=false") - - assert response.status_code == 200 - results = response.json()["results"] - assert len(results) == 4 - - # Ensure all results are not marked as masked by the current user - masked_documents_ids = [str(doc.id) for doc in masked_documents] - for result in results: - assert result["id"] not in masked_documents_ids - - -def test_api_documents_list_filter_is_masked_invalid(): - """Filtering with an invalid `is_masked` value should do nothing.""" - user = factories.UserFactory() - client = APIClient() - client.force_login(user) - - factories.DocumentFactory.create_batch(2, users=[user]) - factories.DocumentFactory.create_batch(3, users=[user], masked_by=[user]) - unmasked_documents = factories.DocumentFactory.create_batch(2, users=[user]) - for document in unmasked_documents: - models.LinkTrace.objects.create(document=document, user=user, is_masked=False) - - response = client.get("/api/v1.0/documents/?is_masked=invalid") - - assert response.status_code == 200 - results = response.json()["results"] - assert len(results) == 7 - - # Filters: title diff --git a/src/backend/core/tests/documents/test_api_documents_mask.py b/src/backend/core/tests/documents/test_api_documents_mask.py deleted file mode 100644 index 8d5e4f9bda..0000000000 --- a/src/backend/core/tests/documents/test_api_documents_mask.py +++ /dev/null @@ -1,353 +0,0 @@ -"""Test mask document API endpoint for users in impress's core app.""" - -import pytest -from rest_framework.test import APIClient - -from core import factories, models - -pytestmark = pytest.mark.django_db - - -@pytest.mark.parametrize( - "reach", - [ - "restricted", - "authenticated", - "public", - ], -) -@pytest.mark.parametrize("method", ["post", "delete"]) -def test_api_document_mask_anonymous_user(method, reach): - """Anonymous users should not be able to mask/unmask documents.""" - document = factories.DocumentFactory(link_reach=reach) - - response = getattr(APIClient(), method)( - f"/api/v1.0/documents/{document.id!s}/mask/" - ) - - assert response.status_code == 401 - assert response.json() == { - "detail": "Authentication credentials were not provided." - } - - # Verify in database - assert models.LinkTrace.objects.exists() is False - - -@pytest.mark.parametrize( - "reach, has_role", - [ - ["restricted", True], - ["authenticated", False], - ["authenticated", True], - ["public", False], - ["public", True], - ], -) -def test_api_document_mask_authenticated_post_allowed(reach, has_role): - """Authenticated users should be able to mask a document to which they have access.""" - user = factories.UserFactory() - client = APIClient() - client.force_login(user) - - document = factories.DocumentFactory(link_reach=reach) - if has_role: - models.DocumentAccess.objects.create(document=document, user=user) - - # Try masking the document without a link trace - response = client.post(f"/api/v1.0/documents/{document.id!s}/mask/") - assert response.status_code == 400 - assert response.json() == {"detail": "User never accessed this document before."} - assert not models.LinkTrace.objects.filter(document=document, user=user).exists() - - models.LinkTrace.objects.create(document=document, user=user) - # Mask document - response = client.post(f"/api/v1.0/documents/{document.id!s}/mask/") - - assert response.status_code == 201 - assert response.json() == {"detail": "Document was masked"} - assert models.LinkTrace.objects.filter( - document=document, user=user, is_masked=True - ).exists() - - -def test_api_document_mask_authenticated_post_forbidden(): - """ - Authenticated users should no be allowed to mask a document - to which they don't have access. - """ - user = factories.UserFactory() - client = APIClient() - client.force_login(user) - - document = factories.DocumentFactory(link_reach="restricted") - - # Try masking - response = client.post(f"/api/v1.0/documents/{document.id!s}/mask/") - - assert response.status_code == 403 - assert response.json() == { - "detail": "You do not have permission to perform this action." - } - - # Verify in database - assert ( - models.LinkTrace.objects.filter(document=document, user=user).exists() is False - ) - - -@pytest.mark.parametrize( - "reach, has_role", - [ - ["restricted", True], - ["authenticated", False], - ["authenticated", True], - ["public", False], - ["public", True], - ], -) -def test_api_document_mask_authenticated_post_already_masked_allowed(reach, has_role): - """POST should not create duplicate link trace if already marked.""" - user = factories.UserFactory() - client = APIClient() - client.force_login(user) - - document = factories.DocumentFactory(link_reach=reach, masked_by=[user]) - if has_role: - models.DocumentAccess.objects.create(document=document, user=user) - - # Try masking again - response = client.post(f"/api/v1.0/documents/{document.id!s}/mask/") - - assert response.status_code == 200 - assert response.json() == {"detail": "Document was already masked"} - assert models.LinkTrace.objects.filter( - document=document, user=user, is_masked=True - ).exists() - - -def test_api_document_mask_authenticated_post_already_masked_forbidden(): - """POST should not create duplicate masks if already marked.""" - user = factories.UserFactory() - client = APIClient() - client.force_login(user) - - document = factories.DocumentFactory(link_reach="restricted", masked_by=[user]) - # Try masking again - response = client.post(f"/api/v1.0/documents/{document.id!s}/mask/") - - assert response.status_code == 403 - assert response.json() == { - "detail": "You do not have permission to perform this action." - } - assert models.LinkTrace.objects.filter(document=document, user=user).exists() - - -@pytest.mark.parametrize( - "reach, has_role", - [ - ["restricted", True], - ["authenticated", False], - ["authenticated", True], - ["public", False], - ["public", True], - ], -) -def test_api_document_mask_authenticated_post_unmasked_allowed(reach, has_role): - """POST should not create duplicate link trace if unmasked.""" - user = factories.UserFactory() - client = APIClient() - client.force_login(user) - - document = factories.DocumentFactory(link_reach=reach) - models.LinkTrace.objects.create(document=document, user=user, is_masked=False) - if has_role: - models.DocumentAccess.objects.create(document=document, user=user) - - # Try masking again - response = client.post(f"/api/v1.0/documents/{document.id!s}/mask/") - - assert response.status_code == 201 - assert response.json() == {"detail": "Document was masked"} - assert models.LinkTrace.objects.filter( - document=document, user=user, is_masked=True - ).exists() - - -def test_api_document_mask_authenticated_post_unmasked_forbidden(): - """POST should not create duplicate masks if unmasked.""" - user = factories.UserFactory() - client = APIClient() - client.force_login(user) - - document = factories.DocumentFactory(link_reach="restricted") - models.LinkTrace.objects.create(document=document, user=user, is_masked=False) - # Try masking again - response = client.post(f"/api/v1.0/documents/{document.id!s}/mask/") - - assert response.status_code == 403 - assert response.json() == { - "detail": "You do not have permission to perform this action." - } - assert models.LinkTrace.objects.filter( - document=document, user=user, is_masked=False - ).exists() - - -@pytest.mark.parametrize( - "reach, has_role", - [ - ["restricted", True], - ["authenticated", False], - ["authenticated", True], - ["public", False], - ["public", True], - ], -) -def test_api_document_mask_authenticated_delete_allowed(reach, has_role): - """Authenticated users should be able to unmask a document using DELETE.""" - user = factories.UserFactory() - client = APIClient() - client.force_login(user) - - document = factories.DocumentFactory(link_reach=reach, masked_by=[user]) - if has_role: - models.DocumentAccess.objects.create(document=document, user=user) - - # Unmask document - response = client.delete(f"/api/v1.0/documents/{document.id!s}/mask/") - - assert response.status_code == 204 - assert response.content == b"" # No body - assert response.text == "" # Empty decoded text - assert "Content-Type" not in response.headers # No Content-Type for 204 - - assert models.LinkTrace.objects.filter( - document=document, user=user, is_masked=False - ).exists() - - -def test_api_document_mask_authenticated_delete_forbidden(): - """ - Authenticated users should not be allowed to unmask a document if - they don't have access to it. - """ - user = factories.UserFactory() - client = APIClient() - client.force_login(user) - - document = factories.DocumentFactory(link_reach="restricted", masked_by=[user]) - - # Unmask document - response = client.delete(f"/api/v1.0/documents/{document.id!s}/mask/") - - assert response.status_code == 403 - assert response.json() == { - "detail": "You do not have permission to perform this action." - } - assert models.LinkTrace.objects.filter( - document=document, user=user, is_masked=True - ).exists() - - -@pytest.mark.parametrize( - "reach, has_role", - [ - ["restricted", True], - ["authenticated", False], - ["authenticated", True], - ["public", False], - ["public", True], - ], -) -def test_api_document_mask_authenticated_delete_not_masked_allowed(reach, has_role): - """DELETE should be idempotent if the document is not masked.""" - user = factories.UserFactory() - client = APIClient() - client.force_login(user) - - document = factories.DocumentFactory(link_reach=reach) - if has_role: - models.DocumentAccess.objects.create(document=document, user=user) - - # Try unmasking the document without a link trace - response = client.delete(f"/api/v1.0/documents/{document.id!s}/mask/") - assert response.status_code == 400 - assert response.json() == {"detail": "User never accessed this document before."} - assert not models.LinkTrace.objects.filter(document=document, user=user).exists() - - models.LinkTrace.objects.create(document=document, user=user, is_masked=False) - # Unmask document - response = client.delete(f"/api/v1.0/documents/{document.id!s}/mask/") - - assert response.status_code == 200 - assert response.json() == {"detail": "Document was already not masked"} - assert models.LinkTrace.objects.filter( - document=document, user=user, is_masked=False - ).exists() - - -def test_api_document_mask_authenticated_delete_not_masked_forbidden(): - """DELETE should be idempotent if the document is not masked.""" - user = factories.UserFactory() - client = APIClient() - client.force_login(user) - - document = factories.DocumentFactory(link_reach="restricted") - - # Try to unmask when no entry exists - response = client.delete(f"/api/v1.0/documents/{document.id!s}/mask/") - - assert response.status_code == 403 - assert response.json() == { - "detail": "You do not have permission to perform this action." - } - assert ( - models.LinkTrace.objects.filter(document=document, user=user).exists() is False - ) - - -@pytest.mark.parametrize( - "reach, has_role", - [ - ["restricted", True], - ["authenticated", False], - ["authenticated", True], - ["public", False], - ["public", True], - ], -) -def test_api_document_mask_authenticated_post_unmark_then_mark_again_allowed( - reach, has_role -): - """A user should be able to mask, unmask, and mask a document again.""" - user = factories.UserFactory() - client = APIClient() - client.force_login(user) - - document = factories.DocumentFactory(link_reach=reach) - if has_role: - models.DocumentAccess.objects.create(document=document, user=user) - models.LinkTrace.objects.create(document=document, user=user, is_masked=False) - - url = f"/api/v1.0/documents/{document.id!s}/mask/" - - # Mask document - response = client.post(url) - assert response.status_code == 201 - - # Unmask document - response = client.delete(url) - assert response.status_code == 204 - assert response.content == b"" # No body - assert response.text == "" # Empty decoded text - assert "Content-Type" not in response.headers # No Content-Type for 204 - - # Mask document again - response = client.post(url) - assert response.status_code == 201 - assert response.json() == {"detail": "Document was masked"} - - assert models.LinkTrace.objects.filter( - document=document, user=user, is_masked=True - ).exists() diff --git a/src/backend/core/tests/documents/test_api_documents_retrieve.py b/src/backend/core/tests/documents/test_api_documents_retrieve.py index 5e3a596ec7..b209fb279c 100644 --- a/src/backend/core/tests/documents/test_api_documents_retrieve.py +++ b/src/backend/core/tests/documents/test_api_documents_retrieve.py @@ -52,7 +52,6 @@ def test_api_documents_retrieve_anonymous_public_standalone(): "public": ["reader", "commenter", "editor"], "restricted": None, }, - "mask": False, "content_patch": document.link_role == "editor", "content_retrieve": True, "media_auth": True, @@ -131,7 +130,6 @@ def test_api_documents_retrieve_anonymous_public_parent(): "link_select_options": models.LinkReachChoices.get_select_options( **links_definition ), - "mask": False, "content_patch": grand_parent.link_role == "editor", "content_retrieve": True, "media_auth": True, @@ -243,7 +241,6 @@ def test_api_documents_retrieve_authenticated_unrelated_public_or_authenticated( "public": ["reader", "commenter", "editor"], "restricted": None, }, - "mask": True, "content_patch": document.link_role == "editor", "content_retrieve": True, "media_auth": True, @@ -329,7 +326,6 @@ def test_api_documents_retrieve_authenticated_public_or_authenticated_parent(rea "link_select_options": models.LinkReachChoices.get_select_options( **links_definition ), - "mask": True, "move": False, "content_patch": grand_parent.link_role == "editor", "content_retrieve": True, @@ -529,7 +525,6 @@ def test_api_documents_retrieve_authenticated_related_parent(): "link_select_options": models.LinkReachChoices.get_select_options( **link_definition ), - "mask": True, "content_patch": access.role not in ["reader", "commenter"], "content_retrieve": True, "media_auth": True, diff --git a/src/backend/core/tests/documents/test_api_documents_trashbin.py b/src/backend/core/tests/documents/test_api_documents_trashbin.py index d6e69b7324..7b90eeeb91 100644 --- a/src/backend/core/tests/documents/test_api_documents_trashbin.py +++ b/src/backend/core/tests/documents/test_api_documents_trashbin.py @@ -94,7 +94,6 @@ def test_api_documents_trashbin_format(): "public": ["reader", "commenter", "editor"], "restricted": None, }, - "mask": False, "content_patch": False, "content_retrieve": True, "media_auth": False, diff --git a/src/backend/core/tests/test_models_documents.py b/src/backend/core/tests/test_models_documents.py index 0c5c8f3cd5..b1b12aece6 100644 --- a/src/backend/core/tests/test_models_documents.py +++ b/src/backend/core/tests/test_models_documents.py @@ -171,7 +171,6 @@ def test_models_documents_get_abilities_forbidden( "favorite": False, "comment": False, "invite_owner": False, - "mask": False, "content_patch": False, "content_retrieve": False, "media_auth": False, @@ -246,7 +245,6 @@ def test_models_documents_get_abilities_reader( "public": ["reader", "commenter", "editor"], "restricted": None, }, - "mask": is_authenticated, "content_patch": False, "content_retrieve": True, "media_auth": True, @@ -320,7 +318,6 @@ def test_models_documents_get_abilities_commenter( "public": ["reader", "commenter", "editor"], "restricted": None, }, - "mask": is_authenticated, "content_patch": False, "content_retrieve": True, "media_auth": True, @@ -391,7 +388,6 @@ def test_models_documents_get_abilities_editor( "public": ["reader", "commenter", "editor"], "restricted": None, }, - "mask": is_authenticated, "content_patch": True, "content_retrieve": True, "media_auth": True, @@ -451,7 +447,6 @@ def test_models_documents_get_abilities_owner(django_assert_num_queries): "public": ["reader", "commenter", "editor"], "restricted": None, }, - "mask": True, "content_patch": True, "content_retrieve": True, "media_auth": True, @@ -497,7 +492,6 @@ def test_models_documents_get_abilities_owner(django_assert_num_queries): "public": ["reader", "commenter", "editor"], "restricted": None, }, - "mask": False, "content_patch": False, "content_retrieve": True, "media_auth": False, @@ -547,7 +541,6 @@ def test_models_documents_get_abilities_administrator(django_assert_num_queries) "public": ["reader", "commenter", "editor"], "restricted": None, }, - "mask": True, "content_patch": True, "content_retrieve": True, "media_auth": True, @@ -607,7 +600,6 @@ def test_models_documents_get_abilities_editor_user(django_assert_num_queries): "public": ["reader", "commenter", "editor"], "restricted": None, }, - "mask": True, "content_patch": True, "content_retrieve": True, "media_auth": True, @@ -675,7 +667,6 @@ def test_models_documents_get_abilities_reader_user( "public": ["reader", "commenter", "editor"], "restricted": None, }, - "mask": True, "content_patch": access_from_link, "content_retrieve": True, "media_auth": True, @@ -744,7 +735,6 @@ def test_models_documents_get_abilities_commenter_user( "public": ["reader", "commenter", "editor"], "restricted": None, }, - "mask": True, "content_patch": access_from_link, "content_retrieve": True, "media_auth": True, @@ -809,7 +799,6 @@ def test_models_documents_get_abilities_preset_role(django_assert_num_queries): "public": ["reader", "commenter", "editor"], "restricted": None, }, - "mask": True, "content_patch": False, "content_retrieve": True, "media_auth": True, diff --git a/src/backend/core/tests/test_models_user_reconciliation.py b/src/backend/core/tests/test_models_user_reconciliation.py index d7a063d1a4..aefe2a155b 100644 --- a/src/backend/core/tests/test_models_user_reconciliation.py +++ b/src/backend/core/tests/test_models_user_reconciliation.py @@ -472,9 +472,7 @@ def test_process_reconciliation_updates_linktraces( models.LinkTrace.objects.create(document=doc_both, user=user_2) doc_inactive_only = userdocs_u2[4].document - models.LinkTrace.objects.create( - document=doc_inactive_only, user=user_2, is_masked=True - ) + models.LinkTrace.objects.create(document=doc_inactive_only, user=user_2) doc_active_only = userdocs_u1[4].document models.LinkTrace.objects.create(document=doc_active_only, user=user_1) @@ -515,12 +513,11 @@ def test_process_reconciliation_updates_linktraces( is False ) - # doc_inactive_only should now be linked to active user and preserve is_masked + # doc_inactive_only should now be linked to active user lt = models.LinkTrace.objects.filter( user=user_1, document=doc_inactive_only ).first() assert lt is not None - assert lt.is_masked is True # doc_active_only should still belong to active user assert models.LinkTrace.objects.filter( diff --git a/src/backend/core/tests/test_services_search_indexers.py b/src/backend/core/tests/test_services_search_indexers.py index da0406fa28..a48d2c8660 100644 --- a/src/backend/core/tests/test_services_search_indexers.py +++ b/src/backend/core/tests/test_services_search_indexers.py @@ -481,7 +481,7 @@ def test_get_visited_document_ids_of(): doc1, doc2, _ = factories.DocumentFactory.create_batch(3) - create_link = partial(models.LinkTrace.objects.create, user=user, is_masked=False) + create_link = partial(models.LinkTrace.objects.create, user=user) create_link(document=doc1) create_link(document=doc2) @@ -515,7 +515,7 @@ def test_get_visited_document_ids_of_deleted(): doc_deleted = factories.DocumentFactory() doc_ancestor_deleted = factories.DocumentFactory(parent=doc_deleted) - create_link = partial(models.LinkTrace.objects.create, user=user, is_masked=False) + create_link = partial(models.LinkTrace.objects.create, user=user) create_link(document=doc) create_link(document=doc_deleted) @@ -566,7 +566,7 @@ def test_services_search_indexers_search(mock_post, indexer_settings): doc1, doc2, _ = factories.DocumentFactory.create_batch(3) - create_link = partial(models.LinkTrace.objects.create, user=user, is_masked=False) + create_link = partial(models.LinkTrace.objects.create, user=user) create_link(document=doc1) create_link(document=doc2) @@ -607,7 +607,7 @@ def test_services_search_indexers_search_nb_results(mock_post, indexer_settings) doc1, doc2, _ = factories.DocumentFactory.create_batch(3) - create_link = partial(models.LinkTrace.objects.create, user=user, is_masked=False) + create_link = partial(models.LinkTrace.objects.create, user=user) create_link(document=doc1) create_link(document=doc2) From 1a231a1acd3f573f08f76da76baceffbcb968c69 Mon Sep 17 00:00:00 2001 From: Manuel Raynaud Date: Fri, 29 May 2026 14:32:16 +0200 Subject: [PATCH 2/2] =?UTF-8?q?=E2=9C=A8(backend)=20allow=20to=20leave=20a?= =?UTF-8?q?=20document?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We want to allow users to leave a document where they have an access or they have visited creating a link_trace. All subdocuments should also be leaved at the same time. To know if the user can leave a doc we have to check when computing the abilities if a record is existing in the LinkTrace table. This is a N+1 query situation. To avoid it, we added an annotation in the DocumentQueryset like we already do to annotate the user role. There is one edge case where the annotation is made to soon, it is when the user is visiting a document for the first time, the `get_object` add the annotation and in the permission, we compute the abilities. The `leave` property is False because the entry in the LinkTrace table is not made, when the serializer ask for the abilities again, it is still False. So in the `retrieve` method in the viewset we force the `user_has_link_trace` to the correct value. --- CHANGELOG.md | 1 + src/backend/core/api/viewsets.py | 53 +++- src/backend/core/models.py | 38 ++- .../documents/test_api_documents_leave.py | 298 ++++++++++++++++++ .../documents/test_api_documents_retrieve.py | 5 + .../documents/test_api_documents_trashbin.py | 1 + .../core/tests/test_models_documents.py | 19 +- 7 files changed, 404 insertions(+), 11 deletions(-) create mode 100644 src/backend/core/tests/documents/test_api_documents_leave.py diff --git a/CHANGELOG.md b/CHANGELOG.md index dc118a1298..318302955f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to - ✨(buildpack) add PaaS deployment support, tested with Scalingo #2293 - 🔧(backend) allow configuring settings OIDC_OP_USER_ENDPOINT_FORMAT - ⚡️(helm) create a dedicated svc and deployment for yprovider converter #2368 +- ✨(backend) allow to leave a document #2365 ### Changed diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index f180cf8dba..71efacad38 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -19,7 +19,7 @@ from django.core.exceptions import ValidationError from django.core.files.storage import default_storage from django.core.validators import URLValidator -from django.db import connection, transaction +from django.db import DatabaseError, connection, transaction from django.db import models as db from django.db.models.expressions import RawSQL from django.db.models.functions import Greatest, Left, Length @@ -598,6 +598,8 @@ def filter_queryset(self, queryset): user = self.request.user queryset = queryset.annotate_is_favorite(user) queryset = queryset.annotate_user_roles(user) + queryset = queryset.annotate_user_has_link_trace(user) + return queryset def get_response_for_queryset(self, queryset, context=None): @@ -638,6 +640,7 @@ def _compute_parents(self, documents): for parent in ( models.Document.objects.annotate_user_roles(self.request.user) .annotate_is_favorite(self.request.user) + .annotate_user_has_link_trace(self.request.user) .filter(path__in=missing_parent_paths) .iterator() ): @@ -675,7 +678,7 @@ def list(self, request, *args, **kwargs): for field in ["is_creator_me", "title", "q"]: queryset = filterset.filters[field].filter(queryset, filter_data[field]) - queryset = queryset.annotate_user_roles(user) + queryset = queryset.annotate_user_roles(user).annotate_user_has_link_trace(user) # 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. @@ -706,7 +709,6 @@ def retrieve(self, request, *args, **kwargs): """ user = self.request.user instance = self.get_object() - serializer = self.get_serializer(instance) # The `create` query generates 5 db queries which are much less efficient than an # `exists` query. The user will visit the document many times after the first visit @@ -717,6 +719,11 @@ def retrieve(self, request, *args, **kwargs): ): models.LinkTrace.objects.create(document=instance, user=request.user) + # To avoid N+1 query, we force the `user_has_link_trace` normally set by the + # queryset.annotate_user_has_link_trace method. If the user is connected, it must be True. + instance.user_has_link_trace = user.is_authenticated + + serializer = self.get_serializer(instance) return drf.response.Response(serializer.data) def _apply_uploaded_file_conversion(self, serializer): @@ -880,7 +887,7 @@ def favorite_list(self, request, *args, **kwargs): queryset = queryset.filter(id__in=favorite_documents_ids) queryset = queryset.filter(ancestors_deleted_at__isnull=True) queryset = queryset.order_by("-updated_at") - queryset = queryset.annotate_user_roles(user) + queryset = queryset.annotate_user_roles(user).annotate_user_has_link_trace(user) queryset = queryset.annotate( is_favorite=db.Value(True, output_field=db.BooleanField()) ) @@ -922,7 +929,9 @@ def trashbin(self, request, *args, **kwargs): deleted_at__isnull=False, deleted_at__gte=models.get_trashbin_cutoff(), ) - queryset = queryset.annotate_user_roles(self.request.user) + queryset = queryset.annotate_user_roles( + self.request.user + ).annotate_user_has_link_trace(self.request.user) return self.get_response_for_queryset(queryset) @@ -1174,7 +1183,7 @@ def all(self, request, *args, **kwargs): for field in ["is_creator_me", "title", "q"]: queryset = filterset.filters[field].filter(queryset, filter_data[field]) - queryset = queryset.annotate_user_roles(user) + queryset = queryset.annotate_user_roles(user).annotate_user_has_link_trace(user) # Annotate favorite status and filter if applicable as late as possible queryset = queryset.annotate_is_favorite(user) @@ -1273,6 +1282,7 @@ def tree(self, request, pk, *args, **kwargs): queryset = queryset.order_by("path") queryset = queryset.annotate_user_roles(user) queryset = queryset.annotate_is_favorite(user) + queryset = queryset.annotate_user_has_link_trace(user) # Pass ancestors' links paths mapping to the serializer as a context variable # in order to allow saving time while computing abilities on the instance @@ -1590,6 +1600,7 @@ def _search_using_database(self, request, validated_data, *args, **kwargs): .filter(ancestors_deleted_at__isnull=True) .annotate_user_roles(user) .annotate_is_favorite(user) + .annotate_user_has_link_trace(user) ) queryset = filterset.filter_queryset(queryset) @@ -2499,6 +2510,36 @@ def formatted_content(self, request, pk=None): } ) + @drf.decorators.action( + detail=True, + methods=["post"], + ) + def leave(self, request, *args, **kwargs): + """ + Remove document_accesses if exists and the link_trace related to the current document + for the connected user. + """ + # Check for permissions. + document = self.get_object() + + try: + with transaction.atomic(): + models.DocumentAccess.objects.filter( + document__path__startswith=document.path, user=request.user + ).delete() + models.LinkTrace.objects.filter( + document__path__startswith=document.path, user=request.user + ).delete() + except DatabaseError: + logger.error( + "Impossible to leave document %s for user %s", + str(document.id), + str(request.user.id), + ) + raise + + return drf.response.Response(status=drf.status.HTTP_204_NO_CONTENT) + class DocumentAccessViewSet( ResourceAccessViewsetMixin, diff --git a/src/backend/core/models.py b/src/backend/core/models.py index 9b4b8a46e3..8ae5b13127 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -852,6 +852,22 @@ def annotate_user_roles(self, user): user_roles=models.Value([], output_field=output_field), ) + def annotate_user_has_link_trace(self, user): + """ + Annotate document queryset with a boolean to know if the current user + has a link_trace on the current document. + """ + + if user.is_authenticated: + link_trace_exists_subquery = LinkTrace.objects.filter( + document_id=models.OuterRef("pk"), user=user + ) + return self.annotate( + user_has_link_trace=models.Exists(link_trace_exists_subquery) + ) + + return self.annotate(user_has_link_trace=models.Value(False)) + class DocumentManager(MP_NodeManager.from_queryset(DocumentQuerySet)): """ @@ -1150,6 +1166,17 @@ def get_role(self, user): return RoleChoices.max(*roles) + def has_link_trace(self, user): + """Return if the user has a link trace on this document.""" + + if not user.is_authenticated: + return False + + try: + return self.user_has_link_trace + except AttributeError: + return LinkTrace.objects.filter(document=self, user=user).exists() + def compute_ancestors_links_paths_mapping(self): """ Compute the ancestors links for the current document up to the highest readable ancestor. @@ -1227,7 +1254,7 @@ def computed_link_role(self): """Actual link role on the document.""" return self.computed_link_definition["link_role"] - def get_abilities(self, user): + def get_abilities(self, user): # pylint: disable=too-many-locals """ Compute and return abilities for a given user on the document. """ @@ -1248,6 +1275,14 @@ def get_abilities(self, user): is_owner_or_admin or role == RoleChoices.EDITOR ) and not is_deleted + # compute can_leave + # A user can leave a document if it has non privileged role on the document or it has + # access to it with a link_trace + can_leave = user.is_authenticated and ( + (has_access_role and not is_owner_or_admin) + or (not has_access_role and self.has_link_trace(user)) + ) + link_select_options = LinkReachChoices.get_select_options( **self.ancestors_link_definition ) @@ -1312,6 +1347,7 @@ def get_abilities(self, user): "favorite": can_get and user.is_authenticated, "link_configuration": is_owner_or_admin, "invite_owner": is_owner and not is_deleted, + "leave": can_leave, "move": is_owner_or_admin and not is_deleted, "partial_update": can_update, "restore": is_owner, diff --git a/src/backend/core/tests/documents/test_api_documents_leave.py b/src/backend/core/tests/documents/test_api_documents_leave.py new file mode 100644 index 0000000000..82c0eded40 --- /dev/null +++ b/src/backend/core/tests/documents/test_api_documents_leave.py @@ -0,0 +1,298 @@ +"""Test for the leave document API""" + +import pytest +from rest_framework import status +from rest_framework.test import APIClient + +from core import factories, models + +pytestmark = pytest.mark.django_db + + +@pytest.mark.parametrize( + "link_reach", + models.LinkReachChoices.values, +) +def test_api_documents_leave_document_anonymous_user(link_reach): + """ + Anonymous user are not allowed to access the leave feature no matter the document link reach. + """ + + document = factories.DocumentFactory(link_reach=link_reach) + + client = APIClient() + response = client.post(f"/api/v1.0/documents/{document.id!s}/leave/") + + assert response.status_code == status.HTTP_401_UNAUTHORIZED + + +@pytest.mark.parametrize( + "link_reach", + models.LinkReachChoices.values, +) +def test_api_documents_leave_connected_user_without_access_nor_link_trace(link_reach): + """ + A connected user with no access or link_trace on the document can not access the leave feature + """ + + user = factories.UserFactory() + other_users = factories.UserFactory.create_batch(3) + + document = factories.DocumentFactory(link_reach=link_reach, link_traces=other_users) + + factories.UserDocumentAccessFactory.create_batch(4, document=document) + + assert not models.LinkTrace.objects.filter(document=document, user=user).exists() + assert not models.DocumentAccess.objects.filter( + document=document, user=user + ).exists() + + assert models.LinkTrace.objects.count() == 3 + assert models.DocumentAccess.objects.count() == 4 + + client = APIClient() + client.force_login(user) + response = client.post(f"/api/v1.0/documents/{document.id!s}/leave/") + + assert response.status_code == status.HTTP_403_FORBIDDEN + + assert models.LinkTrace.objects.count() == 3 + assert models.DocumentAccess.objects.count() == 4 + + +@pytest.mark.parametrize( + "link_reach", + [models.LinkReachChoices.PUBLIC, models.LinkReachChoices.AUTHENTICATED], +) +def test_api_documents_leave_connected_user_with_link_trace(link_reach): + """ + A connected user with link_trace on a document can leave it. + """ + + user = factories.UserFactory() + other_users = factories.UserFactory.create_batch(3) + + document = factories.DocumentFactory( + link_reach=link_reach, link_traces=[user, *other_users] + ) + factories.UserDocumentAccessFactory.create_batch(4, document=document) + + assert models.LinkTrace.objects.filter(document=document, user=user).exists() + assert models.LinkTrace.objects.count() == 4 + assert models.DocumentAccess.objects.count() == 4 + + client = APIClient() + client.force_login(user) + response = client.post(f"/api/v1.0/documents/{document.id!s}/leave/") + + assert response.status_code == status.HTTP_204_NO_CONTENT + + assert not models.LinkTrace.objects.filter(document=document, user=user).exists() + assert models.LinkTrace.objects.count() == 3 + assert models.DocumentAccess.objects.count() == 4 + + +@pytest.mark.parametrize( + "link_reach", + models.LinkReachChoices.values, +) +@pytest.mark.parametrize( + "role", [role for role in models.RoleChoices if role not in models.PRIVILEGED_ROLES] +) +def test_api_documents_leave_connected_user_with_access(role, link_reach): + """Connected user with a DocumentAccess can leave it.""" + + user = factories.UserFactory() + other_users = factories.UserFactory.create_batch(3) + + document = factories.DocumentFactory( + link_reach=link_reach, link_traces=other_users, users=[(user, role)] + ) + factories.UserDocumentAccessFactory.create_batch(4, document=document) + + assert models.DocumentAccess.objects.filter(document=document, user=user).exists() + assert models.LinkTrace.objects.count() == 3 + assert models.DocumentAccess.objects.count() == 5 + + client = APIClient() + client.force_login(user) + response = client.post(f"/api/v1.0/documents/{document.id!s}/leave/") + + assert response.status_code == status.HTTP_204_NO_CONTENT + + assert not models.DocumentAccess.objects.filter( + document=document, user=user + ).exists() + assert models.LinkTrace.objects.count() == 3 + assert models.DocumentAccess.objects.count() == 4 + + +@pytest.mark.parametrize( + "link_reach", + models.LinkReachChoices.values, +) +@pytest.mark.parametrize( + "role", [role for role in models.RoleChoices if role in models.PRIVILEGED_ROLES] +) +def test_api_documents_leave_connected_access_with_privileged_role_not_allowed( + role, link_reach +): + """Connected user with privileged access role can not leave a document.""" + + user = factories.UserFactory() + other_users = factories.UserFactory.create_batch(3) + + document = factories.DocumentFactory( + link_reach=link_reach, link_traces=[user, *other_users], users=[(user, role)] + ) + factories.UserDocumentAccessFactory.create_batch(4, document=document) + + assert models.DocumentAccess.objects.filter(document=document, user=user).exists() + assert models.LinkTrace.objects.count() == 4 + assert models.DocumentAccess.objects.count() == 5 + + client = APIClient() + client.force_login(user) + response = client.post(f"/api/v1.0/documents/{document.id!s}/leave/") + + assert response.status_code == status.HTTP_403_FORBIDDEN + + assert models.DocumentAccess.objects.filter(document=document, user=user).exists() + assert models.LinkTrace.objects.count() == 4 + assert models.DocumentAccess.objects.count() == 5 + + +@pytest.mark.parametrize( + "link_reach", + models.LinkReachChoices.values, +) +@pytest.mark.parametrize( + "role", [role for role in models.RoleChoices if role not in models.PRIVILEGED_ROLES] +) +def test_api_documents_leave_connected_user_with_access_and_link_trace( + role, link_reach +): + """Connected user with a DocumentAccess can leave it.""" + + user = factories.UserFactory() + other_users = factories.UserFactory.create_batch(3) + + document = factories.DocumentFactory( + link_reach=link_reach, link_traces=[user, *other_users], users=[(user, role)] + ) + factories.UserDocumentAccessFactory.create_batch(4, document=document) + + assert models.DocumentAccess.objects.filter(document=document, user=user).exists() + assert models.LinkTrace.objects.filter(document=document, user=user).exists() + assert models.LinkTrace.objects.count() == 4 + assert models.DocumentAccess.objects.count() == 5 + + client = APIClient() + client.force_login(user) + response = client.post(f"/api/v1.0/documents/{document.id!s}/leave/") + + assert response.status_code == status.HTTP_204_NO_CONTENT + + assert not models.DocumentAccess.objects.filter( + document=document, user=user + ).exists() + assert not models.LinkTrace.objects.filter(document=document, user=user).exists() + assert models.LinkTrace.objects.count() == 3 + assert models.DocumentAccess.objects.count() == 4 + + +@pytest.mark.parametrize( + "link_reach", + models.LinkReachChoices.values, +) +@pytest.mark.parametrize( + "role", [role for role in models.RoleChoices if role not in models.PRIVILEGED_ROLES] +) +def test_api_documents_leave_connected_accessing_multiple_documents_leave_only_one( + role, link_reach +): + """Connected user accessing multiple document leaving one should keep access to the others.""" + + user = factories.UserFactory() + other_users = factories.UserFactory.create_batch(3) + + document = factories.DocumentFactory( + link_reach=link_reach, link_traces=[user, *other_users], users=[(user, role)] + ) + factories.UserDocumentAccessFactory.create_batch(4, document=document) + + # Create access to other documents for the same user + factories.UserDocumentAccessFactory.create_batch(4, user=user) + + assert models.DocumentAccess.objects.filter(document=document, user=user).exists() + assert models.LinkTrace.objects.filter(document=document, user=user).exists() + assert models.LinkTrace.objects.count() == 4 + assert models.DocumentAccess.objects.count() == 9 + + client = APIClient() + client.force_login(user) + response = client.post(f"/api/v1.0/documents/{document.id!s}/leave/") + + assert response.status_code == status.HTTP_204_NO_CONTENT + + assert not models.DocumentAccess.objects.filter( + document=document, user=user + ).exists() + assert not models.LinkTrace.objects.filter(document=document, user=user).exists() + assert models.LinkTrace.objects.count() == 3 + assert models.DocumentAccess.objects.count() == 8 + + +@pytest.mark.parametrize( + "link_reach", + models.LinkReachChoices.values, +) +@pytest.mark.parametrize( + "role", [role for role in models.RoleChoices if role not in models.PRIVILEGED_ROLES] +) +def test_api_documents_leave_connected_leave_also_sub_documents(role, link_reach): + """User connected with access and link_trace to a tree should leave all the tree.""" + + user = factories.UserFactory() + other_users = factories.UserFactory.create_batch(3) + + document = factories.DocumentFactory( + link_reach=link_reach, link_traces=[user, *other_users], users=[(user, role)] + ) + child = factories.DocumentFactory(parent=document, link_traces=[user, *other_users]) + grand_child = factories.DocumentFactory( + parent=child, link_traces=[user, *other_users], users=[(user, role)] + ) + + factories.UserDocumentAccessFactory.create_batch(4, document=document) + + # Create access to other documents for the same user + factories.UserDocumentAccessFactory.create_batch(4, user=user) + + assert models.DocumentAccess.objects.filter(document=document, user=user).exists() + assert models.DocumentAccess.objects.filter( + document=grand_child, user=user + ).exists() + assert models.LinkTrace.objects.filter(document=document, user=user).exists() + assert models.LinkTrace.objects.filter(document=child, user=user).exists() + assert models.LinkTrace.objects.filter(document=grand_child, user=user).exists() + assert models.LinkTrace.objects.count() == 12 + assert models.DocumentAccess.objects.count() == 10 + + client = APIClient() + client.force_login(user) + response = client.post(f"/api/v1.0/documents/{document.id!s}/leave/") + + assert response.status_code == status.HTTP_204_NO_CONTENT + + assert not models.DocumentAccess.objects.filter( + document=document, user=user + ).exists() + assert not models.DocumentAccess.objects.filter( + document=grand_child, user=user + ).exists() + assert not models.LinkTrace.objects.filter(document=document, user=user).exists() + assert not models.LinkTrace.objects.filter(document=child, user=user).exists() + assert not models.LinkTrace.objects.filter(document=grand_child, user=user).exists() + assert models.LinkTrace.objects.count() == 9 + assert models.DocumentAccess.objects.count() == 8 diff --git a/src/backend/core/tests/documents/test_api_documents_retrieve.py b/src/backend/core/tests/documents/test_api_documents_retrieve.py index b209fb279c..db7907ffb9 100644 --- a/src/backend/core/tests/documents/test_api_documents_retrieve.py +++ b/src/backend/core/tests/documents/test_api_documents_retrieve.py @@ -54,6 +54,7 @@ def test_api_documents_retrieve_anonymous_public_standalone(): }, "content_patch": document.link_role == "editor", "content_retrieve": True, + "leave": False, "media_auth": True, "media_check": True, "move": False, @@ -132,6 +133,7 @@ def test_api_documents_retrieve_anonymous_public_parent(): ), "content_patch": grand_parent.link_role == "editor", "content_retrieve": True, + "leave": False, "media_auth": True, "media_check": True, "move": False, @@ -243,6 +245,7 @@ def test_api_documents_retrieve_authenticated_unrelated_public_or_authenticated( }, "content_patch": document.link_role == "editor", "content_retrieve": True, + "leave": True, "media_auth": True, "media_check": True, "move": False, @@ -329,6 +332,7 @@ def test_api_documents_retrieve_authenticated_public_or_authenticated_parent(rea "move": False, "content_patch": grand_parent.link_role == "editor", "content_retrieve": True, + "leave": True, "media_auth": True, "media_check": True, "partial_update": grand_parent.link_role == "editor", @@ -527,6 +531,7 @@ def test_api_documents_retrieve_authenticated_related_parent(): ), "content_patch": access.role not in ["reader", "commenter"], "content_retrieve": True, + "leave": access.role not in ["administrator", "owner"], "media_auth": True, "media_check": True, "move": access.role in ["administrator", "owner"], diff --git a/src/backend/core/tests/documents/test_api_documents_trashbin.py b/src/backend/core/tests/documents/test_api_documents_trashbin.py index 7b90eeeb91..d7b61266f8 100644 --- a/src/backend/core/tests/documents/test_api_documents_trashbin.py +++ b/src/backend/core/tests/documents/test_api_documents_trashbin.py @@ -96,6 +96,7 @@ def test_api_documents_trashbin_format(): }, "content_patch": False, "content_retrieve": True, + "leave": False, "media_auth": False, "media_check": False, "move": False, # Can't move a deleted document diff --git a/src/backend/core/tests/test_models_documents.py b/src/backend/core/tests/test_models_documents.py index b1b12aece6..e9405ce38f 100644 --- a/src/backend/core/tests/test_models_documents.py +++ b/src/backend/core/tests/test_models_documents.py @@ -173,6 +173,7 @@ def test_models_documents_get_abilities_forbidden( "invite_owner": False, "content_patch": False, "content_retrieve": False, + "leave": False, "media_auth": False, "media_check": False, "move": False, @@ -192,7 +193,7 @@ def test_models_documents_get_abilities_forbidden( "versions_retrieve": False, "search": False, } - nb_queries = 1 if is_authenticated else 0 + nb_queries = 2 if is_authenticated else 0 with django_assert_num_queries(nb_queries): assert document.get_abilities(user) == expected_abilities document.soft_delete() @@ -247,6 +248,7 @@ def test_models_documents_get_abilities_reader( }, "content_patch": False, "content_retrieve": True, + "leave": False, "media_auth": True, "media_check": True, "move": False, @@ -260,7 +262,7 @@ def test_models_documents_get_abilities_reader( "versions_retrieve": False, "search": True, } - nb_queries = 1 if is_authenticated else 0 + nb_queries = 2 if is_authenticated else 0 with django_assert_num_queries(nb_queries): assert document.get_abilities(user) == expected_abilities @@ -320,6 +322,7 @@ def test_models_documents_get_abilities_commenter( }, "content_patch": False, "content_retrieve": True, + "leave": False, "media_auth": True, "media_check": True, "move": False, @@ -333,7 +336,7 @@ def test_models_documents_get_abilities_commenter( "versions_retrieve": False, "search": True, } - nb_queries = 1 if is_authenticated else 0 + nb_queries = 2 if is_authenticated else 0 with django_assert_num_queries(nb_queries): assert document.get_abilities(user) == expected_abilities @@ -390,6 +393,7 @@ def test_models_documents_get_abilities_editor( }, "content_patch": True, "content_retrieve": True, + "leave": False, "media_auth": True, "media_check": True, "move": False, @@ -403,7 +407,7 @@ def test_models_documents_get_abilities_editor( "versions_retrieve": False, "search": True, } - nb_queries = 1 if is_authenticated else 0 + nb_queries = 2 if is_authenticated else 0 with django_assert_num_queries(nb_queries): assert document.get_abilities(user) == expected_abilities document.soft_delete() @@ -449,6 +453,7 @@ def test_models_documents_get_abilities_owner(django_assert_num_queries): }, "content_patch": True, "content_retrieve": True, + "leave": False, "media_auth": True, "media_check": True, "move": True, @@ -494,6 +499,7 @@ def test_models_documents_get_abilities_owner(django_assert_num_queries): }, "content_patch": False, "content_retrieve": True, + "leave": False, "media_auth": False, "media_check": False, "move": False, @@ -543,6 +549,7 @@ def test_models_documents_get_abilities_administrator(django_assert_num_queries) }, "content_patch": True, "content_retrieve": True, + "leave": False, "media_auth": True, "media_check": True, "move": True, @@ -602,6 +609,7 @@ def test_models_documents_get_abilities_editor_user(django_assert_num_queries): }, "content_patch": True, "content_retrieve": True, + "leave": True, "media_auth": True, "media_check": True, "move": False, @@ -669,6 +677,7 @@ def test_models_documents_get_abilities_reader_user( }, "content_patch": access_from_link, "content_retrieve": True, + "leave": True, "media_auth": True, "media_check": True, "move": False, @@ -737,6 +746,7 @@ def test_models_documents_get_abilities_commenter_user( }, "content_patch": access_from_link, "content_retrieve": True, + "leave": True, "media_auth": True, "media_check": True, "move": False, @@ -801,6 +811,7 @@ def test_models_documents_get_abilities_preset_role(django_assert_num_queries): }, "content_patch": False, "content_retrieve": True, + "leave": True, "media_auth": True, "media_check": True, "move": False,