From f67dd152e66d549dd0c3715c7107e808ce6dd170 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 12 Nov 2025 15:36:48 +0000 Subject: [PATCH] Changes before error encountered Co-authored-by: dawnsystem <42047891+dawnsystem@users.noreply.github.com> --- src/documents/serialisers.py | 58 +++ .../tests/test_api_deletion_requests.py | 359 ++++++++++++++++++ src/documents/views/__init__.py | 5 + src/documents/views/deletion_request.py | 262 +++++++++++++ src/paperless/urls.py | 2 + 5 files changed, 686 insertions(+) create mode 100644 src/documents/tests/test_api_deletion_requests.py create mode 100644 src/documents/views/__init__.py create mode 100644 src/documents/views/deletion_request.py diff --git a/src/documents/serialisers.py b/src/documents/serialisers.py index f04bb70da..91b60e305 100644 --- a/src/documents/serialisers.py +++ b/src/documents/serialisers.py @@ -2696,3 +2696,61 @@ class StoragePathTestSerializer(SerializerWithPerms): label="Document", write_only=True, ) + + +class DeletionRequestSerializer(serializers.ModelSerializer): + """Serializer for DeletionRequest model with document details.""" + + document_details = serializers.SerializerMethodField() + user_username = serializers.CharField(source='user.username', read_only=True) + reviewed_by_username = serializers.CharField( + source='reviewed_by.username', + read_only=True, + allow_null=True, + ) + + class Meta: + from documents.models import DeletionRequest + model = DeletionRequest + fields = [ + 'id', + 'created_at', + 'updated_at', + 'requested_by_ai', + 'ai_reason', + 'user', + 'user_username', + 'status', + 'impact_summary', + 'reviewed_at', + 'reviewed_by', + 'reviewed_by_username', + 'review_comment', + 'completed_at', + 'completion_details', + 'document_details', + ] + read_only_fields = [ + 'id', + 'created_at', + 'updated_at', + 'reviewed_at', + 'reviewed_by', + 'completed_at', + 'completion_details', + ] + + def get_document_details(self, obj): + """Get details of documents in this deletion request.""" + documents = obj.documents.all() + return [ + { + 'id': doc.id, + 'title': doc.title, + 'created': doc.created.isoformat() if doc.created else None, + 'correspondent': doc.correspondent.name if doc.correspondent else None, + 'document_type': doc.document_type.name if doc.document_type else None, + 'tags': [tag.name for tag in doc.tags.all()], + } + for doc in documents + ] diff --git a/src/documents/tests/test_api_deletion_requests.py b/src/documents/tests/test_api_deletion_requests.py new file mode 100644 index 000000000..44bd6375a --- /dev/null +++ b/src/documents/tests/test_api_deletion_requests.py @@ -0,0 +1,359 @@ +""" +API tests for DeletionRequest endpoints. + +Tests cover: +- List and retrieve deletion requests +- Approve endpoint with permissions and status validation +- Reject endpoint with permissions and status validation +- Cancel endpoint with permissions and status validation +- Permission checking (owner vs non-owner vs admin) +- Execution flow when approved +""" + +from django.contrib.auth.models import User +from django.test import override_settings +from rest_framework import status +from rest_framework.test import APITestCase + +from documents.models import ( + Correspondent, + DeletionRequest, + Document, + DocumentType, + Tag, +) + + +class TestDeletionRequestAPI(APITestCase): + """Test DeletionRequest API endpoints.""" + + def setUp(self): + """Set up test data.""" + # Create users + self.user1 = User.objects.create_user(username="user1", password="pass123") + self.user2 = User.objects.create_user(username="user2", password="pass123") + self.admin = User.objects.create_superuser(username="admin", password="admin123") + + # Create test documents + self.doc1 = Document.objects.create( + title="Test Document 1", + content="Content 1", + checksum="checksum1", + mime_type="application/pdf", + ) + self.doc2 = Document.objects.create( + title="Test Document 2", + content="Content 2", + checksum="checksum2", + mime_type="application/pdf", + ) + self.doc3 = Document.objects.create( + title="Test Document 3", + content="Content 3", + checksum="checksum3", + mime_type="application/pdf", + ) + + # Create deletion requests + self.request1 = DeletionRequest.objects.create( + requested_by_ai=True, + ai_reason="Duplicate document detected", + user=self.user1, + status=DeletionRequest.STATUS_PENDING, + impact_summary={"document_count": 1}, + ) + self.request1.documents.add(self.doc1) + + self.request2 = DeletionRequest.objects.create( + requested_by_ai=True, + ai_reason="Low quality document", + user=self.user2, + status=DeletionRequest.STATUS_PENDING, + impact_summary={"document_count": 1}, + ) + self.request2.documents.add(self.doc2) + + def test_list_deletion_requests_as_owner(self): + """Test that users can list their own deletion requests.""" + self.client.force_authenticate(user=self.user1) + response = self.client.get("/api/deletion-requests/") + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(len(response.data["results"]), 1) + self.assertEqual(response.data["results"][0]["id"], self.request1.id) + + def test_list_deletion_requests_as_admin(self): + """Test that admin can list all deletion requests.""" + self.client.force_authenticate(user=self.admin) + response = self.client.get("/api/deletion-requests/") + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(len(response.data["results"]), 2) + + def test_retrieve_deletion_request(self): + """Test retrieving a single deletion request.""" + self.client.force_authenticate(user=self.user1) + response = self.client.get(f"/api/deletion-requests/{self.request1.id}/") + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["id"], self.request1.id) + self.assertEqual(response.data["ai_reason"], "Duplicate document detected") + self.assertEqual(response.data["status"], DeletionRequest.STATUS_PENDING) + self.assertIn("document_details", response.data) + + def test_approve_deletion_request_as_owner(self): + """Test approving a deletion request as the owner.""" + self.client.force_authenticate(user=self.user1) + + # Verify document exists + self.assertTrue(Document.objects.filter(id=self.doc1.id).exists()) + + response = self.client.post( + f"/api/deletion-requests/{self.request1.id}/approve/", + {"comment": "Approved by owner"}, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertIn("message", response.data) + self.assertIn("execution_result", response.data) + self.assertEqual(response.data["execution_result"]["deleted_count"], 1) + + # Verify document was deleted + self.assertFalse(Document.objects.filter(id=self.doc1.id).exists()) + + # Verify deletion request was updated + self.request1.refresh_from_db() + self.assertEqual(self.request1.status, DeletionRequest.STATUS_COMPLETED) + self.assertIsNotNone(self.request1.reviewed_at) + self.assertEqual(self.request1.reviewed_by, self.user1) + self.assertEqual(self.request1.review_comment, "Approved by owner") + + def test_approve_deletion_request_as_admin(self): + """Test approving a deletion request as admin.""" + self.client.force_authenticate(user=self.admin) + + response = self.client.post( + f"/api/deletion-requests/{self.request2.id}/approve/", + {"comment": "Approved by admin"}, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertIn("execution_result", response.data) + + # Verify document was deleted + self.assertFalse(Document.objects.filter(id=self.doc2.id).exists()) + + # Verify deletion request was updated + self.request2.refresh_from_db() + self.assertEqual(self.request2.status, DeletionRequest.STATUS_COMPLETED) + self.assertEqual(self.request2.reviewed_by, self.admin) + + def test_approve_deletion_request_without_permission(self): + """Test that non-owners cannot approve deletion requests.""" + self.client.force_authenticate(user=self.user2) + + response = self.client.post( + f"/api/deletion-requests/{self.request1.id}/approve/", + ) + + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + # Verify document was NOT deleted + self.assertTrue(Document.objects.filter(id=self.doc1.id).exists()) + + # Verify deletion request was NOT updated + self.request1.refresh_from_db() + self.assertEqual(self.request1.status, DeletionRequest.STATUS_PENDING) + + def test_approve_already_approved_request(self): + """Test that already approved requests cannot be approved again.""" + self.request1.status = DeletionRequest.STATUS_APPROVED + self.request1.save() + + self.client.force_authenticate(user=self.user1) + + response = self.client.post( + f"/api/deletion-requests/{self.request1.id}/approve/", + ) + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn("error", response.data) + self.assertIn("pending", response.data["error"].lower()) + + def test_reject_deletion_request_as_owner(self): + """Test rejecting a deletion request as the owner.""" + self.client.force_authenticate(user=self.user1) + + response = self.client.post( + f"/api/deletion-requests/{self.request1.id}/reject/", + {"comment": "Not needed"}, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertIn("message", response.data) + + # Verify document was NOT deleted + self.assertTrue(Document.objects.filter(id=self.doc1.id).exists()) + + # Verify deletion request was updated + self.request1.refresh_from_db() + self.assertEqual(self.request1.status, DeletionRequest.STATUS_REJECTED) + self.assertIsNotNone(self.request1.reviewed_at) + self.assertEqual(self.request1.reviewed_by, self.user1) + self.assertEqual(self.request1.review_comment, "Not needed") + + def test_reject_deletion_request_as_admin(self): + """Test rejecting a deletion request as admin.""" + self.client.force_authenticate(user=self.admin) + + response = self.client.post( + f"/api/deletion-requests/{self.request2.id}/reject/", + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + + # Verify document was NOT deleted + self.assertTrue(Document.objects.filter(id=self.doc2.id).exists()) + + # Verify deletion request was updated + self.request2.refresh_from_db() + self.assertEqual(self.request2.status, DeletionRequest.STATUS_REJECTED) + self.assertEqual(self.request2.reviewed_by, self.admin) + + def test_reject_deletion_request_without_permission(self): + """Test that non-owners cannot reject deletion requests.""" + self.client.force_authenticate(user=self.user2) + + response = self.client.post( + f"/api/deletion-requests/{self.request1.id}/reject/", + ) + + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + # Verify deletion request was NOT updated + self.request1.refresh_from_db() + self.assertEqual(self.request1.status, DeletionRequest.STATUS_PENDING) + + def test_reject_already_rejected_request(self): + """Test that already rejected requests cannot be rejected again.""" + self.request1.status = DeletionRequest.STATUS_REJECTED + self.request1.save() + + self.client.force_authenticate(user=self.user1) + + response = self.client.post( + f"/api/deletion-requests/{self.request1.id}/reject/", + ) + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn("error", response.data) + + def test_cancel_deletion_request_as_owner(self): + """Test canceling a deletion request as the owner.""" + self.client.force_authenticate(user=self.user1) + + response = self.client.post( + f"/api/deletion-requests/{self.request1.id}/cancel/", + {"comment": "Changed my mind"}, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertIn("message", response.data) + + # Verify document was NOT deleted + self.assertTrue(Document.objects.filter(id=self.doc1.id).exists()) + + # Verify deletion request was updated + self.request1.refresh_from_db() + self.assertEqual(self.request1.status, DeletionRequest.STATUS_CANCELLED) + self.assertIsNotNone(self.request1.reviewed_at) + self.assertEqual(self.request1.reviewed_by, self.user1) + self.assertIn("Changed my mind", self.request1.review_comment) + + def test_cancel_deletion_request_without_permission(self): + """Test that non-owners cannot cancel deletion requests.""" + self.client.force_authenticate(user=self.user2) + + response = self.client.post( + f"/api/deletion-requests/{self.request1.id}/cancel/", + ) + + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + # Verify deletion request was NOT updated + self.request1.refresh_from_db() + self.assertEqual(self.request1.status, DeletionRequest.STATUS_PENDING) + + def test_cancel_already_approved_request(self): + """Test that approved requests cannot be cancelled.""" + self.request1.status = DeletionRequest.STATUS_APPROVED + self.request1.save() + + self.client.force_authenticate(user=self.user1) + + response = self.client.post( + f"/api/deletion-requests/{self.request1.id}/cancel/", + ) + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn("error", response.data) + + def test_approve_with_multiple_documents(self): + """Test approving a deletion request with multiple documents.""" + # Create a deletion request with multiple documents + multi_request = DeletionRequest.objects.create( + requested_by_ai=True, + ai_reason="Multiple duplicates", + user=self.user1, + status=DeletionRequest.STATUS_PENDING, + impact_summary={"document_count": 2}, + ) + multi_request.documents.add(self.doc1, self.doc3) + + self.client.force_authenticate(user=self.user1) + + response = self.client.post( + f"/api/deletion-requests/{multi_request.id}/approve/", + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["execution_result"]["deleted_count"], 2) + self.assertEqual(response.data["execution_result"]["total_documents"], 2) + + # Verify both documents were deleted + self.assertFalse(Document.objects.filter(id=self.doc1.id).exists()) + self.assertFalse(Document.objects.filter(id=self.doc3.id).exists()) + + def test_document_details_in_response(self): + """Test that document details are properly included in response.""" + # Add some metadata to the document + tag = Tag.objects.create(name="test-tag") + correspondent = Correspondent.objects.create(name="Test Corp") + doc_type = DocumentType.objects.create(name="Invoice") + + self.doc1.tags.add(tag) + self.doc1.correspondent = correspondent + self.doc1.document_type = doc_type + self.doc1.save() + + self.client.force_authenticate(user=self.user1) + response = self.client.get(f"/api/deletion-requests/{self.request1.id}/") + + self.assertEqual(response.status_code, status.HTTP_200_OK) + doc_details = response.data["document_details"] + self.assertEqual(len(doc_details), 1) + self.assertEqual(doc_details[0]["id"], self.doc1.id) + self.assertEqual(doc_details[0]["title"], "Test Document 1") + self.assertEqual(doc_details[0]["correspondent"], "Test Corp") + self.assertEqual(doc_details[0]["document_type"], "Invoice") + self.assertIn("test-tag", doc_details[0]["tags"]) + + def test_unauthenticated_access(self): + """Test that unauthenticated users cannot access the API.""" + response = self.client.get("/api/deletion-requests/") + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + response = self.client.post( + f"/api/deletion-requests/{self.request1.id}/approve/", + ) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) diff --git a/src/documents/views/__init__.py b/src/documents/views/__init__.py new file mode 100644 index 000000000..d12631b9d --- /dev/null +++ b/src/documents/views/__init__.py @@ -0,0 +1,5 @@ +"""Views module for documents app.""" + +from documents.views.deletion_request import DeletionRequestViewSet + +__all__ = ["DeletionRequestViewSet"] diff --git a/src/documents/views/deletion_request.py b/src/documents/views/deletion_request.py new file mode 100644 index 000000000..22d8e25c3 --- /dev/null +++ b/src/documents/views/deletion_request.py @@ -0,0 +1,262 @@ +""" +API ViewSet for DeletionRequest management. + +Provides endpoints for: +- Listing and retrieving deletion requests +- Approving deletion requests (POST /api/deletion-requests/{id}/approve/) +- Rejecting deletion requests (POST /api/deletion-requests/{id}/reject/) +- Canceling deletion requests (POST /api/deletion-requests/{id}/cancel/) +""" + +import logging + +from django.db import transaction +from django.http import HttpResponseForbidden +from django.utils import timezone +from rest_framework import status +from rest_framework.decorators import action +from rest_framework.response import Response +from rest_framework.viewsets import ModelViewSet + +from documents.models import DeletionRequest +from documents.serialisers import DeletionRequestSerializer + +logger = logging.getLogger("paperless.api") + + +class DeletionRequestViewSet(ModelViewSet): + """ + ViewSet for managing deletion requests. + + Provides CRUD operations plus custom actions for approval workflow. + """ + + model = DeletionRequest + serializer_class = DeletionRequestSerializer + + def get_queryset(self): + """ + Return deletion requests for the current user. + + Superusers can see all requests. + Regular users only see their own requests. + """ + user = self.request.user + if user.is_superuser: + return DeletionRequest.objects.all() + return DeletionRequest.objects.filter(user=user) + + def _can_manage_request(self, deletion_request): + """ + Check if current user can manage (approve/reject/cancel) the request. + + Args: + deletion_request: The DeletionRequest instance + + Returns: + bool: True if user is the owner or a superuser + """ + user = self.request.user + return user.is_superuser or deletion_request.user == user + + @action(methods=["post"], detail=True) + def approve(self, request, pk=None): + """ + Approve a pending deletion request and execute the deletion. + + Validates: + - User has permission (owner or admin) + - Status is pending + + Returns: + Response with execution results + """ + deletion_request = self.get_object() + + # Check permissions + if not self._can_manage_request(deletion_request): + return HttpResponseForbidden( + "You don't have permission to approve this deletion request." + ) + + # Validate status + if deletion_request.status != DeletionRequest.STATUS_PENDING: + return Response( + { + "error": "Only pending deletion requests can be approved.", + "current_status": deletion_request.status, + }, + status=status.HTTP_400_BAD_REQUEST, + ) + + comment = request.data.get("comment", "") + + # Execute approval and deletion in a transaction + try: + with transaction.atomic(): + # Approve the request + if not deletion_request.approve(request.user, comment): + return Response( + {"error": "Failed to approve deletion request."}, + status=status.HTTP_500_INTERNAL_SERVER_ERROR, + ) + + # Execute the deletion + documents = list(deletion_request.documents.all()) + deleted_count = 0 + failed_deletions = [] + + for doc in documents: + try: + doc_id = doc.id + doc_title = doc.title + doc.delete() + deleted_count += 1 + logger.info( + f"Deleted document {doc_id} ('{doc_title}') " + f"as part of deletion request {deletion_request.id}" + ) + except Exception as e: + logger.error( + f"Failed to delete document {doc.id}: {str(e)}" + ) + failed_deletions.append({ + "id": doc.id, + "title": doc.title, + "error": str(e), + }) + + # Update completion status + deletion_request.status = DeletionRequest.STATUS_COMPLETED + deletion_request.completed_at = timezone.now() + deletion_request.completion_details = { + "deleted_count": deleted_count, + "failed_deletions": failed_deletions, + "total_documents": len(documents), + } + deletion_request.save() + + logger.info( + f"Deletion request {deletion_request.id} completed. " + f"Deleted {deleted_count}/{len(documents)} documents." + ) + except Exception as e: + logger.error( + f"Error executing deletion request {deletion_request.id}: {str(e)}" + ) + return Response( + {"error": f"Failed to execute deletion: {str(e)}"}, + status=status.HTTP_500_INTERNAL_SERVER_ERROR, + ) + + serializer = self.get_serializer(deletion_request) + return Response( + { + "message": "Deletion request approved and executed successfully.", + "execution_result": deletion_request.completion_details, + "deletion_request": serializer.data, + }, + status=status.HTTP_200_OK, + ) + + @action(methods=["post"], detail=True) + def reject(self, request, pk=None): + """ + Reject a pending deletion request. + + Validates: + - User has permission (owner or admin) + - Status is pending + + Returns: + Response with updated deletion request + """ + deletion_request = self.get_object() + + # Check permissions + if not self._can_manage_request(deletion_request): + return HttpResponseForbidden( + "You don't have permission to reject this deletion request." + ) + + # Validate status + if deletion_request.status != DeletionRequest.STATUS_PENDING: + return Response( + { + "error": "Only pending deletion requests can be rejected.", + "current_status": deletion_request.status, + }, + status=status.HTTP_400_BAD_REQUEST, + ) + + comment = request.data.get("comment", "") + + # Reject the request + if not deletion_request.reject(request.user, comment): + return Response( + {"error": "Failed to reject deletion request."}, + status=status.HTTP_500_INTERNAL_SERVER_ERROR, + ) + + logger.info( + f"Deletion request {deletion_request.id} rejected by user {request.user.username}" + ) + + serializer = self.get_serializer(deletion_request) + return Response( + { + "message": "Deletion request rejected successfully.", + "deletion_request": serializer.data, + }, + status=status.HTTP_200_OK, + ) + + @action(methods=["post"], detail=True) + def cancel(self, request, pk=None): + """ + Cancel a pending deletion request. + + Validates: + - User has permission (owner or admin) + - Status is pending + + Returns: + Response with updated deletion request + """ + deletion_request = self.get_object() + + # Check permissions + if not self._can_manage_request(deletion_request): + return HttpResponseForbidden( + "You don't have permission to cancel this deletion request." + ) + + # Validate status + if deletion_request.status != DeletionRequest.STATUS_PENDING: + return Response( + { + "error": "Only pending deletion requests can be cancelled.", + "current_status": deletion_request.status, + }, + status=status.HTTP_400_BAD_REQUEST, + ) + + # Cancel the request + deletion_request.status = DeletionRequest.STATUS_CANCELLED + deletion_request.reviewed_by = request.user + deletion_request.reviewed_at = timezone.now() + deletion_request.review_comment = request.data.get("comment", "Cancelled by user") + deletion_request.save() + + logger.info( + f"Deletion request {deletion_request.id} cancelled by user {request.user.username}" + ) + + serializer = self.get_serializer(deletion_request) + return Response( + { + "message": "Deletion request cancelled successfully.", + "deletion_request": serializer.data, + }, + status=status.HTTP_200_OK, + ) diff --git a/src/paperless/urls.py b/src/paperless/urls.py index e24d1a459..3e6b6d87d 100644 --- a/src/paperless/urls.py +++ b/src/paperless/urls.py @@ -43,6 +43,7 @@ from documents.views import WorkflowActionViewSet from documents.views import WorkflowTriggerViewSet from documents.views import WorkflowViewSet from documents.views import serve_logo +from documents.views.deletion_request import DeletionRequestViewSet from paperless.consumers import StatusConsumer from paperless.views import ApplicationConfigurationViewSet from paperless.views import DisconnectSocialAccountView @@ -79,6 +80,7 @@ api_router.register(r"workflows", WorkflowViewSet) api_router.register(r"custom_fields", CustomFieldViewSet) api_router.register(r"config", ApplicationConfigurationViewSet) api_router.register(r"processed_mail", ProcessedMailViewSet) +api_router.register(r"deletion-requests", DeletionRequestViewSet, basename="deletion-requests") urlpatterns = [