From 1af10fc785e0482c328ce05cf3402053e87e0bea Mon Sep 17 00:00:00 2001 From: MohammadPatelNHS <247976665+MohammadPatelNHS@users.noreply.github.com> Date: Wed, 22 Apr 2026 08:24:06 +0000 Subject: [PATCH 1/4] [CDAPI-152]: Commit for pipeline --- .../tests/integration/test_endpoints.py | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/pathology-api/tests/integration/test_endpoints.py b/pathology-api/tests/integration/test_endpoints.py index 6614179..4add64f 100644 --- a/pathology-api/tests/integration/test_endpoints.py +++ b/pathology-api/tests/integration/test_endpoints.py @@ -886,6 +886,66 @@ def test_invalid_organization_resource( ], } + def test_pdm_returns_400( + self, client: Client, build_valid_test_result: Callable[[str, str], Bundle] + ) -> None: + bundle = build_valid_test_result("PDM_VALIDATION_ERROR", "ods_code") + + response = client.send( + data=bundle.model_dump_json(by_alias=True), + path="FHIR/R4/Bundle", + request_method="POST", + headers={"X-Correlation-ID": "bb038f9a-dc45-49e1-bcfd-3ab3c3de5e16"}, + ) + + assert response.status_code == 500 + assert response.headers["Content-Type"] == "application/fhir+json" + assert ( + response.headers["X-Correlation-ID"] + == "bb038f9a-dc45-49e1-bcfd-3ab3c3de5e16" + ) + + response_data = response.json() + operation_outcome = OperationOutcome.model_validate(response_data) + + issue: OperationOutcome.Issue = { + "severity": "fatal", + "code": "exception", + "diagnostics": "Failed to create document", + } + + assert operation_outcome.issue == [issue] + + def test_pdm_returns_500( + self, client: Client, build_valid_test_result: Callable[[str, str], Bundle] + ) -> None: + bundle = build_valid_test_result("PDM_SERVER_ERROR", "ods_code") + + response = client.send( + data=bundle.model_dump_json(by_alias=True), + path="FHIR/R4/Bundle", + request_method="POST", + headers={"X-Correlation-ID": "bb038f9a-dc45-49e1-bcfd-3ab3c3de5e16"}, + ) + + assert response.status_code == 500 + assert response.headers["Content-Type"] == "application/fhir+json" + assert ( + response.headers["X-Correlation-ID"] + == "bb038f9a-dc45-49e1-bcfd-3ab3c3de5e16" + ) + + response_data = response.json() + operation_outcome = OperationOutcome.model_validate(response_data) + + issue: OperationOutcome.Issue = { + "severity": "fatal", + "code": "exception", + "diagnostics": "Failed to create document", + } + + assert operation_outcome.issue == [issue] + @pytest.mark.parametrize( ("subject"), [ From 7126038795445da94908822f0f286c8a736f21d4 Mon Sep 17 00:00:00 2001 From: MohammadPatelNHS <247976665+MohammadPatelNHS@users.noreply.github.com> Date: Wed, 22 Apr 2026 15:27:10 +0000 Subject: [PATCH 2/4] [CDAPI-152]: updated integration tests and mocks --- .../api-gateway-mock/resources/server.py | 7 +- mocks/src/pdm_mock/handler.py | 27 ++++++- pathology-api/src/pathology_api/pdm.py | 2 +- pathology-api/src/pathology_api/test_pdm.py | 4 +- .../tests/integration/test_endpoints.py | 80 ++++++++++++++----- 5 files changed, 93 insertions(+), 27 deletions(-) diff --git a/infrastructure/images/api-gateway-mock/resources/server.py b/infrastructure/images/api-gateway-mock/resources/server.py index 230abf2..2b4a6a6 100644 --- a/infrastructure/images/api-gateway-mock/resources/server.py +++ b/infrastructure/images/api-gateway-mock/resources/server.py @@ -42,7 +42,7 @@ ) @app.route("/", methods=["POST", "GET"]) def forward_request(path_params): - x_correlation_id = request.headers.get("X-Correlation-ID") + x_correlation_id = request.headers.get("X-Correlation-ID", "") forwarded_headers = {k.lower(): v for k, v in request.headers.items()} forwarded_headers["nhsd-correlation-id"] = x_correlation_id @@ -75,6 +75,11 @@ def forward_request(path_params): app.logger.info("response: %s", response.text) response_data = response.json() + # proxygen adds x correlation id to the response headers if one is sent, + # so we can mimic that here, as we currently dont manually return it + if x_correlation_id: + response_data["headers"]["X-Correlation-ID"] = x_correlation_id + output = ( ( response_data["body"], diff --git a/mocks/src/pdm_mock/handler.py b/mocks/src/pdm_mock/handler.py index b14945f..c0637a1 100644 --- a/mocks/src/pdm_mock/handler.py +++ b/mocks/src/pdm_mock/handler.py @@ -73,15 +73,34 @@ def _raise_error(status_code: int, error_text: str, error_code: str) -> RequestH } +def _get_patient_id_from_resource(resource: dict[str, Any]) -> Any: + resource_type = resource.get("resourceType") + if resource_type != "Composition" and resource_type != "Patient": + return None + + if resource_type == "Composition": + resource = resource.get("subject", {}) + + if "identifier" not in resource or len(resource["identifier"]) == 0: + return None + + patient = ( + resource["identifier"][0].get("value") + if type(resource["identifier"]) is list + else resource["identifier"].get("value") + ) + if not patient: + return None + + return patient + + def _fetch_patient_from_payload(payload: dict[str, Any]) -> str | None: patient_values = [ str(patient) for entry in payload.get("entry", []) if (resource := entry.get("resource")) - and resource.get("resourceType") == "Patient" - and "identifier" in resource - and len(resource["identifier"]) > 0 - and (patient := resource.get("identifier")[0].get("value")) + and (patient := _get_patient_id_from_resource(resource)) ] if not patient_values: diff --git a/pathology-api/src/pathology_api/pdm.py b/pathology-api/src/pathology_api/pdm.py index 433b180..ad15373 100644 --- a/pathology-api/src/pathology_api/pdm.py +++ b/pathology-api/src/pathology_api/pdm.py @@ -59,4 +59,4 @@ def post_document(document: Bundle) -> PdmResponse: # all other responses including 5xx and 4xx return same format for now else: pdm_error = response.text - raise PdmException(f"Failed to send document: {pdm_error}") + raise PdmException(f"Failed to store document: {pdm_error}") diff --git a/pathology-api/src/pathology_api/test_pdm.py b/pathology-api/src/pathology_api/test_pdm.py index 3ac4e1a..695e0c6 100644 --- a/pathology-api/src/pathology_api/test_pdm.py +++ b/pathology-api/src/pathology_api/test_pdm.py @@ -172,7 +172,7 @@ def test_post_document_4xx(self) -> None: ) with pytest.raises( - PdmException, match="Failed to send document: error message" + PdmException, match="Failed to store document: error message" ): post_document(bundle) @@ -195,6 +195,6 @@ def test_post_document_5xx(self) -> None: ) with pytest.raises( - PdmException, match="Failed to send document: error message" + PdmException, match="Failed to store document: error message" ): post_document(bundle) diff --git a/pathology-api/tests/integration/test_endpoints.py b/pathology-api/tests/integration/test_endpoints.py index 4add64f..8a15b71 100644 --- a/pathology-api/tests/integration/test_endpoints.py +++ b/pathology-api/tests/integration/test_endpoints.py @@ -33,6 +33,7 @@ def test_bundle_returns_200( response.headers["X-Correlation-ID"] == "bb038f9a-dc45-49e1-bcfd-3ab3c3de5e16" ) + assert response.headers["etag"] == 'W/"1"' response_data = response.json() response_bundle = Bundle.model_validate(response_data, by_alias=True) @@ -46,16 +47,14 @@ def test_bundle_returns_200( assert response_bundle.meta is not None response_meta = response_bundle.meta - print(f"Response meta: {response_meta}") - assert response_meta.last_updated is not None assert response_meta.version_id == "1" - assert response.headers["etag"] == 'W/"1"' - def test_no_payload_returns_error(self, client: Client) -> None: response = client.send_without_payload( - request_method="POST", path="FHIR/R4/Bundle" + request_method="POST", + path="FHIR/R4/Bundle", + headers={"X-Correlation-ID": "bb038f9a-dc45-49e1-bcfd-3ab3c3de5e16"}, ) assert response.status_code == 400 @@ -76,7 +75,12 @@ def test_no_payload_returns_error(self, client: Client) -> None: assert response.status_code == 400 def test_empty_payload_returns_error(self, client: Client) -> None: - response = client.send(data="", request_method="POST", path="FHIR/R4/Bundle") + response = client.send( + data="", + request_method="POST", + path="FHIR/R4/Bundle", + headers={"X-Correlation-ID": "bb038f9a-dc45-49e1-bcfd-3ab3c3de5e16"}, + ) assert response.status_code == 400 response_data = response.json() @@ -346,7 +350,10 @@ def test_invalid_payload_returns_error( self, client: Client, payload: dict[str, Any], expected_diagnostic: str ) -> None: response = client.send( - data=json.dumps(payload), request_method="POST", path="FHIR/R4/Bundle" + data=json.dumps(payload), + request_method="POST", + path="FHIR/R4/Bundle", + headers={"X-Correlation-ID": "bb038f9a-dc45-49e1-bcfd-3ab3c3de5e16"}, ) assert response.status_code == 400 @@ -548,7 +555,10 @@ def test_invalid_composition_resource( } response = client.send( - data=json.dumps(bundle), request_method="POST", path="FHIR/R4/Bundle" + data=json.dumps(bundle), + request_method="POST", + path="FHIR/R4/Bundle", + headers={"X-Correlation-ID": "bb038f9a-dc45-49e1-bcfd-3ab3c3de5e16"}, ) assert response.status_code == 400 @@ -653,7 +663,10 @@ def test_invalid_service_request_resource( } response = client.send( - data=json.dumps(bundle), request_method="POST", path="FHIR/R4/Bundle" + data=json.dumps(bundle), + request_method="POST", + path="FHIR/R4/Bundle", + headers={"X-Correlation-ID": "bb038f9a-dc45-49e1-bcfd-3ab3c3de5e16"}, ) assert response.status_code == 400 @@ -759,7 +772,10 @@ def test_invalid_practitioner_role_resource( } response = client.send( - data=json.dumps(bundle), request_method="POST", path="FHIR/R4/Bundle" + data=json.dumps(bundle), + request_method="POST", + path="FHIR/R4/Bundle", + headers={"X-Correlation-ID": "bb038f9a-dc45-49e1-bcfd-3ab3c3de5e16"}, ) assert response.status_code == 400 @@ -870,7 +886,10 @@ def test_invalid_organization_resource( } response = client.send( - data=json.dumps(bundle), request_method="POST", path="FHIR/R4/Bundle" + data=json.dumps(bundle), + request_method="POST", + path="FHIR/R4/Bundle", + headers={"X-Correlation-ID": "bb038f9a-dc45-49e1-bcfd-3ab3c3de5e16"}, ) assert response.status_code == 400 @@ -886,7 +905,7 @@ def test_invalid_organization_resource( ], } - def test_pdm_returns_400( + def test_pdm_returns_validation_error( self, client: Client, build_valid_test_result: Callable[[str, str], Bundle] ) -> None: bundle = build_valid_test_result("PDM_VALIDATION_ERROR", "ods_code") @@ -908,15 +927,28 @@ def test_pdm_returns_400( response_data = response.json() operation_outcome = OperationOutcome.model_validate(response_data) + pdm_diagnostic = { + "resourceType": "OperationOutcome", + "issue": [ + { + "severity": "error", + "code": "invariant", + "details": { + "text": "Bundle size exceeds maximum allowed size or" + " number of entries." + }, + } + ], + } issue: OperationOutcome.Issue = { - "severity": "fatal", - "code": "exception", - "diagnostics": "Failed to create document", + "severity": "error", + "code": "invalid", + "diagnostics": f"Failed to store document: {json.dumps(pdm_diagnostic)}", } assert operation_outcome.issue == [issue] - def test_pdm_returns_500( + def test_pdm_returns_internal_server_error( self, client: Client, build_valid_test_result: Callable[[str, str], Bundle] ) -> None: bundle = build_valid_test_result("PDM_SERVER_ERROR", "ods_code") @@ -938,10 +970,20 @@ def test_pdm_returns_500( response_data = response.json() operation_outcome = OperationOutcome.model_validate(response_data) + pdm_diagnostic = { + "resourceType": "OperationOutcome", + "issue": [ + { + "severity": "error", + "code": "exception", + "details": {"text": "Internal server error"}, + } + ], + } issue: OperationOutcome.Issue = { - "severity": "fatal", - "code": "exception", - "diagnostics": "Failed to create document", + "severity": "error", + "code": "invalid", + "diagnostics": f"Failed to store document: {json.dumps(pdm_diagnostic)}", } assert operation_outcome.issue == [issue] From 38e73b9fd33bb69600158f7e5efce70d3b30a712 Mon Sep 17 00:00:00 2001 From: MohammadPatelNHS <247976665+MohammadPatelNHS@users.noreply.github.com> Date: Thu, 23 Apr 2026 09:00:20 +0000 Subject: [PATCH 3/4] [CDAPI-152]: fix failing acceptence test --- mocks/src/pdm_mock/handler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mocks/src/pdm_mock/handler.py b/mocks/src/pdm_mock/handler.py index c0637a1..804ffe5 100644 --- a/mocks/src/pdm_mock/handler.py +++ b/mocks/src/pdm_mock/handler.py @@ -106,7 +106,7 @@ def _fetch_patient_from_payload(payload: dict[str, Any]) -> str | None: if not patient_values: return None - if len(patient_values) > 1: + if len(set(patient_values)) > 1: raise ValueError("Multiple patients referenced within the same bundle") return str(patient_values[0]) From 32cbee543444358fa0bd558b2c298e962a80b313 Mon Sep 17 00:00:00 2001 From: MohammadPatelNHS <247976665+MohammadPatelNHS@users.noreply.github.com> Date: Thu, 23 Apr 2026 14:49:00 +0000 Subject: [PATCH 4/4] [CDAPI-152]: add test to improve coverage --- mocks/src/pdm_mock/test_handler.py | 69 ++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/mocks/src/pdm_mock/test_handler.py b/mocks/src/pdm_mock/test_handler.py index ce022c9..038940e 100644 --- a/mocks/src/pdm_mock/test_handler.py +++ b/mocks/src/pdm_mock/test_handler.py @@ -193,6 +193,75 @@ def test_handle_post_request_multiple_patients( assert not boto3_resource_mock.called + @patch("common.storage_helper.StorageHelper.put_item") + @patch("pdm_mock.handler.uuid4") + @patch("pdm_mock.handler.datetime") + @pytest.mark.parametrize( + ("payload_entry", "expected_status_code"), + [ + pytest.param({"resource": {"resourceType": "Observation"}}, 201), + pytest.param( + { + "resource": { + "resourceType": "Composition", + "subject": {"identifier": {"value": "test_number"}}, + } + }, + 201, + ), + pytest.param({"resource": {"resourceType": "Patient"}}, 201), + pytest.param( + { + "resource": { + "resourceType": "Patient", + "identifier": [{"system": "https://fhir.nhs.uk/Id/nhs-number"}], + } + }, + 201, + ), + pytest.param( + { + "resource": { + "resourceType": "Composition", + "subject": {"identifier": {"value": "PDM_VALIDATION_ERROR"}}, + } + }, + 422, + ), + pytest.param( + { + "resource": { + "resourceType": "Patient", + "identifier": [{"value": "PDM_SERVER_ERROR"}], + } + }, + 500, + ), + ], + ) + def test_magic_patient_id_in_payload( + self, + mock_datetime: MagicMock, + mock_uuid: MagicMock, + mock_storage_helper_put_item: MagicMock, + basic_document_payload: dict[str, Any], + payload_entry: dict[str, Any], + expected_status_code: int, + handler: ModuleType, + ) -> None: + + mock_datetime.now.return_value = datetime.datetime( + 2024, 6, 1, 0, 0, 0, tzinfo=datetime.timezone.utc + ) + mock_uuid.return_value = "uuid4" + payload = {**basic_document_payload, "entry": [payload_entry]} + + response = handler.handle_post_request(payload) + + assert response["status_code"] == expected_status_code + if expected_status_code == 201: + assert mock_storage_helper_put_item.called + @patch("boto3.resource") @patch("pdm_mock.handler.uuid4") @patch("pdm_mock.handler.datetime")