From 0ee85aae212e4782e3f70a64f3638829f877ac32 Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Mon, 5 Aug 2024 17:01:01 -0700 Subject: [PATCH] Enhancement: log when pre-check fails for documents in trash (#7355) --- src-ui/messages.xlf | 40 +++++++++----- .../app/services/consumer-status.service.ts | 2 + src/documents/consumer.py | 30 ++++++++--- src/documents/tests/test_consumer.py | 53 +++++++++++++++++++ 4 files changed, 105 insertions(+), 20 deletions(-) diff --git a/src-ui/messages.xlf b/src-ui/messages.xlf index d5ae2afe8..34096d3a2 100644 --- a/src-ui/messages.xlf +++ b/src-ui/messages.xlf @@ -8325,25 +8325,39 @@ 17 + + Document already exists. Note: existing document is in the trash. + + src/app/services/consumer-status.service.ts + 18 + + Document with ASN already exists. src/app/services/consumer-status.service.ts - 18 + 19 + + + + Document with ASN already exists. Note: existing document is in the trash. + + src/app/services/consumer-status.service.ts + 20 File not found. src/app/services/consumer-status.service.ts - 19 + 21 Pre-consume script does not exist. src/app/services/consumer-status.service.ts - 20 + 22 Pre-Consume is a term that appears like that in the documentation as well and does not need a specific translation @@ -8351,7 +8365,7 @@ Error while executing pre-consume script. src/app/services/consumer-status.service.ts - 21 + 23 Pre-Consume is a term that appears like that in the documentation as well and does not need a specific translation @@ -8359,7 +8373,7 @@ Post-consume script does not exist. src/app/services/consumer-status.service.ts - 22 + 24 Post-Consume is a term that appears like that in the documentation as well and does not need a specific translation @@ -8367,7 +8381,7 @@ Error while executing post-consume script. src/app/services/consumer-status.service.ts - 23 + 25 Post-Consume is a term that appears like that in the documentation as well and does not need a specific translation @@ -8375,49 +8389,49 @@ Received new file. src/app/services/consumer-status.service.ts - 24 + 26 File type not supported. src/app/services/consumer-status.service.ts - 25 + 27 Processing document... src/app/services/consumer-status.service.ts - 26 + 28 Generating thumbnail... src/app/services/consumer-status.service.ts - 27 + 29 Retrieving date from document... src/app/services/consumer-status.service.ts - 28 + 30 Saving document... src/app/services/consumer-status.service.ts - 29 + 31 Finished. src/app/services/consumer-status.service.ts - 30 + 32 diff --git a/src-ui/src/app/services/consumer-status.service.ts b/src-ui/src/app/services/consumer-status.service.ts index d8e8ffe28..40641ff81 100644 --- 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 b6d0ea455..d90b88f5a 100644 --- 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 b585d70c5..5b56e2cca 100644 --- 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 = []