From 2a90736430d1fab8f8895b548229cea0da4b0377 Mon Sep 17 00:00:00 2001 From: kdeterme Date: Thu, 6 Aug 2026 13:28:45 +0200 Subject: [PATCH] feat: consolidate document deletion logic and expose deletion UI based on granular permissions --- streetup/documents/permissions.py | 19 ++-- .../templates/documents/document_detail.html | 2 + .../templates/documents/document_list.html | 2 +- .../documents/templatetags/documents_tags.py | 18 +++- streetup/documents/tests.py | 89 ++++++++++++------- streetup/documents/views.py | 15 ++-- 6 files changed, 98 insertions(+), 47 deletions(-) diff --git a/streetup/documents/permissions.py b/streetup/documents/permissions.py index 2f27a3b..e0d594e 100644 --- a/streetup/documents/permissions.py +++ b/streetup/documents/permissions.py @@ -159,11 +159,20 @@ def user_is_internal_manager(user) -> bool: return config.roles.filter(name="manager").exists() -def document_has_project_or_intervention_attachment(document: ManagedDocument) -> bool: - return ( - document.attachments.filter(content_type__app_label="projects", content_type__model="project").exists() - or document.attachments.filter(content_type__app_label="interventions", content_type__model="intervention").exists() - ) +def user_can_delete_document( + user: settings.AUTH_USER_MODEL | AnonymousUser, + document: ManagedDocument, +) -> bool: + """Return ``True`` if ``user`` can delete ``document``.""" + if user is None or not getattr(user, "is_authenticated", False): + return False + if user_is_documents_admin(user): + return True + if document.created_by_id and document.created_by_id == getattr(user, "pk", None): + return True + return user_has_document_permission(user, document, required_permission=ManagedDocument.PERMISSION_EDIT) + + def _get_accessible_thematic_ids(user) -> set[int]: diff --git a/streetup/documents/templates/documents/document_detail.html b/streetup/documents/templates/documents/document_detail.html index f48bc58..cce1bd0 100644 --- a/streetup/documents/templates/documents/document_detail.html +++ b/streetup/documents/templates/documents/document_detail.html @@ -42,6 +42,8 @@ + {% endif %} + {% if can_delete_document %} {% translate "Delete" %} {% endif %} diff --git a/streetup/documents/templates/documents/document_list.html b/streetup/documents/templates/documents/document_list.html index 8d2c833..47d48bf 100644 --- a/streetup/documents/templates/documents/document_list.html +++ b/streetup/documents/templates/documents/document_list.html @@ -205,7 +205,7 @@ {% endfor %} - {% if document.created_by_id == request.user.id or is_documents_admin %} + {% if item.can_delete %} diff --git a/streetup/documents/templatetags/documents_tags.py b/streetup/documents/templatetags/documents_tags.py index f51ad61..3264ee7 100644 --- a/streetup/documents/templatetags/documents_tags.py +++ b/streetup/documents/templatetags/documents_tags.py @@ -46,4 +46,20 @@ def render_folder_tree(nodes, active_slug=None, trail_slugs=None): "nodes": nodes or [], "active_slug": active_slug, "trail_slugs": trail_slugs or [], - } \ No newline at end of file + } + + +@register.filter +def can_delete_document(document, user): + from documents.permissions import user_can_delete_document + if not document or not user: + return False + return user_can_delete_document(user, document) + + +@register.simple_tag +def user_can_delete_document_tag(user, document): + from documents.permissions import user_can_delete_document + if not document or not user: + return False + return user_can_delete_document(user, document) \ No newline at end of file diff --git a/streetup/documents/tests.py b/streetup/documents/tests.py index d780f8f..da13a9e 100644 --- a/streetup/documents/tests.py +++ b/streetup/documents/tests.py @@ -395,36 +395,6 @@ class DocumentDeletionPermissionTests(TestCase): created_by=self.owner, ) - def test_internal_manager_can_delete_project_document(self): - document = ManagedDocument.objects.create(title="Project document", created_by=self.owner) - DocumentAttachment.objects.create( - document=document, - content_type=ContentType.objects.get_for_model(Project), - object_id=self.project.pk, - ) - - ensure_document_deletable(document, self.manager) - - def test_internal_manager_can_delete_intervention_document(self): - document = ManagedDocument.objects.create(title="Intervention document", created_by=self.owner) - DocumentAttachment.objects.create( - document=document, - content_type=ContentType.objects.get_for_model(Intervention), - object_id=self.intervention.pk, - ) - - ensure_document_deletable(document, self.manager) - self.client.force_login(self.manager) - response = self.client.post( - reverse("documents:folder_edit", args=[self.folder.pk]), - { - "name": "Still locked", - "description": "", - "application_label": "", - "parent_folders": [], - }, - ) - self.assertEqual(response.status_code, 403) def test_internal_manager_can_delete_thematic_asset_document_owned_by_others(self): from common.models import Thematic, UserThematics @@ -484,6 +454,65 @@ class DocumentDeletionPermissionTests(TestCase): with self.assertRaises(PermissionDenied): ensure_document_deletable(other_doc, self.manager) + def test_document_list_view_renders_delete_button_when_user_has_permission(self): + from common.models import Thematic, UserThematics + from assets.models import TrafficLightIntersection + from documents.models import DocumentFolderAttachment + + thematic, _ = Thematic.objects.get_or_create( + code="trafficlights", + defaults={"name_fr": "Signalisation Lumineuse Tricolore", "name_nl": "Verkeerslichten"} + ) + UserThematics.objects.create(user_config=self.manager_config, thematic=thematic, can_edit_assets=True) + + intersection = TrafficLightIntersection.objects.create(code="TL-LIST-1", name_fr="Carrefour List 1") + root_folder = DocumentFolder.objects.create(name="Root Intersection Folder List", created_by=self.owner) + + DocumentFolderAttachment.objects.create( + folder=root_folder, + content_type=ContentType.objects.get_for_model(TrafficLightIntersection), + object_id=intersection.pk, + ) + + other_doc = ManagedDocument.objects.create(title="Other User Doc List Test", created_by=self.owner) + other_doc.folders.add(root_folder) + + self.client.force_login(self.manager) + response = self.client.get(reverse("documents:list")) + self.assertEqual(response.status_code, 200) + delete_url = reverse("documents:delete", args=[other_doc.pk]) + self.assertContains(response, delete_url) + + def test_document_list_view_hides_delete_button_when_user_lacks_permission(self): + from common.models import Thematic, UserThematics + from assets.models import TrafficLightIntersection + from documents.models import DocumentFolderAttachment + + thematic, _ = Thematic.objects.get_or_create( + code="trafficlights", + defaults={"name_fr": "Signalisation Lumineuse Tricolore", "name_nl": "Verkeerslichten"} + ) + UserThematics.objects.create(user_config=self.manager_config, thematic=thematic, can_edit_assets=False) + + intersection = TrafficLightIntersection.objects.create(code="TL-LIST-2", name_fr="Carrefour List 2") + root_folder = DocumentFolder.objects.create(name="Root Intersection Folder List 2", created_by=self.owner) + + DocumentFolderAttachment.objects.create( + folder=root_folder, + content_type=ContentType.objects.get_for_model(TrafficLightIntersection), + object_id=intersection.pk, + ) + + other_doc = ManagedDocument.objects.create(title="Other User Doc List Test 2", created_by=self.owner) + other_doc.folders.add(root_folder) + + self.client.force_login(self.manager) + response = self.client.get(reverse("documents:list")) + self.assertEqual(response.status_code, 200) + delete_url = reverse("documents:delete", args=[other_doc.pk]) + self.assertNotContains(response, delete_url) + + class SearchObjectsViewTests(TestCase): def setUp(self): diff --git a/streetup/documents/views.py b/streetup/documents/views.py index 6b255d5..8ba7e6d 100644 --- a/streetup/documents/views.py +++ b/streetup/documents/views.py @@ -40,10 +40,10 @@ from .models import ( ManagedDocument, ) from .permissions import ( - document_has_project_or_intervention_attachment, filter_documents_for_user, filter_folders_for_user, get_user_visible_folder_ids, + user_can_delete_document, user_is_internal_manager, user_has_document_permission, user_is_documents_admin, @@ -224,15 +224,8 @@ def ensure_folder_editable(folder, user): def ensure_document_deletable(document, user): """Vérifie si l'utilisateur peut supprimer le document.""" - if user_is_documents_admin(user): - return - if user_is_internal_manager(user) and document_has_project_or_intervention_attachment(document): - return - if document.created_by_id and document.created_by_id == getattr(user, "pk", None): - return - if user_has_document_permission(user, document, required_permission=ManagedDocument.PERMISSION_EDIT): - return - raise PermissionDenied + if not user_can_delete_document(user, document): + raise PermissionDenied def ensure_folder_shareable(folder, user): @@ -432,6 +425,7 @@ class DocumentListView(DocumentsAppAccessMixin, LoginRequiredMixin, ListView): { "type": "document", "object": document, + "can_delete": user_can_delete_document(self.request.user, document), "updated_at": document.updated_at, "updated_by": document.latest_version.uploaded_by.get_short_name() if document.latest_version and document.latest_version.uploaded_by else "/", "name_key": document.title.casefold(), @@ -1069,6 +1063,7 @@ class DocumentDetailView(DocumentsAppAccessMixin, LoginRequiredMixin, DetailView document, required_permission=ManagedDocument.PERMISSION_EDIT, ) + context["can_delete_document"] = user_can_delete_document(self.request.user, document) # Available folders/tags for add forms (exclude already associated ones) existing_folder_ids = list(document.folders.values_list("pk", flat=True))