From 375301f782cd763618e4b16b3061eb829d870cec Mon Sep 17 00:00:00 2001 From: jameslinnell Date: Wed, 25 Feb 2026 11:13:13 +0000 Subject: [PATCH 01/35] [NDR-423] FHIR GET and SEARCH by ID --- .../get_fhir_document_reference_handler.py | 75 ++++++++++++----- .../delete_fhir_document_reference_service.py | 80 ++++++++++++++----- .../document_reference_search_service.py | 1 + lambdas/services/document_service.py | 4 +- .../get_fhir_document_reference_service.py | 39 ++++----- 5 files changed, 138 insertions(+), 61 deletions(-) diff --git a/lambdas/handlers/get_fhir_document_reference_handler.py b/lambdas/handlers/get_fhir_document_reference_handler.py index 25df6ba44f..bdda76f1b5 100644 --- a/lambdas/handlers/get_fhir_document_reference_handler.py +++ b/lambdas/handlers/get_fhir_document_reference_handler.py @@ -1,5 +1,9 @@ -from enums.lambda_error import LambdaError from oauthlib.oauth2 import WebApplicationClient + +from enums.lambda_error import LambdaError +from enums.mtls import MtlsCommonNames +from enums.snomed_codes import SnomedCode, SnomedCodes +from lambdas.utils.lambda_header_utils import validate_common_name_in_mtls from services.base.ssm_service import SSMService from services.dynamic_configuration_service import DynamicConfigurationService from services.get_fhir_document_reference_service import GetFhirDocumentReferenceService @@ -29,44 +33,51 @@ "APPCONFIG_ENVIRONMENT", "PRESIGNED_ASSUME_ROLE", "CLOUDFRONT_URL", - ] + ], ) def lambda_handler(event, context): try: + common_name = validate_common_name_in_mtls(event.get("requestContext")) bearer_token = extract_bearer_token(event, context) selected_role_id = event.get("headers", {}).get("cis2-urid", None) - document_id, snomed_code = extract_document_parameters(event) + document_id = extract_document_parameters(event) + snomed_code = _determine_document_type(common_name=common_name) get_document_service = GetFhirDocumentReferenceService() document_reference = get_document_service.handle_get_document_reference_request( - snomed_code, document_id + snomed_code, + document_id, ) if selected_role_id and bearer_token: verify_user_authorisation( - bearer_token, selected_role_id, document_reference.nhs_number + bearer_token, + selected_role_id, + document_reference.nhs_number, ) document_reference_response = ( get_document_service.create_document_reference_fhir_response( - document_reference + document_reference, ) ) logger.info( - f"Successfully retrieved document reference for document_id: {document_id}, snomed_code: {snomed_code}" + f"Successfully retrieved document reference for document_id: {document_id}, snomed_code: {snomed_code}", ) return ApiGatewayResponse( - status_code=200, body=document_reference_response, methods="GET" + status_code=200, + body=document_reference_response, + methods="GET", ).create_api_gateway_response() except GetFhirDocumentReferenceException as exception: return ApiGatewayResponse( status_code=exception.status_code, body=exception.error.create_error_response().create_error_fhir_response( - exception.error.value.get("fhir_coding") + exception.error.value.get("fhir_coding"), ), methods="GET", ).create_api_gateway_response() @@ -75,15 +86,16 @@ def lambda_handler(event, context): def extract_document_parameters(event): """Extract document ID and SNOMED code from path parameters""" path_params = event.get("pathParameters", {}).get("id", None) - document_id, snomed_code = get_id_and_snomed_from_path_parameters(path_params) + document_id = get_id_and_snomed_from_path_parameters(path_params) - if not document_id or not snomed_code: + if not document_id: logger.error("Missing document id or snomed code in request path parameters.") raise GetFhirDocumentReferenceException( - 400, LambdaError.DocumentReferenceMissingParameters + 400, + LambdaError.DocumentReferenceMissingParameters, ) - return document_id, snomed_code + return document_id def verify_user_authorisation(bearer_token, selected_role_id, nhs_number): @@ -100,17 +112,21 @@ def verify_user_authorisation(bearer_token, selected_role_id, nhs_number): org_ods_code = oidc_service.fetch_user_org_code(userinfo, selected_role_id) smartcard_role_code, _ = oidc_service.fetch_user_role_code( - userinfo, selected_role_id, "R" + userinfo, + selected_role_id, + "R", ) except (OidcApiException, AuthorisationException) as e: logger.error(f"Authorization error: {str(e)}") raise GetFhirDocumentReferenceException( - 403, LambdaError.DocumentReferenceUnauthorised + 403, + LambdaError.DocumentReferenceUnauthorised, ) try: search_patient_service = SearchPatientDetailsService( - smartcard_role_code, org_ods_code + smartcard_role_code, + org_ods_code, ) search_patient_service.handle_search_patient_request(nhs_number, False) except SearchPatientException as e: @@ -118,9 +134,24 @@ def verify_user_authorisation(bearer_token, selected_role_id, nhs_number): def get_id_and_snomed_from_path_parameters(path_parameters): - """Extract document ID and SNOMED code from path parameters""" - if path_parameters: - params = path_parameters.split("~") - if len(params) == 2: - return params[1], params[0] - return None, None + """Extract uuid from path parameters. + + Accepts: + - '1234~uuid' + - 'uuid' + """ + if not path_parameters: + return None + + return path_parameters.split("~")[-1] + + +def _determine_document_type(common_name: MtlsCommonNames | None) -> SnomedCode: + if not common_name: + return SnomedCodes.LLOYD_GEORGE.value + + if common_name not in MtlsCommonNames: + logger.error(f"mTLS common name {common_name} - is not supported") + raise GetFhirDocumentReferenceException(400, LambdaError.DocRefInvalidType) + + return SnomedCodes.PATIENT_DATA.value diff --git a/lambdas/services/delete_fhir_document_reference_service.py b/lambdas/services/delete_fhir_document_reference_service.py index a1150fb953..3238f228b9 100644 --- a/lambdas/services/delete_fhir_document_reference_service.py +++ b/lambdas/services/delete_fhir_document_reference_service.py @@ -1,8 +1,9 @@ import uuid -from datetime import datetime, timezone from typing import Dict, Optional from botocore.exceptions import ClientError +from pydantic import ValidationError + from enums.document_retention import DocumentRetentionDays from enums.lambda_error import LambdaError from enums.metadata_field_names import DocumentReferenceMetadataFields @@ -10,7 +11,6 @@ from enums.snomed_codes import SnomedCode, SnomedCodes from enums.supported_document_types import SupportedDocumentTypes from models.document_reference import DocumentReference -from pydantic import ValidationError from services.document_deletion_service import DocumentDeletionService from services.document_service import DocumentService from services.fhir_document_reference_service_base import ( @@ -38,7 +38,8 @@ def __init__(self): super().__init__() def process_fhir_document_reference( - self, event: dict = {} + self, + event: dict = {}, ) -> list[DocumentReference]: """ Process a FHIR Document Reference DELETE request @@ -52,7 +53,8 @@ def process_fhir_document_reference( if any(v is None for v in deletion_identifiers): logger.error("FHIR document validation error: NhsNumber/id") raise DocumentRefException( - 400, LambdaError.DocumentReferenceMissingParameters + 400, + LambdaError.DocumentReferenceMissingParameters, ) if len(deletion_identifiers) < 2: @@ -61,7 +63,8 @@ def process_fhir_document_reference( if not self.is_uuid(deletion_identifiers[0]): logger.error("FHIR document validation error: Id") raise DocumentRefException( - 400, LambdaError.DocumentReferenceMissingParameters + 400, + LambdaError.DocumentReferenceMissingParameters, ) doc_type = self._determine_document_type_based_on_common_name(common_name) @@ -69,7 +72,8 @@ def process_fhir_document_reference( if not validate_nhs_number(deletion_identifiers[1]): logger.error("FHIR document validation error: NhsNumber") raise DocumentRefException( - 400, LambdaError.DocumentReferenceMissingParameters + 400, + LambdaError.DocumentReferenceMissingParameters, ) request_context.patient_nhs_no = deletion_identifiers[1] @@ -84,12 +88,9 @@ def process_fhir_document_reference( fhir=True, ) else: - files_deleted = ( - self.delete_fhir_document_reference_by_nhs_id_and_doc_id( - nhs_number=deletion_identifiers[1], - doc_id=deletion_identifiers[0], - doc_type=doc_type, - ) + files_deleted = self.delete_fhir_document_reference_by_doc_id( + doc_id=deletion_identifiers[0], + doc_type=doc_type, ) return files_deleted @@ -100,13 +101,52 @@ def process_fhir_document_reference( logger.error(f"AWS client error: {str(e)}") raise DocumentRefException(500, LambdaError.InternalServerError) + def delete_fhir_document_reference_by_doc_id( + self, + doc_id: str, + doc_type: SnomedCode, + ) -> DocumentReference | None: + dynamo_table = self._get_dynamo_table_for_doc_type(doc_type) + document_service = DocumentService() + document = document_service.get_item_agnostic( + partition_key={DocumentReferenceMetadataFields.ID.value: doc_id}, + table_name=dynamo_table, + ) + if not document: + return None + try: + document_service.delete_document_reference( + table_name=dynamo_table, + document_reference=document, + document_ttl_days=DocumentRetentionDays.SOFT_DELETE, + key_pair={ + DocumentReferenceMetadataFields.ID.value: doc_id, + }, + ) + logger.info( + f"Deleted document of type {doc_type.display_name}", + {"Result": "Successful deletion"}, + ) + return document + except (ClientError, DynamoServiceException) as e: + logger.error( + f"{LambdaError.DocDelClient.to_str()}: {str(e)}", + {"Results": "Failed to delete documents"}, + ) + raise DocumentDeletionServiceException(500, LambdaError.DocDelClient) + def delete_fhir_document_reference_by_nhs_id_and_doc_id( - self, nhs_number: str, doc_id: str, doc_type: SnomedCode + self, + nhs_number: str, + doc_id: str, + doc_type: SnomedCode, ) -> DocumentReference | None: dynamo_table = self._get_dynamo_table_for_doc_type(doc_type) document_service = DocumentService() document = document_service.get_item_agnostic( - partion_key={DocumentReferenceMetadataFields.NHS_NUMBER.value: nhs_number}, + partition_key={ + DocumentReferenceMetadataFields.NHS_NUMBER.value: nhs_number, + }, sort_key={DocumentReferenceMetadataFields.ID.value: doc_id}, table_name=dynamo_table, ) @@ -135,7 +175,9 @@ def delete_fhir_document_reference_by_nhs_id_and_doc_id( raise DocumentDeletionServiceException(500, LambdaError.DocDelClient) def delete_fhir_document_references_by_nhs_id( - self, nhs_number: str, doc_type: SnomedCode + self, + nhs_number: str, + doc_type: SnomedCode, ) -> list[DocumentReference] | None: dynamo_table = self._get_dynamo_table_for_doc_type(doc_type) document_service = DocumentService() @@ -166,7 +208,8 @@ def delete_fhir_document_references_by_nhs_id( raise DocumentDeletionServiceException(500, LambdaError.DocDelClient) def _determine_document_type_based_on_common_name( - self, common_name: MtlsCommonNames | None + self, + common_name: MtlsCommonNames | None, ) -> SnomedCode: if not common_name: """Determine the document type based on common_name""" @@ -186,7 +229,7 @@ def is_uuid(self, value: str) -> bool: def extract_parameters(self, event) -> list[str]: nhs_id, document_reference_id = self.extract_document_query_parameters( - event.get("queryStringParameters") or {} + event.get("queryStringParameters") or {}, ) if not nhs_id or not document_reference_id: @@ -201,7 +244,8 @@ def extract_document_path_parameters(self, event): return doc_ref_id def extract_document_query_parameters( - self, query_string: Dict[str, str] + self, + query_string: Dict[str, str], ) -> tuple[Optional[str], Optional[str]]: nhs_number = None document_reference_id = None diff --git a/lambdas/services/document_reference_search_service.py b/lambdas/services/document_reference_search_service.py index 2fe77fdbc8..f4f91ca50c 100644 --- a/lambdas/services/document_reference_search_service.py +++ b/lambdas/services/document_reference_search_service.py @@ -108,6 +108,7 @@ def _search_tables_for_documents( documents = self.fetch_documents_from_table( search_condition=nhs_number, search_key="NhsNumber", + index_name="idx_gsi_nhs_number", table_name=table_name, query_filter=filter_expression, ) diff --git a/lambdas/services/document_service.py b/lambdas/services/document_service.py index 32a2927d18..065cdf4549 100644 --- a/lambdas/services/document_service.py +++ b/lambdas/services/document_service.py @@ -123,7 +123,7 @@ def _get_item(self, table_name, key, model_class): def get_item_agnostic( self, - partion_key: dict, + partition_key: dict, sort_key: dict | None = None, table_name: str | None = None, model_class: type[BaseModel] | None = None, @@ -133,7 +133,7 @@ def get_item_agnostic( return self._get_item( table_name=table_name, - key=(partion_key or {}) | (sort_key or {}), + key=(partition_key or {}) | (sort_key or {}), model_class=model_class, ) diff --git a/lambdas/services/get_fhir_document_reference_service.py b/lambdas/services/get_fhir_document_reference_service.py index 183bbc3175..0bf3281608 100644 --- a/lambdas/services/get_fhir_document_reference_service.py +++ b/lambdas/services/get_fhir_document_reference_service.py @@ -24,7 +24,7 @@ def __init__(self): get_document_presign_url_aws_role_arn = os.getenv("PRESIGNED_ASSUME_ROLE") self.cloudfront_url = os.environ.get("CLOUDFRONT_URL") self.s3_service = S3Service( - custom_aws_role=get_document_presign_url_aws_role_arn + custom_aws_role=get_document_presign_url_aws_role_arn, ) self.ssm_service = SSMService() self.document_service = DocumentService() @@ -37,7 +37,8 @@ def handle_get_document_reference_request(self, snomed_code, document_id): document_reference = self.get_document_references(document_id, dynamo_table) else: document_reference = self.get_core_document_references( - document_id=document_id, snomed_code=snomed_code, table=dynamo_table + document_id=document_id, + table=dynamo_table, ) return document_reference @@ -47,26 +48,25 @@ def _get_dynamo_table_for_doc_type(self, doc_type: SnomedCode) -> str: return self.doc_router.resolve(doc_type) except KeyError: logger.error( - f"SNOMED code {doc_type.code} - {doc_type.display_name} is not supported" + f"SNOMED code {doc_type.code} - {doc_type.display_name} is not supported", ) raise GetFhirDocumentReferenceException(400, LambdaError.DocTypeInvalid) def get_document_references(self, document_id: str, table) -> DocumentReference: return self.fetch_documents( - search_key="ID", search_condition=document_id, table=table + search_key="ID", + search_condition=document_id, + table=table, ) def get_core_document_references( - self, document_id: str, snomed_code: str, table + self, + document_id: str, + table, ) -> DocumentReference: - index_name = "idx_gsi_snomed_code" - search_key = ["DocumentSnomedCodeType", "ID"] - search_condition = [snomed_code, document_id] - return self.fetch_documents( - search_key=search_key, - search_condition=search_condition, - index_name=index_name, - table=table, + return self.document_service.get_item( + document_id=document_id, + table_name=table, ) def fetch_documents( @@ -86,10 +86,10 @@ def fetch_documents( if len(documents) > 0: logger.info("Document found for given id") return documents[0] - else: - raise GetFhirDocumentReferenceException( - 404, LambdaError.DocumentReferenceNotFound - ) + raise GetFhirDocumentReferenceException( + 404, + LambdaError.DocumentReferenceNotFound, + ) def get_presigned_url(self, bucket_name, file_location): """ @@ -109,7 +109,8 @@ def get_presigned_url(self, bucket_name, file_location): return presign_url_response def create_document_reference_fhir_response( - self, document_reference: DocumentReference + self, + document_reference: DocumentReference, ) -> str: """ Creates a FHIR-compliant DocumentReference response for a given document. @@ -161,7 +162,7 @@ def create_document_reference_fhir_response( custodian=document_reference.current_gp_ods, attachment=document_details, snomed_code_doc_type=SnomedCodes.find_by_code( - document_reference.document_snomed_code_type + document_reference.document_snomed_code_type, ), ) .create_fhir_document_reference_object(document_reference) From c7f461683481a79ebf414452478e4b170d28ffe9 Mon Sep 17 00:00:00 2001 From: jameslinnell Date: Wed, 25 Feb 2026 14:50:04 +0000 Subject: [PATCH 02/35] [NDR-423] Unit tests --- apim/specification.yaml | 311 +++++++++--------- .../get_fhir_document_reference_handler.py | 24 +- .../models/fhir/R4/fhir_document_reference.py | 2 +- .../delete_fhir_document_reference_service.py | 34 -- .../document_reference_search_service.py | 6 +- .../fhir_document_reference_service_base.py | 6 +- .../get_fhir_document_reference_service.py | 2 +- lambdas/tests/unit/conftest.py | 2 +- ...est_get_fhir_document_reference_handler.py | 118 ++++--- ...t_fhir_document_reference_by_id_handler.py | 17 +- .../test_document_reference_search_service.py | 8 +- ...st_fhir_document_reference_service_base.py | 2 +- ...ir_document_reference_by_nhs_id_service.py | 84 ++--- ...t_fhir_document_reference_by_id_service.py | 28 +- ..._fhir_document_reference_search_service.py | 12 +- 15 files changed, 300 insertions(+), 356 deletions(-) diff --git a/apim/specification.yaml b/apim/specification.yaml index edc13fccd4..680115b201 100644 --- a/apim/specification.yaml +++ b/apim/specification.yaml @@ -177,14 +177,14 @@ info: version: 1.0.0 contact: name: NHS Digital API Management - url: 'https://digital.nhs.uk/developer/help-and-support' + url: "https://digital.nhs.uk/developer/help-and-support" email: api.management@nhs.net servers: - - url: 'https://sandbox.api.service.nhs.uk/national-document-repository/FHIR/R4' + - url: "https://sandbox.api.service.nhs.uk/national-document-repository/FHIR/R4" description: Sandbox environment. - - url: 'https://int.api.service.nhs.uk/national-document-repository/FHIR/R4' + - url: "https://int.api.service.nhs.uk/national-document-repository/FHIR/R4" description: Integration test environment. - - url: 'https://api.service.nhs.uk/national-document-repository/FHIR/R4' + - url: "https://api.service.nhs.uk/national-document-repository/FHIR/R4" description: Production environment. paths: /DocumentReference: @@ -356,41 +356,41 @@ paths: summary: Get a single document pointer operationId: readDocumentReference parameters: - - $ref: '#/components/parameters/id' - - $ref: '#/components/parameters/type' - - $ref: '#/components/parameters/requestId' - - $ref: '#/components/parameters/correlationId' + - $ref: "#/components/parameters/id" + - $ref: "#/components/parameters/type" + - $ref: "#/components/parameters/requestId" + - $ref: "#/components/parameters/correlationId" responses: - '200': + "200": description: Search successful response content: application/fhir+json;version=1: schema: - $ref: '#/components/schemas/DocumentReference' + $ref: "#/components/schemas/DocumentReference" example: resourceType: DocumentReference status: current docStatus: final type: coding: - - system: 'http://snomed.info/sct' - code: '16521000000101' + - system: "http://snomed.info/sct" + code: "16521000000101" display: Lloyd George record folder subject: identifier: - system: 'https://fhir.nhs.uk/Id/nhs-number' - value: '4409815415' - date: '2022-12-22T11:45:41+11:00' + system: "https://fhir.nhs.uk/Id/nhs-number" + value: "4409815415" + date: "2022-12-22T11:45:41+11:00" author: - identifier: - system: 'https://fhir.nhs.uk/Id/ods-organization-code' + system: "https://fhir.nhs.uk/Id/ods-organization-code" value: Organization/Y05868 authenticator: identifier: value: Organization/Y05868 custodian: identifier: - system: 'https://fhir.nhs.uk/Id/ods-organization-code' + system: "https://fhir.nhs.uk/Id/ods-organization-code" value: Y05868 content: - attachment: @@ -398,14 +398,14 @@ paths: language: en-US url: pre-signed url size: 100 - title: '1_of_1[example].pdf' - creation: '2022-12-22T09:45:41+11:00' - '400': + title: "1_of_1[example].pdf" + creation: "2022-12-22T09:45:41+11:00" + "400": description: Invalid request, missing id path parameter content: application/fhir+json;verson=1: schema: - $ref: '#/components/schemas/OperationOutcome' + $ref: "#/components/schemas/OperationOutcome" example: resourceType: OperationOutcome issue: @@ -413,16 +413,16 @@ paths: code: exception details: coding: - - system: 'http://hl7.org/fhir/issue-type' - code: 'invalid' - display: 'Invaild' - diagnostics: 'Invalid request' - '401': + - system: "http://hl7.org/fhir/issue-type" + code: "invalid" + display: "Invaild" + diagnostics: "Invalid request" + "401": description: User has no auth, missing token content: application/fhir+json;verson=1: schema: - $ref: '#/components/schemas/OperationOutcome' + $ref: "#/components/schemas/OperationOutcome" example: resourceType: OperationOutcome issue: @@ -430,16 +430,16 @@ paths: code: exception details: coding: - - system: 'http://hl7.org/fhir/issue-type' - code: 'unknown' - display: 'Unknown' - diagnostics: 'The user was not able to be authenticated' - '403': + - system: "http://hl7.org/fhir/issue-type" + code: "unknown" + display: "Unknown" + diagnostics: "The user was not able to be authenticated" + "403": description: User is unauthorised to view record content: application/fhir+json;verson=1: schema: - $ref: '#/components/schemas/OperationOutcome' + $ref: "#/components/schemas/OperationOutcome" example: resourceType: OperationOutcome issue: @@ -447,16 +447,16 @@ paths: code: exception details: coding: - - system: 'http://hl7.org/fhir/issue-type' - code: 'forbidden' - display: 'Forbidden' - diagnostics: 'User is unauthorised to view record' - '404': + - system: "http://hl7.org/fhir/issue-type" + code: "forbidden" + display: "Forbidden" + diagnostics: "User is unauthorised to view record" + "404": description: ID provided does not match any document held content: application/fhir+json;verson=1: schema: - $ref: '#/components/schemas/OperationOutcome' + $ref: "#/components/schemas/OperationOutcome" example: resourceType: OperationOutcome issue: @@ -464,17 +464,16 @@ paths: code: exception details: coding: - - system: 'http://hl7.org/fhir/issue-type' - code: 'not-found' - display: 'Not Found' - diagnostics: 'Document reference not found' - + - system: "http://hl7.org/fhir/issue-type" + code: "not-found" + display: "Not Found" + diagnostics: "Document reference not found" headers: X-Correlation-Id: - $ref: '#/components/headers/CorrelationId' + $ref: "#/components/headers/CorrelationId" X-Request-Id: - $ref: '#/components/headers/RequestId' + $ref: "#/components/headers/RequestId" 4XX: description: | @@ -493,7 +492,7 @@ paths: content: application/fhir+json;version=1: schema: - $ref: '#/components/schemas/OperationOutcome' + $ref: "#/components/schemas/OperationOutcome" example: resourceType: OperationOutcome issue: @@ -501,8 +500,8 @@ paths: code: value details: coding: - - system: 'https://fhir.nhs.uk/CodeSystem/NHSD-API-ErrorOrWarningCode' - version: '1' + - system: "https://fhir.nhs.uk/CodeSystem/NHSD-API-ErrorOrWarningCode" + version: "1" code: ACCESS_DENIED display: A parameter or value has resulted in a validation error diagnostics: The requested document cannot be read because it belongs to another organisation @@ -519,7 +518,7 @@ components: content: application/fhir+json;version=1: schema: - $ref: '#/components/schemas/DocumentReference' + $ref: "#/components/schemas/DocumentReference" required: true schemas: OperationOutcome: @@ -532,26 +531,26 @@ components: id: type: string pattern: '[A-Za-z0-9\-\.]{1,64}' - description: 'The logical id of the resource, as used in the URL for the resource. Once assigned, this value never changes.' + description: "The logical id of the resource, as used in the URL for the resource. Once assigned, this value never changes." meta: - $ref: '#/components/schemas/Meta' + $ref: "#/components/schemas/Meta" description: The metadata about the resource. This is content that is maintained by the infrastructure. Changes to the content might not always be associated with version changes to the resource. implicitRules: type: string pattern: \S* - description: 'A reference to a set of rules that were followed when the resource was constructed, and which must be understood when processing the content. Often, this is a reference to an implementation guide that defines the special rules along with other profiles etc.' + description: "A reference to a set of rules that were followed when the resource was constructed, and which must be understood when processing the content. Often, this is a reference to an implementation guide that defines the special rules along with other profiles etc." language: type: string pattern: '[^\s]+(\s[^\s]+)*' description: The base language in which the resource is written. text: - $ref: '#/components/schemas/Narrative' + $ref: "#/components/schemas/Narrative" description: 'A human–readable narrative that contains a summary of the resource and can be used to represent the content of the resource to a human. The narrative need not encode all the structured data, but is required to contain sufficient detail to make it "clinically safe" for a human to just read the narrative. Resource definitions may define what content should be represented in the narrative to ensure clinical safety.' issue: type: array items: - $ref: '#/components/schemas/OperationOutcomeIssue' - description: 'An error, warning, or information message that results from a system action.' + $ref: "#/components/schemas/OperationOutcomeIssue" + description: "An error, warning, or information message that results from a system action." minItems: 1 required: - resourceType @@ -570,9 +569,9 @@ components: code: type: string pattern: '[^\s]+(\s[^\s]+)*' - description: 'Describes the type of the issue. The system that creates an OperationOutcome SHALL choose the most applicable code from the IssueType value set, and may additional provide its own code for the error in the details element.' + description: "Describes the type of the issue. The system that creates an OperationOutcome SHALL choose the most applicable code from the IssueType value set, and may additional provide its own code for the error in the details element." details: - $ref: '#/components/schemas/CodeableConcept' + $ref: "#/components/schemas/CodeableConcept" description: Additional details about the error. This may be a text description of the error or a system code that identifies the error. diagnostics: type: string @@ -592,7 +591,7 @@ components: items: type: string pattern: '[ \r\n\t\S]+' - description: 'A [simple subset of FHIRPath](fhirpath.html#simple) limited to element names, repetition indicators and the default child accessor that identifies one of the elements in the resource that caused this issue to be raised.' + description: "A [simple subset of FHIRPath](fhirpath.html#simple) limited to element names, repetition indicators and the default child accessor that identifies one of the elements in the resource that caused this issue to be raised." required: - severity - code @@ -605,30 +604,30 @@ components: - DocumentReference id: type: string - pattern: '16521000000101~[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[0-9a-f]{4}-[0-9a-f]{12}' - description: 'The logical id of the resource, as used in the URL for the resource. Once assigned, this value never changes.' + pattern: "[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[0-9a-f]{4}-[0-9a-f]{12}" + description: "The logical id of the resource, as used in the URL for the resource. Once assigned, this value never changes." meta: - $ref: '#/components/schemas/Meta' + $ref: "#/components/schemas/Meta" description: The metadata about the resource. This is content that is maintained by the infrastructure. Changes to the content might not always be associated with version changes to the resource. implicitRules: type: string pattern: \S* - description: 'A reference to a set of rules that were followed when the resource was constructed, and which must be understood when processing the content. Often, this is a reference to an implementation guide that defines the special rules along with other profiles etc.' + description: "A reference to a set of rules that were followed when the resource was constructed, and which must be understood when processing the content. Often, this is a reference to an implementation guide that defines the special rules along with other profiles etc." language: type: string pattern: '[^\s]+(\s[^\s]+)*' description: The base language in which the resource is written. text: - $ref: '#/components/schemas/Narrative' + $ref: "#/components/schemas/Narrative" description: 'A human–readable narrative that contains a summary of the resource and can be used to represent the content of the resource to a human. The narrative need not encode all the structured data, but is required to contain sufficient detail to make it "clinically safe" for a human to just read the narrative. Resource definitions may define what content should be represented in the narrative to ensure clinical safety.' masterIdentifier: - $ref: '#/components/schemas/Identifier' + $ref: "#/components/schemas/Identifier" description: Document identifier as assigned by the source of the document. This identifier is specific to this version of the document. This unique identifier may be used elsewhere to identify this version of the document. identifier: type: array items: - $ref: '#/components/schemas/Identifier' - description: 'Other identifiers associated with the document, including version independent identifiers.' + $ref: "#/components/schemas/Identifier" + description: "Other identifiers associated with the document, including version independent identifiers." status: type: string pattern: '[^\s]+(\s[^\s]+)*' @@ -638,16 +637,16 @@ components: pattern: '[^\s]+(\s[^\s]+)*' description: The status of the underlying document. type: - $ref: '#/components/schemas/CodeableConcept' - description: 'Specifies the particular kind of document referenced (e.g. History and Physical, Discharge Summary, Progress Note). This usually equates to the purpose of making the document referenced.' + $ref: "#/components/schemas/CodeableConcept" + description: "Specifies the particular kind of document referenced (e.g. History and Physical, Discharge Summary, Progress Note). This usually equates to the purpose of making the document referenced." category: type: array items: - $ref: '#/components/schemas/CodeableConcept' + $ref: "#/components/schemas/CodeableConcept" description: A categorization for the type of document referenced – helps for indexing and searching. This may be implied by or derived from the code specified in the DocumentReference.type. subject: - $ref: '#/components/schemas/Reference' - description: 'Who or what the document is about. The document can be about a person, (patient or healthcare practitioner), a device (e.g. a machine) or even a group of subjects (such as a document about a herd of farm animals, or a set of patients that share a common exposure).' + $ref: "#/components/schemas/Reference" + description: "Who or what the document is about. The document can be about a person, (patient or healthcare practitioner), a device (e.g. a machine) or even a group of subjects (such as a document about a herd of farm animals, or a set of patients that share a common exposure)." date: type: string pattern: '([0-9]([0-9]([0-9][1-9]|[1-9]0)|[1-9]00)|[1-9]000)-(0[1-9]|1[0-2])-(0[1-9]|[1-2][0-9]|3[0-1])T([01][0-9]|2[0-3]):[0-5][0-9]:([0-5][0-9]|60)(\.[0-9]+)?(Z|(\+|-)((0[0-9]|1[0-3]):[0-5][0-9]|14:00))' @@ -655,18 +654,18 @@ components: author: type: array items: - $ref: '#/components/schemas/Reference' + $ref: "#/components/schemas/Reference" description: Identifies the Organisation or group who is responsible for adding the information to the document. authenticator: - $ref: '#/components/schemas/Reference' + $ref: "#/components/schemas/Reference" description: Which person or organization authenticates that this document is valid. custodian: - $ref: '#/components/schemas/Reference' + $ref: "#/components/schemas/Reference" description: Identifies the organization or group who is responsible for ongoing maintenance of and access to the document. relatesTo: type: array items: - $ref: '#/components/schemas/DocumentReferenceRelatesTo' + $ref: "#/components/schemas/DocumentReferenceRelatesTo" description: Relationships that this document has with other document references that already exist. description: type: string @@ -675,16 +674,16 @@ components: securityLabel: type: array items: - $ref: '#/components/schemas/CodeableConcept' + $ref: "#/components/schemas/CodeableConcept" description: 'A set of Security–Tag codes specifying the level of privacy/security of the Document. Note that DocumentReference.meta.security contains the security labels of the "reference" to the document, while DocumentReference.securityLabel contains a snapshot of the security labels on the document the reference refers to.' content: type: array items: - $ref: '#/components/schemas/DocumentReferenceContent' - description: 'The document and format referenced. There may be multiple content element repetitions, each with a different format.' + $ref: "#/components/schemas/DocumentReferenceContent" + description: "The document and format referenced. There may be multiple content element repetitions, each with a different format." minItems: 1 context: - $ref: '#/components/schemas/DocumentReferenceContext' + $ref: "#/components/schemas/DocumentReferenceContext" description: The clinical context in which the document was prepared. required: - resourceType @@ -875,29 +874,29 @@ components: encounter: type: array items: - $ref: '#/components/schemas/Reference' + $ref: "#/components/schemas/Reference" description: Describes the clinical encounter or type of care that the document content is associated with. event: type: array items: - $ref: '#/components/schemas/CodeableConcept' + $ref: "#/components/schemas/CodeableConcept" description: 'This list of codes represents the main clinical acts, such as a colonoscopy or an appendectomy, being documented. In some cases, the event is inherent in the type Code, such as a "History and Physical Report" in which the procedure being documented is necessarily a "History and Physical" act.' period: - $ref: '#/components/schemas/Period' + $ref: "#/components/schemas/Period" description: The time period over which the service that is described by the document was provided. facilityType: - $ref: '#/components/schemas/CodeableConcept' + $ref: "#/components/schemas/CodeableConcept" description: The kind of facility where the patient was seen. practiceSetting: - $ref: '#/components/schemas/CodeableConcept' - description: 'This property may convey specifics about the practice setting where the content was created, often reflecting the clinical specialty.' + $ref: "#/components/schemas/CodeableConcept" + description: "This property may convey specifics about the practice setting where the content was created, often reflecting the clinical specialty." sourcePatientInfo: - $ref: '#/components/schemas/Reference' - description: 'The Patient Information as known when the document was published. May be a reference to a version specific, or contained.' + $ref: "#/components/schemas/Reference" + description: "The Patient Information as known when the document was published. May be a reference to a version specific, or contained." related: type: array items: - $ref: '#/components/schemas/Reference' + $ref: "#/components/schemas/Reference" description: Related identifiers or resources associated with the DocumentReference. DocumentReferenceContent: type: object @@ -907,11 +906,11 @@ components: pattern: '[A-Za-z0-9\-\.]{1,64}' description: Unique id for the element within a resource (for internal references). This may be any string value that does not contain spaces. attachment: - $ref: '#/components/schemas/Attachment' + $ref: "#/components/schemas/Attachment" description: The document or URL of the document along with critical metadata to prove content has integrity. format: - $ref: '#/components/schemas/Coding' - description: 'An identifier of the document encoding, structure, and template that the document conforms to beyond the base format indicated in the mimeType.' + $ref: "#/components/schemas/Coding" + description: "An identifier of the document encoding, structure, and template that the document conforms to beyond the base format indicated in the mimeType." required: - attachment DocumentReferenceRelatesTo: @@ -926,7 +925,7 @@ components: pattern: '[^\s]+(\s[^\s]+)*' description: The type of relationship that this document has with anther document. target: - $ref: '#/components/schemas/Reference' + $ref: "#/components/schemas/Reference" description: The target document of this relationship. required: - code @@ -949,7 +948,7 @@ components: data: type: string pattern: '(\s*([0-9a-zA-Z\+/=]){4}\s*)+' - description: 'The actual data of the attachment – a sequence of bytes, base64 encoded.' + description: "The actual data of the attachment – a sequence of bytes, base64 encoded." url: type: string pattern: \S* @@ -957,7 +956,7 @@ components: size: type: integer format: int32 - description: 'The number of bytes of data that make up this attachment (before base64 encoding, if that is done).' + description: "The number of bytes of data that make up this attachment (before base64 encoding, if that is done)." hash: type: string pattern: '(\s*([0-9a-zA-Z\+/=]){4}\s*)+' @@ -980,7 +979,7 @@ components: coding: type: array items: - $ref: '#/components/schemas/Coding' + $ref: "#/components/schemas/Coding" description: A reference to a code defined by a terminology system. text: type: string @@ -1000,7 +999,7 @@ components: version: type: string pattern: '[ \r\n\t\S]+' - description: 'The version of the code system which was used when choosing this code. Note that a well–maintained code system does not need the version reported, because the meaning of codes is consistent across versions. However this cannot consistently be assured, and when the meaning is not guaranteed to be consistent, the version SHOULD be exchanged.' + description: "The version of the code system which was used when choosing this code. Note that a well–maintained code system does not need the version reported, because the meaning of codes is consistent across versions. However this cannot consistently be assured, and when the meaning is not guaranteed to be consistent, the version SHOULD be exchanged." code: type: string pattern: '[^\s]+(\s[^\s]+)*' @@ -1008,7 +1007,7 @@ components: display: type: string pattern: '[ \r\n\t\S]+' - description: 'A representation of the meaning of the code in the system, following the rules of the system.' + description: "A representation of the meaning of the code in the system, following the rules of the system." userSelected: type: boolean description: Indicates that this coding was chosen by a user directly – e.g. off a pick list of available items (codes or displays). @@ -1024,21 +1023,21 @@ components: pattern: '[^\s]+(\s[^\s]+)*' description: The purpose of this identifier. type: - $ref: '#/components/schemas/CodeableConcept' + $ref: "#/components/schemas/CodeableConcept" description: A coded type for the identifier that can be used to determine which identifier to use for a specific purpose. system: type: string pattern: \S* - description: 'Establishes the namespace for the value – that is, a URL that describes a set values that are unique.' + description: "Establishes the namespace for the value – that is, a URL that describes a set values that are unique." value: type: string pattern: '[ \r\n\t\S]+' description: The portion of the identifier typically relevant to the user and which is unique within the context of the system. period: - $ref: '#/components/schemas/Period' + $ref: "#/components/schemas/Period" description: Time period during which identifier is/was valid for use. assigner: - $ref: '#/components/schemas/Reference' + $ref: "#/components/schemas/Reference" description: Organization that issued/manages the identifier. Period: type: object @@ -1054,7 +1053,7 @@ components: end: type: string pattern: '([0-9]([0-9]([0-9][1-9]|[1-9]0)|[1-9]00)|[1-9]000)(-(0[1-9]|1[0-2])(-(0[1-9]|[1-2][0-9]|3[0-1])(T([01][0-9]|2[0-3]):[0-5][0-9]:([0-5][0-9]|60)(\.[0-9]+)?(Z|(\+|-)((0[0-9]|1[0-3]):[0-5][0-9]|14:00)))?)?)?' - description: 'The end of the period. If the end of the period is missing, it means no end was known or planned at the time the instance was created. The start may be in the past, and the end date in the future, which means that period is expected/planned to end at that time.' + description: "The end of the period. If the end of the period is missing, it means no end was known or planned at the time the instance was created. The start may be in the past, and the end date in the future, which means that period is expected/planned to end at that time." Quantity: type: object properties: @@ -1091,7 +1090,7 @@ components: reference: type: string pattern: '[ \r\n\t\S]+' - description: 'A reference to a location at which the other resource is found. The reference may be a relative reference, in which case it is relative to the service base URL, or an absolute URL that resolves to the location where the resource is found. The reference may be version specific or not. If the reference is not to a FHIR RESTful server, then it should be assumed to be version specific. Internal fragment references (start with ''#'') refer to contained resources.' + description: "A reference to a location at which the other resource is found. The reference may be a relative reference, in which case it is relative to the service base URL, or an absolute URL that resolves to the location where the resource is found. The reference may be version specific or not. If the reference is not to a FHIR RESTful server, then it should be assumed to be version specific. Internal fragment references (start with '#') refer to contained resources." type: type: string pattern: \S* @@ -1099,8 +1098,8 @@ components: The expected type of the target of the reference. If both Reference.type and Reference.reference are populated and Reference.reference is a FHIR URL, both SHALL be consistent. The type is the Canonical URL of Resource Definition that is the type this reference refers to. References are URLs that are relative to http://hl7.org/fhir/StructureDefinition/ e.g. "Patient" is a reference to http://hl7.org/fhir/StructureDefinition/Patient. Absolute URLs are only allowed for logical models (and can only be used in references in logical models, not resources). identifier: - $ref: '#/components/schemas/Identifier' - description: 'An identifier for the target resource. This is used when there is no way to reference the other resource directly, either because the entity it represents is not available through a FHIR server, or because there is no way for the author of the resource to convert a known identifier to an actual location. There is no requirement that a Reference.identifier point to something that is actually exposed as a FHIR instance, but it SHALL point to a business concept that would be expected to be exposed as a FHIR instance, and that instance would need to be of a FHIR resource type allowed by the reference.' + $ref: "#/components/schemas/Identifier" + description: "An identifier for the target resource. This is used when there is no way to reference the other resource directly, either because the entity it represents is not available through a FHIR server, or because there is no way for the author of the resource to convert a known identifier to an actual location. There is no requirement that a Reference.identifier point to something that is actually exposed as a FHIR instance, but it SHALL point to a business concept that would be expected to be exposed as a FHIR instance, and that instance would need to be of a FHIR resource type allowed by the reference." display: type: string pattern: '[ \r\n\t\S]+' @@ -1115,7 +1114,7 @@ components: type: type: array items: - $ref: '#/components/schemas/Coding' + $ref: "#/components/schemas/Coding" description: An indication of the reason that the entity signed this document. This may be explicitly included as part of the signature information and can be used when determining accountability for various actions concerning the document. minItems: 1 when: @@ -1123,10 +1122,10 @@ components: pattern: '([0-9]([0-9]([0-9][1-9]|[1-9]0)|[1-9]00)|[1-9]000)-(0[1-9]|1[0-2])-(0[1-9]|[1-2][0-9]|3[0-1])T([01][0-9]|2[0-3]):[0-5][0-9]:([0-5][0-9]|60)(\.[0-9]+)?(Z|(\+|-)((0[0-9]|1[0-3]):[0-5][0-9]|14:00))' description: When the digital signature was signed. who: - $ref: '#/components/schemas/Reference' + $ref: "#/components/schemas/Reference" description: A reference to an application–usable description of the identity that signed (e.g. the signature used their private key). onBehalfOf: - $ref: '#/components/schemas/Reference' + $ref: "#/components/schemas/Reference" description: A reference to an application–usable description of the identity that is represented by the signature. targetFormat: type: string @@ -1135,7 +1134,7 @@ components: sigFormat: type: string pattern: '[^\s]+(\s[^\s]+)*' - description: 'A mime type that indicates the technical format of the signature. Important mime types are application/signature+xml for X ML DigSig, application/jose for JWS, and image/* for a graphical image of a signature, etc.' + description: "A mime type that indicates the technical format of the signature. Important mime types are application/signature+xml for X ML DigSig, application/jose for JWS, and image/* for a graphical image of a signature, etc." data: type: string pattern: '(\s*([0-9a-zA-Z\+/=]){4}\s*)+' @@ -1154,7 +1153,7 @@ components: versionId: type: string pattern: '[A-Za-z0-9\-\.]{1,64}' - description: 'The version specific identifier, as it appears in the version portion of the URL. This value changes when the resource is created, updated, or deleted.' + description: "The version specific identifier, as it appears in the version portion of the URL. This value changes when the resource is created, updated, or deleted." lastUpdated: type: string pattern: '([0-9]([0-9]([0-9][1-9]|[1-9]0)|[1-9]00)|[1-9]000)-(0[1-9]|1[0-2])-(0[1-9]|[1-2][0-9]|3[0-1])T([01][0-9]|2[0-3]):[0-5][0-9]:([0-5][0-9]|60)(\.[0-9]+)?(Z|(\+|-)((0[0-9]|1[0-3]):[0-5][0-9]|14:00))' @@ -1162,23 +1161,23 @@ components: source: type: string pattern: \S* - description: 'A uri that identifies the source system of the resource. This provides a minimal amount of [Provenance](provenance.html#) information that can be used to track or differentiate the source of information in the resource. The source may identify another FHIR server, document, message, database, etc.' + description: "A uri that identifies the source system of the resource. This provides a minimal amount of [Provenance](provenance.html#) information that can be used to track or differentiate the source of information in the resource. The source may identify another FHIR server, document, message, database, etc." profile: type: array items: type: string pattern: \S* - description: 'A list of profiles (references to [StructureDefinition](structuredefinition.html#) resources) that this resource claims to conform to. The URL is a reference to [StructureDefinition.url](structuredefinition–definitions.html#StructureDefinition.url).' + description: "A list of profiles (references to [StructureDefinition](structuredefinition.html#) resources) that this resource claims to conform to. The URL is a reference to [StructureDefinition.url](structuredefinition–definitions.html#StructureDefinition.url)." security: type: array items: - $ref: '#/components/schemas/Coding' + $ref: "#/components/schemas/Coding" description: Security labels applied to this resource. These tags connect specific resources to the overall security policy and infrastructure. tag: type: array items: - $ref: '#/components/schemas/Coding' - description: 'Tags applied to this resource. Tags are intended to be used to identify and relate resources to process and workflow, and applications are not required to consider the tags when interpreting the meaning of a resource.' + $ref: "#/components/schemas/Coding" + description: "Tags applied to this resource. Tags are intended to be used to identify and relate resources to process and workflow, and applications are not required to consider the tags when interpreting the meaning of a resource." Narrative: type: object properties: @@ -1189,71 +1188,71 @@ components: status: type: string pattern: '[^\s]+(\s[^\s]+)*' - description: 'The status of the narrative – whether it''s entirely generated (from just the defined data or the extensions too), or whether a human authored it and it may contain additional data.' + description: "The status of the narrative – whether it's entirely generated (from just the defined data or the extensions too), or whether a human authored it and it may contain additional data." div: type: string - description: 'The actual narrative content, a stripped down version of XHTML.' + description: "The actual narrative content, a stripped down version of XHTML." required: - status - div DocumentId: type: string - pattern: '16521000000101~[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[0-9a-f]{4}-[0-9a-f]{12}' + pattern: "[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[0-9a-f]{4}-[0-9a-f]{12}" RequestPathParams: type: object properties: id: - $ref: '#/components/schemas/DocumentId' + $ref: "#/components/schemas/DocumentId" required: - id RequestHeader: type: object properties: odsCode: - $ref: '#/components/schemas/RequestHeaderOdsCode' + $ref: "#/components/schemas/RequestHeaderOdsCode" required: - odsCode RequestParams: type: object properties: - 'subject:identifier': - $ref: '#/components/schemas/RequestQuerySubject' - 'custodian:identifier': - $ref: '#/components/schemas/RequestQueryCustodian' + "subject:identifier": + $ref: "#/components/schemas/RequestQuerySubject" + "custodian:identifier": + $ref: "#/components/schemas/RequestQueryCustodian" type: - $ref: '#/components/schemas/RequestQueryType' + $ref: "#/components/schemas/RequestQueryType" category: - $ref: '#/components/schemas/RequestQueryCategory' + $ref: "#/components/schemas/RequestQueryCategory" next-page-token: - $ref: '#/components/schemas/NextPageToken' + $ref: "#/components/schemas/NextPageToken" required: - - 'subject:identifier' + - "subject:identifier" CountRequestParams: type: object properties: - 'subject:identifier': - $ref: '#/components/schemas/RequestQuerySubject' + "subject:identifier": + $ref: "#/components/schemas/RequestQuerySubject" required: - - 'subject:identifier' + - "subject:identifier" RequestQuerySubject: type: string pattern: '^https\:\/\/fhir\.nhs\.uk\/Id\/nhs-number\|(\d+)$' - example: 'https://fhir.nhs.uk/Id/nhs-number|4409815415' + example: "https://fhir.nhs.uk/Id/nhs-number|4409815415" RequestQueryCustodian: type: string pattern: '^https\:\/\/fhir\.nhs\.uk\/Id\/ods-organization-code\|(\w+)$' - example: 'https://fhir.nhs.uk/Id/ods-organization-code|Y05868' + example: "https://fhir.nhs.uk/Id/ods-organization-code|Y05868" RequestQueryType: type: string - example: 'http://snomed.info/sct|736253002' + example: "http://snomed.info/sct|736253002" RequestQueryCategory: type: string - example: 'http://snomed.info/sct|734163000' + example: "http://snomed.info/sct|734163000" NextPageToken: type: string RequestHeaderRequestId: type: string - pattern: '^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$' + pattern: "^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$" example: 60E0B220-8136-4CA5-AE46-1D97EF59D068 RequestHeaderCorrelationId: type: string @@ -1265,11 +1264,11 @@ components: required: true description: logical identifier schema: - $ref: '#/components/schemas/DocumentId' + $ref: "#/components/schemas/DocumentId" examples: valid: summary: Valid ID, document held, user has correct auth (200) - value: 16521000000101~f9ed81db-f90a-42d4-b7e4-d554d8f338fd + value: f9ed81db-f90a-42d4-b7e4-d554d8f338fd invalid_400: summary: Invalid request, no ID present (400) value: 400 @@ -1283,24 +1282,24 @@ components: summary: Document not Found, {id} does not match any documents held. (404) value: 404 subject: - name: 'subject:identifier' + name: "subject:identifier" in: query schema: - $ref: '#/components/schemas/RequestQuerySubject' + $ref: "#/components/schemas/RequestQuerySubject" required: true examples: none: summary: None - value: '' + value: "" valid_1: - summary: 'Valid #1' - value: 'https://fhir.nhs.uk/Id/nhs-number|4409815415' + summary: "Valid #1" + value: "https://fhir.nhs.uk/Id/nhs-number|4409815415" valid_2: - summary: 'Valid #2' - value: 'https://fhir.nhs.uk/Id/nhs-number|3495456481' + summary: "Valid #2" + value: "https://fhir.nhs.uk/Id/nhs-number|3495456481" invalid: summary: Unknown - value: 'https://fhir.nhs.uk/Id/nhs-number|3495456001' + value: "https://fhir.nhs.uk/Id/nhs-number|3495456001" custodian: name: custodian:identifier in: query @@ -1320,17 +1319,17 @@ components: name: type:identifier in: query schema: - $ref: '#/components/schemas/RequestQueryType' + $ref: "#/components/schemas/RequestQueryType" examples: none: summary: None - value: '' + value: "" SNOMED_CODES_LLOYD_GEORGE_RECORD_FOLDER: summary: Lloyd George record folder - value: 'http://snomed.info/sct|16521000000101' + value: "http://snomed.info/sct|16521000000101" invalid: summary: Unknown - value: 'http://snomed.info/sct|410970009' + value: "http://snomed.info/sct|410970009" nextPageToken: name: next-page-token description: | @@ -1350,7 +1349,7 @@ components: Mirrored back in a response header. in: header schema: - $ref: '#/components/schemas/RequestHeaderRequestId' + $ref: "#/components/schemas/RequestHeaderRequestId" correlationId: name: X-Correlation-ID description: | @@ -1364,20 +1363,20 @@ components: in: header required: false schema: - $ref: '#/components/schemas/RequestHeaderCorrelationId' + $ref: "#/components/schemas/RequestHeaderCorrelationId" responses: Success: description: Success Response content: application/fhir+json;version=1: schema: - $ref: '#/components/schemas/OperationOutcome' + $ref: "#/components/schemas/OperationOutcome" Error: description: Error Response content: application/fhir+json;version=1: schema: - $ref: '#/components/schemas/OperationOutcome' + $ref: "#/components/schemas/OperationOutcome" headers: CorrelationId: schema: @@ -1388,7 +1387,7 @@ components: RequestId: schema: type: string - pattern: '^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$' + pattern: "^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$" example: 60E0B220-8136-4CA5-AE46-1D97EF59D068 description: | The X-Request-ID from the request header mirrored back. diff --git a/lambdas/handlers/get_fhir_document_reference_handler.py b/lambdas/handlers/get_fhir_document_reference_handler.py index bdda76f1b5..0de1627d48 100644 --- a/lambdas/handlers/get_fhir_document_reference_handler.py +++ b/lambdas/handlers/get_fhir_document_reference_handler.py @@ -1,3 +1,5 @@ +import uuid + from oauthlib.oauth2 import WebApplicationClient from enums.lambda_error import LambdaError @@ -46,7 +48,7 @@ def lambda_handler(event, context): get_document_service = GetFhirDocumentReferenceService() document_reference = get_document_service.handle_get_document_reference_request( - snomed_code, + snomed_code.code, document_id, ) @@ -86,7 +88,7 @@ def lambda_handler(event, context): def extract_document_parameters(event): """Extract document ID and SNOMED code from path parameters""" path_params = event.get("pathParameters", {}).get("id", None) - document_id = get_id_and_snomed_from_path_parameters(path_params) + document_id = get_id_from_path_parameters(path_params) if not document_id: logger.error("Missing document id or snomed code in request path parameters.") @@ -133,7 +135,7 @@ def verify_user_authorisation(bearer_token, selected_role_id, nhs_number): raise GetFhirDocumentReferenceException(e.status_code, e.error) -def get_id_and_snomed_from_path_parameters(path_parameters): +def get_id_from_path_parameters(path_parameters): """Extract uuid from path parameters. Accepts: @@ -143,7 +145,21 @@ def get_id_and_snomed_from_path_parameters(path_parameters): if not path_parameters: return None - return path_parameters.split("~")[-1] + id = path_parameters.split("~")[-1] + if not is_uuid(id): + raise GetFhirDocumentReferenceException( + 400, + LambdaError.DocRefInvalidFiles, + ) + return id + + +def is_uuid(value: str) -> bool: + try: + uuid.UUID(value) + return True + except (ValueError, TypeError): + return False def _determine_document_type(common_name: MtlsCommonNames | None) -> SnomedCode: diff --git a/lambdas/models/fhir/R4/fhir_document_reference.py b/lambdas/models/fhir/R4/fhir_document_reference.py index 00f5b8c638..9699ffd3b3 100644 --- a/lambdas/models/fhir/R4/fhir_document_reference.py +++ b/lambdas/models/fhir/R4/fhir_document_reference.py @@ -255,7 +255,7 @@ def create_fhir_document_reference_object( fhir_doc_ref = DocumentReference( resourceType="DocumentReference", - id=f"{self.snomed_code_doc_type.code}~{document.id}", + id=document.id, docStatus=document.doc_status, type=CodeableConcept( coding=self._create_snomed_coding(self.snomed_code_doc_type), diff --git a/lambdas/services/delete_fhir_document_reference_service.py b/lambdas/services/delete_fhir_document_reference_service.py index 3238f228b9..122755b555 100644 --- a/lambdas/services/delete_fhir_document_reference_service.py +++ b/lambdas/services/delete_fhir_document_reference_service.py @@ -17,7 +17,6 @@ FhirDocumentReferenceServiceBase, ) from utils.audit_logging_setup import LoggingService -from utils.common_query_filters import NotDeleted from utils.exceptions import DynamoServiceException, InvalidNhsNumberException from utils.lambda_exceptions import ( DocumentDeletionServiceException, @@ -174,39 +173,6 @@ def delete_fhir_document_reference_by_nhs_id_and_doc_id( ) raise DocumentDeletionServiceException(500, LambdaError.DocDelClient) - def delete_fhir_document_references_by_nhs_id( - self, - nhs_number: str, - doc_type: SnomedCode, - ) -> list[DocumentReference] | None: - dynamo_table = self._get_dynamo_table_for_doc_type(doc_type) - document_service = DocumentService() - documents = document_service.fetch_documents_from_table( - search_condition=nhs_number, - search_key="NhsNumber", - table_name=dynamo_table, - query_filter=NotDeleted, - ) - if not documents: - return None - try: - document_service.delete_document_references( - table_name=dynamo_table, - document_references=documents, - document_ttl_days=DocumentRetentionDays.SOFT_DELETE, - ) - logger.info( - f"Deleted document of type {doc_type.display_name}", - {"Result": "Successful deletion"}, - ) - return documents - except (ClientError, DynamoServiceException) as e: - logger.error( - f"{LambdaError.DocDelClient.to_str()}: {str(e)}", - {"Results": "Failed to delete documents"}, - ) - raise DocumentDeletionServiceException(500, LambdaError.DocDelClient) - def _determine_document_type_based_on_common_name( self, common_name: MtlsCommonNames | None, diff --git a/lambdas/services/document_reference_search_service.py b/lambdas/services/document_reference_search_service.py index f4f91ca50c..119506bf90 100644 --- a/lambdas/services/document_reference_search_service.py +++ b/lambdas/services/document_reference_search_service.py @@ -234,11 +234,7 @@ def create_document_reference_fhir_response( title=document_reference.file_name, creation=document_reference.document_scan_creation or document_reference.created, - url=DOCUMENT_RETRIEVE_ENDPOINT - + "/" - + document_reference.document_snomed_code_type - + "~" - + document_reference.id, + url=DOCUMENT_RETRIEVE_ENDPOINT + "/" + document_reference.id, ) fhir_document_reference = ( DocumentReferenceInfo( diff --git a/lambdas/services/fhir_document_reference_service_base.py b/lambdas/services/fhir_document_reference_service_base.py index 75f6c2caf2..3fab347339 100644 --- a/lambdas/services/fhir_document_reference_service_base.py +++ b/lambdas/services/fhir_document_reference_service_base.py @@ -206,11 +206,7 @@ def _create_fhir_response( attachment_url = presigned_url else: attachment_url = ( - DOCUMENT_RETRIEVE_ENDPOINT - + "/" - + document_reference_ndr.document_snomed_code_type - + "~" - + document_reference_ndr.id + DOCUMENT_RETRIEVE_ENDPOINT + "/" + document_reference_ndr.id ) document_details = Attachment( title=document_reference_ndr.file_name, diff --git a/lambdas/services/get_fhir_document_reference_service.py b/lambdas/services/get_fhir_document_reference_service.py index 0bf3281608..b3de5be31d 100644 --- a/lambdas/services/get_fhir_document_reference_service.py +++ b/lambdas/services/get_fhir_document_reference_service.py @@ -63,7 +63,7 @@ def get_core_document_references( self, document_id: str, table, - ) -> DocumentReference: + ) -> DocumentReference | None: return self.document_service.get_item( document_id=document_id, table_name=table, diff --git a/lambdas/tests/unit/conftest.py b/lambdas/tests/unit/conftest.py index 75fd49077a..d873b2516d 100644 --- a/lambdas/tests/unit/conftest.py +++ b/lambdas/tests/unit/conftest.py @@ -84,7 +84,7 @@ MOCK_STATISTICS_REPORT_BUCKET_NAME = "test_statistics_report_bucket" REVIEW_SQS_QUEUE_URL = "test_review_queue" TEST_NHS_NUMBER = "9000000009" -TEST_UUID = "1234-4567-8912-HSDF-TEST" +TEST_UUID = "550e8400-e29b-41d4-a716-446655440000" TEST_FILE_KEY = "test_file_key" TEST_FILE_NAME = "test.pdf" TEST_FILE_SIZE = 24000 diff --git a/lambdas/tests/unit/handlers/test_get_fhir_document_reference_handler.py b/lambdas/tests/unit/handlers/test_get_fhir_document_reference_handler.py index cdd3e6f873..124aec526f 100644 --- a/lambdas/tests/unit/handlers/test_get_fhir_document_reference_handler.py +++ b/lambdas/tests/unit/handlers/test_get_fhir_document_reference_handler.py @@ -2,11 +2,12 @@ from copy import deepcopy import pytest + from enums.lambda_error import LambdaError from enums.snomed_codes import SnomedCodes from handlers.get_fhir_document_reference_handler import ( extract_document_parameters, - get_id_and_snomed_from_path_parameters, + get_id_from_path_parameters, lambda_handler, verify_user_authorisation, ) @@ -40,7 +41,7 @@ MOCK_EVENT_APPLICATION["headers"] = {"Authorization": f"Bearer {TEST_UUID}"} MOCK_DOCUMENT_REFERENCE = DocumentReference.model_validate( - MOCK_SEARCH_RESPONSE["Items"][0] + MOCK_SEARCH_RESPONSE["Items"][0], ) @@ -58,7 +59,7 @@ def mock_oidc_service(mocker): @pytest.fixture def mock_config_service(mocker): mock_config = mocker.patch( - "handlers.get_fhir_document_reference_handler.DynamicConfigurationService" + "handlers.get_fhir_document_reference_handler.DynamicConfigurationService", ) mock_config_instance = mock_config.return_value return mock_config_instance @@ -67,7 +68,7 @@ def mock_config_service(mocker): @pytest.fixture def mock_document_service(mocker): mock_service = mocker.patch( - "handlers.get_fhir_document_reference_handler.GetFhirDocumentReferenceService" + "handlers.get_fhir_document_reference_handler.GetFhirDocumentReferenceService", ) mock_service_instance = mock_service.return_value mock_service_instance.handle_get_document_reference_request.return_value = ( @@ -79,7 +80,7 @@ def mock_document_service(mocker): @pytest.fixture def mock_search_patient_service(mocker): mock_service = mocker.patch( - "handlers.get_fhir_document_reference_handler.SearchPatientDetailsService" + "handlers.get_fhir_document_reference_handler.SearchPatientDetailsService", ) mock_service_instance = mock_service.return_value return mock_service_instance @@ -106,13 +107,15 @@ def test_lambda_handler_happy_path_with_cis2_login( mock_oidc_service.fetch_user_org_code.assert_called_once() mock_oidc_service.fetch_user_role_code.assert_called_once() mock_document_service.handle_get_document_reference_request.assert_called_once_with( - SNOMED_CODE, TEST_UUID + SNOMED_CODE, + TEST_UUID, ) mock_search_patient_service.handle_search_patient_request.assert_called_once_with( - "9000000009", False + "9000000009", + False, ) mock_document_service.create_document_reference_fhir_response.assert_called_once_with( - MOCK_DOCUMENT_REFERENCE + MOCK_DOCUMENT_REFERENCE, ) @@ -137,11 +140,12 @@ def test_lambda_handler_happy_path_with_application_login( mock_oidc_service.fetch_user_org_code.assert_not_called() mock_oidc_service.fetch_user_role_code.assert_not_called() mock_document_service.handle_get_document_reference_request.assert_called_once_with( - SNOMED_CODE, TEST_UUID + SNOMED_CODE, + TEST_UUID, ) mock_search_patient_service.handle_search_patient_request.assert_not_called() mock_document_service.create_document_reference_fhir_response.assert_called_once_with( - MOCK_DOCUMENT_REFERENCE + MOCK_DOCUMENT_REFERENCE, ) @@ -161,20 +165,19 @@ def test_extract_missing_bearer_token(context): def test_extract_document_parameters_valid(): - document_id, snomed_code = extract_document_parameters(MOCK_CIS2_VALID_EVENT) + document_id = extract_document_parameters(MOCK_CIS2_VALID_EVENT) assert document_id == TEST_UUID - assert snomed_code == SNOMED_CODE def test_extract_document_parameters_invalid(): - with pytest.raises(GetFhirDocumentReferenceException) as e: - extract_document_parameters(MOCK_INVALID_EVENT_ID_MALFORMED) - assert e.value.status_code == 400 - assert e.value.error == LambdaError.DocumentReferenceMissingParameters + document_id = extract_document_parameters(MOCK_INVALID_EVENT_ID_MALFORMED) + assert document_id == TEST_UUID def test_verify_user_authorisation( - mock_oidc_service, mock_config_service, mock_search_patient_service + mock_oidc_service, + mock_config_service, + mock_search_patient_service, ): try: verify_user_authorisation(f"Bearer {TEST_UUID}", TEST_UUID, "9000000009") @@ -183,7 +186,9 @@ def test_verify_user_authorisation( def test_verify_user_authorization_raise_oidc_error( - mock_oidc_service, mock_config_service, mock_search_patient_service + mock_oidc_service, + mock_config_service, + mock_search_patient_service, ): mock_oidc_service.fetch_userinfo.side_effect = OidcApiException("OIDC error") with pytest.raises(GetFhirDocumentReferenceException) as excinfo: @@ -193,7 +198,9 @@ def test_verify_user_authorization_raise_oidc_error( def test_verify_user_authorization_raise_search_patient_error( - mock_oidc_service, mock_config_service, mock_search_patient_service + mock_oidc_service, + mock_config_service, + mock_search_patient_service, ): mock_search_patient_service.handle_search_patient_request.side_effect = ( SearchPatientException(403, LambdaError.MockError) @@ -205,7 +212,11 @@ def test_verify_user_authorization_raise_search_patient_error( def test_lambda_handler_missing_auth( - set_env, mock_oidc_service, mock_config_service, mock_document_service, context + set_env, + mock_oidc_service, + mock_config_service, + mock_document_service, + context, ): event_without_auth = deepcopy(MOCK_CIS2_VALID_EVENT) event_without_auth["headers"] = {} @@ -215,16 +226,12 @@ def test_lambda_handler_missing_auth( mock_document_service.handle_get_document_reference_request.assert_not_called() -def test_lambda_handler_id_malformed( - set_env, mock_oidc_service, mock_config_service, mock_document_service, context -): - response = lambda_handler(MOCK_INVALID_EVENT_ID_MALFORMED, context) - assert response["statusCode"] == 400 - mock_document_service.handle_get_document_reference_request.assert_not_called() - - def test_lambda_handler_oidc_error( - set_env, mock_config_service, mock_document_service, context, mocker + set_env, + mock_config_service, + mock_document_service, + context, + mocker, ): mocker.patch( "handlers.get_fhir_document_reference_handler.OidcService.set_up_oidc_parameters", @@ -238,7 +245,11 @@ def test_lambda_handler_oidc_error( def test_lambda_handler_invalid_path_parameters( - set_env, mock_oidc_service, mock_config_service, mock_document_service, context + set_env, + mock_oidc_service, + mock_config_service, + mock_document_service, + context, ): event_with_invalid_path = deepcopy(MOCK_CIS2_VALID_EVENT) event_with_invalid_path["pathParameters"] = {"id": "invalid_format_no_tilde"} @@ -312,34 +323,37 @@ def test_lambda_handler_search_service_errors( == LambdaError.MockError.value.get("fhir_coding").display ) assert response_body["issue"][0]["diagnostics"] == LambdaError.MockError.value.get( - "message" + "message", ) -def test_get_id_and_snomed_from_path_parameters(): - path_parameter = f"{SNOMED_CODE}~{TEST_UUID}" - - document_id, snomed = get_id_and_snomed_from_path_parameters(path_parameter) +@pytest.mark.parametrize( + "path_parameter", + [ + (f"{SNOMED_CODE}~{TEST_UUID}"), + (f"~{TEST_UUID}"), + (f"{TEST_UUID}"), + ], +) +def test_get_id_from_path_parameters(path_parameter): + document_id = get_id_from_path_parameters(path_parameter) assert document_id == TEST_UUID - assert snomed == SNOMED_CODE - - -def test_get_id_and_snomed_from_path_parameters_no_tilde(): - document_id, snomed = get_id_and_snomed_from_path_parameters("notildePresent") - assert document_id is None - assert snomed is None - -def test_get_id_and_snomed_from_path_parameters_extra_tildes(): - # Test with extra tildes - path_parameter = f"{SNOMED_CODE}~{TEST_UUID}~extra" - document_id, snomed = get_id_and_snomed_from_path_parameters(path_parameter) - assert document_id is None - assert snomed is None +@pytest.mark.parametrize( + "path_parameter", + [ + ("notauuid"), + (f"{SNOMED_CODE}~{TEST_UUID}~extra"), + ], +) +def test_get_id_from_path_parameters_raises_error(path_parameter): + with pytest.raises(GetFhirDocumentReferenceException) as excinfo: + get_id_from_path_parameters(path_parameter) + assert excinfo.value.status_code == 400 + assert excinfo.value.error == LambdaError.DocRefInvalidFiles -def test_get_id_and_snomed_from_path_parameters_empty(): - document_id, snomed = get_id_and_snomed_from_path_parameters("") +def test_get_id_from_path_parameters_empty(): + document_id = get_id_from_path_parameters("") assert document_id is None - assert snomed is None diff --git a/lambdas/tests/unit/handlers/test_pdm_get_fhir_document_reference_by_id_handler.py b/lambdas/tests/unit/handlers/test_pdm_get_fhir_document_reference_by_id_handler.py index ad1521349f..077b9b87b3 100644 --- a/lambdas/tests/unit/handlers/test_pdm_get_fhir_document_reference_by_id_handler.py +++ b/lambdas/tests/unit/handlers/test_pdm_get_fhir_document_reference_by_id_handler.py @@ -1,4 +1,5 @@ import pytest + from enums.mtls import MtlsCommonNames from enums.snomed_codes import SnomedCodes from handlers.get_fhir_document_reference_handler import ( @@ -15,7 +16,7 @@ MOCK_MTLS_VALID_EVENT = { "httpMethod": "GET", "headers": {}, - "pathParameters": {"id": f"{SNOMED_CODE}~{TEST_UUID}"}, + "pathParameters": {"id": f"{TEST_UUID}"}, "body": None, "requestContext": { "accountId": "123456789012", @@ -39,14 +40,14 @@ } MOCK_DOCUMENT_REFERENCE = DocumentReference.model_validate( - MOCK_SEARCH_RESPONSE["Items"][0] + MOCK_SEARCH_RESPONSE["Items"][0], ) @pytest.fixture def mock_config_service(mocker): mock_config = mocker.patch( - "handlers.get_fhir_document_reference_handler.DynamicConfigurationService" + "handlers.get_fhir_document_reference_handler.DynamicConfigurationService", ) mock_config_instance = mock_config.return_value return mock_config_instance @@ -55,7 +56,7 @@ def mock_config_service(mocker): @pytest.fixture def mock_document_service(mocker): mock_service = mocker.patch( - "handlers.get_fhir_document_reference_handler.GetFhirDocumentReferenceService" + "handlers.get_fhir_document_reference_handler.GetFhirDocumentReferenceService", ) mock_service_instance = mock_service.return_value mock_service_instance.handle_get_document_reference_request.return_value = ( @@ -89,10 +90,11 @@ def test_lambda_handler_happy_path_with_mtls_pdm_login( assert response["body"] == "test_document_reference" # Verify correct method calls mock_document_service.handle_get_document_reference_request.assert_called_once_with( - SNOMED_CODE, TEST_UUID + SNOMED_CODE, + TEST_UUID, ) mock_document_service.create_document_reference_fhir_response.assert_called_once_with( - MOCK_DOCUMENT_REFERENCE + MOCK_DOCUMENT_REFERENCE, ) @@ -102,6 +104,5 @@ def test_extract_bearer_token_when_pdm(context, mock_mtls_common_names): def test_extract_document_parameters_valid_pdm(): - document_id, snomed_code = extract_document_parameters(MOCK_MTLS_VALID_EVENT) + document_id = extract_document_parameters(MOCK_MTLS_VALID_EVENT) assert document_id == TEST_UUID - assert snomed_code == SNOMED_CODE diff --git a/lambdas/tests/unit/services/test_document_reference_search_service.py b/lambdas/tests/unit/services/test_document_reference_search_service.py index b9d751d899..bd9bf58988 100644 --- a/lambdas/tests/unit/services/test_document_reference_search_service.py +++ b/lambdas/tests/unit/services/test_document_reference_search_service.py @@ -329,7 +329,7 @@ def test_create_document_reference_fhir_response(mock_document_service, mocker): ) expected_fhir_response = { - "id": "16521000000101~Y05868-1634567890", + "id": "Y05868-1634567890", "resourceType": "DocumentReference", "status": "current", "docStatus": "final", @@ -374,7 +374,7 @@ def test_create_document_reference_fhir_response(mock_document_service, mocker): mock_attachment.assert_called_once_with( title=mock_document_reference.file_name, creation=mock_document_reference.document_scan_creation, - url=f"{APIM_API_URL}/DocumentReference/{SnomedCodes.LLOYD_GEORGE.value.code}~{mock_document_reference.id}", + url=f"{APIM_API_URL}/DocumentReference/{mock_document_reference.id}", ) mock_doc_ref_info.assert_called_once_with( @@ -409,7 +409,7 @@ def test_create_document_reference_fhir_response_integration( mock_document_reference.version = "1" expected_fhir_response = { - "id": "16521000000101~Y05868-1634567890", + "id": "Y05868-1634567890", "resourceType": "DocumentReference", "status": "current", "docStatus": "final", @@ -427,7 +427,7 @@ def test_create_document_reference_fhir_response_integration( "language": "en-GB", "title": "test_document.pdf", "creation": "2023-05-01", - "url": f"{APIM_API_URL}/DocumentReference/16521000000101~Y05868-1634567890", + "url": f"{APIM_API_URL}/DocumentReference/Y05868-1634567890", }, }, ], diff --git a/lambdas/tests/unit/services/test_fhir_document_reference_service_base.py b/lambdas/tests/unit/services/test_fhir_document_reference_service_base.py index 99ecdf03fc..ce29b597c9 100644 --- a/lambdas/tests/unit/services/test_fhir_document_reference_service_base.py +++ b/lambdas/tests/unit/services/test_fhir_document_reference_service_base.py @@ -593,7 +593,7 @@ def test_create_fhir_response_without_presigned_url(set_env, mock_service, mocke result_json = json.loads(result) assert result_json["resourceType"] == "DocumentReference" - expected_url = f"{APIM_API_URL}/DocumentReference/{SnomedCodes.LLOYD_GEORGE.value.code}~{document_ref.id}" + expected_url = f"{APIM_API_URL}/DocumentReference/{document_ref.id}" assert result_json["content"][0]["attachment"]["url"] == expected_url diff --git a/lambdas/tests/unit/services/test_pdm_delete_fhir_document_reference_by_nhs_id_service.py b/lambdas/tests/unit/services/test_pdm_delete_fhir_document_reference_by_nhs_id_service.py index 938f714e03..05d91fb49b 100644 --- a/lambdas/tests/unit/services/test_pdm_delete_fhir_document_reference_by_nhs_id_service.py +++ b/lambdas/tests/unit/services/test_pdm_delete_fhir_document_reference_by_nhs_id_service.py @@ -1,27 +1,25 @@ import uuid -from datetime import datetime, timezone from unittest.mock import MagicMock, patch -from lambdas.models import document_reference import pytest from botocore.exceptions import ClientError + from enums.document_retention import DocumentRetentionDays from enums.metadata_field_names import DocumentReferenceMetadataFields from enums.mtls import MtlsCommonNames from enums.snomed_codes import SnomedCodes -from models.document_reference import DocumentReference, Identifier +from models.document_reference import DocumentReference from services.delete_fhir_document_reference_service import ( DeleteFhirDocumentReferenceService, ) from tests.unit.conftest import TEST_NHS_NUMBER from tests.unit.helpers.data.dynamo.dynamo_responses import MOCK_SEARCH_RESPONSE -from utils.common_query_filters import NotDeleted from utils.lambda_exceptions import ( DocumentRefException, ) MOCK_DOCUMENT_REFERENCE = DocumentReference.model_validate( - MOCK_SEARCH_RESPONSE["Items"][0] + MOCK_SEARCH_RESPONSE["Items"][0], ) TEST_DOCUMENT_ID = "3d8683b9-1665-40d2-8499-6e8302d507ff" @@ -73,7 +71,7 @@ "httpMethod": "DELETE", "headers": {}, "queryStringParameters": { - "foo": f"https://fhir.nhs.uk/Id/nhs-number|{TEST_NHS_NUMBER}" + "foo": f"https://fhir.nhs.uk/Id/nhs-number|{TEST_NHS_NUMBER}", }, "body": None, } @@ -99,7 +97,7 @@ "httpMethod": "DELETE", "headers": {}, "queryStringParameters": { - "subject:identifier": f"https://fhir.nhs.uk/Id/nhs-number|{TEST_NHS_NUMBER}" + "subject:identifier": f"https://fhir.nhs.uk/Id/nhs-number|{TEST_NHS_NUMBER}", }, "pathParameters": {"id": f"{TEST_DOCUMENT_ID}"}, "body": None, @@ -125,7 +123,7 @@ def test_none(service): def test_extract_path_parameter_no_path_param_exists_for_id(service): identifier = service.extract_document_path_parameters( - MOCK_MTLS_VALID_EVENT_BY_NHS_ID + MOCK_MTLS_VALID_EVENT_BY_NHS_ID, ) assert identifier is None @@ -134,21 +132,21 @@ def test_extract_path_parameter_no_path_param_exists_for_id_but_pathParameters_e service, ): identifier = service.extract_document_path_parameters( - MOCK_MTLS_VALID_EVENT_WITH_INVALID_PATH_PARAM + MOCK_MTLS_VALID_EVENT_WITH_INVALID_PATH_PARAM, ) assert identifier is None def test_extract_path_parameter_path_param_exists_for_id(service): identifier = service.extract_document_path_parameters( - MOCK_MTLS_VALID_EVENT_WITH_VALID_PATH_PARAM_NO_QUERY_PARAM + MOCK_MTLS_VALID_EVENT_WITH_VALID_PATH_PARAM_NO_QUERY_PARAM, ) assert identifier is TEST_DOCUMENT_ID def test_extract_query_parameter_for_id_and_nhsnumber(service): identifiers = service.extract_document_query_parameters( - MOCK_MTLS_VALID_EVENT_BY_NHS_ID["queryStringParameters"] + MOCK_MTLS_VALID_EVENT_BY_NHS_ID["queryStringParameters"], ) assert identifiers[0] == TEST_NHS_NUMBER assert identifiers[1] == TEST_DOCUMENT_ID @@ -156,21 +154,21 @@ def test_extract_query_parameter_for_id_and_nhsnumber(service): def test_extract_query_parameter_with_invalid_query_parameter(service): identifiers = service.extract_document_query_parameters( - MOCK_MTLS_VALID_EVENT_WITH_INVALID_QUERY_PARAM["queryStringParameters"] + MOCK_MTLS_VALID_EVENT_WITH_INVALID_QUERY_PARAM["queryStringParameters"], ) assert identifiers == (None, None) def test_extract_query_parameter_when_non_existent(service): identifiers = service.extract_document_query_parameters( - MOCK_MTLS_VALID_EVENT_WITH_VALID_PATH_PARAM_NO_QUERY_PARAM + MOCK_MTLS_VALID_EVENT_WITH_VALID_PATH_PARAM_NO_QUERY_PARAM, ) assert identifiers == (None, None) def test_extract_query_parameter_with_too_many(service): identifiers = service.extract_document_query_parameters( - MOCK_MTLS_VALID_EVENT_WITH_INVALID_QUERY_PARAM_AND_VALID_QUERY_PARAMS + MOCK_MTLS_VALID_EVENT_WITH_INVALID_QUERY_PARAM_AND_VALID_QUERY_PARAMS, ) assert identifiers == (None, None) @@ -190,7 +188,7 @@ def test_extract_parameters_when_no_query_and_no_path(service): def test_doc_type_by_common_name_pdm(service): doc_type = service._determine_document_type_based_on_common_name( - MtlsCommonNames.PDM + MtlsCommonNames.PDM, ) assert doc_type.code == SnomedCodes.PATIENT_DATA.value.code @@ -201,7 +199,6 @@ def test_doc_type_by_common_name_None(service): def test_delete_fhir_document_references_by_nhs_id_and_doc_id_happy_path(service): - nhs_number = "9000000009" doc_type = MagicMock() doc_type.value = "test-doc-type" @@ -213,15 +210,14 @@ def test_delete_fhir_document_references_by_nhs_id_and_doc_id_happy_path(service service.delete_document_reference = MagicMock() with patch( - "services.delete_fhir_document_reference_service.DocumentService" + "services.delete_fhir_document_reference_service.DocumentService", ) as mock_document_service_cls: mock_document_service = MagicMock() mock_document_service.get_item_agnostic.return_value = mock_document mock_document_service_cls.return_value = mock_document_service - result = service.delete_fhir_document_reference_by_nhs_id_and_doc_id( - nhs_number=nhs_number, + result = service.delete_fhir_document_reference_by_doc_id( doc_id=TEST_DOCUMENT_ID, doc_type=doc_type, ) @@ -231,8 +227,7 @@ def test_delete_fhir_document_references_by_nhs_id_and_doc_id_happy_path(service service._get_dynamo_table_for_doc_type.assert_called_once_with(doc_type) mock_document_service.get_item_agnostic.assert_called_once_with( - partion_key={DocumentReferenceMetadataFields.NHS_NUMBER.value: nhs_number}, - sort_key={DocumentReferenceMetadataFields.ID.value: TEST_DOCUMENT_ID}, + partition_key={DocumentReferenceMetadataFields.ID.value: TEST_DOCUMENT_ID}, table_name=dynamo_table, ) @@ -241,7 +236,6 @@ def test_delete_fhir_document_references_by_nhs_id_and_doc_id_happy_path(service document_reference=mock_document, document_ttl_days=DocumentRetentionDays.SOFT_DELETE, key_pair={ - DocumentReferenceMetadataFields.NHS_NUMBER.value: nhs_number, DocumentReferenceMetadataFields.ID.value: TEST_DOCUMENT_ID, }, ) @@ -253,15 +247,14 @@ def test_delete_fhir_document_references_by_nhs_id_no_documents(service): service.delete_document_reference = MagicMock() with patch( - "services.delete_fhir_document_reference_service.DocumentService" + "services.delete_fhir_document_reference_service.DocumentService", ) as mock_document_service_cls: mock_document_service = MagicMock() mock_document_service.get_item_agnostic.return_value = [] mock_document_service_cls.return_value = mock_document_service - result = service.delete_fhir_document_reference_by_nhs_id_and_doc_id( - nhs_number="9000000009", + result = service.delete_fhir_document_reference_by_doc_id( doc_id=TEST_DOCUMENT_ID, doc_type=MagicMock(), ) @@ -274,12 +267,13 @@ def test_delete_fhir_document_references_by_nhs_id_propagates_client_error(servi service._get_dynamo_table_for_doc_type = MagicMock(return_value="mock-table") with patch( - "services.delete_fhir_document_reference_service.DocumentService" + "services.delete_fhir_document_reference_service.DocumentService", ) as mock_document_service_cls: mock_document_service = MagicMock() mock_document_service.get_item_agnostic.side_effect = ClientError( - error_response={}, operation_name="Query" + error_response={}, + operation_name="Query", ) mock_document_service_cls.return_value = mock_document_service @@ -291,42 +285,6 @@ def test_delete_fhir_document_references_by_nhs_id_propagates_client_error(servi ) -def test_process_calls_delete_by_nhs_id_for_pdm(service): - event = {"requestContext": {}} - deletion_identifiers = [TEST_DOCUMENT_ID, "9000000009"] - - with patch( - "services.delete_fhir_document_reference_service.validate_common_name_in_mtls", - return_value="CN", - ), patch.object( - service, - "extract_parameters", - return_value=deletion_identifiers, - ), patch.object( - service, - "_determine_document_type_based_on_common_name", - return_value=SnomedCodes.PATIENT_DATA.value, - ), patch.object( - service, - "is_uuid", - return_value=True, - ), patch.object( - service, - "delete_fhir_document_reference_by_nhs_id_and_doc_id", - return_value=MOCK_DOCUMENT_REFERENCE, - ) as mock_delete: - - result = service.process_fhir_document_reference(event) - - assert result == MOCK_DOCUMENT_REFERENCE - - mock_delete.assert_called_once_with( - nhs_number=deletion_identifiers[1], - doc_id=deletion_identifiers[0], - doc_type=SnomedCodes.PATIENT_DATA.value, - ) - - def test_process_returns_none_when_only_one_identifier(service): event = {"requestContext": {}} diff --git a/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_by_id_service.py b/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_by_id_service.py index 4f051123c2..7fcd06b515 100644 --- a/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_by_id_service.py +++ b/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_by_id_service.py @@ -1,4 +1,5 @@ import pytest + from enums.infrastructure import DynamoTables from enums.lambda_error import LambdaError from enums.snomed_codes import SnomedCode, SnomedCodes @@ -6,7 +7,6 @@ from tests.unit.conftest import MOCK_PDM_TABLE_NAME from tests.unit.helpers.data.test_documents import create_test_doc_store_refs from utils.lambda_exceptions import ( - GetFhirDocumentReferenceException, InvalidDocTypeException, ) @@ -24,11 +24,10 @@ def patched_service(mocker, set_env, context): def test_get_document_reference_service(patched_service): documents = create_test_doc_store_refs() - patched_service.document_service.fetch_documents_from_table.return_value = documents + patched_service.document_service.get_item.return_value = documents[0] actual = patched_service.get_core_document_references( document_id="3d8683b9-1665-40d2-8499-6e8302d507ff", - snomed_code=SnomedCodes.PATIENT_DATA.value.code, table=MOCK_PDM_TABLE_NAME, ) assert actual == documents[0] @@ -40,11 +39,14 @@ def test_handle_get_document_reference_request(patched_service, mocker, set_env) expected = documents[0] mock_document_ref = documents[0] mocker.patch.object( - patched_service, "get_core_document_references", return_value=mock_document_ref + patched_service, + "get_core_document_references", + return_value=mock_document_ref, ) actual = patched_service.handle_get_document_reference_request( - SnomedCodes.PATIENT_DATA.value.code, "test-id" + SnomedCodes.PATIENT_DATA.value.code, + "test-id", ) assert expected == actual @@ -84,14 +86,10 @@ def test_get_dynamo_table_for_lloyd_george_doc_type(patched_service): def test_get_document_references_empty_result(patched_service): # Test when no documents are found - patched_service.document_service.fetch_documents_from_table.return_value = [] - - with pytest.raises(GetFhirDocumentReferenceException) as exc_info: - patched_service.get_core_document_references( - document_id="test-id", - snomed_code=SnomedCodes.PATIENT_DATA.value.code, - table=MOCK_PDM_TABLE_NAME, - ) + patched_service.document_service.get_item.return_value = None - assert exc_info.value.error == LambdaError.DocumentReferenceNotFound - assert exc_info.value.status_code == 404 + result = patched_service.get_core_document_references( + document_id="test-id", + table=MOCK_PDM_TABLE_NAME, + ) + assert result is None diff --git a/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_search_service.py b/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_search_service.py index be00f4b7da..b70123cd91 100644 --- a/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_search_service.py +++ b/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_search_service.py @@ -123,7 +123,7 @@ def test_create_document_reference_fhir_response(mock_document_service, mocker): ) expected_fhir_response = { - "id": "717391000000106~Y05868-1634567890", + "id": "Y05868-1634567890", "resourceType": "DocumentReference", "status": "current", "docStatus": "final", @@ -168,7 +168,7 @@ def test_create_document_reference_fhir_response(mock_document_service, mocker): mock_attachment.assert_called_once_with( title=mock_document_reference.file_name, creation=mock_document_reference.document_scan_creation, - url=f"{APIM_API_URL}/DocumentReference/{SnomedCodes.PATIENT_DATA.value.code}~{mock_document_reference.id}", + url=f"{APIM_API_URL}/DocumentReference/{mock_document_reference.id}", ) mock_doc_ref_info.assert_called_once_with( @@ -203,7 +203,7 @@ def test_create_document_reference_fhir_response_integration( mock_document_reference.version = "1" expected_fhir_response = { - "id": "717391000000106~Y05868-1634567890", + "id": "Y05868-1634567890", "resourceType": "DocumentReference", "status": "current", "docStatus": "final", @@ -221,7 +221,7 @@ def test_create_document_reference_fhir_response_integration( "language": "en-GB", "title": "test_document.pdf", "creation": "2023-05-01", - "url": f"{APIM_API_URL}/DocumentReference/717391000000106~Y05868-1634567890", + "url": f"{APIM_API_URL}/DocumentReference/Y05868-1634567890", }, }, ], @@ -278,7 +278,7 @@ def test_create_document_reference_fhir_response_no_title( mock_document_reference.version = "1" expected_fhir_response = { - "id": "717391000000106~Y05868-1634567890", + "id": "Y05868-1634567890", "resourceType": "DocumentReference", "status": "current", "docStatus": "final", @@ -295,7 +295,7 @@ def test_create_document_reference_fhir_response_no_title( "contentType": "application/pdf", "language": "en-GB", "creation": "2023-05-01", - "url": f"{APIM_API_URL}/DocumentReference/717391000000106~Y05868-1634567890", + "url": f"{APIM_API_URL}/DocumentReference/Y05868-1634567890", }, }, ], From fe671038786e646ea19c1de465d0feca1ec8376d Mon Sep 17 00:00:00 2001 From: jameslinnell Date: Wed, 25 Feb 2026 15:25:21 +0000 Subject: [PATCH 03/35] [NDR-423] e2e tests --- .../get_fhir_document_reference_handler.py | 2 +- .../get_fhir_document_reference_service.py | 8 ++++++- lambdas/tests/e2e/api/fhir/conftest.py | 2 +- ...test_retrieve_document_fhir_api_failure.py | 24 +++++-------------- lambdas/tests/e2e/helpers/data_helper.py | 4 ++-- 5 files changed, 17 insertions(+), 23 deletions(-) diff --git a/lambdas/handlers/get_fhir_document_reference_handler.py b/lambdas/handlers/get_fhir_document_reference_handler.py index 0de1627d48..f3a5663ade 100644 --- a/lambdas/handlers/get_fhir_document_reference_handler.py +++ b/lambdas/handlers/get_fhir_document_reference_handler.py @@ -5,7 +5,6 @@ from enums.lambda_error import LambdaError from enums.mtls import MtlsCommonNames from enums.snomed_codes import SnomedCode, SnomedCodes -from lambdas.utils.lambda_header_utils import validate_common_name_in_mtls from services.base.ssm_service import SSMService from services.dynamic_configuration_service import DynamicConfigurationService from services.get_fhir_document_reference_service import GetFhirDocumentReferenceService @@ -21,6 +20,7 @@ SearchPatientException, ) from utils.lambda_handler_utils import extract_bearer_token +from utils.lambda_header_utils import validate_common_name_in_mtls from utils.lambda_response import ApiGatewayResponse logger = LoggingService(__name__) diff --git a/lambdas/services/get_fhir_document_reference_service.py b/lambdas/services/get_fhir_document_reference_service.py index b3de5be31d..f74c23a91d 100644 --- a/lambdas/services/get_fhir_document_reference_service.py +++ b/lambdas/services/get_fhir_document_reference_service.py @@ -64,10 +64,16 @@ def get_core_document_references( document_id: str, table, ) -> DocumentReference | None: - return self.document_service.get_item( + documentreference = self.document_service.get_item( document_id=document_id, table_name=table, ) + if not documentreference: + raise GetFhirDocumentReferenceException( + 404, + LambdaError.DocumentReferenceNotFound, + ) + return documentreference def fetch_documents( self, diff --git a/lambdas/tests/e2e/api/fhir/conftest.py b/lambdas/tests/e2e/api/fhir/conftest.py index 891ac53f46..695fd49386 100644 --- a/lambdas/tests/e2e/api/fhir/conftest.py +++ b/lambdas/tests/e2e/api/fhir/conftest.py @@ -105,7 +105,7 @@ def get_pdm_document_reference( endpoint_override=None, ): if not endpoint_override: - url = f"https://{MTLS_ENDPOINT}/{resource_type}/{pdm_snomed}~{record_id}" + url = f"https://{MTLS_ENDPOINT}/{resource_type}/{record_id}" else: url = f"https://{MTLS_ENDPOINT}/{resource_type}/{endpoint_override}" headers = { diff --git a/lambdas/tests/e2e/api/fhir/test_retrieve_document_fhir_api_failure.py b/lambdas/tests/e2e/api/fhir/test_retrieve_document_fhir_api_failure.py index e5a4df3f9f..457a369b54 100644 --- a/lambdas/tests/e2e/api/fhir/test_retrieve_document_fhir_api_failure.py +++ b/lambdas/tests/e2e/api/fhir/test_retrieve_document_fhir_api_failure.py @@ -76,11 +76,11 @@ def test_retrieve_non_existant_document_reference( @pytest.mark.parametrize( "param,expected_status,expected_code", [ - (f"{pdm_data_helper.snomed_code}-{str(uuid.uuid4())}", 400, "MISSING_VALUE"), - (f"{pdm_data_helper.snomed_code}+{str(uuid.uuid4())}", 400, "MISSING_VALUE"), - (f"{pdm_data_helper.snomed_code}&{str(uuid.uuid4())}", 400, "MISSING_VALUE"), - (f"{pdm_data_helper.snomed_code}{str(uuid.uuid4())}", 400, "MISSING_VALUE"), - (f"{str(uuid.uuid4())}~{pdm_data_helper.snomed_code}", 500, "exception"), + (f"{pdm_data_helper.snomed_code}-{str(uuid.uuid4())}", 400, "invalid"), + (f"{pdm_data_helper.snomed_code}+{str(uuid.uuid4())}", 400, "invalid"), + (f"{pdm_data_helper.snomed_code}&{str(uuid.uuid4())}", 400, "invalid"), + (f"{pdm_data_helper.snomed_code}{str(uuid.uuid4())}", 400, "invalid"), + (f"{str(uuid.uuid4())}~{pdm_data_helper.snomed_code}", 400, "invalid"), ], ) def test_incorrectly_formatted_path_param_id( @@ -98,18 +98,6 @@ def test_incorrectly_formatted_path_param_id( _assert_operation_outcome(body=body, code=expected_code) -def test_no_snomed_code_in_path_param_id(test_data): - pdm_record = create_and_store_pdm_record(test_data) - response = get_pdm_document_reference( - record_id=pdm_record["id"], - pdm_snomed="", - ) - - body = response.json() - assert response.status_code == 400 - _assert_operation_outcome(body=body, code="MISSING_VALUE") - - def test_no_document_id_in_path_param_id(): response = get_pdm_document_reference() @@ -137,4 +125,4 @@ def test_extra_parameter_in_id_in_path_param_id(test_data): body = response.json() assert response.status_code == 400 - _assert_operation_outcome(body=body, code="MISSING_VALUE") + _assert_operation_outcome(body=body, code="invalid") diff --git a/lambdas/tests/e2e/helpers/data_helper.py b/lambdas/tests/e2e/helpers/data_helper.py index aa708b821b..6e29b5e8b0 100644 --- a/lambdas/tests/e2e/helpers/data_helper.py +++ b/lambdas/tests/e2e/helpers/data_helper.py @@ -219,13 +219,13 @@ def __init__(self): def retrieve_document_reference(self, record): return self.dynamo_service.get_item( table_name=self.dynamo_table, - key={"NhsNumber": record["nhs_number"], "ID": record["id"]}, + key={"ID": record["id"]}, ) def tidyup(self, record): self.dynamo_service.delete_item( table_name=self.dynamo_table, - key={"NhsNumber": record["nhs_number"], "ID": record["id"]}, + key={"ID": record["id"]}, ) self.s3_service.delete_object( self.s3_bucket, From b2e2decc74df6bf25549ac70424dd96544e09e18 Mon Sep 17 00:00:00 2001 From: jameslinnell Date: Wed, 25 Feb 2026 15:31:24 +0000 Subject: [PATCH 04/35] [NDR-423] 404 test --- ...pdm_get_fhir_document_reference_by_id_service.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_by_id_service.py b/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_by_id_service.py index 7fcd06b515..b6f4a84557 100644 --- a/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_by_id_service.py +++ b/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_by_id_service.py @@ -7,6 +7,7 @@ from tests.unit.conftest import MOCK_PDM_TABLE_NAME from tests.unit.helpers.data.test_documents import create_test_doc_store_refs from utils.lambda_exceptions import ( + GetFhirDocumentReferenceException, InvalidDocTypeException, ) @@ -88,8 +89,10 @@ def test_get_document_references_empty_result(patched_service): # Test when no documents are found patched_service.document_service.get_item.return_value = None - result = patched_service.get_core_document_references( - document_id="test-id", - table=MOCK_PDM_TABLE_NAME, - ) - assert result is None + with pytest.raises(GetFhirDocumentReferenceException) as excinfo: + patched_service.get_core_document_references( + document_id="test-id", + table=MOCK_PDM_TABLE_NAME, + ) + assert excinfo.value.status_code == 404 + assert excinfo.value.error == LambdaError.DocumentReferenceNotFound From a8d4da25e6c3d3c7e0b9b4a288fb73d2acf123f0 Mon Sep 17 00:00:00 2001 From: jameslinnell Date: Wed, 25 Feb 2026 16:18:29 +0000 Subject: [PATCH 05/35] [NDR-423] Don't return deleted items on get_item --- lambdas/services/document_service.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/lambdas/services/document_service.py b/lambdas/services/document_service.py index 065cdf4549..0472c1afb1 100644 --- a/lambdas/services/document_service.py +++ b/lambdas/services/document_service.py @@ -106,13 +106,21 @@ def fetch_documents_from_table( continue return documents - def _get_item(self, table_name, key, model_class): + def _get_item(self, table_name, key, model_class, return_deleted=False): try: response = self.dynamo_service.get_item(table_name=table_name, key=key) if "Item" not in response: logger.info("No document found") return None + if not return_deleted: + deleted = response.get("Item").get("deleted", None) + if deleted in (None, ""): + document = model_class.model_validate(response["Item"]) + return document + + return None + document = model_class.model_validate(response["Item"]) return document @@ -143,6 +151,7 @@ def get_item( sort_key: dict | None = None, table_name: str = None, model_class: type[BaseModel] = None, + return_deleted: bool = False, ) -> Optional[BaseModel]: """Fetch a single document by ID from a specified or configured table. @@ -164,6 +173,7 @@ def get_item( table_name=table_to_use, key=document_key, model_class=model_to_use, + return_deleted=return_deleted, ) def get_nhs_numbers_based_on_ods_code( From b19ef8063483630525f78b69eabb4a50d7425bbf Mon Sep 17 00:00:00 2001 From: jameslinnell Date: Wed, 25 Feb 2026 17:10:31 +0000 Subject: [PATCH 06/35] [NDR-423] Search tests --- lambdas/services/document_service.py | 2 +- .../fhir_document_reference_service_base.py | 2 +- .../api/fhir/test_search_patient_fhir_api.py | 160 ++++++++++++++++++ .../test_upload_document_fhir_api_failure.py | 2 +- .../test_upload_document_fhir_api_success.py | 10 +- 5 files changed, 168 insertions(+), 8 deletions(-) create mode 100644 lambdas/tests/e2e/api/fhir/test_search_patient_fhir_api.py diff --git a/lambdas/services/document_service.py b/lambdas/services/document_service.py index 0472c1afb1..217b94b2eb 100644 --- a/lambdas/services/document_service.py +++ b/lambdas/services/document_service.py @@ -114,7 +114,7 @@ def _get_item(self, table_name, key, model_class, return_deleted=False): return None if not return_deleted: - deleted = response.get("Item").get("deleted", None) + deleted = response.get("Item").get("Deleted", None) if deleted in (None, ""): document = model_class.model_validate(response["Item"]) return document diff --git a/lambdas/services/fhir_document_reference_service_base.py b/lambdas/services/fhir_document_reference_service_base.py index 3fab347339..0178d8cd85 100644 --- a/lambdas/services/fhir_document_reference_service_base.py +++ b/lambdas/services/fhir_document_reference_service_base.py @@ -227,7 +227,7 @@ def _create_fhir_response( .model_dump_json(exclude_none=True) ) - document_id = f"{document_reference_ndr.document_snomed_code_type}~{document_reference_ndr.id}" + document_id = f"{document_reference_ndr.id}" return fhir_document_reference, document_id diff --git a/lambdas/tests/e2e/api/fhir/test_search_patient_fhir_api.py b/lambdas/tests/e2e/api/fhir/test_search_patient_fhir_api.py new file mode 100644 index 0000000000..f561f71c2c --- /dev/null +++ b/lambdas/tests/e2e/api/fhir/test_search_patient_fhir_api.py @@ -0,0 +1,160 @@ +from datetime import datetime, timezone + +import pytest + +from enums.document_retention import DocumentRetentionDays +from tests.e2e.api.fhir.conftest import ( + MTLS_ENDPOINT, + PDM_SNOMED, + TEST_NHS_NUMBER, + UNKNOWN_TEST_NHS_NUMBER, + create_and_store_pdm_record, + create_mtls_session, +) +from tests.e2e.conftest import APIM_ENDPOINT +from tests.e2e.helpers.data_helper import PdmDataHelper + +pdm_data_helper = PdmDataHelper() + + +def search_document_reference( + nhs_number, + client_cert_path=None, + client_key_path=None, + resource_type="DocumentReference", +): + """Helper to perform search by NHS number with optional mTLS certs.""" + url = f"https://{MTLS_ENDPOINT}/{resource_type}?subject:identifier=https://fhir.nhs.uk/Id/nhs-number|{nhs_number}" + headers = { + "X-Correlation-Id": "1234", + } + + # Use provided certs if available, else defaults + if client_cert_path and client_key_path: + session = create_mtls_session(client_cert_path, client_key_path) + else: + session = create_mtls_session() + + return session.get(url, headers=headers) + + +def test_search_nonexistent_document_references_for_patient_details(): + response = search_document_reference(UNKNOWN_TEST_NHS_NUMBER) + assert response.status_code == 200 + + bundle = response.json() + assert bundle["resourceType"] == "Bundle" + assert bundle["type"] == "searchset" + assert bundle["total"] == 0 + assert "entry" in bundle + assert bundle["entry"] == [] + + +def test_search_patient_details(test_data): + created_record = create_and_store_pdm_record(test_data) + expected_record_id = created_record["id"] + + response = search_document_reference(TEST_NHS_NUMBER) + assert response.status_code == 200 + + bundle = response.json() + entries = bundle.get("entry", []) + assert entries + + # Find the entry with the matching record_id + matching_entry = next( + (e for e in entries if e["resource"].get("id") == f"{expected_record_id}"), + None, + ) + assert matching_entry + + attachment_url = matching_entry["resource"]["content"][0]["attachment"]["url"] + assert ( + f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{expected_record_id}" + in attachment_url + ) + + +def test_multiple_cancelled_search_patient_details(test_data): + record_ids = [ + create_and_store_pdm_record(test_data, doc_status="cancelled")["id"] + for _ in range(2) + ] + + response = search_document_reference(TEST_NHS_NUMBER) + assert response.status_code == 200 + + bundle = response.json() + entries = bundle.get("entry", []) + assert len(entries) >= 2 + + # Validate all created records exist and have status cancelled + for record_id in record_ids: + entry = next( + (e for e in entries if e["resource"].get("id") == f"{record_id}"), + None, + ) + assert entry + assert entry["resource"].get("docStatus") == "cancelled" + + +@pytest.mark.parametrize( + "nhs_number,expected_status,expected_code,expected_diagnostics", + [ + ("9999999993", 400, "INVALID_SEARCH_DATA", "Invalid patient number 9999999993"), + ("123", 400, "INVALID_SEARCH_DATA", "Invalid patient number 123"), + ], +) +def test_search_edge_cases( + nhs_number, + expected_status, + expected_code, + expected_diagnostics, +): + response = search_document_reference(nhs_number) + assert response.status_code == expected_status + + body = response.json() + issue = body["issue"][0] + details = issue.get("details", {}) + coding = details.get("coding", [{}])[0] + assert coding.get("code") == expected_code + assert issue.get("diagnostics") == expected_diagnostics + + +def test_search_patient_details_deleted_are_not_returned(test_data): + created_record_1 = create_and_store_pdm_record(test_data) + expected_record_id_1 = created_record_1["id"] + + deletion_date = datetime.now(timezone.utc) + document_ttl_days = DocumentRetentionDays.SOFT_DELETE + ttl_seconds = document_ttl_days * 24 * 60 * 60 + document_reference_ttl = int(deletion_date.timestamp() + ttl_seconds) + created_record_2 = create_and_store_pdm_record( + test_data, + doc_status="deprecated", + Deleted=deletion_date.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), + ttl=document_reference_ttl, + ) + expected_record_id_2 = created_record_2["id"] + + response = search_document_reference(TEST_NHS_NUMBER) + assert response.status_code == 200 + + bundle = response.json() + assert bundle["total"] < 2 + entries = bundle.get("entry", []) + assert entries + + # Find the entry with the matching record_id + matching_entry = next( + (e for e in entries if e["resource"].get("id") == f"{expected_record_id_1}"), + None, + ) + assert matching_entry + # Assert deleted item doesn't exist + non_matching_entry = next( + (e for e in entries if e["resource"].get("id") == f"{expected_record_id_2}"), + None, + ) + assert non_matching_entry is None diff --git a/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_failure.py b/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_failure.py index 73241a3b4f..621fec877b 100644 --- a/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_failure.py +++ b/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_failure.py @@ -50,7 +50,7 @@ def test_create_document_virus(test_data): assert raw_upload_response.status_code == 201 upload_response = raw_upload_response.json() - record["id"] = upload_response["id"].split("~")[1] + record["id"] = upload_response["id"] test_data.append(record) assert "Location" in raw_upload_response.headers diff --git a/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_success.py b/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_success.py index d3eedc8c46..964c41fc1c 100644 --- a/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_success.py +++ b/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_success.py @@ -37,7 +37,7 @@ def test_create_document_base64(test_data): raw_upload_response = upload_document(payload) assert raw_upload_response.status_code == 201 upload_response = raw_upload_response.json() - record["id"] = upload_response["id"].split("~")[1] + record["id"] = upload_response["id"] test_data.append(record) assert "Location" in raw_upload_response.headers @@ -47,7 +47,7 @@ def test_create_document_base64(test_data): # Validate attachment URL attachment_url = upload_response["content"][0]["attachment"]["url"] assert ( - f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{PDM_SNOMED}~" + f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{upload_response['id']}" in attachment_url ) @@ -129,7 +129,7 @@ def test_create_document_saves_raw(test_data): raw_upload_response = upload_document(payload) assert raw_upload_response.status_code == 201 upload_response = raw_upload_response.json() - record["id"] = upload_response["id"].split("~")[1] + record["id"] = upload_response["id"] test_data.append(record) assert "Location" in raw_upload_response.headers @@ -163,7 +163,7 @@ def test_create_document_without_author_or_type(test_data): raw_upload_response = upload_document(payload) assert raw_upload_response.status_code == 201 upload_response = raw_upload_response.json() - record["id"] = upload_response["id"].split("~")[1] + record["id"] = upload_response["id"] test_data.append(record) assert "Location" in raw_upload_response.headers @@ -195,7 +195,7 @@ def test_create_document_without_title(test_data): raw_upload_response = upload_document(payload) assert raw_upload_response.status_code == 201 upload_response = raw_upload_response.json() - record["id"] = upload_response["id"].split("~")[1] + record["id"] = upload_response["id"] test_data.append(record) assert "Location" in raw_upload_response.headers From af359e8a78b34c27f042edea05ed660978deba24 Mon Sep 17 00:00:00 2001 From: jameslinnell Date: Thu, 26 Feb 2026 09:36:20 +0000 Subject: [PATCH 07/35] [NDR-423] Docker fix --- .devcontainer/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile index 300ca809eb..6446d82352 100644 --- a/.devcontainer/Dockerfile +++ b/.devcontainer/Dockerfile @@ -1,4 +1,4 @@ -FROM --platform=linux/amd64 mcr.microsoft.com/devcontainers/python:3.11 +FROM --platform=linux/amd64 mcr.microsoft.com/devcontainers/python:3.11-bookworm RUN apt-get update && apt-get upgrade -y && apt-get install -y \ git \ From 471d5a8e184eb8dd2ff7f6a7358da865753a7a86 Mon Sep 17 00:00:00 2001 From: jameslinnell Date: Thu, 26 Feb 2026 10:22:27 +0000 Subject: [PATCH 08/35] [NDR-423] Update unit tests --- lambdas/tests/e2e/api/fhir/test_search_patient_fhir_api.py | 1 - .../e2e/api/fhir/test_upload_document_fhir_api_success.py | 1 - .../services/test_fhir_document_reference_service_base.py | 2 +- .../services/test_pdm_post_fhir_document_reference_service.py | 2 +- .../services/test_post_fhir_document_reference_service.py | 4 ++-- 5 files changed, 4 insertions(+), 6 deletions(-) diff --git a/lambdas/tests/e2e/api/fhir/test_search_patient_fhir_api.py b/lambdas/tests/e2e/api/fhir/test_search_patient_fhir_api.py index f561f71c2c..362e851e0b 100644 --- a/lambdas/tests/e2e/api/fhir/test_search_patient_fhir_api.py +++ b/lambdas/tests/e2e/api/fhir/test_search_patient_fhir_api.py @@ -5,7 +5,6 @@ from enums.document_retention import DocumentRetentionDays from tests.e2e.api.fhir.conftest import ( MTLS_ENDPOINT, - PDM_SNOMED, TEST_NHS_NUMBER, UNKNOWN_TEST_NHS_NUMBER, create_and_store_pdm_record, diff --git a/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_success.py b/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_success.py index 964c41fc1c..e08ecfb27b 100644 --- a/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_success.py +++ b/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_success.py @@ -4,7 +4,6 @@ import os from tests.e2e.api.fhir.conftest import ( - PDM_SNOMED, TEST_NHS_NUMBER, retrieve_document_with_retry, upload_document, diff --git a/lambdas/tests/unit/services/test_fhir_document_reference_service_base.py b/lambdas/tests/unit/services/test_fhir_document_reference_service_base.py index ce29b597c9..43ff574edf 100644 --- a/lambdas/tests/unit/services/test_fhir_document_reference_service_base.py +++ b/lambdas/tests/unit/services/test_fhir_document_reference_service_base.py @@ -565,7 +565,7 @@ def test_create_fhir_response_with_presigned_url(mock_service, mocker): result_json = json.loads(result) assert result_json["resourceType"] == "DocumentReference" assert result_json["content"][0]["attachment"]["url"] == presigned_url - assert document_id == f"{SnomedCodes.LLOYD_GEORGE.value.code}~test-id" + assert document_id == "test-id" def test_create_fhir_response_without_presigned_url(set_env, mock_service, mocker): diff --git a/lambdas/tests/unit/services/test_pdm_post_fhir_document_reference_service.py b/lambdas/tests/unit/services/test_pdm_post_fhir_document_reference_service.py index 1a06703617..0891cdefd2 100644 --- a/lambdas/tests/unit/services/test_pdm_post_fhir_document_reference_service.py +++ b/lambdas/tests/unit/services/test_pdm_post_fhir_document_reference_service.py @@ -443,7 +443,7 @@ def test_process_mtls_fhir_document_reference_with_binary( valid_mtls_request_context, ) ) - expected_document_id = f"{SnomedCodes.PATIENT_DATA.value.code}~{TEST_UUID}" + expected_document_id = TEST_UUID assert isinstance(json_result, str) result_json = json.loads(json_result) diff --git a/lambdas/tests/unit/services/test_post_fhir_document_reference_service.py b/lambdas/tests/unit/services/test_post_fhir_document_reference_service.py index 911ec2ca5a..6db6a3d952 100644 --- a/lambdas/tests/unit/services/test_post_fhir_document_reference_service.py +++ b/lambdas/tests/unit/services/test_post_fhir_document_reference_service.py @@ -182,7 +182,7 @@ def test_process_fhir_document_reference_with_presigned_url( ) ) expected_pre_sign_url = mock_presigned_url_response - expected_document_id = f"{SnomedCodes.LLOYD_GEORGE.value.code}~{TEST_UUID}" + expected_document_id = TEST_UUID assert isinstance(json_result, str) result_json = json.loads(json_result) @@ -207,7 +207,7 @@ def test_process_fhir_document_reference_with_binary( valid_fhir_doc_with_binary, ) ) - expected_document_id = f"{SnomedCodes.LLOYD_GEORGE.value.code}~{TEST_UUID}" + expected_document_id = TEST_UUID assert isinstance(json_result, str) result_json = json.loads(json_result) From 949be15277f81cc09739856f7450d6c27aeba252 Mon Sep 17 00:00:00 2001 From: jameslinnell Date: Thu, 26 Feb 2026 14:55:09 +0000 Subject: [PATCH 09/35] [NDR-423] upload service now checks PDM by ID --- .../upload_document_reference_service.py | 19 ++------------- .../test_upload_document_fhir_api_failure.py | 2 +- lambdas/tests/e2e/helpers/data_helper.py | 16 ------------- ...t_pdm_upload_document_reference_service.py | 23 ++----------------- 4 files changed, 5 insertions(+), 55 deletions(-) diff --git a/lambdas/services/upload_document_reference_service.py b/lambdas/services/upload_document_reference_service.py index 564df4b083..f729c0be89 100644 --- a/lambdas/services/upload_document_reference_service.py +++ b/lambdas/services/upload_document_reference_service.py @@ -58,14 +58,10 @@ def handle_upload_document_reference_request( try: object_parts = object_key.split("/") document_key = object_parts[-1] - nhs_number = None - if len(object_parts) > 1: - nhs_number = object_parts[-2] self._get_infrastructure_for_document_key(object_parts) preliminary_document_reference = self._fetch_preliminary_document_reference( document_key, - nhs_number, ) if not preliminary_document_reference: return @@ -102,22 +98,11 @@ def _get_infrastructure_for_document_key(self, object_parts: list[str]) -> None: def _fetch_preliminary_document_reference( self, document_key: str, - nhs_number: str | None = None, ) -> Optional[DocumentReference]: """Fetch document reference from the database""" try: - if self.doc_type.code != SnomedCodes.PATIENT_DATA.value.code: - search_key = "ID" - search_condition = document_key - else: - if not nhs_number: - logger.error( - f"Failed to process object key with ID: {document_key}", - ) - raise FileProcessingException(400, LambdaError.DocRefInvalidFiles) - - search_key = ["NhsNumber", "ID"] - search_condition = [nhs_number, document_key] + search_key = "ID" + search_condition = document_key documents = self.document_service.fetch_documents_from_table( search_key=search_key, diff --git a/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_failure.py b/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_failure.py index 621fec877b..98576411dc 100644 --- a/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_failure.py +++ b/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_failure.py @@ -54,7 +54,7 @@ def test_create_document_virus(test_data): test_data.append(record) assert "Location" in raw_upload_response.headers - expected_location = f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{upload_response['id']}" + expected_location = f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{record['id']}" assert raw_upload_response.headers["Location"] == expected_location # Poll until processing/scan completes diff --git a/lambdas/tests/e2e/helpers/data_helper.py b/lambdas/tests/e2e/helpers/data_helper.py index 6e29b5e8b0..c91fb46911 100644 --- a/lambdas/tests/e2e/helpers/data_helper.py +++ b/lambdas/tests/e2e/helpers/data_helper.py @@ -216,22 +216,6 @@ def __init__(self): "pdm_record", ) - def retrieve_document_reference(self, record): - return self.dynamo_service.get_item( - table_name=self.dynamo_table, - key={"ID": record["id"]}, - ) - - def tidyup(self, record): - self.dynamo_service.delete_item( - table_name=self.dynamo_table, - key={"ID": record["id"]}, - ) - self.s3_service.delete_object( - self.s3_bucket, - f"{record['nhs_number']}/{record['id']}", - ) - class LloydGeorgeDataHelper(DataHelper): def __init__(self): diff --git a/lambdas/tests/unit/services/test_pdm_upload_document_reference_service.py b/lambdas/tests/unit/services/test_pdm_upload_document_reference_service.py index 4bbc9c636f..2a0fd4a1f0 100644 --- a/lambdas/tests/unit/services/test_pdm_upload_document_reference_service.py +++ b/lambdas/tests/unit/services/test_pdm_upload_document_reference_service.py @@ -164,21 +164,19 @@ def test_fetch_preliminary_document_reference_success( ): """Test successful document reference fetching""" document_key = "test-doc-id" - nhs_number = "12345" pdm_service.document_service.fetch_documents_from_table.return_value = [ mock_pdm_document_reference, ] result = pdm_service._fetch_preliminary_document_reference( document_key=document_key, - nhs_number=nhs_number, ) assert result == mock_pdm_document_reference pdm_service.document_service.fetch_documents_from_table.assert_called_once_with( table_name=MOCK_PDM_TABLE_NAME, - search_condition=[nhs_number, document_key], - search_key=["NhsNumber", "ID"], + search_condition=document_key, + search_key="ID", query_filter=PreliminaryStatus, ) @@ -186,35 +184,21 @@ def test_fetch_preliminary_document_reference_success( def test_fetch_preliminary_document_reference_no_documents_found(pdm_service): """Test handling when no documents are found""" document_key = "test-doc-id" - nhs_number = "12345" pdm_service.document_service.fetch_documents_from_table.return_value = [] result = pdm_service._fetch_preliminary_document_reference( document_key=document_key, - nhs_number=nhs_number, ) assert result is None -def test_fetch_preliminary_document_reference_no_nhs_number(pdm_service): - """Test handling when no documents are found""" - document_key = "test-doc-id" - nhs_number = None - with pytest.raises(FileProcessingException): - pdm_service._fetch_preliminary_document_reference( - document_key=document_key, - nhs_number=nhs_number, - ) - - def test_fetch_preliminary_document_reference_multiple_documents_warning( pdm_service, mock_document_reference, ): """Test handling when multiple documents are found""" document_key = "test-doc-id" - nhs_number = "12345" mock_doc_2 = Mock(spec=DocumentReference) pdm_service.document_service.fetch_documents_from_table.return_value = [ mock_document_reference, @@ -223,7 +207,6 @@ def test_fetch_preliminary_document_reference_multiple_documents_warning( result = pdm_service._fetch_preliminary_document_reference( document_key=document_key, - nhs_number=nhs_number, ) assert result == mock_document_reference @@ -232,7 +215,6 @@ def test_fetch_preliminary_document_reference_multiple_documents_warning( def test_fetch_preliminary_document_reference_exception(pdm_service): """Test handling of exceptions during document fetching""" document_key = "test-doc-id" - nhs_number = "12345" pdm_service.document_service.fetch_documents_from_table.side_effect = ( ClientError({"error": "test error message"}, "test"), ) @@ -240,7 +222,6 @@ def test_fetch_preliminary_document_reference_exception(pdm_service): with pytest.raises(DocumentServiceException): pdm_service._fetch_preliminary_document_reference( document_key=document_key, - nhs_number=nhs_number, ) From e33cc2b0a0ad2b6327ea2488d5dac64413a04454 Mon Sep 17 00:00:00 2001 From: jameslinnell Date: Thu, 26 Feb 2026 15:12:35 +0000 Subject: [PATCH 10/35] [NDR-423] LG FHIR E2E tests fix --- lambdas/tests/e2e/api/test_search_patient_api.py | 11 +++++------ lambdas/tests/e2e/api/test_upload_document_api.py | 6 +++--- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/lambdas/tests/e2e/api/test_search_patient_api.py b/lambdas/tests/e2e/api/test_search_patient_api.py index 2a28349c66..7e75af1804 100644 --- a/lambdas/tests/e2e/api/test_search_patient_api.py +++ b/lambdas/tests/e2e/api/test_search_patient_api.py @@ -4,7 +4,8 @@ import requests from syrupy.filters import paths -from tests.e2e.conftest import API_ENDPOINT, API_KEY, APIM_ENDPOINT, LLOYD_GEORGE_SNOMED + +from tests.e2e.conftest import API_ENDPOINT, API_KEY, APIM_ENDPOINT from tests.e2e.helpers.data_helper import LloydGeorgeDataHelper data_helper = LloydGeorgeDataHelper() @@ -33,7 +34,7 @@ def test_search_patient_details(test_data, snapshot_json): attachment_url = bundle["entry"][0]["resource"]["content"][0]["attachment"]["url"] assert ( - f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{LLOYD_GEORGE_SNOMED}~" + f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{lloyd_george_record['id']}" in attachment_url ) @@ -182,8 +183,7 @@ def test_search_patient_details_deleted_are_not_returned(test_data): ( e for e in entries - if e["resource"].get("id") - == f"{LLOYD_GEORGE_SNOMED}~{lloyd_george_record['id']}" + if e["resource"].get("id") == f"{lloyd_george_record['id']}" ), None, ) @@ -193,8 +193,7 @@ def test_search_patient_details_deleted_are_not_returned(test_data): ( e for e in entries - if e["resource"].get("id") - == f"{LLOYD_GEORGE_SNOMED}~{second_lloyd_george_record['id']}" + if e["resource"].get("id") == f"{second_lloyd_george_record['id']}" ), None, ) diff --git a/lambdas/tests/e2e/api/test_upload_document_api.py b/lambdas/tests/e2e/api/test_upload_document_api.py index bc018ef247..3e6f866fed 100644 --- a/lambdas/tests/e2e/api/test_upload_document_api.py +++ b/lambdas/tests/e2e/api/test_upload_document_api.py @@ -137,7 +137,7 @@ def test_create_document_presign(test_data, snapshot_json): retrieve_response = requests.post(url, headers=headers, data=payload) upload_response = retrieve_response.json() - lloyd_george_record["id"] = upload_response["id"].split("~")[1] + lloyd_george_record["id"] = upload_response["id"] test_data.append(lloyd_george_record) presign_uri = upload_response["content"][0]["attachment"]["url"] del upload_response["content"][0]["attachment"]["url"] @@ -193,7 +193,7 @@ def test_create_document_virus(test_data, snapshot_json): retrieve_response = requests.post(url, headers=headers, data=payload) upload_response = retrieve_response.json() - lloyd_george_record["id"] = upload_response["id"].split("~")[1] + lloyd_george_record["id"] = upload_response["id"] test_data.append(lloyd_george_record) retrieve_url = ( @@ -233,7 +233,7 @@ def test_create_document_does_not_save_raw(test_data): retrieve_response = requests.post(url, headers=headers, data=payload) json_response = retrieve_response.json() - lloyd_george_record["id"] = json_response.get("id").split("~")[1] + lloyd_george_record["id"] = json_response.get("id") test_data.append(lloyd_george_record) doc_ref = data_helper.retrieve_document_reference(record=lloyd_george_record) assert "Item" in doc_ref From d8e697af6577387964c41b02d0f6733745d154af Mon Sep 17 00:00:00 2001 From: jameslinnell Date: Thu, 26 Feb 2026 16:05:20 +0000 Subject: [PATCH 11/35] [NDR-423] Update file update --- lambdas/services/upload_document_reference_service.py | 1 - lambdas/tests/e2e/api/test_upload_document_api.py | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/lambdas/services/upload_document_reference_service.py b/lambdas/services/upload_document_reference_service.py index f729c0be89..eb56176513 100644 --- a/lambdas/services/upload_document_reference_service.py +++ b/lambdas/services/upload_document_reference_service.py @@ -469,7 +469,6 @@ def _update_dynamo_table( if self.doc_type.code == SnomedCodes.PATIENT_DATA.value.code: update_fields.add("s3_file_key") update_key = { - DocumentReferenceMetadataFields.NHS_NUMBER.value: document.nhs_number, DocumentReferenceMetadataFields.ID.value: document.id, } diff --git a/lambdas/tests/e2e/api/test_upload_document_api.py b/lambdas/tests/e2e/api/test_upload_document_api.py index 3e6f866fed..88fa364f71 100644 --- a/lambdas/tests/e2e/api/test_upload_document_api.py +++ b/lambdas/tests/e2e/api/test_upload_document_api.py @@ -95,7 +95,7 @@ def test_create_document_base64(test_data, snapshot_json): retrieve_response = requests.post(url, headers=headers, data=payload) upload_response = retrieve_response.json() - lloyd_george_record["id"] = upload_response["id"].split("~")[1] + lloyd_george_record["id"] = upload_response["id"] test_data.append(lloyd_george_record) retrieve_url = ( From 4ab55b63d0df0b36b13b03ca1ff17bf33b3c1647 Mon Sep 17 00:00:00 2001 From: jameslinnell Date: Thu, 26 Feb 2026 16:10:59 +0000 Subject: [PATCH 12/35] [NDR-423] Fix upload test --- .../unit/services/test_pdm_upload_document_reference_service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lambdas/tests/unit/services/test_pdm_upload_document_reference_service.py b/lambdas/tests/unit/services/test_pdm_upload_document_reference_service.py index 2a0fd4a1f0..80f7c44ae4 100644 --- a/lambdas/tests/unit/services/test_pdm_upload_document_reference_service.py +++ b/lambdas/tests/unit/services/test_pdm_upload_document_reference_service.py @@ -454,7 +454,7 @@ def test_update_dynamo_table_clean_scan_result( pdm_service.document_service.update_document.assert_called_once_with( table_name=MOCK_PDM_TABLE_NAME, - key_pair={"NhsNumber": "9000000001", "ID": "test-doc-id"}, + key_pair={"ID": "test-doc-id"}, document=mock_pdm_document_reference, update_fields_name={ "virus_scanner_result", From e79e7584e69f6d2eb35637ac3f28b441e6a44e53 Mon Sep 17 00:00:00 2001 From: jameslinnell Date: Thu, 26 Feb 2026 16:12:38 +0000 Subject: [PATCH 13/35] [NDR-423] LG FHIR test --- lambdas/tests/e2e/api/test_upload_document_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lambdas/tests/e2e/api/test_upload_document_api.py b/lambdas/tests/e2e/api/test_upload_document_api.py index 88fa364f71..b88deb040f 100644 --- a/lambdas/tests/e2e/api/test_upload_document_api.py +++ b/lambdas/tests/e2e/api/test_upload_document_api.py @@ -111,7 +111,7 @@ def condition(response_json): attachment_url = upload_response["content"][0]["attachment"]["url"] assert ( - f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{LLOYD_GEORGE_SNOMED}~" + f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{lloyd_george_record['id']}" in attachment_url ) From f94837edaaeca7e4adb97f12ccff4f52dc3efbd3 Mon Sep 17 00:00:00 2001 From: jameslinnell Date: Fri, 27 Feb 2026 11:23:33 +0000 Subject: [PATCH 14/35] [NDR-423] Removed now broken 500 error --- .../e2e/api/fhir/test_mtls_api_failure.py | 33 ++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/lambdas/tests/e2e/api/fhir/test_mtls_api_failure.py b/lambdas/tests/e2e/api/fhir/test_mtls_api_failure.py index 03404131e3..375901b303 100644 --- a/lambdas/tests/e2e/api/fhir/test_mtls_api_failure.py +++ b/lambdas/tests/e2e/api/fhir/test_mtls_api_failure.py @@ -83,21 +83,24 @@ def test_unsupported_http_method_returns_fhir_error(http_method): ) -def test_5xx_response_is_fhir_compliant(test_data): - """Verify that a Lambda error returns a FHIR-compliant OperationOutcome via DEFAULT_5XX.""" - - reversed_id = f"{str(uuid.uuid4())}~{pdm_data_helper.snomed_code}" - - response = get_pdm_document_reference( - endpoint_override=reversed_id, - ) - - assert response.status_code == 500 - body = response.json() - assert body["resourceType"] == "OperationOutcome" - assert len(body["issue"]) > 0 - issue = body["issue"][0] - assert issue["severity"] == "error" +# Commented this out for now as this will no longer retunr a 500 but now a 400 +# invalid and I can't think of a way to get it to return a 500. Definietly something that needs to be tested though. +# +# def test_5xx_response_is_fhir_compliant(test_data): +# """Verify that a Lambda error returns a FHIR-compliant OperationOutcome via DEFAULT_5XX.""" +# +# reversed_id = f"{str(uuid.uuid4())}~{pdm_data_helper.snomed_code}" +# +# response = get_pdm_document_reference( +# endpoint_override=reversed_id, +# ) +# +# assert response.status_code == 500 +# body = response.json() +# assert body["resourceType"] == "OperationOutcome" +# assert len(body["issue"]) > 0 +# issue = body["issue"][0] +# assert issue["severity"] == "error" def test_mtls_invalid_common_name(): From d7155af26f5d907bf60e3e5080622de948e80940 Mon Sep 17 00:00:00 2001 From: jameslinnell Date: Tue, 3 Mar 2026 17:21:02 +0000 Subject: [PATCH 15/35] [NDR-423] Update tests --- lambdas/services/document_service.py | 29 +- .../upload_document_reference_service.py | 41 ++- lambdas/tests/unit/conftest.py | 2 +- .../unit/services/test_document_service.py | 162 +++++++++++ .../services/test_pdm_document_service.py | 160 +++++++++++ ...t_pdm_upload_document_reference_service.py | 255 +++++++++--------- 6 files changed, 498 insertions(+), 151 deletions(-) create mode 100644 lambdas/tests/unit/services/test_pdm_document_service.py diff --git a/lambdas/services/document_service.py b/lambdas/services/document_service.py index 217b94b2eb..f4ab11269d 100644 --- a/lambdas/services/document_service.py +++ b/lambdas/services/document_service.py @@ -1,6 +1,6 @@ import os from datetime import datetime, timezone -from typing import Optional +from typing import List, Optional from boto3.dynamodb.conditions import Attr, ConditionBase from botocore.exceptions import ClientError @@ -106,7 +106,16 @@ def fetch_documents_from_table( continue return documents - def _get_item(self, table_name, key, model_class, return_deleted=False): + def filter_item(self, response, filters=[]): + if len(filters) > 0: + for filter in filters: + for key in filter: + attribute = response.get("Item").get(key, None) + if attribute != filter[key]: + return False + return True + + def _get_item(self, table_name, key, model_class, return_deleted=False, filters=[]): try: response = self.dynamo_service.get_item(table_name=table_name, key=key) if "Item" not in response: @@ -116,16 +125,20 @@ def _get_item(self, table_name, key, model_class, return_deleted=False): if not return_deleted: deleted = response.get("Item").get("Deleted", None) if deleted in (None, ""): - document = model_class.model_validate(response["Item"]) - return document + if self.filter_item(response, filters): + document = model_class.model_validate(response.get("Item")) + return document return None - document = model_class.model_validate(response["Item"]) - return document + if self.filter_item(response, filters): + document = model_class.model_validate(response.get("Item")) + return document + + return None except ValidationError as e: - logger.error(f"Validation error on document: {response.get('Item')}") + logger.error(f"Validation error on document: {response.get('Item') or ''}") logger.error(f"{e}") return None @@ -152,6 +165,7 @@ def get_item( table_name: str = None, model_class: type[BaseModel] = None, return_deleted: bool = False, + filters: List[dict] = [], ) -> Optional[BaseModel]: """Fetch a single document by ID from a specified or configured table. @@ -174,6 +188,7 @@ def get_item( key=document_key, model_class=model_to_use, return_deleted=return_deleted, + filters=filters, ) def get_nhs_numbers_based_on_ods_code( diff --git a/lambdas/services/upload_document_reference_service.py b/lambdas/services/upload_document_reference_service.py index eb56176513..fd28b3a059 100644 --- a/lambdas/services/upload_document_reference_service.py +++ b/lambdas/services/upload_document_reference_service.py @@ -101,29 +101,44 @@ def _fetch_preliminary_document_reference( ) -> Optional[DocumentReference]: """Fetch document reference from the database""" try: - search_key = "ID" - search_condition = document_key + if self.doc_type.code != SnomedCodes.PATIENT_DATA.value.code: + search_key = "ID" + search_condition = document_key + documents = self.document_service.fetch_documents_from_table( + search_key=search_key, + search_condition=search_condition, + table_name=self.table_name, + query_filter=PreliminaryStatus, + ) + if not documents: + logger.error( + f"No document with the following key found in {self.table_name} table: {document_key}", + ) + logger.info("Skipping this object") + return None - documents = self.document_service.fetch_documents_from_table( - search_key=search_key, - search_condition=search_condition, + if len(documents) > 1: + logger.warning( + f"Multiple documents found for key {document_key}, using first one", + ) + + return documents[0] + + document = self.document_service.get_item( + document_id=document_key, table_name=self.table_name, - query_filter=PreliminaryStatus, + return_deleted=False, + filters=[{"doc_status": "preliminary"}], ) - if not documents: + if not document: logger.error( f"No document with the following key found in {self.table_name} table: {document_key}", ) logger.info("Skipping this object") return None - if len(documents) > 1: - logger.warning( - f"Multiple documents found for key {document_key}, using first one", - ) - - return documents[0] + return document except ClientError as e: logger.error( diff --git a/lambdas/tests/unit/conftest.py b/lambdas/tests/unit/conftest.py index d873b2516d..750ebf9861 100644 --- a/lambdas/tests/unit/conftest.py +++ b/lambdas/tests/unit/conftest.py @@ -68,7 +68,7 @@ MOCK_STATISTICAL_REPORTS_BUCKET_ENV_NAME = "STATISTICAL_REPORTS_BUCKET" MOCK_ARF_TABLE_NAME = "test_arf_dynamoDB_table" -MOCK_PDM_TABLE_NAME = "test_pdm_dynamoDB_table" +MOCK_PDM_TABLE_NAME = "COREDocumentMetadata" MOCK_LG_TABLE_NAME = "test_lg_dynamoDB_table" MOCK_UNSTITCHED_LG_TABLE_NAME = "test_unstitched_lg_table" MOCK_BULK_REPORT_TABLE_NAME = "test_report_dynamoDB_table" diff --git a/lambdas/tests/unit/services/test_document_service.py b/lambdas/tests/unit/services/test_document_service.py index a2fddb79c1..4fbfff213b 100644 --- a/lambdas/tests/unit/services/test_document_service.py +++ b/lambdas/tests/unit/services/test_document_service.py @@ -723,3 +723,165 @@ def test_get_item_document_id_with_sort_key(mock_service, mock_dynamo_service): table_name=MOCK_LG_TABLE_NAME, key={"ID": document_id, **sort_key}, ) + + +@pytest.mark.parametrize( + "document_id,table_name,file_name,expected_table,return_deleted", + [ + ( + "test-doc-id-123", + MOCK_LG_TABLE_NAME, + "test-file.pdf", + MOCK_LG_TABLE_NAME, + False, + ), + ("test-doc-id-456", MOCK_TABLE_NAME, "lg-file.pdf", MOCK_TABLE_NAME, False), + ( + "test-doc-id-123", + MOCK_LG_TABLE_NAME, + "test-file.pdf", + MOCK_LG_TABLE_NAME, + True, + ), + ("test-doc-id-456", MOCK_TABLE_NAME, "lg-file.pdf", MOCK_TABLE_NAME, True), + ], +) +def test_get_item_returns_deleted( + mock_service, + mock_dynamo_service, + document_id, + table_name, + file_name, + expected_table, + return_deleted, +): + """Test successful retrieval of a document with default or custom table.""" + mock_dynamo_response = { + "Item": { + "ID": document_id, + "NhsNumber": TEST_NHS_NUMBER, + "FileName": file_name, + "Created": "2023-01-01T00:00:00Z", + "Deleted": "foobar", + "VirusScannerResult": "Clean", + }, + } + + mock_dynamo_service.get_item.return_value = mock_dynamo_response + + result = mock_service._get_item( + table_name=table_name, + key={"ID": document_id}, + model_class=DocumentReference, + return_deleted=return_deleted, + ) + + mock_dynamo_service.get_item.assert_called_once_with( + table_name=expected_table, + key={"ID": document_id}, + ) + if return_deleted: + assert result is not None + assert isinstance(result, DocumentReference) + assert result.id == document_id + assert result.nhs_number == TEST_NHS_NUMBER + else: + assert result is None + + +@pytest.mark.parametrize( + "filters", + [ + ([{"DocStatus": "preliminary"}]), + ([{"DocStatus": "current"}]), + ], +) +def test_filters_on_get_item( + mock_service, + filters, +): + mock_dynamo_response = { + "Item": { + "ID": "test-doc-id-123", + "NhsNumber": TEST_NHS_NUMBER, + "FileName": "test-file.pdf", + "Created": "2023-01-01T00:00:00Z", + "Deleted": "foobar", + "VirusScannerResult": "Clean", + "DocStatus": "preliminary", + }, + } + + print(filters) + result = mock_service.filter_item(response=mock_dynamo_response, filters=filters) + if filters[0].get("DocStatus") == "preliminary": + assert result is True + else: + assert result is False + + +@pytest.mark.parametrize( + "document_id,table_name,file_name,expected_table,return_deleted,filters", + [ + ( + "test-doc-id-123", + MOCK_LG_TABLE_NAME, + "test-file.pdf", + MOCK_LG_TABLE_NAME, + False, + [{"DocStatus": "preliminary"}], + ), + ( + "test-doc-id-456", + MOCK_TABLE_NAME, + "lg-file.pdf", + MOCK_TABLE_NAME, + False, + [{"DocStatus": "current"}], + ), + ], +) +def test_get_item_returns_filtered( + mock_service, + mock_dynamo_service, + document_id, + table_name, + file_name, + expected_table, + return_deleted, + filters, +): + """Test successful retrieval of a document with default or custom table.""" + mock_dynamo_response = { + "Item": { + "ID": document_id, + "NhsNumber": TEST_NHS_NUMBER, + "FileName": file_name, + "Created": "2023-01-01T00:00:00Z", + "Deleted": "", + "VirusScannerResult": "Clean", + "DocStatus": "preliminary", + }, + } + + mock_dynamo_service.get_item.return_value = mock_dynamo_response + + result = mock_service._get_item( + table_name=table_name, + key={"ID": document_id}, + model_class=DocumentReference, + return_deleted=return_deleted, + filters=filters, + ) + + mock_dynamo_service.get_item.assert_called_once_with( + table_name=expected_table, + key={"ID": document_id}, + ) + if filters[0].get("DocStatus") == "preliminary": + assert result is not None + assert isinstance(result, DocumentReference) + assert result.id == document_id + assert result.nhs_number == TEST_NHS_NUMBER + else: + assert result is None diff --git a/lambdas/tests/unit/services/test_pdm_document_service.py b/lambdas/tests/unit/services/test_pdm_document_service.py new file mode 100644 index 0000000000..381689059e --- /dev/null +++ b/lambdas/tests/unit/services/test_pdm_document_service.py @@ -0,0 +1,160 @@ +from unittest.mock import MagicMock + +import pytest + +from enums.snomed_codes import SnomedCodes +from models.document_reference import DocumentReference +from services.document_service import DocumentService + + +@pytest.fixture +def preliminary_dynamo_item_dict(): + return { + "Item": { + "id": "test-doc-id", + "nhs_number": "9000000001", + "s3_file_key": f"fhir_upload/{SnomedCodes.PATIENT_DATA.value.code}/9000000001/test-doc-id", + "s3_bucket_name": "test-staging-bucket", + "file_size": 1234567890, + "doc_status": "preliminary", + "status": "current", + "file_name": None, + }, + } + + +@pytest.fixture +def service(): + service = DocumentService() + service.filter_item = MagicMock(return_value=True) + service.dynamo_service = MagicMock() + return service + + +@pytest.mark.parametrize( + "db_filters,expected_result", + [ + ( + [], + True, + ), + ( + [{"doc_status": "preliminary"}], + True, + ), + ( + [{"doc_status": "preliminary"}, {"status": "current"}], + True, + ), + ( + [{"doc_status": "final"}], + False, + ), + ( + [{"doc_status": "final"}, {"status": "foobar"}], + False, + ), + ( + [{"doc_status": "preliminary"}, {"status": "foobar"}], + False, + ), + ], +) +def test_filter_returns_true(db_filters, expected_result, preliminary_dynamo_item_dict): + service = DocumentService() + result = service.filter_item(preliminary_dynamo_item_dict, filters=db_filters) + assert result is expected_result + + +def test_get_item_returns_none_when_no_item(service): + service.dynamo_service.get_item.return_value = {} + + result = service._get_item( + table_name="dev_COREDocumentMetadata", + key={"ID": "123"}, + model_class=DocumentReference, + ) + + assert result is None + service.dynamo_service.get_item.assert_called_once() + + +def test_get_item_returns_successful_document_reference( + service, + preliminary_dynamo_item_dict, +): + service.dynamo_service.get_item.return_value = preliminary_dynamo_item_dict + + result = service._get_item( + table_name="dev_COREDocumentMetadata", + key={"ID": "test-doc-id"}, + model_class=DocumentReference, + ) + + assert isinstance(result, DocumentReference) + assert result.id == "test-doc-id" + + +def test_get_item_does_not_return_successful_deleted_document_reference( + service, + preliminary_dynamo_item_dict, +): + preliminary_dynamo_item_dict["Item"]["Deleted"] = "foobar" + service.dynamo_service.get_item.return_value = preliminary_dynamo_item_dict + + result = service._get_item( + table_name="dev_COREDocumentMetadata", + key={"ID": "test-doc-id"}, + model_class=DocumentReference, + ) + + assert result is None + service.dynamo_service.get_item.assert_called_once() + + +def test_get_item_returns_successful_deleted_document_reference( + service, + preliminary_dynamo_item_dict, +): + preliminary_dynamo_item_dict["Item"]["Deleted"] = "foobar" + service.dynamo_service.get_item.return_value = preliminary_dynamo_item_dict + + result = service._get_item( + table_name="dev_COREDocumentMetadata", + key={"ID": "test-doc-id"}, + model_class=DocumentReference, + return_deleted=True, + ) + + assert isinstance(result, DocumentReference) + assert result.id == "test-doc-id" + + +def test_get_item_returns_none_when_filters_do_not_match( + service, + preliminary_dynamo_item_dict, +): + service.filter_item.return_value = False + service.dynamo_service.get_item.return_value = preliminary_dynamo_item_dict + + result = service._get_item( + table_name="dev_COREDocumentMetadata", + key={"ID": "test-doc-id"}, + model_class=DocumentReference, + ) + + assert result is None + service.dynamo_service.get_item.assert_called_once() + + +def test_get_item_validation_error_returns_none(service): + # invalid doc (id must be string) + service.dynamo_service.get_item.return_value = {"Item": {"ID": None}} + + result = service._get_item( + table_name="table", + key={"ID": "123"}, + model_class=DocumentReference, + ) + + assert result is None diff --git a/lambdas/tests/unit/services/test_pdm_upload_document_reference_service.py b/lambdas/tests/unit/services/test_pdm_upload_document_reference_service.py index 80f7c44ae4..b2a87bc04c 100644 --- a/lambdas/tests/unit/services/test_pdm_upload_document_reference_service.py +++ b/lambdas/tests/unit/services/test_pdm_upload_document_reference_service.py @@ -1,4 +1,4 @@ -from unittest.mock import Mock, patch +from unittest.mock import MagicMock, Mock, patch import pytest from botocore.exceptions import ClientError @@ -6,6 +6,7 @@ from enums.infrastructure import DynamoTables from enums.virus_scan_result import VirusScanResult from lambdas.enums.snomed_codes import SnomedCodes +from lambdas.services.document_service import DocumentService from models.document_reference import DocumentReference from services.mock_virus_scan_service import MockVirusScanService from services.upload_document_reference_service import UploadDocumentReferenceService @@ -14,8 +15,8 @@ MOCK_PDM_BUCKET, MOCK_PDM_TABLE_NAME, MOCK_STAGING_STORE_BUCKET, + WORKSPACE, ) -from utils.common_query_filters import PreliminaryStatus from utils.exceptions import DocumentServiceException, FileProcessingException from utils.lambda_exceptions import InvalidDocTypeException @@ -88,103 +89,134 @@ def mock_pdm_document_reference(): return doc_ref +@pytest.fixture +def dynamo_item_dict(): + return { + "Item": { + "id": "test-doc-id", + "nhs_number": "9000000001", + "s3_file_key": f"fhir_upload/{SnomedCodes.PATIENT_DATA.value.code}/9000000001/test-doc-id", + "s3_bucket_name": "test-staging-bucket", + "file_size": 1234567890, + "doc_status": "preliminary", + "status": "current", + "file_name": None, + }, + } + + +@pytest.fixture +def pdm_document_reference(): + return DocumentReference( + id="test-doc-id", + nhs_number="9000000001", + s3_file_key=f"fhir_upload/{SnomedCodes.PATIENT_DATA.value.code}/9000000001/test-doc-id", + s3_bucket_name="test-staging-bucket", + file_size=1234567890, + doc_status="preliminary", + status="current", + file_name=None, + author=None, + content_type="application/pdf", + created="2026-03-03T16:10:10.165878Z", + document_scan_creation="2026-03-03", + document_snomed_code_type="16521000000101", + file_location="s3://test-staging-bucket/9000000001/test-doc-id", + last_updated=1772554210, + raw_request=None, + s3_version_id=None, + s3_upload_key="9000000001/test-doc-id", + ttl=None, + version="1", + ) + + @pytest.fixture def pdm_service(set_env, mock_virus_scan_service): - with patch.multiple( - "services.upload_document_reference_service", - DocumentService=Mock(), - DynamoDBService=Mock(), - S3Service=Mock(), - ): - service = UploadDocumentReferenceService() - service.document_service = Mock() - service.dynamo_service = Mock() - service.virus_scan_service = MockVirusScanService() - service.s3_service = Mock() - service.table_name = MOCK_PDM_TABLE_NAME - service.destination_bucket_name = MOCK_PDM_BUCKET - service.doc_type = SnomedCodes.PATIENT_DATA.value - return service + service = UploadDocumentReferenceService() + service.dynamo_service = MagicMock() + service.virus_scan_service = MockVirusScanService() + service.s3_service = MagicMock() + service.table_name = "dev_COREDocumentMetadata" + service.destination_bucket_name = MOCK_PDM_BUCKET + service.doc_type = SnomedCodes.PATIENT_DATA.value + service.document_service = DocumentService() + service.document_service.dynamo_service = service.dynamo_service + return service def test_handle_upload_document_reference_request_with_empty_object_key(pdm_service): """Test handling of an empty object key""" - pdm_service.handle_upload_document_reference_request("", 122) - - pdm_service.document_service.fetch_documents_from_table.assert_not_called() + result = pdm_service.handle_upload_document_reference_request("", 122) + assert result is None def test_handle_upload_document_reference_request_with_none_object_key(pdm_service): """Test handling of a None object key""" - pdm_service.handle_upload_document_reference_request(None, 122) - - pdm_service.document_service.fetch_documents_from_table.assert_not_called() + result = pdm_service.handle_upload_document_reference_request(None, 122) + assert result is None def test_handle_upload_document_reference_request_success( - service, - mock_pdm_document_reference, + pdm_service, + dynamo_item_dict, mocker, ): """Test successful handling of the upload document reference request""" object_key = ( f"fhir_upload/{SnomedCodes.PATIENT_DATA.value.code}/9000000001/test-doc-id" ) - object_size = 1111 + object_size = dynamo_item_dict["Item"]["file_size"] - service.document_service.fetch_documents_from_table.side_effect = [ - [mock_pdm_document_reference], - ] - service.virus_scan_service.scan_file = mocker.MagicMock( + pdm_service.dynamo_service.get_item.return_value = { + "Item": dynamo_item_dict["Item"], + } + pdm_service.virus_scan_service.scan_file = mocker.MagicMock( return_value=VirusScanResult.CLEAN, ) - service.handle_upload_document_reference_request(object_key, object_size) - - service.document_service.fetch_documents_from_table.assert_called_once() - service.s3_service.copy_across_bucket.assert_called_once() - service.s3_service.delete_object.assert_called_once() - service.virus_scan_service.scan_file.assert_called_once() - + pdm_service.handle_upload_document_reference_request(object_key, object_size) + pdm_service.s3_service.copy_across_bucket.assert_called_once() + pdm_service.s3_service.delete_object.assert_called_once() + pdm_service.virus_scan_service.scan_file.assert_called_once() -def test_handle_upload_document_reference_request_with_exception(pdm_service): - """Test handling of exceptions during processing""" - object_key = "staging/test-doc-id" - pdm_service.document_service.fetch_documents_from_table.side_effect = Exception( - "Test error", - ) - - pdm_service.handle_upload_document_reference_request(object_key) +def test_handle_upload_document_reference_with_no_object_key(pdm_service): + object_key = "" + result = pdm_service.handle_upload_document_reference_request(object_key=object_key) + assert result is None def test_fetch_preliminary_document_reference_success( pdm_service, - mock_pdm_document_reference, + dynamo_item_dict, + pdm_document_reference, + mocker, ): """Test successful document reference fetching""" - document_key = "test-doc-id" - pdm_service.document_service.fetch_documents_from_table.return_value = [ - mock_pdm_document_reference, - ] + spy = mocker.spy(pdm_service.document_service, "get_item") + document_key = dynamo_item_dict["Item"]["id"] + pdm_service.dynamo_service.get_item.return_value = { + "Item": dynamo_item_dict["Item"], + } result = pdm_service._fetch_preliminary_document_reference( document_key=document_key, ) - - assert result == mock_pdm_document_reference - pdm_service.document_service.fetch_documents_from_table.assert_called_once_with( - table_name=MOCK_PDM_TABLE_NAME, - search_condition=document_key, - search_key="ID", - query_filter=PreliminaryStatus, + assert isinstance(result, DocumentReference) + assert result.id == pdm_document_reference.id + spy.assert_called_once_with( + document_id=document_key, + table_name=f"dev_{MOCK_PDM_TABLE_NAME}", + return_deleted=False, + filters=[{"doc_status": "preliminary"}], ) def test_fetch_preliminary_document_reference_no_documents_found(pdm_service): """Test handling when no documents are found""" document_key = "test-doc-id" - pdm_service.document_service.fetch_documents_from_table.return_value = [] + pdm_service.dynamo_service.get_item.return_value = {} result = pdm_service._fetch_preliminary_document_reference( document_key=document_key, @@ -193,29 +225,10 @@ def test_fetch_preliminary_document_reference_no_documents_found(pdm_service): assert result is None -def test_fetch_preliminary_document_reference_multiple_documents_warning( - pdm_service, - mock_document_reference, -): - """Test handling when multiple documents are found""" - document_key = "test-doc-id" - mock_doc_2 = Mock(spec=DocumentReference) - pdm_service.document_service.fetch_documents_from_table.return_value = [ - mock_document_reference, - mock_doc_2, - ] - - result = pdm_service._fetch_preliminary_document_reference( - document_key=document_key, - ) - - assert result == mock_document_reference - - -def test_fetch_preliminary_document_reference_exception(pdm_service): +def test_fetch_preliminary_document_reference_exception(pdm_service, dynamo_item_dict): """Test handling of exceptions during document fetching""" - document_key = "test-doc-id" - pdm_service.document_service.fetch_documents_from_table.side_effect = ( + document_key = dynamo_item_dict["Item"]["id"] + pdm_service.dynamo_service.get_item.side_effect = ( ClientError({"error": "test error message"}, "test"), ) @@ -261,7 +274,7 @@ def test__process_preliminary_document_reference_clean_virus_scan( def test__process_preliminary_document_reference_infected_virus_scan( pdm_service, - mock_document_reference, + mock_pdm_document_reference, mocker, ): """Test processing document reference with an infected virus scan""" @@ -277,7 +290,7 @@ def test__process_preliminary_document_reference_infected_virus_scan( mock_process_clean = mocker.patch.object(pdm_service, "_process_clean_document") mock_update_dynamo = mocker.patch.object(pdm_service, "_update_dynamo_table") pdm_service._process_preliminary_document_reference( - mock_document_reference, + mock_pdm_document_reference, object_key, 1222, ) @@ -289,11 +302,11 @@ def test__process_preliminary_document_reference_infected_virus_scan( def test_perform_virus_scan_returns_clean_hardcoded( pdm_service, - mock_document_reference, + mock_pdm_document_reference, ): """Test virus scan returns hardcoded CLEAN result""" object_key = "staging/test-doc-id" - result = pdm_service._perform_virus_scan(mock_document_reference, object_key) + result = pdm_service._perform_virus_scan(mock_pdm_document_reference, object_key) assert result == VirusScanResult.CLEAN @@ -448,12 +461,13 @@ def test_delete_file_from_staging_bucket_client_error(pdm_service): def test_update_dynamo_table_clean_scan_result( pdm_service, mock_pdm_document_reference, + mocker, ): + spy = mocker.spy(pdm_service.document_service, "update_document") """Test updating DynamoDB table with a clean scan result""" pdm_service._update_dynamo_table(mock_pdm_document_reference) - - pdm_service.document_service.update_document.assert_called_once_with( - table_name=MOCK_PDM_TABLE_NAME, + spy.assert_called_once_with( + table_name=f"{WORKSPACE}_{MOCK_PDM_TABLE_NAME}", key_pair={"ID": "test-doc-id"}, document=mock_pdm_document_reference, update_fields_name={ @@ -468,14 +482,19 @@ def test_update_dynamo_table_clean_scan_result( ) -def test_update_dynamo_table_infected_scan_result(pdm_service, mock_document_reference): +def test_update_dynamo_table_infected_scan_result( + pdm_service, + mock_document_reference, + mocker, +): """Test updating DynamoDB table with an infected scan result""" + spy = mocker.spy(pdm_service.document_service, "update_document") pdm_service._update_dynamo_table(mock_document_reference) - pdm_service.document_service.update_document.assert_called_once() + spy.assert_called_once() -def test_update_dynamo_table_client_error(pdm_service, mock_document_reference): +def test_update_dynamo_table_client_error(pdm_service, pdm_document_reference, mocker): """Test handling of ClientError during DynamoDB update""" client_error = ClientError( error_response={ @@ -486,10 +505,11 @@ def test_update_dynamo_table_client_error(pdm_service, mock_document_reference): }, operation_name="UpdateItem", ) + pdm_service.document_service = MagicMock() pdm_service.document_service.update_document.side_effect = client_error with pytest.raises(DocumentServiceException): - pdm_service._update_dynamo_table(mock_document_reference) + pdm_service._update_dynamo_table(pdm_document_reference) def test_handle_upload_document_reference_request_no_document_found(pdm_service): @@ -497,14 +517,14 @@ def test_handle_upload_document_reference_request_no_document_found(pdm_service) object_key = "staging/test-doc-id" object_size = 1234 - pdm_service.document_service.fetch_documents_from_table.return_value = [] + pdm_service.dynamo_service.get_item.return_value = {} - pdm_service.handle_upload_document_reference_request(object_key, object_size) + result = pdm_service.handle_upload_document_reference_request( + object_key, + object_size, + ) - # Should fetch but not proceed with processing - pdm_service.document_service.fetch_documents_from_table.assert_called_once() - pdm_service.s3_service.copy_across_bucket.assert_not_called() - pdm_service.document_service.update_document.assert_not_called() + assert result is None def test_process_preliminary_document_reference_exception_during_processing( @@ -635,44 +655,19 @@ def test_document_type_extraction_from_object_key( assert service.doc_type.code == expected_doctype.code -def test_handle_upload_pdm_document_reference_request_success( - service, - mock_document_reference, - mocker, -): - """Test successful handling of the upload document reference request""" - pdm_snomed = SnomedCodes.PATIENT_DATA.value - object_key = f"fhir_upload/{pdm_snomed.code}/staging/test-doc-id" - object_size = 1111 - service.document_service.fetch_documents_from_table.return_value = [ - mock_document_reference, - ] - service.virus_scan_service.scan_file = mocker.MagicMock( - return_value=VirusScanResult.CLEAN, - ) - - service.handle_upload_document_reference_request(object_key, object_size) - - service.document_service.fetch_documents_from_table.assert_called_once() - service.document_service.update_document.assert_called_once() - service.s3_service.copy_across_bucket.assert_called_once() - service.s3_service.delete_object.assert_called_once() - service.virus_scan_service.scan_file.assert_called_once() - - def test_copy_files_from_staging_bucket_to_pdm_success( pdm_service, - mock_pdm_document_reference, + dynamo_item_dict, + pdm_document_reference, + mocker, ): """Test successful file copying from staging bucket""" - source_file_key = ( - f"fhir_upload/{SnomedCodes.PATIENT_DATA.value.code}/staging/test-doc-id" - ) + source_file_key = dynamo_item_dict["Item"]["s3_file_key"] expected_dest_key = ( - f"{mock_pdm_document_reference.nhs_number}/{mock_pdm_document_reference.id}" + f"{pdm_document_reference.nhs_number}/{pdm_document_reference.id}" ) pdm_service.copy_files_from_staging_bucket( - mock_pdm_document_reference, + pdm_document_reference, source_file_key, ) pdm_service.s3_service.copy_across_bucket.assert_called_once_with( @@ -682,5 +677,5 @@ def test_copy_files_from_staging_bucket_to_pdm_success( dest_file_key=expected_dest_key, ) - assert mock_pdm_document_reference.s3_file_key == expected_dest_key - assert mock_pdm_document_reference.s3_bucket_name == MOCK_PDM_BUCKET + assert pdm_document_reference.s3_file_key == expected_dest_key + assert pdm_document_reference.s3_bucket_name == MOCK_PDM_BUCKET From 6f2464c16e99fbf663f09933987356fd29aefc88 Mon Sep 17 00:00:00 2001 From: jameslinnell Date: Wed, 4 Mar 2026 10:16:57 +0000 Subject: [PATCH 16/35] [NDR-423] Update tests --- .../e2e/api/fhir/test_upload_document_fhir_api_failure.py | 2 +- .../e2e/api/fhir/test_upload_document_fhir_api_success.py | 4 ++-- lambdas/tests/e2e/api/test_upload_document_api.py | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_failure.py b/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_failure.py index 98576411dc..f852de211a 100644 --- a/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_failure.py +++ b/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_failure.py @@ -178,7 +178,7 @@ def test_create_document_password_protected_docx(test_data): raw_upload_response = upload_document(payload) assert raw_upload_response.status_code == 201 upload_response = raw_upload_response.json() - record["id"] = upload_response["id"].split("~")[1] + record["id"] = upload_response["id"] test_data.append(record) def condition(response_json): diff --git a/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_success.py b/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_success.py index e08ecfb27b..4cc4b0f8c7 100644 --- a/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_success.py +++ b/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_success.py @@ -86,7 +86,7 @@ def test_create_document_base64_medium_file(test_data): raw_upload_response = upload_document(payload) assert raw_upload_response.status_code == 201 upload_response = raw_upload_response.json() - record["id"] = upload_response["id"].split("~")[1] + record["id"] = upload_response["id"] test_data.append(record) assert "Location" in raw_upload_response.headers @@ -95,7 +95,7 @@ def test_create_document_base64_medium_file(test_data): attachment_url = upload_response["content"][0]["attachment"]["url"] assert ( - f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{PDM_SNOMED}~" + f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{upload_response['id']}" in attachment_url ) diff --git a/lambdas/tests/e2e/api/test_upload_document_api.py b/lambdas/tests/e2e/api/test_upload_document_api.py index b88deb040f..38593546ba 100644 --- a/lambdas/tests/e2e/api/test_upload_document_api.py +++ b/lambdas/tests/e2e/api/test_upload_document_api.py @@ -296,7 +296,7 @@ def test_create_document_password_protected_docx(test_data, snapshot_json): retrieve_response = requests.post(url, headers=headers, data=payload) upload_response = retrieve_response.json() - lloyd_george_record["id"] = upload_response["id"].split("~")[1] + lloyd_george_record["id"] = upload_response["id"] test_data.append(lloyd_george_record) retrieve_url = ( @@ -336,7 +336,7 @@ def test_create_document_corrupted_png_returns_error(test_data, snapshot_json): retrieve_response = requests.post(url, headers=headers, data=payload) upload_response = retrieve_response.json() - lloyd_george_record["id"] = upload_response["id"].split("~")[1] + lloyd_george_record["id"] = upload_response["id"] test_data.append(lloyd_george_record) presign_uri = upload_response["content"][0]["attachment"]["url"] del upload_response["content"][0]["attachment"]["url"] From b399b7ecba441bdc86ea1494fe41a974f98d8a92 Mon Sep 17 00:00:00 2001 From: jameslinnell Date: Wed, 4 Mar 2026 12:24:58 +0000 Subject: [PATCH 17/35] [NDR-423] Fix DocStatus tests --- lambdas/services/upload_document_reference_service.py | 2 +- lambdas/tests/e2e/api/fhir/test_search_patient_fhir_api.py | 2 +- .../services/test_pdm_upload_document_reference_service.py | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lambdas/services/upload_document_reference_service.py b/lambdas/services/upload_document_reference_service.py index fd28b3a059..722d90ffc0 100644 --- a/lambdas/services/upload_document_reference_service.py +++ b/lambdas/services/upload_document_reference_service.py @@ -128,7 +128,7 @@ def _fetch_preliminary_document_reference( document_id=document_key, table_name=self.table_name, return_deleted=False, - filters=[{"doc_status": "preliminary"}], + filters=[{"DocStatus": "preliminary"}], ) if not document: diff --git a/lambdas/tests/e2e/api/fhir/test_search_patient_fhir_api.py b/lambdas/tests/e2e/api/fhir/test_search_patient_fhir_api.py index 362e851e0b..65c31ee7dc 100644 --- a/lambdas/tests/e2e/api/fhir/test_search_patient_fhir_api.py +++ b/lambdas/tests/e2e/api/fhir/test_search_patient_fhir_api.py @@ -131,7 +131,7 @@ def test_search_patient_details_deleted_are_not_returned(test_data): document_reference_ttl = int(deletion_date.timestamp() + ttl_seconds) created_record_2 = create_and_store_pdm_record( test_data, - doc_status="deprecated", + DocStatus="deprecated", Deleted=deletion_date.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), ttl=document_reference_ttl, ) diff --git a/lambdas/tests/unit/services/test_pdm_upload_document_reference_service.py b/lambdas/tests/unit/services/test_pdm_upload_document_reference_service.py index b2a87bc04c..9102f64ef5 100644 --- a/lambdas/tests/unit/services/test_pdm_upload_document_reference_service.py +++ b/lambdas/tests/unit/services/test_pdm_upload_document_reference_service.py @@ -98,7 +98,7 @@ def dynamo_item_dict(): "s3_file_key": f"fhir_upload/{SnomedCodes.PATIENT_DATA.value.code}/9000000001/test-doc-id", "s3_bucket_name": "test-staging-bucket", "file_size": 1234567890, - "doc_status": "preliminary", + "DocStatus": "preliminary", "status": "current", "file_name": None, }, @@ -209,7 +209,7 @@ def test_fetch_preliminary_document_reference_success( document_id=document_key, table_name=f"dev_{MOCK_PDM_TABLE_NAME}", return_deleted=False, - filters=[{"doc_status": "preliminary"}], + filters=[{"DocStatus": "preliminary"}], ) From 1db1e183365c5b8df218962fe719e5a31f50581a Mon Sep 17 00:00:00 2001 From: jameslinnell Date: Wed, 4 Mar 2026 13:48:54 +0000 Subject: [PATCH 18/35] [NDR-423] Update DocStatus --- .../tests/unit/services/test_pdm_document_service.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lambdas/tests/unit/services/test_pdm_document_service.py b/lambdas/tests/unit/services/test_pdm_document_service.py index 381689059e..6f843f059b 100644 --- a/lambdas/tests/unit/services/test_pdm_document_service.py +++ b/lambdas/tests/unit/services/test_pdm_document_service.py @@ -16,7 +16,7 @@ def preliminary_dynamo_item_dict(): "s3_file_key": f"fhir_upload/{SnomedCodes.PATIENT_DATA.value.code}/9000000001/test-doc-id", "s3_bucket_name": "test-staging-bucket", "file_size": 1234567890, - "doc_status": "preliminary", + "DocStatus": "preliminary", "status": "current", "file_name": None, }, @@ -39,23 +39,23 @@ def service(): True, ), ( - [{"doc_status": "preliminary"}], + [{"DocStatus": "preliminary"}], True, ), ( - [{"doc_status": "preliminary"}, {"status": "current"}], + [{"DocStatus": "preliminary"}, {"status": "current"}], True, ), ( - [{"doc_status": "final"}], + [{"DocStatus": "final"}], False, ), ( - [{"doc_status": "final"}, {"status": "foobar"}], + [{"DocStatus": "final"}, {"status": "foobar"}], False, ), ( - [{"doc_status": "preliminary"}, {"status": "foobar"}], + [{"DocStatus": "preliminary"}, {"status": "foobar"}], False, ), ], From b144569270c7a75aa0686a45de30c3db9b200d00 Mon Sep 17 00:00:00 2001 From: jameslinnell Date: Wed, 4 Mar 2026 14:11:01 +0000 Subject: [PATCH 19/35] [NDR-423] Remove print --- lambdas/tests/unit/services/test_document_service.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lambdas/tests/unit/services/test_document_service.py b/lambdas/tests/unit/services/test_document_service.py index 4fbfff213b..1418000b1a 100644 --- a/lambdas/tests/unit/services/test_document_service.py +++ b/lambdas/tests/unit/services/test_document_service.py @@ -812,7 +812,6 @@ def test_filters_on_get_item( }, } - print(filters) result = mock_service.filter_item(response=mock_dynamo_response, filters=filters) if filters[0].get("DocStatus") == "preliminary": assert result is True From 1d704db08194564d1cb964dedc3c767553e6e87f Mon Sep 17 00:00:00 2001 From: jameslinnell Date: Thu, 12 Mar 2026 09:51:57 +0000 Subject: [PATCH 20/35] [NDR-423] PR changes --- .devcontainer/Dockerfile | 2 +- .tool-versions | 2 +- lambdas/handlers/get_fhir_document_reference_handler.py | 2 +- lambdas/services/document_service.py | 8 ++------ lambdas/services/get_fhir_document_reference_service.py | 6 +++--- 5 files changed, 8 insertions(+), 12 deletions(-) diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile index 6446d82352..300ca809eb 100644 --- a/.devcontainer/Dockerfile +++ b/.devcontainer/Dockerfile @@ -1,4 +1,4 @@ -FROM --platform=linux/amd64 mcr.microsoft.com/devcontainers/python:3.11-bookworm +FROM --platform=linux/amd64 mcr.microsoft.com/devcontainers/python:3.11 RUN apt-get update && apt-get upgrade -y && apt-get install -y \ git \ diff --git a/.tool-versions b/.tool-versions index e6b411675e..8bdd5e18b9 100644 --- a/.tool-versions +++ b/.tool-versions @@ -9,4 +9,4 @@ python 3.11.14 shellcheck 0.11.0 terraform 1.14.6 terraform-docs 0.20.0 -trivy 0.69.2 \ No newline at end of file +trivy 0.69.2 diff --git a/lambdas/handlers/get_fhir_document_reference_handler.py b/lambdas/handlers/get_fhir_document_reference_handler.py index f3a5663ade..b8423404af 100644 --- a/lambdas/handlers/get_fhir_document_reference_handler.py +++ b/lambdas/handlers/get_fhir_document_reference_handler.py @@ -66,7 +66,7 @@ def lambda_handler(event, context): ) logger.info( - f"Successfully retrieved document reference for document_id: {document_id}, snomed_code: {snomed_code}", + f"Successfully retrieved document reference for document_id: {document_id}", ) return ApiGatewayResponse( diff --git a/lambdas/services/document_service.py b/lambdas/services/document_service.py index f4ab11269d..f0ebd042e4 100644 --- a/lambdas/services/document_service.py +++ b/lambdas/services/document_service.py @@ -124,12 +124,8 @@ def _get_item(self, table_name, key, model_class, return_deleted=False, filters= if not return_deleted: deleted = response.get("Item").get("Deleted", None) - if deleted in (None, ""): - if self.filter_item(response, filters): - document = model_class.model_validate(response.get("Item")) - return document - - return None + if deleted not in (None, ""): + return None if self.filter_item(response, filters): document = model_class.model_validate(response.get("Item")) diff --git a/lambdas/services/get_fhir_document_reference_service.py b/lambdas/services/get_fhir_document_reference_service.py index f74c23a91d..e3f3f6469d 100644 --- a/lambdas/services/get_fhir_document_reference_service.py +++ b/lambdas/services/get_fhir_document_reference_service.py @@ -64,16 +64,16 @@ def get_core_document_references( document_id: str, table, ) -> DocumentReference | None: - documentreference = self.document_service.get_item( + document_reference = self.document_service.get_item( document_id=document_id, table_name=table, ) - if not documentreference: + if not document_reference: raise GetFhirDocumentReferenceException( 404, LambdaError.DocumentReferenceNotFound, ) - return documentreference + return document_reference def fetch_documents( self, From 89e08521a09d14970f98dc2423b94796a7d35acb Mon Sep 17 00:00:00 2001 From: jameslinnell Date: Thu, 12 Mar 2026 10:05:55 +0000 Subject: [PATCH 21/35] [NDR-423] PR changes --- .../handlers/get_fhir_document_reference_handler.py | 2 +- ..._pdm_get_fhir_document_reference_by_id_service.py | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lambdas/handlers/get_fhir_document_reference_handler.py b/lambdas/handlers/get_fhir_document_reference_handler.py index b8423404af..0fd947bc52 100644 --- a/lambdas/handlers/get_fhir_document_reference_handler.py +++ b/lambdas/handlers/get_fhir_document_reference_handler.py @@ -91,7 +91,7 @@ def extract_document_parameters(event): document_id = get_id_from_path_parameters(path_params) if not document_id: - logger.error("Missing document id or snomed code in request path parameters.") + logger.error("Missing document id in request path parameters.") raise GetFhirDocumentReferenceException( 400, LambdaError.DocumentReferenceMissingParameters, diff --git a/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_by_id_service.py b/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_by_id_service.py index b6f4a84557..04d4cfe178 100644 --- a/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_by_id_service.py +++ b/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_by_id_service.py @@ -68,11 +68,11 @@ def test_get_dynamo_table_for_unsupported_doc_type(patched_service): non_lg_code = SnomedCode(code="non-lg-code", display_name="Non Lloyd George") - with pytest.raises(InvalidDocTypeException) as excinfo: + with pytest.raises(InvalidDocTypeException) as exc_info: patched_service._get_dynamo_table_for_doc_type(non_lg_code) - assert excinfo.value.status_code == 400 - assert excinfo.value.error == LambdaError.DocTypeDB + assert exc_info.value.status_code == 400 + assert exc_info.value.error == LambdaError.DocTypeDB # Not PDM however the code that this relates to was introduced because of PDM @@ -89,10 +89,10 @@ def test_get_document_references_empty_result(patched_service): # Test when no documents are found patched_service.document_service.get_item.return_value = None - with pytest.raises(GetFhirDocumentReferenceException) as excinfo: + with pytest.raises(GetFhirDocumentReferenceException) as exc_info: patched_service.get_core_document_references( document_id="test-id", table=MOCK_PDM_TABLE_NAME, ) - assert excinfo.value.status_code == 404 - assert excinfo.value.error == LambdaError.DocumentReferenceNotFound + assert exc_info.value.status_code == 404 + assert exc_info.value.error == LambdaError.DocumentReferenceNotFound From 7b40f1cc97a914b649c3e6d227532e312b70f114 Mon Sep 17 00:00:00 2001 From: jameslinnell Date: Thu, 12 Mar 2026 10:21:11 +0000 Subject: [PATCH 22/35] [NDR-423] Small change --- lambdas/handlers/get_fhir_document_reference_handler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lambdas/handlers/get_fhir_document_reference_handler.py b/lambdas/handlers/get_fhir_document_reference_handler.py index 0fd947bc52..3d00a499e5 100644 --- a/lambdas/handlers/get_fhir_document_reference_handler.py +++ b/lambdas/handlers/get_fhir_document_reference_handler.py @@ -91,7 +91,7 @@ def extract_document_parameters(event): document_id = get_id_from_path_parameters(path_params) if not document_id: - logger.error("Missing document id in request path parameters.") + logger.error("Missing document ID in request path parameters.") raise GetFhirDocumentReferenceException( 400, LambdaError.DocumentReferenceMissingParameters, From 6f4eca0570b2c03e81bd9e9f92dbc44f76bbd839 Mon Sep 17 00:00:00 2001 From: jameslinnell Date: Thu, 12 Mar 2026 10:45:10 +0000 Subject: [PATCH 23/35] [NDR-423] Fix search tests --- .../test_search_document_fhir_api_success.py | 30 +++++-------------- 1 file changed, 7 insertions(+), 23 deletions(-) diff --git a/lambdas/tests/e2e/api/fhir/test_search_document_fhir_api_success.py b/lambdas/tests/e2e/api/fhir/test_search_document_fhir_api_success.py index dd0dc9172b..f0b2c816d7 100644 --- a/lambdas/tests/e2e/api/fhir/test_search_document_fhir_api_success.py +++ b/lambdas/tests/e2e/api/fhir/test_search_document_fhir_api_success.py @@ -62,7 +62,7 @@ def test_search_document_reference_for_valid_patient_details(test_data): attachment_url = matching_entry["resource"]["content"][0]["attachment"]["url"] assert ( - f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{PDM_SNOMED}~{expected_record_id}" + f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{expected_record_id}" in attachment_url ) @@ -82,18 +82,14 @@ def test_search_multiple_document_references_for_valid_patient_details(test_data # Find the entries with the matching record_id's for record_id in expected_record_ids: matching_entry = next( - ( - e - for e in entries - if e["resource"].get("id") == f"{PDM_SNOMED}~{record_id}" - ), + (e for e in entries if e["resource"].get("id") == f"{record_id}"), None, ) assert matching_entry attachment_url = matching_entry["resource"]["content"][0]["attachment"]["url"] assert ( - f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{PDM_SNOMED}~{record_id}" + f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{record_id}" in attachment_url ) @@ -131,7 +127,7 @@ def test_search_document_reference_filters_by_custodian(test_data): f"{PDM_SNOMED}~{rec2['id']}", } - unexpected_id = f"{PDM_SNOMED}~{rec3['id']}" + unexpected_id = f"{rec3['id']}" assert expected_ids.issubset(returned_ids) assert unexpected_id not in returned_ids @@ -187,11 +183,7 @@ def test_search_multiple_cancelled_document_references_for_valid_patient_details # Validate all created records exist and have status cancelled for record_id in record_ids: entry = next( - ( - e - for e in entries - if e["resource"].get("id") == f"{PDM_SNOMED}~{record_id}" - ), + (e for e in entries if e["resource"].get("id") == f"{record_id}"), None, ) assert entry @@ -254,22 +246,14 @@ def test_search_mixed_deleted_and_not_deleted_document_references(test_data): # Assert the non-deleted record is returned matching_non_deleted = next( - ( - e - for e in entries - if e["resource"].get("id") == f"{PDM_SNOMED}~{expected_non_deleted_id}" - ), + (e for e in entries if e["resource"].get("id") == f"{expected_non_deleted_id}"), None, ) assert matching_non_deleted # Assert the deleted record isn't returned matching_deleted = next( - ( - e - for e in entries - if e["resource"].get("id") == f"{PDM_SNOMED}~{deleted_record_id}" - ), + (e for e in entries if e["resource"].get("id") == f"{deleted_record_id}"), None, ) assert matching_deleted is None From 22ad8df98812ebbdbe5955fefe4ed70e21af517e Mon Sep 17 00:00:00 2001 From: jameslinnell Date: Thu, 12 Mar 2026 11:10:36 +0000 Subject: [PATCH 24/35] [NDR-423] Search fix --- .../api/fhir/test_search_document_fhir_api_success.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/lambdas/tests/e2e/api/fhir/test_search_document_fhir_api_success.py b/lambdas/tests/e2e/api/fhir/test_search_document_fhir_api_success.py index f0b2c816d7..87fee87a61 100644 --- a/lambdas/tests/e2e/api/fhir/test_search_document_fhir_api_success.py +++ b/lambdas/tests/e2e/api/fhir/test_search_document_fhir_api_success.py @@ -4,7 +4,6 @@ from enums.document_retention import DocumentRetentionDays from tests.e2e.api.fhir.conftest import ( - PDM_SNOMED, TEST_NHS_NUMBER, UNKNOWN_TEST_NHS_NUMBER, create_and_store_pdm_record, @@ -51,11 +50,7 @@ def test_search_document_reference_for_valid_patient_details(test_data): # Find the entry with the matching record_id matching_entry = next( - ( - e - for e in entries - if e["resource"].get("id") == f"{PDM_SNOMED}~{expected_record_id}" - ), + (e for e in entries if e["resource"].get("id") == f"{expected_record_id}"), None, ) assert matching_entry @@ -123,8 +118,8 @@ def test_search_document_reference_filters_by_custodian(test_data): # Only records with custodian A11111 should be returned expected_ids = { - f"{PDM_SNOMED}~{rec1['id']}", - f"{PDM_SNOMED}~{rec2['id']}", + f"{rec1['id']}", + f"{rec2['id']}", } unexpected_id = f"{rec3['id']}" From 0d47413202c0053c0d4751326dac97aad2ff844b Mon Sep 17 00:00:00 2001 From: jameslinnell Date: Thu, 12 Mar 2026 14:27:44 +0000 Subject: [PATCH 25/35] [NDR-423] Fix e2e test help message --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index f8093725b3..3bb820b7d2 100644 --- a/Makefile +++ b/Makefile @@ -124,14 +124,14 @@ download-api-certs: ## Downloads mTLS certificates (use with dev envs only). Usa rm -rf ./lambdas/mtls_env_certs/$(WORKSPACE) ./scripts/aws/download-api-certs.sh $(WORKSPACE) -test-lg-fhir-api-e2e: ## Runs LG FHIR API E2E tests. See readme for required environment variables. Usage: make test-fhir-api-e2e-lg CONTAINER= +test-lg-fhir-api-e2e: ## Runs LG FHIR API E2E tests. See readme for required environment variables. Usage: make test-lg-fhir-api-e2e CONTAINER= ifeq ($(CONTAINER), true) cd ./lambdas && PYTHONPATH=. poetry run pytest tests/e2e/api --ignore=tests/e2e/api/fhir -vv else cd ./lambdas && ./venv/bin/python3 -m pytest tests/e2e/api --ignore=tests/e2e/api/fhir -vv endif -test-core-fhir-api-e2e: guard-WORKSPACE ## Runs Core FHIR API E2E tests. Usage: make test-fhir-api-e2e-core WORKSPACE= CONTAINER= +test-core-fhir-api-e2e: guard-WORKSPACE ## Runs Core FHIR API E2E tests. Usage: make test-core-fhir-api-e2e WORKSPACE= CONTAINER= ./scripts/test/run-e2e-fhir-api-tests.sh --workspace $(WORKSPACE) --container $(CONTAINER) rm -rf ./lambdas/mtls_env_certs/$(WORKSPACE) From 15317c6e9b0325ea9d5ed0fa23b165461ee403f3 Mon Sep 17 00:00:00 2001 From: jameslinnell Date: Thu, 12 Mar 2026 15:46:32 +0000 Subject: [PATCH 26/35] [NDR-423] Remove search test file that has been renamed in a previous PR --- .../api/fhir/test_search_patient_fhir_api.py | 159 ------------------ 1 file changed, 159 deletions(-) delete mode 100644 lambdas/tests/e2e/api/fhir/test_search_patient_fhir_api.py diff --git a/lambdas/tests/e2e/api/fhir/test_search_patient_fhir_api.py b/lambdas/tests/e2e/api/fhir/test_search_patient_fhir_api.py deleted file mode 100644 index 65c31ee7dc..0000000000 --- a/lambdas/tests/e2e/api/fhir/test_search_patient_fhir_api.py +++ /dev/null @@ -1,159 +0,0 @@ -from datetime import datetime, timezone - -import pytest - -from enums.document_retention import DocumentRetentionDays -from tests.e2e.api.fhir.conftest import ( - MTLS_ENDPOINT, - TEST_NHS_NUMBER, - UNKNOWN_TEST_NHS_NUMBER, - create_and_store_pdm_record, - create_mtls_session, -) -from tests.e2e.conftest import APIM_ENDPOINT -from tests.e2e.helpers.data_helper import PdmDataHelper - -pdm_data_helper = PdmDataHelper() - - -def search_document_reference( - nhs_number, - client_cert_path=None, - client_key_path=None, - resource_type="DocumentReference", -): - """Helper to perform search by NHS number with optional mTLS certs.""" - url = f"https://{MTLS_ENDPOINT}/{resource_type}?subject:identifier=https://fhir.nhs.uk/Id/nhs-number|{nhs_number}" - headers = { - "X-Correlation-Id": "1234", - } - - # Use provided certs if available, else defaults - if client_cert_path and client_key_path: - session = create_mtls_session(client_cert_path, client_key_path) - else: - session = create_mtls_session() - - return session.get(url, headers=headers) - - -def test_search_nonexistent_document_references_for_patient_details(): - response = search_document_reference(UNKNOWN_TEST_NHS_NUMBER) - assert response.status_code == 200 - - bundle = response.json() - assert bundle["resourceType"] == "Bundle" - assert bundle["type"] == "searchset" - assert bundle["total"] == 0 - assert "entry" in bundle - assert bundle["entry"] == [] - - -def test_search_patient_details(test_data): - created_record = create_and_store_pdm_record(test_data) - expected_record_id = created_record["id"] - - response = search_document_reference(TEST_NHS_NUMBER) - assert response.status_code == 200 - - bundle = response.json() - entries = bundle.get("entry", []) - assert entries - - # Find the entry with the matching record_id - matching_entry = next( - (e for e in entries if e["resource"].get("id") == f"{expected_record_id}"), - None, - ) - assert matching_entry - - attachment_url = matching_entry["resource"]["content"][0]["attachment"]["url"] - assert ( - f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{expected_record_id}" - in attachment_url - ) - - -def test_multiple_cancelled_search_patient_details(test_data): - record_ids = [ - create_and_store_pdm_record(test_data, doc_status="cancelled")["id"] - for _ in range(2) - ] - - response = search_document_reference(TEST_NHS_NUMBER) - assert response.status_code == 200 - - bundle = response.json() - entries = bundle.get("entry", []) - assert len(entries) >= 2 - - # Validate all created records exist and have status cancelled - for record_id in record_ids: - entry = next( - (e for e in entries if e["resource"].get("id") == f"{record_id}"), - None, - ) - assert entry - assert entry["resource"].get("docStatus") == "cancelled" - - -@pytest.mark.parametrize( - "nhs_number,expected_status,expected_code,expected_diagnostics", - [ - ("9999999993", 400, "INVALID_SEARCH_DATA", "Invalid patient number 9999999993"), - ("123", 400, "INVALID_SEARCH_DATA", "Invalid patient number 123"), - ], -) -def test_search_edge_cases( - nhs_number, - expected_status, - expected_code, - expected_diagnostics, -): - response = search_document_reference(nhs_number) - assert response.status_code == expected_status - - body = response.json() - issue = body["issue"][0] - details = issue.get("details", {}) - coding = details.get("coding", [{}])[0] - assert coding.get("code") == expected_code - assert issue.get("diagnostics") == expected_diagnostics - - -def test_search_patient_details_deleted_are_not_returned(test_data): - created_record_1 = create_and_store_pdm_record(test_data) - expected_record_id_1 = created_record_1["id"] - - deletion_date = datetime.now(timezone.utc) - document_ttl_days = DocumentRetentionDays.SOFT_DELETE - ttl_seconds = document_ttl_days * 24 * 60 * 60 - document_reference_ttl = int(deletion_date.timestamp() + ttl_seconds) - created_record_2 = create_and_store_pdm_record( - test_data, - DocStatus="deprecated", - Deleted=deletion_date.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), - ttl=document_reference_ttl, - ) - expected_record_id_2 = created_record_2["id"] - - response = search_document_reference(TEST_NHS_NUMBER) - assert response.status_code == 200 - - bundle = response.json() - assert bundle["total"] < 2 - entries = bundle.get("entry", []) - assert entries - - # Find the entry with the matching record_id - matching_entry = next( - (e for e in entries if e["resource"].get("id") == f"{expected_record_id_1}"), - None, - ) - assert matching_entry - # Assert deleted item doesn't exist - non_matching_entry = next( - (e for e in entries if e["resource"].get("id") == f"{expected_record_id_2}"), - None, - ) - assert non_matching_entry is None From b9e43fbd973652b1a82cd6a240ca1bdb2bf497ed Mon Sep 17 00:00:00 2001 From: jameslinnell Date: Fri, 13 Mar 2026 10:44:56 +0000 Subject: [PATCH 27/35] [NDR-423] Removed MTLS 500 error test --- .../e2e/api/fhir/test_mtls_api_failure.py | 20 ------------------- 1 file changed, 20 deletions(-) diff --git a/lambdas/tests/e2e/api/fhir/test_mtls_api_failure.py b/lambdas/tests/e2e/api/fhir/test_mtls_api_failure.py index 375901b303..0d9d76c570 100644 --- a/lambdas/tests/e2e/api/fhir/test_mtls_api_failure.py +++ b/lambdas/tests/e2e/api/fhir/test_mtls_api_failure.py @@ -83,26 +83,6 @@ def test_unsupported_http_method_returns_fhir_error(http_method): ) -# Commented this out for now as this will no longer retunr a 500 but now a 400 -# invalid and I can't think of a way to get it to return a 500. Definietly something that needs to be tested though. -# -# def test_5xx_response_is_fhir_compliant(test_data): -# """Verify that a Lambda error returns a FHIR-compliant OperationOutcome via DEFAULT_5XX.""" -# -# reversed_id = f"{str(uuid.uuid4())}~{pdm_data_helper.snomed_code}" -# -# response = get_pdm_document_reference( -# endpoint_override=reversed_id, -# ) -# -# assert response.status_code == 500 -# body = response.json() -# assert body["resourceType"] == "OperationOutcome" -# assert len(body["issue"]) > 0 -# issue = body["issue"][0] -# assert issue["severity"] == "error" - - def test_mtls_invalid_common_name(): record_id = str(uuid.uuid4()) response = get_pdm_document_reference( From ede15dbde3112909f5c710e02c84a6046d09d041 Mon Sep 17 00:00:00 2001 From: jameslinnell Date: Mon, 16 Mar 2026 14:00:24 +0000 Subject: [PATCH 28/35] [NDR-423] Still return SNOMED --- .../get_fhir_document_reference_handler.py | 42 ++++++++++++------- ...est_get_fhir_document_reference_handler.py | 17 ++++++-- ...t_fhir_document_reference_by_id_handler.py | 3 +- 3 files changed, 43 insertions(+), 19 deletions(-) diff --git a/lambdas/handlers/get_fhir_document_reference_handler.py b/lambdas/handlers/get_fhir_document_reference_handler.py index 3d00a499e5..4b66c15756 100644 --- a/lambdas/handlers/get_fhir_document_reference_handler.py +++ b/lambdas/handlers/get_fhir_document_reference_handler.py @@ -1,10 +1,11 @@ import uuid +from typing import Optional, Tuple from oauthlib.oauth2 import WebApplicationClient from enums.lambda_error import LambdaError from enums.mtls import MtlsCommonNames -from enums.snomed_codes import SnomedCode, SnomedCodes +from enums.snomed_codes import SnomedCodes from services.base.ssm_service import SSMService from services.dynamic_configuration_service import DynamicConfigurationService from services.get_fhir_document_reference_service import GetFhirDocumentReferenceService @@ -43,12 +44,13 @@ def lambda_handler(event, context): bearer_token = extract_bearer_token(event, context) selected_role_id = event.get("headers", {}).get("cis2-urid", None) - document_id = extract_document_parameters(event) - snomed_code = _determine_document_type(common_name=common_name) + document_id, snomed_code = extract_document_parameters(event) + if not snomed_code: + snomed_code = _determine_document_type(common_name=common_name) get_document_service = GetFhirDocumentReferenceService() document_reference = get_document_service.handle_get_document_reference_request( - snomed_code.code, + snomed_code, document_id, ) @@ -88,7 +90,7 @@ def lambda_handler(event, context): def extract_document_parameters(event): """Extract document ID and SNOMED code from path parameters""" path_params = event.get("pathParameters", {}).get("id", None) - document_id = get_id_from_path_parameters(path_params) + document_id, snomed_code = get_id_from_path_parameters(path_params) if not document_id: logger.error("Missing document ID in request path parameters.") @@ -97,7 +99,7 @@ def extract_document_parameters(event): LambdaError.DocumentReferenceMissingParameters, ) - return document_id + return document_id, snomed_code def verify_user_authorisation(bearer_token, selected_role_id, nhs_number): @@ -135,23 +137,35 @@ def verify_user_authorisation(bearer_token, selected_role_id, nhs_number): raise GetFhirDocumentReferenceException(e.status_code, e.error) -def get_id_from_path_parameters(path_parameters): +def get_id_from_path_parameters(path_parameters) -> Tuple[Optional[str], Optional[str]]: """Extract uuid from path parameters. Accepts: - '1234~uuid' - 'uuid' """ + snomed_code = None if not path_parameters: - return None + return None, None - id = path_parameters.split("~")[-1] - if not is_uuid(id): + params = path_parameters.split("~") + if len(params) > 2: raise GetFhirDocumentReferenceException( 400, LambdaError.DocRefInvalidFiles, ) - return id + + if len(params) > 1: + snomed_code = params[0] if params[0] else None + doc_id = params[1] + else: + doc_id = params[-1] + if not is_uuid(doc_id): + raise GetFhirDocumentReferenceException( + 400, + LambdaError.DocRefInvalidFiles, + ) + return doc_id, snomed_code def is_uuid(value: str) -> bool: @@ -162,12 +176,12 @@ def is_uuid(value: str) -> bool: return False -def _determine_document_type(common_name: MtlsCommonNames | None) -> SnomedCode: +def _determine_document_type(common_name: MtlsCommonNames | None) -> str: if not common_name: - return SnomedCodes.LLOYD_GEORGE.value + return SnomedCodes.LLOYD_GEORGE.value.code if common_name not in MtlsCommonNames: logger.error(f"mTLS common name {common_name} - is not supported") raise GetFhirDocumentReferenceException(400, LambdaError.DocRefInvalidType) - return SnomedCodes.PATIENT_DATA.value + return SnomedCodes.PATIENT_DATA.value.code diff --git a/lambdas/tests/unit/handlers/test_get_fhir_document_reference_handler.py b/lambdas/tests/unit/handlers/test_get_fhir_document_reference_handler.py index 124aec526f..feb5a76967 100644 --- a/lambdas/tests/unit/handlers/test_get_fhir_document_reference_handler.py +++ b/lambdas/tests/unit/handlers/test_get_fhir_document_reference_handler.py @@ -165,13 +165,17 @@ def test_extract_missing_bearer_token(context): def test_extract_document_parameters_valid(): - document_id = extract_document_parameters(MOCK_CIS2_VALID_EVENT) + document_id, snomed_code = extract_document_parameters(MOCK_CIS2_VALID_EVENT) assert document_id == TEST_UUID + assert snomed_code == SNOMED_CODE def test_extract_document_parameters_invalid(): - document_id = extract_document_parameters(MOCK_INVALID_EVENT_ID_MALFORMED) + document_id, snomed_code = extract_document_parameters( + MOCK_INVALID_EVENT_ID_MALFORMED, + ) assert document_id == TEST_UUID + assert snomed_code is None def test_verify_user_authorisation( @@ -336,8 +340,12 @@ def test_lambda_handler_search_service_errors( ], ) def test_get_id_from_path_parameters(path_parameter): - document_id = get_id_from_path_parameters(path_parameter) + document_id, snomed_code = get_id_from_path_parameters(path_parameter) assert document_id == TEST_UUID + if snomed_code: + assert isinstance(snomed_code, str) + else: + assert snomed_code is None @pytest.mark.parametrize( @@ -355,5 +363,6 @@ def test_get_id_from_path_parameters_raises_error(path_parameter): def test_get_id_from_path_parameters_empty(): - document_id = get_id_from_path_parameters("") + document_id, snomed_code = get_id_from_path_parameters("") assert document_id is None + assert snomed_code is None diff --git a/lambdas/tests/unit/handlers/test_pdm_get_fhir_document_reference_by_id_handler.py b/lambdas/tests/unit/handlers/test_pdm_get_fhir_document_reference_by_id_handler.py index 077b9b87b3..147e7b0891 100644 --- a/lambdas/tests/unit/handlers/test_pdm_get_fhir_document_reference_by_id_handler.py +++ b/lambdas/tests/unit/handlers/test_pdm_get_fhir_document_reference_by_id_handler.py @@ -104,5 +104,6 @@ def test_extract_bearer_token_when_pdm(context, mock_mtls_common_names): def test_extract_document_parameters_valid_pdm(): - document_id = extract_document_parameters(MOCK_MTLS_VALID_EVENT) + document_id, snomed_code = extract_document_parameters(MOCK_MTLS_VALID_EVENT) + assert snomed_code is None assert document_id == TEST_UUID From 89d693be621ccca2319a764d8196fb75fd85cc09 Mon Sep 17 00:00:00 2001 From: jameslinnell Date: Mon, 16 Mar 2026 15:43:15 +0000 Subject: [PATCH 29/35] [NDR-436] Use CORE table --- apim/specification.yaml | 6 +++--- lambdas/models/fhir/R4/fhir_document_reference.py | 2 +- lambdas/services/document_reference_search_service.py | 6 +++++- lambdas/services/fhir_document_reference_service_base.py | 8 ++++++-- .../e2e/api/fhir/test_search_document_fhir_api_success.py | 5 +++-- .../e2e/api/fhir/test_upload_document_fhir_api_failure.py | 6 +++--- .../e2e/api/fhir/test_upload_document_fhir_api_success.py | 8 ++++---- 7 files changed, 25 insertions(+), 16 deletions(-) diff --git a/apim/specification.yaml b/apim/specification.yaml index 680115b201..3a90ee30a6 100644 --- a/apim/specification.yaml +++ b/apim/specification.yaml @@ -604,7 +604,7 @@ components: - DocumentReference id: type: string - pattern: "[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[0-9a-f]{4}-[0-9a-f]{12}" + pattern: "16521000000101~[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[0-9a-f]{4}-[0-9a-f]{12}" description: "The logical id of the resource, as used in the URL for the resource. Once assigned, this value never changes." meta: $ref: "#/components/schemas/Meta" @@ -1197,7 +1197,7 @@ components: - div DocumentId: type: string - pattern: "[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[0-9a-f]{4}-[0-9a-f]{12}" + pattern: "16521000000101~[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[0-9a-f]{4}-[0-9a-f]{12}" RequestPathParams: type: object properties: @@ -1268,7 +1268,7 @@ components: examples: valid: summary: Valid ID, document held, user has correct auth (200) - value: f9ed81db-f90a-42d4-b7e4-d554d8f338fd + value: 16521000000101~f9ed81db-f90a-42d4-b7e4-d554d8f338fd invalid_400: summary: Invalid request, no ID present (400) value: 400 diff --git a/lambdas/models/fhir/R4/fhir_document_reference.py b/lambdas/models/fhir/R4/fhir_document_reference.py index 9699ffd3b3..00f5b8c638 100644 --- a/lambdas/models/fhir/R4/fhir_document_reference.py +++ b/lambdas/models/fhir/R4/fhir_document_reference.py @@ -255,7 +255,7 @@ def create_fhir_document_reference_object( fhir_doc_ref = DocumentReference( resourceType="DocumentReference", - id=document.id, + id=f"{self.snomed_code_doc_type.code}~{document.id}", docStatus=document.doc_status, type=CodeableConcept( coding=self._create_snomed_coding(self.snomed_code_doc_type), diff --git a/lambdas/services/document_reference_search_service.py b/lambdas/services/document_reference_search_service.py index 119506bf90..f4f91ca50c 100644 --- a/lambdas/services/document_reference_search_service.py +++ b/lambdas/services/document_reference_search_service.py @@ -234,7 +234,11 @@ def create_document_reference_fhir_response( title=document_reference.file_name, creation=document_reference.document_scan_creation or document_reference.created, - url=DOCUMENT_RETRIEVE_ENDPOINT + "/" + document_reference.id, + url=DOCUMENT_RETRIEVE_ENDPOINT + + "/" + + document_reference.document_snomed_code_type + + "~" + + document_reference.id, ) fhir_document_reference = ( DocumentReferenceInfo( diff --git a/lambdas/services/fhir_document_reference_service_base.py b/lambdas/services/fhir_document_reference_service_base.py index 0178d8cd85..75f6c2caf2 100644 --- a/lambdas/services/fhir_document_reference_service_base.py +++ b/lambdas/services/fhir_document_reference_service_base.py @@ -206,7 +206,11 @@ def _create_fhir_response( attachment_url = presigned_url else: attachment_url = ( - DOCUMENT_RETRIEVE_ENDPOINT + "/" + document_reference_ndr.id + DOCUMENT_RETRIEVE_ENDPOINT + + "/" + + document_reference_ndr.document_snomed_code_type + + "~" + + document_reference_ndr.id ) document_details = Attachment( title=document_reference_ndr.file_name, @@ -227,7 +231,7 @@ def _create_fhir_response( .model_dump_json(exclude_none=True) ) - document_id = f"{document_reference_ndr.id}" + document_id = f"{document_reference_ndr.document_snomed_code_type}~{document_reference_ndr.id}" return fhir_document_reference, document_id diff --git a/lambdas/tests/e2e/api/fhir/test_search_document_fhir_api_success.py b/lambdas/tests/e2e/api/fhir/test_search_document_fhir_api_success.py index 87fee87a61..bbbf12a669 100644 --- a/lambdas/tests/e2e/api/fhir/test_search_document_fhir_api_success.py +++ b/lambdas/tests/e2e/api/fhir/test_search_document_fhir_api_success.py @@ -4,6 +4,7 @@ from enums.document_retention import DocumentRetentionDays from tests.e2e.api.fhir.conftest import ( + PDM_SNOMED, TEST_NHS_NUMBER, UNKNOWN_TEST_NHS_NUMBER, create_and_store_pdm_record, @@ -57,7 +58,7 @@ def test_search_document_reference_for_valid_patient_details(test_data): attachment_url = matching_entry["resource"]["content"][0]["attachment"]["url"] assert ( - f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{expected_record_id}" + f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{PDM_SNOMED}~{expected_record_id}" in attachment_url ) @@ -84,7 +85,7 @@ def test_search_multiple_document_references_for_valid_patient_details(test_data attachment_url = matching_entry["resource"]["content"][0]["attachment"]["url"] assert ( - f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{record_id}" + f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{PDM_SNOMED}~{record_id}" in attachment_url ) diff --git a/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_failure.py b/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_failure.py index f852de211a..73241a3b4f 100644 --- a/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_failure.py +++ b/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_failure.py @@ -50,11 +50,11 @@ def test_create_document_virus(test_data): assert raw_upload_response.status_code == 201 upload_response = raw_upload_response.json() - record["id"] = upload_response["id"] + record["id"] = upload_response["id"].split("~")[1] test_data.append(record) assert "Location" in raw_upload_response.headers - expected_location = f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{record['id']}" + expected_location = f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{upload_response['id']}" assert raw_upload_response.headers["Location"] == expected_location # Poll until processing/scan completes @@ -178,7 +178,7 @@ def test_create_document_password_protected_docx(test_data): raw_upload_response = upload_document(payload) assert raw_upload_response.status_code == 201 upload_response = raw_upload_response.json() - record["id"] = upload_response["id"] + record["id"] = upload_response["id"].split("~")[1] test_data.append(record) def condition(response_json): diff --git a/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_success.py b/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_success.py index 4cc4b0f8c7..5e9a376d6f 100644 --- a/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_success.py +++ b/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_success.py @@ -36,7 +36,7 @@ def test_create_document_base64(test_data): raw_upload_response = upload_document(payload) assert raw_upload_response.status_code == 201 upload_response = raw_upload_response.json() - record["id"] = upload_response["id"] + record["id"] = upload_response["id"].split("~")[1] test_data.append(record) assert "Location" in raw_upload_response.headers @@ -86,7 +86,7 @@ def test_create_document_base64_medium_file(test_data): raw_upload_response = upload_document(payload) assert raw_upload_response.status_code == 201 upload_response = raw_upload_response.json() - record["id"] = upload_response["id"] + record["id"] = upload_response["id"].split("~")[1] test_data.append(record) assert "Location" in raw_upload_response.headers @@ -162,7 +162,7 @@ def test_create_document_without_author_or_type(test_data): raw_upload_response = upload_document(payload) assert raw_upload_response.status_code == 201 upload_response = raw_upload_response.json() - record["id"] = upload_response["id"] + record["id"] = upload_response["id"].split("~")[1] test_data.append(record) assert "Location" in raw_upload_response.headers @@ -194,7 +194,7 @@ def test_create_document_without_title(test_data): raw_upload_response = upload_document(payload) assert raw_upload_response.status_code == 201 upload_response = raw_upload_response.json() - record["id"] = upload_response["id"] + record["id"] = upload_response["id"].split("~")[1] test_data.append(record) assert "Location" in raw_upload_response.headers From 25c3c5f1e444d5730911690a4505f5452ad10e2e Mon Sep 17 00:00:00 2001 From: jameslinnell Date: Mon, 16 Mar 2026 16:07:04 +0000 Subject: [PATCH 30/35] [NDR-436] More tests --- .../test_document_reference_search_service.py | 8 ++++---- .../test_fhir_document_reference_service_base.py | 4 ++-- ...pdm_get_fhir_document_reference_search_service.py | 12 ++++++------ .../test_pdm_post_fhir_document_reference_service.py | 2 +- .../test_post_fhir_document_reference_service.py | 4 ++-- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/lambdas/tests/unit/services/test_document_reference_search_service.py b/lambdas/tests/unit/services/test_document_reference_search_service.py index bd9bf58988..b9d751d899 100644 --- a/lambdas/tests/unit/services/test_document_reference_search_service.py +++ b/lambdas/tests/unit/services/test_document_reference_search_service.py @@ -329,7 +329,7 @@ def test_create_document_reference_fhir_response(mock_document_service, mocker): ) expected_fhir_response = { - "id": "Y05868-1634567890", + "id": "16521000000101~Y05868-1634567890", "resourceType": "DocumentReference", "status": "current", "docStatus": "final", @@ -374,7 +374,7 @@ def test_create_document_reference_fhir_response(mock_document_service, mocker): mock_attachment.assert_called_once_with( title=mock_document_reference.file_name, creation=mock_document_reference.document_scan_creation, - url=f"{APIM_API_URL}/DocumentReference/{mock_document_reference.id}", + url=f"{APIM_API_URL}/DocumentReference/{SnomedCodes.LLOYD_GEORGE.value.code}~{mock_document_reference.id}", ) mock_doc_ref_info.assert_called_once_with( @@ -409,7 +409,7 @@ def test_create_document_reference_fhir_response_integration( mock_document_reference.version = "1" expected_fhir_response = { - "id": "Y05868-1634567890", + "id": "16521000000101~Y05868-1634567890", "resourceType": "DocumentReference", "status": "current", "docStatus": "final", @@ -427,7 +427,7 @@ def test_create_document_reference_fhir_response_integration( "language": "en-GB", "title": "test_document.pdf", "creation": "2023-05-01", - "url": f"{APIM_API_URL}/DocumentReference/Y05868-1634567890", + "url": f"{APIM_API_URL}/DocumentReference/16521000000101~Y05868-1634567890", }, }, ], diff --git a/lambdas/tests/unit/services/test_fhir_document_reference_service_base.py b/lambdas/tests/unit/services/test_fhir_document_reference_service_base.py index 43ff574edf..99ecdf03fc 100644 --- a/lambdas/tests/unit/services/test_fhir_document_reference_service_base.py +++ b/lambdas/tests/unit/services/test_fhir_document_reference_service_base.py @@ -565,7 +565,7 @@ def test_create_fhir_response_with_presigned_url(mock_service, mocker): result_json = json.loads(result) assert result_json["resourceType"] == "DocumentReference" assert result_json["content"][0]["attachment"]["url"] == presigned_url - assert document_id == "test-id" + assert document_id == f"{SnomedCodes.LLOYD_GEORGE.value.code}~test-id" def test_create_fhir_response_without_presigned_url(set_env, mock_service, mocker): @@ -593,7 +593,7 @@ def test_create_fhir_response_without_presigned_url(set_env, mock_service, mocke result_json = json.loads(result) assert result_json["resourceType"] == "DocumentReference" - expected_url = f"{APIM_API_URL}/DocumentReference/{document_ref.id}" + expected_url = f"{APIM_API_URL}/DocumentReference/{SnomedCodes.LLOYD_GEORGE.value.code}~{document_ref.id}" assert result_json["content"][0]["attachment"]["url"] == expected_url diff --git a/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_search_service.py b/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_search_service.py index b70123cd91..be00f4b7da 100644 --- a/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_search_service.py +++ b/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_search_service.py @@ -123,7 +123,7 @@ def test_create_document_reference_fhir_response(mock_document_service, mocker): ) expected_fhir_response = { - "id": "Y05868-1634567890", + "id": "717391000000106~Y05868-1634567890", "resourceType": "DocumentReference", "status": "current", "docStatus": "final", @@ -168,7 +168,7 @@ def test_create_document_reference_fhir_response(mock_document_service, mocker): mock_attachment.assert_called_once_with( title=mock_document_reference.file_name, creation=mock_document_reference.document_scan_creation, - url=f"{APIM_API_URL}/DocumentReference/{mock_document_reference.id}", + url=f"{APIM_API_URL}/DocumentReference/{SnomedCodes.PATIENT_DATA.value.code}~{mock_document_reference.id}", ) mock_doc_ref_info.assert_called_once_with( @@ -203,7 +203,7 @@ def test_create_document_reference_fhir_response_integration( mock_document_reference.version = "1" expected_fhir_response = { - "id": "Y05868-1634567890", + "id": "717391000000106~Y05868-1634567890", "resourceType": "DocumentReference", "status": "current", "docStatus": "final", @@ -221,7 +221,7 @@ def test_create_document_reference_fhir_response_integration( "language": "en-GB", "title": "test_document.pdf", "creation": "2023-05-01", - "url": f"{APIM_API_URL}/DocumentReference/Y05868-1634567890", + "url": f"{APIM_API_URL}/DocumentReference/717391000000106~Y05868-1634567890", }, }, ], @@ -278,7 +278,7 @@ def test_create_document_reference_fhir_response_no_title( mock_document_reference.version = "1" expected_fhir_response = { - "id": "Y05868-1634567890", + "id": "717391000000106~Y05868-1634567890", "resourceType": "DocumentReference", "status": "current", "docStatus": "final", @@ -295,7 +295,7 @@ def test_create_document_reference_fhir_response_no_title( "contentType": "application/pdf", "language": "en-GB", "creation": "2023-05-01", - "url": f"{APIM_API_URL}/DocumentReference/Y05868-1634567890", + "url": f"{APIM_API_URL}/DocumentReference/717391000000106~Y05868-1634567890", }, }, ], diff --git a/lambdas/tests/unit/services/test_pdm_post_fhir_document_reference_service.py b/lambdas/tests/unit/services/test_pdm_post_fhir_document_reference_service.py index 0891cdefd2..1a06703617 100644 --- a/lambdas/tests/unit/services/test_pdm_post_fhir_document_reference_service.py +++ b/lambdas/tests/unit/services/test_pdm_post_fhir_document_reference_service.py @@ -443,7 +443,7 @@ def test_process_mtls_fhir_document_reference_with_binary( valid_mtls_request_context, ) ) - expected_document_id = TEST_UUID + expected_document_id = f"{SnomedCodes.PATIENT_DATA.value.code}~{TEST_UUID}" assert isinstance(json_result, str) result_json = json.loads(json_result) diff --git a/lambdas/tests/unit/services/test_post_fhir_document_reference_service.py b/lambdas/tests/unit/services/test_post_fhir_document_reference_service.py index 6db6a3d952..911ec2ca5a 100644 --- a/lambdas/tests/unit/services/test_post_fhir_document_reference_service.py +++ b/lambdas/tests/unit/services/test_post_fhir_document_reference_service.py @@ -182,7 +182,7 @@ def test_process_fhir_document_reference_with_presigned_url( ) ) expected_pre_sign_url = mock_presigned_url_response - expected_document_id = TEST_UUID + expected_document_id = f"{SnomedCodes.LLOYD_GEORGE.value.code}~{TEST_UUID}" assert isinstance(json_result, str) result_json = json.loads(json_result) @@ -207,7 +207,7 @@ def test_process_fhir_document_reference_with_binary( valid_fhir_doc_with_binary, ) ) - expected_document_id = TEST_UUID + expected_document_id = f"{SnomedCodes.LLOYD_GEORGE.value.code}~{TEST_UUID}" assert isinstance(json_result, str) result_json = json.loads(json_result) From 5c8b2dff2c0d275115ac5e54ee6dddba17bb2420 Mon Sep 17 00:00:00 2001 From: jameslinnell Date: Mon, 16 Mar 2026 17:31:58 +0000 Subject: [PATCH 31/35] [NDR-436] Tests --- .../test_search_document_fhir_api_success.py | 38 +++++++++++++++---- .../test_upload_document_fhir_api_success.py | 2 +- .../tests/e2e/api/test_search_patient_api.py | 4 +- .../tests/e2e/api/test_upload_document_api.py | 14 +++---- 4 files changed, 40 insertions(+), 18 deletions(-) diff --git a/lambdas/tests/e2e/api/fhir/test_search_document_fhir_api_success.py b/lambdas/tests/e2e/api/fhir/test_search_document_fhir_api_success.py index bbbf12a669..2183ffe2af 100644 --- a/lambdas/tests/e2e/api/fhir/test_search_document_fhir_api_success.py +++ b/lambdas/tests/e2e/api/fhir/test_search_document_fhir_api_success.py @@ -42,7 +42,9 @@ def test_search_document_reference_for_valid_patient_details(test_data): created_record = create_and_store_pdm_record(test_data) expected_record_id = created_record["id"] + print(expected_record_id) response = search_document_reference(TEST_NHS_NUMBER) + print(response.json()) assert response.status_code == 200 bundle = response.json() @@ -51,7 +53,11 @@ def test_search_document_reference_for_valid_patient_details(test_data): # Find the entry with the matching record_id matching_entry = next( - (e for e in entries if e["resource"].get("id") == f"{expected_record_id}"), + ( + e + for e in entries + if e["resource"].get("id") == f"{PDM_SNOMED}~{expected_record_id}" + ), None, ) assert matching_entry @@ -78,7 +84,11 @@ def test_search_multiple_document_references_for_valid_patient_details(test_data # Find the entries with the matching record_id's for record_id in expected_record_ids: matching_entry = next( - (e for e in entries if e["resource"].get("id") == f"{record_id}"), + ( + e + for e in entries + if e["resource"].get("id") == f"{PDM_SNOMED}~{record_id}" + ), None, ) assert matching_entry @@ -119,11 +129,11 @@ def test_search_document_reference_filters_by_custodian(test_data): # Only records with custodian A11111 should be returned expected_ids = { - f"{rec1['id']}", - f"{rec2['id']}", + f"{PDM_SNOMED}~{rec1['id']}", + f"{PDM_SNOMED}~{rec2['id']}", } - unexpected_id = f"{rec3['id']}" + unexpected_id = f"{PDM_SNOMED}~{rec3['id']}" assert expected_ids.issubset(returned_ids) assert unexpected_id not in returned_ids @@ -179,7 +189,11 @@ def test_search_multiple_cancelled_document_references_for_valid_patient_details # Validate all created records exist and have status cancelled for record_id in record_ids: entry = next( - (e for e in entries if e["resource"].get("id") == f"{record_id}"), + ( + e + for e in entries + if e["resource"].get("id") == f"{PDM_SNOMED}~{record_id}" + ), None, ) assert entry @@ -242,14 +256,22 @@ def test_search_mixed_deleted_and_not_deleted_document_references(test_data): # Assert the non-deleted record is returned matching_non_deleted = next( - (e for e in entries if e["resource"].get("id") == f"{expected_non_deleted_id}"), + ( + e + for e in entries + if e["resource"].get("id") == f"{PDM_SNOMED}~{expected_non_deleted_id}" + ), None, ) assert matching_non_deleted # Assert the deleted record isn't returned matching_deleted = next( - (e for e in entries if e["resource"].get("id") == f"{deleted_record_id}"), + ( + e + for e in entries + if e["resource"].get("id") == f"{PDM_SNOMED}~{deleted_record_id}" + ), None, ) assert matching_deleted is None diff --git a/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_success.py b/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_success.py index 5e9a376d6f..82fdb0c05f 100644 --- a/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_success.py +++ b/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_success.py @@ -128,7 +128,7 @@ def test_create_document_saves_raw(test_data): raw_upload_response = upload_document(payload) assert raw_upload_response.status_code == 201 upload_response = raw_upload_response.json() - record["id"] = upload_response["id"] + record["id"] = upload_response["id"].split("~")[1] test_data.append(record) assert "Location" in raw_upload_response.headers diff --git a/lambdas/tests/e2e/api/test_search_patient_api.py b/lambdas/tests/e2e/api/test_search_patient_api.py index 7e75af1804..eecbf48fea 100644 --- a/lambdas/tests/e2e/api/test_search_patient_api.py +++ b/lambdas/tests/e2e/api/test_search_patient_api.py @@ -5,7 +5,7 @@ import requests from syrupy.filters import paths -from tests.e2e.conftest import API_ENDPOINT, API_KEY, APIM_ENDPOINT +from tests.e2e.conftest import API_ENDPOINT, API_KEY, APIM_ENDPOINT, LLOYD_GEORGE_SNOMED from tests.e2e.helpers.data_helper import LloydGeorgeDataHelper data_helper = LloydGeorgeDataHelper() @@ -22,7 +22,7 @@ def test_search_patient_details(test_data, snapshot_json): data_helper.create_metadata(lloyd_george_record) data_helper.create_resource(lloyd_george_record) - url = f"https://{API_ENDPOINT}/FhirDocumentReference?subject:identifier=https://fhir.nhs.uk/Id/nhs-number|{lloyd_george_record['nhs_number']}" + url = f"https://{API_ENDPOINT}/FhirDocumentReference?subject:identifier=https://fhir.nhs.uk/Id/nhs-number|{LLOYD_GEORGE_SNOMED}~" headers = { "Authorization": "Bearer 123", "X-Api-Key": API_KEY, diff --git a/lambdas/tests/e2e/api/test_upload_document_api.py b/lambdas/tests/e2e/api/test_upload_document_api.py index 38593546ba..bc018ef247 100644 --- a/lambdas/tests/e2e/api/test_upload_document_api.py +++ b/lambdas/tests/e2e/api/test_upload_document_api.py @@ -95,7 +95,7 @@ def test_create_document_base64(test_data, snapshot_json): retrieve_response = requests.post(url, headers=headers, data=payload) upload_response = retrieve_response.json() - lloyd_george_record["id"] = upload_response["id"] + lloyd_george_record["id"] = upload_response["id"].split("~")[1] test_data.append(lloyd_george_record) retrieve_url = ( @@ -111,7 +111,7 @@ def condition(response_json): attachment_url = upload_response["content"][0]["attachment"]["url"] assert ( - f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{lloyd_george_record['id']}" + f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{LLOYD_GEORGE_SNOMED}~" in attachment_url ) @@ -137,7 +137,7 @@ def test_create_document_presign(test_data, snapshot_json): retrieve_response = requests.post(url, headers=headers, data=payload) upload_response = retrieve_response.json() - lloyd_george_record["id"] = upload_response["id"] + lloyd_george_record["id"] = upload_response["id"].split("~")[1] test_data.append(lloyd_george_record) presign_uri = upload_response["content"][0]["attachment"]["url"] del upload_response["content"][0]["attachment"]["url"] @@ -193,7 +193,7 @@ def test_create_document_virus(test_data, snapshot_json): retrieve_response = requests.post(url, headers=headers, data=payload) upload_response = retrieve_response.json() - lloyd_george_record["id"] = upload_response["id"] + lloyd_george_record["id"] = upload_response["id"].split("~")[1] test_data.append(lloyd_george_record) retrieve_url = ( @@ -233,7 +233,7 @@ def test_create_document_does_not_save_raw(test_data): retrieve_response = requests.post(url, headers=headers, data=payload) json_response = retrieve_response.json() - lloyd_george_record["id"] = json_response.get("id") + lloyd_george_record["id"] = json_response.get("id").split("~")[1] test_data.append(lloyd_george_record) doc_ref = data_helper.retrieve_document_reference(record=lloyd_george_record) assert "Item" in doc_ref @@ -296,7 +296,7 @@ def test_create_document_password_protected_docx(test_data, snapshot_json): retrieve_response = requests.post(url, headers=headers, data=payload) upload_response = retrieve_response.json() - lloyd_george_record["id"] = upload_response["id"] + lloyd_george_record["id"] = upload_response["id"].split("~")[1] test_data.append(lloyd_george_record) retrieve_url = ( @@ -336,7 +336,7 @@ def test_create_document_corrupted_png_returns_error(test_data, snapshot_json): retrieve_response = requests.post(url, headers=headers, data=payload) upload_response = retrieve_response.json() - lloyd_george_record["id"] = upload_response["id"] + lloyd_george_record["id"] = upload_response["id"].split("~")[1] test_data.append(lloyd_george_record) presign_uri = upload_response["content"][0]["attachment"]["url"] del upload_response["content"][0]["attachment"]["url"] From b101345ef01e1211d3e3d62c098bfc855e5e274b Mon Sep 17 00:00:00 2001 From: jameslinnell Date: Mon, 16 Mar 2026 17:44:00 +0000 Subject: [PATCH 32/35] [NDR-436] Lloyd George e2e tests --- lambdas/tests/e2e/api/test_search_patient_api.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lambdas/tests/e2e/api/test_search_patient_api.py b/lambdas/tests/e2e/api/test_search_patient_api.py index eecbf48fea..11bee3c1f8 100644 --- a/lambdas/tests/e2e/api/test_search_patient_api.py +++ b/lambdas/tests/e2e/api/test_search_patient_api.py @@ -22,7 +22,7 @@ def test_search_patient_details(test_data, snapshot_json): data_helper.create_metadata(lloyd_george_record) data_helper.create_resource(lloyd_george_record) - url = f"https://{API_ENDPOINT}/FhirDocumentReference?subject:identifier=https://fhir.nhs.uk/Id/nhs-number|{LLOYD_GEORGE_SNOMED}~" + url = f"https://{API_ENDPOINT}/FhirDocumentReference?subject:identifier=https://fhir.nhs.uk/Id/nhs-number|{lloyd_george_record["nhs_number"]}" headers = { "Authorization": "Bearer 123", "X-Api-Key": API_KEY, @@ -34,7 +34,7 @@ def test_search_patient_details(test_data, snapshot_json): attachment_url = bundle["entry"][0]["resource"]["content"][0]["attachment"]["url"] assert ( - f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{lloyd_george_record['id']}" + f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{LLOYD_GEORGE_SNOMED}~" in attachment_url ) @@ -183,7 +183,8 @@ def test_search_patient_details_deleted_are_not_returned(test_data): ( e for e in entries - if e["resource"].get("id") == f"{lloyd_george_record['id']}" + if e["resource"].get("id") + == f"{LLOYD_GEORGE_SNOMED}~{lloyd_george_record['id']}" ), None, ) @@ -193,7 +194,8 @@ def test_search_patient_details_deleted_are_not_returned(test_data): ( e for e in entries - if e["resource"].get("id") == f"{second_lloyd_george_record['id']}" + if e["resource"].get("id") + == f"{LLOYD_GEORGE_SNOMED}~{second_lloyd_george_record['id']}" ), None, ) From e719067ec67f48fb0c8418e2c6513478b63fbee5 Mon Sep 17 00:00:00 2001 From: jameslinnell Date: Mon, 16 Mar 2026 18:20:34 +0000 Subject: [PATCH 33/35] [NDR-436] Fix test --- lambdas/tests/e2e/api/test_search_patient_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lambdas/tests/e2e/api/test_search_patient_api.py b/lambdas/tests/e2e/api/test_search_patient_api.py index 11bee3c1f8..69c13b1dd1 100644 --- a/lambdas/tests/e2e/api/test_search_patient_api.py +++ b/lambdas/tests/e2e/api/test_search_patient_api.py @@ -22,7 +22,7 @@ def test_search_patient_details(test_data, snapshot_json): data_helper.create_metadata(lloyd_george_record) data_helper.create_resource(lloyd_george_record) - url = f"https://{API_ENDPOINT}/FhirDocumentReference?subject:identifier=https://fhir.nhs.uk/Id/nhs-number|{lloyd_george_record["nhs_number"]}" + url = f"https://{API_ENDPOINT}/FhirDocumentReference?subject:identifier=https://fhir.nhs.uk/Id/nhs-number|{lloyd_george_record['nhs_number']}" headers = { "Authorization": "Bearer 123", "X-Api-Key": API_KEY, From d5df7695531a8cbf38b4afcfb03019ee489cc001 Mon Sep 17 00:00:00 2001 From: jameslinnell Date: Tue, 17 Mar 2026 09:09:40 +0000 Subject: [PATCH 34/35] [NDR-436] Remove prints --- .tool-versions | 1 - .../tests/e2e/api/fhir/test_search_document_fhir_api_success.py | 2 -- 2 files changed, 3 deletions(-) diff --git a/.tool-versions b/.tool-versions index 8bdd5e18b9..43012f108b 100644 --- a/.tool-versions +++ b/.tool-versions @@ -9,4 +9,3 @@ python 3.11.14 shellcheck 0.11.0 terraform 1.14.6 terraform-docs 0.20.0 -trivy 0.69.2 diff --git a/lambdas/tests/e2e/api/fhir/test_search_document_fhir_api_success.py b/lambdas/tests/e2e/api/fhir/test_search_document_fhir_api_success.py index 2183ffe2af..dd0dc9172b 100644 --- a/lambdas/tests/e2e/api/fhir/test_search_document_fhir_api_success.py +++ b/lambdas/tests/e2e/api/fhir/test_search_document_fhir_api_success.py @@ -42,9 +42,7 @@ def test_search_document_reference_for_valid_patient_details(test_data): created_record = create_and_store_pdm_record(test_data) expected_record_id = created_record["id"] - print(expected_record_id) response = search_document_reference(TEST_NHS_NUMBER) - print(response.json()) assert response.status_code == 200 bundle = response.json() From e23ea1dcd836c3df914055f90652ad629f00e394 Mon Sep 17 00:00:00 2001 From: jameslinnell Date: Tue, 17 Mar 2026 10:12:12 +0000 Subject: [PATCH 35/35] [NDR-436] Trivy in tool-versions --- .tool-versions | 1 + 1 file changed, 1 insertion(+) diff --git a/.tool-versions b/.tool-versions index 43012f108b..8bdd5e18b9 100644 --- a/.tool-versions +++ b/.tool-versions @@ -9,3 +9,4 @@ python 3.11.14 shellcheck 0.11.0 terraform 1.14.6 terraform-docs 0.20.0 +trivy 0.69.2