From 8676e4b7f8d35039f4f9cf34dcff6ef7d9a80974 Mon Sep 17 00:00:00 2001 From: robg-test <106234256+robg-test@users.noreply.github.com> Date: Tue, 10 Mar 2026 17:37:47 +0000 Subject: [PATCH] [PRMP-1491-2] Fix Upload Statistics --- .../handlers/document_status_check_handler.py | 15 ++- .../services/get_document_upload_status.py | 19 ++-- .../api/performance/cloudfront_performance.py | 0 lambdas/tests/e2e/api/performance/data.csv | 0 .../test_document_status_check_handler.py | 18 ++-- .../test_get_document_upload_status.py | 94 ++++++++++--------- 6 files changed, 83 insertions(+), 63 deletions(-) create mode 100644 lambdas/tests/e2e/api/performance/cloudfront_performance.py create mode 100644 lambdas/tests/e2e/api/performance/data.csv diff --git a/lambdas/handlers/document_status_check_handler.py b/lambdas/handlers/document_status_check_handler.py index 905a7a28c6..1a9580ceb3 100644 --- a/lambdas/handlers/document_status_check_handler.py +++ b/lambdas/handlers/document_status_check_handler.py @@ -54,15 +54,20 @@ def lambda_handler(event, context): documents_id_list = set(documents_list_query_string.split(",")) upload_confirm_result_service = GetDocumentUploadStatusService() - results = upload_confirm_result_service.get_document_references_by_id( + document_status_reports, document_types = upload_confirm_result_service.get_document_status_info( document_ids=documents_id_list, nhs_number=nhs_number_query_string ) - if results: - logger.info("All documents processed successfully") + if document_status_reports: + if all(document_status_report.get("status") == "SUCCESS" for document_status_report in document_status_reports.values()): + logger.info("All documents processed successfully") + for document_type in document_types: + logger.info( + f"A document of type: {document_type} has been processed successfully" + ) return ApiGatewayResponse( - status_code=200, body=json.dumps(results), methods="GET" + status_code=200, body=json.dumps(document_status_reports), methods="GET" ).create_api_gateway_response() else: return ApiGatewayResponse( - status_code=404, body=json.dumps(results), methods="GET" + status_code=404, body=json.dumps(document_status_reports), methods="GET" ) diff --git a/lambdas/services/get_document_upload_status.py b/lambdas/services/get_document_upload_status.py index 734671a3ec..4b36f12241 100644 --- a/lambdas/services/get_document_upload_status.py +++ b/lambdas/services/get_document_upload_status.py @@ -30,11 +30,11 @@ def _determine_document_status(self, doc_ref, nhs_number): return doc_ref.doc_status, None - def get_document_references_by_id( + def get_document_status_info( self, nhs_number: str, document_ids: list[str], - ) -> dict: + ) -> tuple[dict, list]: """ Checks the status of a list of documents for a given patient. @@ -50,18 +50,23 @@ def get_document_references_by_id( SupportedDocumentTypes.LG, ) found_docs_by_id = {doc.id: doc for doc in found_docs} - results = {} + document_statuses = {} + document_types = [] for doc_id in document_ids: doc_ref = found_docs_by_id.get(doc_id) status, error_code = self._determine_document_status(doc_ref, nhs_number) + document_snomed_code = doc_ref.document_snomed_code_type if status is None: continue - result = {"status": status} + status = { "status": status } + type = {"snomed_code": document_snomed_code} + if error_code: - result["error_code"] = error_code - results[doc_id] = result + status["error_code"] = error_code + document_statuses[doc_id] = status + document_types.append(type) - return results + return document_statuses, document_types diff --git a/lambdas/tests/e2e/api/performance/cloudfront_performance.py b/lambdas/tests/e2e/api/performance/cloudfront_performance.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lambdas/tests/e2e/api/performance/data.csv b/lambdas/tests/e2e/api/performance/data.csv new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lambdas/tests/unit/handlers/test_document_status_check_handler.py b/lambdas/tests/unit/handlers/test_document_status_check_handler.py index 1b139e5281..7e4a464bf5 100644 --- a/lambdas/tests/unit/handlers/test_document_status_check_handler.py +++ b/lambdas/tests/unit/handlers/test_document_status_check_handler.py @@ -2,6 +2,7 @@ import pytest from enums.lambda_error import LambdaError +from enums.snomed_codes import SnomedCodes from handlers.document_status_check_handler import lambda_handler from tests.unit.conftest import MOCK_INTERACTION_ID, TEST_NHS_NUMBER from utils.lambda_exceptions import UploadConfirmResultException @@ -36,8 +37,11 @@ def mock_get_document_upload_status_service(mocker, mock_upload_lambda_enabled): ) mocker.patch.object( mock_service_obj, - "get_document_references_by_id", - return_value={"test-doc-id": {"status": "SUCCESS"}}, + "get_document_status_info", + return_value=( + {"test-doc-id": {"status": "SUCCESS"}}, + [SnomedCodes.LLOYD_GEORGE.value.code], + ), ) mocked_instance = mocked_class.return_value yield mocked_instance @@ -55,7 +59,7 @@ def test_document_status_check_handler_success( assert expected == actual - mock_get_document_upload_status_service.get_document_references_by_id.assert_called_with( + mock_get_document_upload_status_service.get_document_status_info.assert_called_with( document_ids=set(["doc-id-1", "doc-id-2"]), nhs_number=TEST_NHS_NUMBER ) @@ -63,7 +67,7 @@ def test_document_status_check_handler_success( def test_document_status_check_handler_empty_result( set_env, context, mock_get_document_upload_status_service ): - mock_get_document_upload_status_service.get_document_references_by_id.return_value = ( + mock_get_document_upload_status_service.get_document_status_info.return_value = ( [] ) @@ -72,7 +76,7 @@ def test_document_status_check_handler_empty_result( actual = lambda_handler(MOCK_VALID_EVENT, context) assert expected == actual - mock_get_document_upload_status_service.get_document_references_by_id.assert_called_with( + mock_get_document_upload_status_service.get_document_status_info.assert_called_with( document_ids=set(["doc-id-1", "doc-id-2"]), nhs_number=TEST_NHS_NUMBER ) @@ -136,7 +140,7 @@ def test_lambda_handler_service_raises_error( mock_get_document_upload_status_service, mock_upload_lambda_enabled, ): - mock_get_document_upload_status_service.get_document_references_by_id.side_effect = UploadConfirmResultException( + mock_get_document_upload_status_service.get_document_status_info.side_effect = UploadConfirmResultException( 400, LambdaError.MockError ) @@ -148,7 +152,7 @@ def test_lambda_handler_service_raises_error( "GET", ).create_api_gateway_response() assert expected == actual - mock_get_document_upload_status_service.get_document_references_by_id.assert_called_with( + mock_get_document_upload_status_service.get_document_status_info.assert_called_with( document_ids=set(["doc-id-1", "doc-id-2"]), nhs_number=TEST_NHS_NUMBER ) diff --git a/lambdas/tests/unit/services/test_get_document_upload_status.py b/lambdas/tests/unit/services/test_get_document_upload_status.py index 3000e11a6c..00437340a6 100644 --- a/lambdas/tests/unit/services/test_get_document_upload_status.py +++ b/lambdas/tests/unit/services/test_get_document_upload_status.py @@ -2,6 +2,7 @@ import pytest from enums.document_status import DocumentStatus +from enums.snomed_codes import SnomedCodes from enums.supported_document_types import SupportedDocumentTypes from enums.virus_scan_result import VirusScanResult from models.document_reference import DocumentReference @@ -32,6 +33,7 @@ def sample_document_references(): file_name="test_file1.pdf", doc_status="final", virus_scanner_result=VirusScanResult.CLEAN, + document_snomed_code_type=SnomedCodes.LLOYD_GEORGE.value.code, ) doc2 = DocumentReference( @@ -40,6 +42,7 @@ def sample_document_references(): file_name="test_file2.pdf", doc_status="final", virus_scanner_result=VirusScanResult.CLEAN, + document_snomed_code_type=SnomedCodes.LETTERS_AND_DOCUMENTS.value.code, ) doc3 = DocumentReference( @@ -82,7 +85,7 @@ def test_get_document_references_by_id_found_documents( sample_document_references[:2] ) - result = get_document_upload_status_service.get_document_references_by_id( + document_statuses, document_types = get_document_upload_status_service.get_document_status_info( nhs_number, document_ids, ) @@ -91,11 +94,14 @@ def test_get_document_references_by_id_found_documents( document_ids, SupportedDocumentTypes.LG, ) - assert len(result) == 2 - assert result["doc-id-1"]["status"] == "final" - assert result["doc-id-2"]["status"] == "final" - assert "error_code" not in result["doc-id-1"] - assert "error_code" not in result["doc-id-2"] + + assert len(document_statuses) == 2 + assert document_statuses["doc-id-1"]["status"] == "final" + assert document_statuses["doc-id-2"]["status"] == "final" + assert document_statuses["doc-id-1"]["snomed_code"] == SnomedCodes.LLOYD_GEORGE.value.code + assert document_statuses["doc-id-2"]["snomed_code"] == SnomedCodes.LETTERS_AND_DOCUMENTS.value.code + assert "error_code" not in document_statuses["doc-id-1"] + assert "error_code" not in document_statuses["doc-id-2"] def test_get_document_references_by_id_not_found_documents( @@ -109,16 +115,16 @@ def test_get_document_references_by_id_not_found_documents( sample_document_references[0], ] - result = get_document_upload_status_service.get_document_references_by_id( + document_statuses, document_types = get_document_upload_status_service.get_document_status_info( nhs_number, document_ids, ) - assert len(result) == 2 - assert result["doc-id-1"]["status"] == "final" - assert result["non-existent-id"]["status"] == DocumentStatus.NOT_FOUND.display - assert "error_code" not in result["doc-id-1"] - assert result["non-existent-id"]["error_code"] == DocumentStatus.NOT_FOUND.code + assert len(document_statuses) == 2 + assert document_statuses["doc-id-1"]["status"] == "final" + assert document_statuses["non-existent-id"]["status"] == DocumentStatus.NOT_FOUND.display + assert "error_code" not in document_statuses["doc-id-1"] + assert document_statuses["non-existent-id"]["error_code"] == DocumentStatus.NOT_FOUND.code def test_get_document_references_by_id_access_denied( @@ -132,14 +138,14 @@ def test_get_document_references_by_id_access_denied( sample_document_references[2], ] - result = get_document_upload_status_service.get_document_references_by_id( + document_statuses, document_types = get_document_upload_status_service.get_document_status_info( nhs_number, document_ids, ) - assert len(result) == 1 - assert result["doc-id-3"]["status"] == DocumentStatus.FORBIDDEN.display - assert result["doc-id-3"]["error_code"] == DocumentStatus.FORBIDDEN.code + assert len(document_statuses) == 1 + assert document_statuses["doc-id-3"]["status"] == DocumentStatus.FORBIDDEN.display + assert document_statuses["doc-id-3"]["error_code"] == DocumentStatus.FORBIDDEN.code def test_get_document_references_by_id_infected_document( @@ -153,14 +159,14 @@ def test_get_document_references_by_id_infected_document( sample_document_references[3], ] - result = get_document_upload_status_service.get_document_references_by_id( + document_statuses, document_types = get_document_upload_status_service.get_document_status_info( nhs_number, document_ids, ) - assert len(result) == 1 - assert result["doc-id-4"]["status"] == DocumentStatus.INFECTED.display - assert result["doc-id-4"]["error_code"] == DocumentStatus.INFECTED.code + assert len(document_statuses) == 1 + assert document_statuses["doc-id-4"]["status"] == DocumentStatus.INFECTED.display + assert document_statuses["doc-id-4"]["error_code"] == DocumentStatus.INFECTED.code def test_get_document_references_by_id_invalid_document( @@ -182,14 +188,14 @@ def test_get_document_references_by_id_invalid_document( cancelled_doc, ] - result = get_document_upload_status_service.get_document_references_by_id( + document_statuses, document_types = get_document_upload_status_service.get_document_status_info( nhs_number, document_ids, ) - assert len(result) == 1 - assert result["doc-id-invalid"]["status"] == DocumentStatus.INVALID.display - assert result["doc-id-invalid"]["error_code"] == DocumentStatus.INVALID.code + assert len(document_statuses) == 1 + assert document_statuses["doc-id-invalid"]["status"] == DocumentStatus.INVALID.display + assert document_statuses["doc-id-invalid"]["error_code"] == DocumentStatus.INVALID.code def test_get_document_references_by_id_cancelled_document( @@ -212,14 +218,14 @@ def test_get_document_references_by_id_cancelled_document( cancelled_doc, ] - result = get_document_upload_status_service.get_document_references_by_id( + document_statuses, document_types = get_document_upload_status_service.get_document_status_info( nhs_number, document_ids, ) - assert len(result) == 1 - assert result["doc-id-cancelled"]["status"] == DocumentStatus.CANCELLED.display - assert result["doc-id-cancelled"]["error_code"] == DocumentStatus.CANCELLED.code + assert len(document_statuses) == 1 + assert document_statuses["doc-id-cancelled"]["status"] == DocumentStatus.CANCELLED.display + assert document_statuses["doc-id-cancelled"]["error_code"] == DocumentStatus.CANCELLED.code def test_get_document_references_by_id_deleted_document( @@ -233,12 +239,12 @@ def test_get_document_references_by_id_deleted_document( sample_document_references[4], ] - result = get_document_upload_status_service.get_document_references_by_id( + document_statuses, document_types = get_document_upload_status_service.get_document_status_info( nhs_number, document_ids, ) - assert len(result) == 0 + assert len(document_statuses) == 0 def test_get_document_references_by_id_multiple_mixed_statuses( @@ -255,25 +261,25 @@ def test_get_document_references_by_id_multiple_mixed_statuses( sample_document_references[4], ] - result = get_document_upload_status_service.get_document_references_by_id( + document_statuses, document_types = get_document_upload_status_service.get_document_status_info( nhs_number, document_ids, ) - assert len(result) == 4 - assert result["doc-id-1"]["status"] == "final" - assert "error_code" not in result["doc-id-1"] + assert len(document_statuses) == 4 + assert document_statuses["doc-id-1"]["status"] == "final" + assert "error_code" not in document_statuses["doc-id-1"] - assert result["doc-id-3"]["status"] == DocumentStatus.FORBIDDEN.display - assert result["doc-id-3"]["error_code"] == DocumentStatus.FORBIDDEN.code + assert document_statuses["doc-id-3"]["status"] == DocumentStatus.FORBIDDEN.display + assert document_statuses["doc-id-3"]["error_code"] == DocumentStatus.FORBIDDEN.code - assert result["doc-id-4"]["status"] == DocumentStatus.INFECTED.display - assert result["doc-id-4"]["error_code"] == DocumentStatus.INFECTED.code + assert document_statuses["doc-id-4"]["status"] == DocumentStatus.INFECTED.display + assert document_statuses["doc-id-4"]["error_code"] == DocumentStatus.INFECTED.code - assert result["non-existent-id"]["status"] == DocumentStatus.NOT_FOUND.display - assert result["non-existent-id"]["error_code"] == DocumentStatus.NOT_FOUND.code + assert document_statuses["non-existent-id"]["status"] == DocumentStatus.NOT_FOUND.display + assert document_statuses["non-existent-id"]["error_code"] == DocumentStatus.NOT_FOUND.code - assert "doc-id-5" not in result + assert "doc-id-5" not in document_statuses def test_get_document_references_by_id_no_results( @@ -284,7 +290,7 @@ def test_get_document_references_by_id_no_results( document_ids = ["doc-id-6"] mock_document_service.get_batch_document_references_by_id.return_value = [] - result = get_document_upload_status_service.get_document_references_by_id( + document_statuses, document_types = get_document_upload_status_service.get_document_status_info( nhs_number, document_ids, ) @@ -293,5 +299,5 @@ def test_get_document_references_by_id_no_results( document_ids, SupportedDocumentTypes.LG, ) - assert result["doc-id-6"]["status"] == DocumentStatus.NOT_FOUND.display - assert result["doc-id-6"]["error_code"] == DocumentStatus.NOT_FOUND.code + assert document_statuses["doc-id-6"]["status"] == DocumentStatus.NOT_FOUND.display + assert document_statuses["doc-id-6"]["error_code"] == DocumentStatus.NOT_FOUND.code