Enhancement: log when pre-check fails for documents in trash (#7355)

diff --git a/src-ui/messages.xlf b/src-ui/messages.xlf
index d5ae2afe8f89a8c6978f66da1b1ea395b9c5a7d7..34096d3a276335ef5c0e63961721eecadb77986d 100644 (file)
--- a/src-ui/messages.xlf
+++ b/src-ui/messages.xlf
@@ -8325,25 +8325,39 @@
           <context context-type="linenumber">17</context>
         </context-group>
       </trans-unit>
+      <trans-unit id="5103087968344279314" datatype="html">
+        <source>Document already exists. Note: existing document is in the trash.</source>
+        <context-group purpose="location">
+          <context context-type="sourcefile">src/app/services/consumer-status.service.ts</context>
+          <context context-type="linenumber">18</context>
+        </context-group>
+      </trans-unit>
       <trans-unit id="6108404046106249255" datatype="html">
         <source>Document with ASN already exists.</source>
         <context-group purpose="location">
           <context context-type="sourcefile">src/app/services/consumer-status.service.ts</context>
-          <context context-type="linenumber">18</context>
+          <context context-type="linenumber">19</context>
+        </context-group>
+      </trans-unit>
+      <trans-unit id="65951081560571094" datatype="html">
+        <source>Document with ASN already exists. Note: existing document is in the trash.</source>
+        <context-group purpose="location">
+          <context context-type="sourcefile">src/app/services/consumer-status.service.ts</context>
+          <context context-type="linenumber">20</context>
         </context-group>
       </trans-unit>
       <trans-unit id="148389968432135849" datatype="html">
         <source>File not found.</source>
         <context-group purpose="location">
           <context context-type="sourcefile">src/app/services/consumer-status.service.ts</context>
-          <context context-type="linenumber">19</context>
+          <context context-type="linenumber">21</context>
         </context-group>
       </trans-unit>
       <trans-unit id="1520671543092565667" datatype="html">
         <source>Pre-consume script does not exist.</source>
         <context-group purpose="location">
           <context context-type="sourcefile">src/app/services/consumer-status.service.ts</context>
-          <context context-type="linenumber">20</context>
+          <context context-type="linenumber">22</context>
         </context-group>
         <note priority="1" from="description">Pre-Consume is a term that appears like that in the documentation as well and does not need a specific translation</note>
       </trans-unit>
@@ -8351,7 +8365,7 @@
         <source>Error while executing pre-consume script.</source>
         <context-group purpose="location">
           <context context-type="sourcefile">src/app/services/consumer-status.service.ts</context>
-          <context context-type="linenumber">21</context>
+          <context context-type="linenumber">23</context>
         </context-group>
         <note priority="1" from="description">Pre-Consume is a term that appears like that in the documentation as well and does not need a specific translation</note>
       </trans-unit>
@@ -8359,7 +8373,7 @@
         <source>Post-consume script does not exist.</source>
         <context-group purpose="location">
           <context context-type="sourcefile">src/app/services/consumer-status.service.ts</context>
-          <context context-type="linenumber">22</context>
+          <context context-type="linenumber">24</context>
         </context-group>
         <note priority="1" from="description">Post-Consume is a term that appears like that in the documentation as well and does not need a specific translation</note>
       </trans-unit>
@@ -8367,7 +8381,7 @@
         <source>Error while executing post-consume script.</source>
         <context-group purpose="location">
           <context context-type="sourcefile">src/app/services/consumer-status.service.ts</context>
-          <context context-type="linenumber">23</context>
+          <context context-type="linenumber">25</context>
         </context-group>
         <note priority="1" from="description">Post-Consume is a term that appears like that in the documentation as well and does not need a specific translation</note>
       </trans-unit>
@@ -8375,49 +8389,49 @@
         <source>Received new file.</source>
         <context-group purpose="location">
           <context context-type="sourcefile">src/app/services/consumer-status.service.ts</context>
-          <context context-type="linenumber">24</context>
+          <context context-type="linenumber">26</context>
         </context-group>
       </trans-unit>
       <trans-unit id="7337565919209746135" datatype="html">
         <source>File type not supported.</source>
         <context-group purpose="location">
           <context context-type="sourcefile">src/app/services/consumer-status.service.ts</context>
-          <context context-type="linenumber">25</context>
+          <context context-type="linenumber">27</context>
         </context-group>
       </trans-unit>
       <trans-unit id="5002399167376099234" datatype="html">
         <source>Processing document...</source>
         <context-group purpose="location">
           <context context-type="sourcefile">src/app/services/consumer-status.service.ts</context>
-          <context context-type="linenumber">26</context>
+          <context context-type="linenumber">28</context>
         </context-group>
       </trans-unit>
       <trans-unit id="1085975194762600381" datatype="html">
         <source>Generating thumbnail...</source>
         <context-group purpose="location">
           <context context-type="sourcefile">src/app/services/consumer-status.service.ts</context>
-          <context context-type="linenumber">27</context>
+          <context context-type="linenumber">29</context>
         </context-group>
       </trans-unit>
       <trans-unit id="3280851677698431426" datatype="html">
         <source>Retrieving date from document...</source>
         <context-group purpose="location">
           <context context-type="sourcefile">src/app/services/consumer-status.service.ts</context>
-          <context context-type="linenumber">28</context>
+          <context context-type="linenumber">30</context>
         </context-group>
       </trans-unit>
       <trans-unit id="7162102384876037296" datatype="html">
         <source>Saving document...</source>
         <context-group purpose="location">
           <context context-type="sourcefile">src/app/services/consumer-status.service.ts</context>
-          <context context-type="linenumber">29</context>
+          <context context-type="linenumber">31</context>
         </context-group>
       </trans-unit>
       <trans-unit id="4550450765009165976" datatype="html">
         <source>Finished.</source>
         <context-group purpose="location">
           <context context-type="sourcefile">src/app/services/consumer-status.service.ts</context>
-          <context context-type="linenumber">30</context>
+          <context context-type="linenumber">32</context>
         </context-group>
       </trans-unit>
       <trans-unit id="5523607037798226031" datatype="html">

diff --git a/src-ui/src/app/services/consumer-status.service.ts b/src-ui/src/app/services/consumer-status.service.ts
index d8e8ffe28c26e658eaa2392a7ee818ab290f500a..40641ff81b685aad7dbf9f3e4b99493c49b843af 100644 (file)
--- a/src-ui/src/app/services/consumer-status.service.ts
+++ b/src-ui/src/app/services/consumer-status.service.ts
@@ -15,7 +15,9 @@ export enum FileStatusPhase {
 
 export const FILE_STATUS_MESSAGES = {
   document_already_exists: $localize`Document already exists.`,
+  document_already_exists_in_trash: $localize`Document already exists. Note: existing document is in the trash.`,
   asn_already_exists: $localize`Document with ASN already exists.`,
+  asn_already_exists_in_trash: $localize`Document with ASN already exists. Note: existing document is in the trash.`,
   file_not_found: $localize`File not found.`,
   pre_consume_script_not_found: $localize`:Pre-Consume is a term that appears like that in the documentation as well and does not need a specific translation:Pre-consume script does not exist.`,
   pre_consume_script_error: $localize`:Pre-Consume is a term that appears like that in the documentation as well and does not need a specific translation:Error while executing pre-consume script.`,

diff --git a/src/documents/consumer.py b/src/documents/consumer.py
index b6d0ea455827a4d4c07a3a74403bd3ec37e73178..d90b88f5a829adb719bb801f7e576f38067c1a07 100644 (file)
--- a/src/documents/consumer.py
+++ b/src/documents/consumer.py
@@ -231,7 +231,9 @@ class ConsumerError(Exception):
 
 class ConsumerStatusShortMessage(str, Enum):
     DOCUMENT_ALREADY_EXISTS = "document_already_exists"
+    DOCUMENT_ALREADY_EXISTS_IN_TRASH = "document_already_exists_in_trash"
     ASN_ALREADY_EXISTS = "asn_already_exists"
+    ASN_ALREADY_EXISTS_IN_TRASH = "asn_already_exists_in_trash"
     ASN_RANGE = "asn_value_out_of_range"
     FILE_NOT_FOUND = "file_not_found"
     PRE_CONSUME_SCRIPT_NOT_FOUND = "pre_consume_script_not_found"
@@ -321,12 +323,18 @@ class ConsumerPlugin(
             Q(checksum=checksum) | Q(archive_checksum=checksum),
         )
         if existing_doc.exists():
+            msg = ConsumerStatusShortMessage.DOCUMENT_ALREADY_EXISTS
+            log_msg = f"Not consuming {self.filename}: It is a duplicate of {existing_doc.get().title} (#{existing_doc.get().pk})."
+
+            if existing_doc.first().deleted_at is not None:
+                msg = ConsumerStatusShortMessage.DOCUMENT_ALREADY_EXISTS_IN_TRASH
+                log_msg += " Note: existing document is in the trash."
+
             if settings.CONSUMER_DELETE_DUPLICATES:
                 os.unlink(self.input_doc.original_file)
             self._fail(
-                ConsumerStatusShortMessage.DOCUMENT_ALREADY_EXISTS,
-                f"Not consuming {self.filename}: It is a duplicate of"
-                f" {existing_doc.get().title} (#{existing_doc.get().pk})",
+                msg,
+                log_msg,
             )
 
     def pre_check_directories(self):
@@ -358,12 +366,20 @@ class ConsumerPlugin(
                 f"[{Document.ARCHIVE_SERIAL_NUMBER_MIN:,}, "
                 f"{Document.ARCHIVE_SERIAL_NUMBER_MAX:,}]",
             )
-        if Document.global_objects.filter(
+        existing_asn_doc = Document.global_objects.filter(
             archive_serial_number=self.metadata.asn,
-        ).exists():
+        )
+        if existing_asn_doc.exists():
+            msg = ConsumerStatusShortMessage.ASN_ALREADY_EXISTS
+            log_msg = f"Not consuming {self.filename}: Given ASN {self.metadata.asn} already exists!"
+
+            if existing_asn_doc.first().deleted_at is not None:
+                msg = ConsumerStatusShortMessage.ASN_ALREADY_EXISTS_IN_TRASH
+                log_msg += " Note: existing document is in the trash."
+
             self._fail(
-                ConsumerStatusShortMessage.ASN_ALREADY_EXISTS,
-                f"Not consuming {self.filename}: Given ASN {self.metadata.asn} already exists!",
+                msg,
+                log_msg,
             )
 
     def run_pre_consume_script(self):

diff --git a/src/documents/tests/test_consumer.py b/src/documents/tests/test_consumer.py
index b585d70c5f96a3a74b83c8e6680f302c85033be9..5b56e2ccaad0f7130a6a5d44feb9c953fde6a6bf 100644 (file)
--- a/src/documents/tests/test_consumer.py
+++ b/src/documents/tests/test_consumer.py
@@ -322,6 +322,18 @@ class TestConsumer(
         shutil.copy(src, dst)
         return dst
 
+    def get_test_file2(self):
+        src = (
+            Path(__file__).parent
+            / "samples"
+            / "documents"
+            / "originals"
+            / "0000002.pdf"
+        )
+        dst = self.dirs.scratch_dir / "sample2.pdf"
+        shutil.copy(src, dst)
+        return dst
+
     def get_test_archive_file(self):
         src = (
             Path(__file__).parent / "samples" / "documents" / "archive" / "0000001.pdf"
@@ -642,6 +654,47 @@ class TestConsumer(
         with self.get_consumer(self.get_test_file()) as consumer:
             consumer.run()
 
+    def testDuplicateInTrash(self):
+        with self.get_consumer(self.get_test_file()) as consumer:
+            consumer.run()
+
+        Document.objects.all().delete()
+
+        with self.get_consumer(self.get_test_file()) as consumer:
+            with self.assertRaisesMessage(ConsumerError, "document is in the trash"):
+                consumer.run()
+
+    def testAsnExists(self):
+        with self.get_consumer(
+            self.get_test_file(),
+            DocumentMetadataOverrides(asn=123),
+        ) as consumer:
+            consumer.run()
+
+        with self.get_consumer(
+            self.get_test_file2(),
+            DocumentMetadataOverrides(asn=123),
+        ) as consumer:
+            with self.assertRaisesMessage(ConsumerError, "ASN 123 already exists"):
+                consumer.run()
+
+    def testAsnExistsInTrash(self):
+        with self.get_consumer(
+            self.get_test_file(),
+            DocumentMetadataOverrides(asn=123),
+        ) as consumer:
+            consumer.run()
+
+            document = Document.objects.first()
+            document.delete()
+
+        with self.get_consumer(
+            self.get_test_file2(),
+            DocumentMetadataOverrides(asn=123),
+        ) as consumer:
+            with self.assertRaisesMessage(ConsumerError, "document is in the trash"):
+                consumer.run()
+
     @mock.patch("documents.parsers.document_consumer_declaration.send")
     def testNoParsers(self, m):
         m.return_value = []