add detail to log message

nhsconnect · Dec 13, 2024 · fd01048 · fd01048
1 parent 51b9a33
commit fd01048
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 14 deletions.
diff --git a/lambdas/services/document_service.py b/lambdas/services/document_service.py
@@ -103,7 +103,9 @@ def delete_document_object(self, bucket: str, key: str):
         if not file_exists:
             raise DocumentServiceException("Document does not exist in S3")
 
-        logger.info("Located file, attempting S3 object deletion")
+        logger.info(
+            f"Located file `{key}` in `{bucket}`, attempting S3 object deletion"
+        )
         self.s3_service.delete_object(s3_bucket_name=bucket, file_key=key)
 
         file_exists = self.s3_service.file_exist_on_s3(

diff --git a/lambdas/tests/unit/services/test_document_service.py b/lambdas/tests/unit/services/test_document_service.py
@@ -289,7 +289,7 @@ def test_delete_document_object_successfully_deletes_s3_object(mock_service, cap
     test_bucket = "test-s3-bucket"
     test_file_key = "9000000000/test-file.pdf"
 
-    expected_log_message = "Located file, attempting S3 object deletion"
+    expected_log_message = f"Located file `{test_file_key}` in `{test_bucket}`, attempting S3 object deletion"
 
     mock_service.s3_service.file_exist_on_s3.side_effect = [
         True,
@@ -313,7 +313,7 @@ def test_delete_document_object_fails_to_delete_s3_object(mock_service, caplog):
     test_bucket = "test-s3-bucket"
     test_file_key = "9000000000/test-file.pdf"
 
-    expected_err_msg = "Document located in S3 after deletion" ""
+    expected_err_msg = "Document located in S3 after deletion"
 
     mock_service.s3_service.file_exist_on_s3.side_effect = [
         True,

diff --git a/lambdas/tests/unit/utils/test_dynamo_utils.py b/lambdas/tests/unit/utils/test_dynamo_utils.py
@@ -2,7 +2,13 @@
 
 import pytest
 from enums.metadata_field_names import DocumentReferenceMetadataFields
-from tests.unit.conftest import TEST_DOCUMENT_LOCATION, TEST_NHS_NUMBER, TEST_UUID
+from tests.unit.conftest import (
+    TEST_CURRENT_GP_ODS,
+    TEST_DOCUMENT_LOCATION,
+    TEST_FILE_KEY,
+    TEST_NHS_NUMBER,
+    TEST_UUID,
+)
 from tests.unit.helpers.data.dynamo.dynamo_stream import (
     MOCK_OLD_IMAGE_EVENT,
     MOCK_OLD_IMAGE_MODEL,
@@ -117,20 +123,21 @@ def test_create_expression_attribute_placeholder_camel_case():
 
 def test_parse_dynamo_record_parses_correctly():
     test_data = MOCK_OLD_IMAGE_EVENT
+    test_image = MOCK_OLD_IMAGE_MODEL
 
     expected = {
-        "ContentType": MOCK_OLD_IMAGE_MODEL.content_type,
-        "FileName": MOCK_OLD_IMAGE_MODEL.file_name,
-        "Uploading": MOCK_OLD_IMAGE_MODEL.uploading,
-        "TTL": MOCK_OLD_IMAGE_MODEL.ttl,
-        "Created": MOCK_OLD_IMAGE_MODEL.created,
-        "Uploaded": MOCK_OLD_IMAGE_MODEL.uploaded,
+        "ContentType": test_image.content_type,
+        "FileName": TEST_FILE_KEY,
+        "Uploading": test_image.uploading,
+        "TTL": test_image.ttl,
+        "Created": test_image.created,
+        "Uploaded": test_image.uploaded,
         "FileLocation": TEST_DOCUMENT_LOCATION,
-        "CurrentGpOds": MOCK_OLD_IMAGE_MODEL.current_gp_ods,
-        "VirusScannerResult": MOCK_OLD_IMAGE_MODEL.virus_scanner_result,
-        "Deleted": MOCK_OLD_IMAGE_MODEL.deleted,
+        "CurrentGpOds": TEST_CURRENT_GP_ODS,
+        "VirusScannerResult": test_image.virus_scanner_result,
+        "Deleted": test_image.deleted,
         "ID": TEST_UUID,
-        "LastUpdated": MOCK_OLD_IMAGE_MODEL.last_updated,
+        "LastUpdated": test_image.last_updated,
         "NhsNumber": TEST_NHS_NUMBER,
     }