From bc49dd9236ba7cad7954d9f4f3148a52357126bb Mon Sep 17 00:00:00 2001 From: Fox Maltas Date: Mon, 22 Dec 2025 10:51:44 +0000 Subject: [PATCH 1/3] [PRMP-1080] Refactored to use the DocumentUploadReviewService --- .../services/document_review_processor_service.py | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/lambdas/services/document_review_processor_service.py b/lambdas/services/document_review_processor_service.py index 5e2162ba98..05dcd6949e 100644 --- a/lambdas/services/document_review_processor_service.py +++ b/lambdas/services/document_review_processor_service.py @@ -12,6 +12,7 @@ from models.sqs.review_message_body import ReviewMessageBody from services.base.dynamo_service import DynamoDBService from services.base.s3_service import S3Service +from services.document_upload_review_service import DocumentUploadReviewService from utils.audit_logging_setup import LoggingService from utils.request_context import request_context @@ -25,8 +26,8 @@ class ReviewProcessorService: def __init__(self): """Initialize the review processor service with required AWS services.""" - self.dynamo_service = DynamoDBService() self.s3_service = S3Service() + self.review_document_service = DocumentUploadReviewService() self.review_table_name = os.environ["DOCUMENT_REVIEW_DYNAMODB_NAME"] self.staging_bucket_name = os.environ["STAGING_STORE_BUCKET_NAME"] @@ -54,16 +55,11 @@ def process_review_message(self, review_message: ReviewMessageBody) -> None: document_upload_review = self._build_review_record( review_message, review_id, review_files ) + try: - self.dynamo_service.create_item( - table_name=self.review_table_name, - item=document_upload_review.model_dump( - by_alias=True, exclude_none=True - ), - key_name=DocumentReferenceMetadataFields.ID.value, + self.review_document_service.create_dynamo_entry( + document_upload_review ) - - logger.info(f"Created review record {document_upload_review.id}") except ClientError as e: if e.response["Error"]["Code"] == "ConditionalCheckFailedException": logger.info("Entry already exists on Document Review table") From d1a55b0002e2797aee070efae09ba85a3c6deee5 Mon Sep 17 00:00:00 2001 From: Fox Maltas Date: Mon, 22 Dec 2025 11:09:10 +0000 Subject: [PATCH 2/3] [PRMP-1080] Fixed unit tests --- .../document_review_processor_service.py | 1 - .../test_document_review_processor_service.py | 36 +++++++++---------- 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/lambdas/services/document_review_processor_service.py b/lambdas/services/document_review_processor_service.py index 05dcd6949e..232ef4207e 100644 --- a/lambdas/services/document_review_processor_service.py +++ b/lambdas/services/document_review_processor_service.py @@ -29,7 +29,6 @@ def __init__(self): self.s3_service = S3Service() self.review_document_service = DocumentUploadReviewService() - self.review_table_name = os.environ["DOCUMENT_REVIEW_DYNAMODB_NAME"] self.staging_bucket_name = os.environ["STAGING_STORE_BUCKET_NAME"] self.review_bucket_name = os.environ["PENDING_REVIEW_BUCKET_NAME"] diff --git a/lambdas/tests/unit/services/test_document_review_processor_service.py b/lambdas/tests/unit/services/test_document_review_processor_service.py index df1d3fbaeb..c76ffee4d4 100644 --- a/lambdas/tests/unit/services/test_document_review_processor_service.py +++ b/lambdas/tests/unit/services/test_document_review_processor_service.py @@ -9,12 +9,6 @@ from services.document_review_processor_service import ReviewProcessorService -@pytest.fixture -def mock_dynamo_service(mocker): - """Mock the DynamoDBService.""" - return mocker.patch("services.document_review_processor_service.DynamoDBService") - - @pytest.fixture def mock_s3_service(mocker): """Mock the S3Service.""" @@ -22,7 +16,12 @@ def mock_s3_service(mocker): @pytest.fixture -def service_under_test(set_env, mock_dynamo_service, mock_s3_service): +def mock_document_review_service(mocker): + return mocker.patch("services.document_review_processor_service.DocumentUploadReviewService") + + +@pytest.fixture +def service_under_test(set_env, mock_document_review_service, mock_s3_service): """Create a ReviewProcessorService instance with mocked dependencies.""" service = ReviewProcessorService() return service @@ -48,14 +47,13 @@ def sample_review_message(): def test_service_initializes_with_correct_environment_variables( - set_env, mock_dynamo_service, mock_s3_service + set_env, mock_document_review_service, mock_s3_service ): service = ReviewProcessorService() - assert service.review_table_name == "test_document_review" assert service.staging_bucket_name == "test_staging_bulk_store" assert service.review_bucket_name == "test_document_review_bucket" - mock_dynamo_service.assert_called_once() + mock_document_review_service.assert_called_once() mock_s3_service.assert_called_once() @@ -76,7 +74,7 @@ def test_process_review_message_success( service_under_test.process_review_message(sample_review_message) mock_move.assert_called_once() - service_under_test.dynamo_service.create_item.assert_called_once() + service_under_test.review_document_service.create_dynamo_entry.assert_called_once() mock_delete.assert_called_once_with(sample_review_message) @@ -118,7 +116,7 @@ def test_process_review_message_multiple_files(service_under_test, mocker): service_under_test.process_review_message(message) mock_move.assert_called_once() - service_under_test.dynamo_service.create_item.assert_called_once() + service_under_test.review_document_service.create_dynamo_entry.assert_called_once() mock_delete.assert_called_once_with(message) @@ -153,7 +151,7 @@ def test_process_review_message_dynamo_error_not_precondition( ) ], ) - service_under_test.dynamo_service.create_item.side_effect = ClientError( + service_under_test.review_document_service.create_dynamo_entry.side_effect = ClientError( {"Error": {"Code": "InternalServerError", "Message": "DynamoDB error"}}, "PutItem", ) @@ -177,7 +175,7 @@ def test_process_review_message_continues_dynamo_conditional_check_failure( ], ) mocker.patch.object(service_under_test, "_delete_files_from_staging") - service_under_test.dynamo_service.create_item.side_effect = ClientError( + service_under_test.review_document_service.create_dynamo_entry.side_effect = ClientError( { "Error": { "Code": "ConditionalCheckFailedException", @@ -348,7 +346,7 @@ def test_move_files_to_review_bucket_continues_file_already_exists_in_review_buc ) service_under_test.process_review_message(sample_review_message) - service_under_test.dynamo_service.create_item.assert_called() + service_under_test.review_document_service.create_dynamo_entry.assert_called() # Tests for _delete_files_from_staging method @@ -382,20 +380,20 @@ def test_delete_from_staging_handles_errors(service_under_test, sample_review_me def test_full_workflow_with_valid_message(service_under_test, sample_review_message): """Test complete workflow from message to final record creation.""" - service_under_test.dynamo_service.create_item.return_value = None + service_under_test.review_document_service.create_dynamo_entry.return_value = None service_under_test.s3_service.copy_across_bucket.return_value = None service_under_test.s3_service.delete_object.return_value = None service_under_test.process_review_message(sample_review_message) - service_under_test.dynamo_service.create_item.assert_called_once() + service_under_test.review_document_service.create_dynamo_entry.assert_called_once() service_under_test.s3_service.copy_across_bucket.assert_called_once() service_under_test.s3_service.delete_object.assert_called_once() def test_workflow_handles_multiple_different_patients(service_under_test): """Test processing messages for different patients.""" - service_under_test.dynamo_service.create_item.return_value = None + service_under_test.review_document_service.create_dynamo_entry.return_value = None service_under_test.s3_service.copy_across_bucket.return_value = None service_under_test.s3_service.delete_object.return_value = None @@ -420,5 +418,5 @@ def test_workflow_handles_multiple_different_patients(service_under_test): for message in messages: service_under_test.process_review_message(message) - assert service_under_test.dynamo_service.create_item.call_count == 3 + assert service_under_test.review_document_service.create_dynamo_entry.call_count == 3 assert service_under_test.s3_service.copy_across_bucket.call_count == 3 From dd6e7e0f34b4ddb035a5ee0c19870b91d325f8f8 Mon Sep 17 00:00:00 2001 From: Fox Maltas Date: Mon, 22 Dec 2025 11:10:41 +0000 Subject: [PATCH 3/3] [PRMP-1080] Fixed formatting --- .../document_review_processor_service.py | 8 ++--- .../test_document_review_processor_service.py | 34 ++++++++++++------- 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/lambdas/services/document_review_processor_service.py b/lambdas/services/document_review_processor_service.py index 232ef4207e..164c9dfcb7 100644 --- a/lambdas/services/document_review_processor_service.py +++ b/lambdas/services/document_review_processor_service.py @@ -4,13 +4,11 @@ from botocore.exceptions import ClientError from enums.document_review_status import DocumentReviewStatus -from models.document_reference import DocumentReferenceMetadataFields from models.document_review import ( DocumentReviewFileDetails, DocumentUploadReviewReference, ) from models.sqs.review_message_body import ReviewMessageBody -from services.base.dynamo_service import DynamoDBService from services.base.s3_service import S3Service from services.document_upload_review_service import DocumentUploadReviewService from utils.audit_logging_setup import LoggingService @@ -54,11 +52,9 @@ def process_review_message(self, review_message: ReviewMessageBody) -> None: document_upload_review = self._build_review_record( review_message, review_id, review_files ) - + try: - self.review_document_service.create_dynamo_entry( - document_upload_review - ) + self.review_document_service.create_dynamo_entry(document_upload_review) except ClientError as e: if e.response["Error"]["Code"] == "ConditionalCheckFailedException": logger.info("Entry already exists on Document Review table") diff --git a/lambdas/tests/unit/services/test_document_review_processor_service.py b/lambdas/tests/unit/services/test_document_review_processor_service.py index c76ffee4d4..f002ada21e 100644 --- a/lambdas/tests/unit/services/test_document_review_processor_service.py +++ b/lambdas/tests/unit/services/test_document_review_processor_service.py @@ -17,7 +17,9 @@ def mock_s3_service(mocker): @pytest.fixture def mock_document_review_service(mocker): - return mocker.patch("services.document_review_processor_service.DocumentUploadReviewService") + return mocker.patch( + "services.document_review_processor_service.DocumentUploadReviewService" + ) @pytest.fixture @@ -151,9 +153,11 @@ def test_process_review_message_dynamo_error_not_precondition( ) ], ) - service_under_test.review_document_service.create_dynamo_entry.side_effect = ClientError( - {"Error": {"Code": "InternalServerError", "Message": "DynamoDB error"}}, - "PutItem", + service_under_test.review_document_service.create_dynamo_entry.side_effect = ( + ClientError( + {"Error": {"Code": "InternalServerError", "Message": "DynamoDB error"}}, + "PutItem", + ) ) with pytest.raises(ClientError): @@ -175,14 +179,16 @@ def test_process_review_message_continues_dynamo_conditional_check_failure( ], ) mocker.patch.object(service_under_test, "_delete_files_from_staging") - service_under_test.review_document_service.create_dynamo_entry.side_effect = ClientError( - { - "Error": { - "Code": "ConditionalCheckFailedException", - "Message": "DynamoDB error", - } - }, - "PutItem", + service_under_test.review_document_service.create_dynamo_entry.side_effect = ( + ClientError( + { + "Error": { + "Code": "ConditionalCheckFailedException", + "Message": "DynamoDB error", + } + }, + "PutItem", + ) ) service_under_test.process_review_message(sample_review_message) @@ -418,5 +424,7 @@ def test_workflow_handles_multiple_different_patients(service_under_test): for message in messages: service_under_test.process_review_message(message) - assert service_under_test.review_document_service.create_dynamo_entry.call_count == 3 + assert ( + service_under_test.review_document_service.create_dynamo_entry.call_count == 3 + ) assert service_under_test.s3_service.copy_across_bucket.call_count == 3