From 1472a570376127e1f986fd30c6e7c8d183b6dcb0 Mon Sep 17 00:00:00 2001
From: shamoon <4887959+shamoon@users.noreply.github.com>
Date: Sat, 8 Jun 2024 07:27:34 -0700
Subject: [PATCH] DRY, fix split test, add coverage for split with delete

---
 src/documents/bulk_edit.py            |  7 +++--
 src/documents/tasks.py                |  6 ----
 src/documents/tests/test_bulk_edit.py | 42 +++++++++++++++++++++++++--
 3 files changed, 44 insertions(+), 11 deletions(-)

diff --git a/src/documents/bulk_edit.py b/src/documents/bulk_edit.py
index 3501d3c6d..7733a8d68 100644
--- a/src/documents/bulk_edit.py
+++ b/src/documents/bulk_edit.py
@@ -7,6 +7,7 @@ from typing import Optional
 from celery import chain
 from celery import chord
 from celery import group
+from celery import shared_task
 from django.conf import settings
 from django.db.models import Q
 
@@ -21,7 +22,6 @@ from documents.models import StoragePath
 from documents.permissions import set_permissions_for_object
 from documents.tasks import bulk_update_documents
 from documents.tasks import consume_file
-from documents.tasks import delete_documents
 from documents.tasks import update_document_archive_file
 
 logger = logging.getLogger("paperless.bulk_edit")
@@ -156,6 +156,7 @@ def modify_custom_fields(doc_ids: list[int], add_custom_fields, remove_custom_fi
     return "OK"
 
 
+@shared_task
 def delete(doc_ids: list[int]):
     Document.objects.filter(id__in=doc_ids).delete()
 
@@ -297,7 +298,7 @@ def merge(
         logger.info(
             "Queueing removal of original documents after consumption of merged document",
         )
-        chain(consume_task, delete_documents.si(affected_docs)).delay()
+        chain(consume_task, delete.si(affected_docs)).delay()
     else:
         consume_task.delay()
 
@@ -346,7 +347,7 @@ def split(doc_ids: list[int], pages: list[list[int]], delete_originals: bool = F
                 logger.info(
                     "Queueing removal of original document after consumption of the split documents",
                 )
-                chord(header=consume_tasks, body=delete_documents.si([doc.id])).delay()
+                chord(header=consume_tasks, body=delete.si([doc.id])).delay()
             else:
                 group(consume_tasks).delay()
 
diff --git a/src/documents/tasks.py b/src/documents/tasks.py
index f49ea594e..e3b2df990 100644
--- a/src/documents/tasks.py
+++ b/src/documents/tasks.py
@@ -15,7 +15,6 @@ from django.db.models.signals import post_save
 from filelock import FileLock
 from whoosh.writing import AsyncWriter
 
-from documents import bulk_edit
 from documents import index
 from documents import sanity_checker
 from documents.barcodes import BarcodePlugin
@@ -293,8 +292,3 @@ def update_document_archive_file(document_id):
         )
     finally:
         parser.cleanup()
-
-
-@shared_task
-def delete_documents(doc_ids: list[int]):
-    bulk_edit.delete(doc_ids)
diff --git a/src/documents/tests/test_bulk_edit.py b/src/documents/tests/test_bulk_edit.py
index 5c5e7ef5b..37ebc35fc 100644
--- a/src/documents/tests/test_bulk_edit.py
+++ b/src/documents/tests/test_bulk_edit.py
@@ -444,7 +444,7 @@ class TestPDFActions(DirectoriesMixin, TestCase):
 
         self.assertEqual(result, "OK")
 
-    @mock.patch("documents.tasks.delete_documents.si")
+    @mock.patch("documents.bulk_edit.delete.si")
     @mock.patch("documents.tasks.consume_file.s")
     @mock.patch("documents.bulk_edit.chain")
     def test_merge_and_delete_originals(
@@ -513,7 +513,7 @@ class TestPDFActions(DirectoriesMixin, TestCase):
 
         mock_consume_file.assert_not_called()
 
-    @mock.patch("documents.tasks.consume_file.delay")
+    @mock.patch("documents.tasks.consume_file.s")
     def test_split(self, mock_consume_file):
         """
         GIVEN:
@@ -532,6 +532,44 @@ class TestPDFActions(DirectoriesMixin, TestCase):
 
         self.assertEqual(result, "OK")
 
+    @mock.patch("documents.bulk_edit.delete.si")
+    @mock.patch("documents.tasks.consume_file.s")
+    @mock.patch("documents.bulk_edit.chord")
+    def test_split_and_delete_originals(
+        self,
+        mock_chord,
+        mock_consume_file,
+        mock_delete_documents,
+    ):
+        """
+        GIVEN:
+            - Existing documents
+        WHEN:
+            - Split action with deleting documents is called with 1 document and 2 page groups
+            - delete_originals is set to True
+        THEN:
+            - Consume file should be called twice
+            - Document deletion task should be called
+        """
+        doc_ids = [self.doc2.id]
+        pages = [[1, 2], [3]]
+
+        result = bulk_edit.split(doc_ids, pages, delete_originals=True)
+        self.assertEqual(result, "OK")
+
+        self.assertEqual(mock_consume_file.call_count, 2)
+        consume_file_args, _ = mock_consume_file.call_args
+        self.assertEqual(consume_file_args[1].title, "B (split 2)")
+
+        mock_delete_documents.assert_called()
+        mock_chord.assert_called_once()
+
+        delete_documents_args, _ = mock_delete_documents.call_args
+        self.assertEqual(
+            delete_documents_args[0],
+            doc_ids,
+        )
+
     @mock.patch("documents.tasks.consume_file.delay")
     @mock.patch("pikepdf.Pdf.save")
     def test_split_with_errors(self, mock_save_pdf, mock_consume_file):