From 873ad91b307c1494517b6d01fe32d9b3dc7c283e Mon Sep 17 00:00:00 2001 From: Matt Dean Date: Thu, 9 Oct 2025 16:54:24 +0100 Subject: [PATCH 1/5] [NRL-1631] WIP - Add logs for create/upsert of unexpected dupe pointers --- .../create_document_reference.py | 18 +++++++ .../tests/test_upsert_document_reference.py | 49 +++++++++++++++++++ .../upsert_document_reference.py | 18 +++++++ layer/nrlf/core/constants.py | 8 +++ layer/nrlf/core/log_references.py | 6 +++ layer/nrlf/core/logger.py | 10 +++- layer/nrlf/tests/events.py | 4 +- 7 files changed, 109 insertions(+), 4 deletions(-) diff --git a/api/producer/createDocumentReference/create_document_reference.py b/api/producer/createDocumentReference/create_document_reference.py index 72450ea3f..7069d57c8 100644 --- a/api/producer/createDocumentReference/create_document_reference.py +++ b/api/producer/createDocumentReference/create_document_reference.py @@ -4,6 +4,7 @@ from nrlf.core.constants import ( PERMISSION_AUDIT_DATES_FROM_PAYLOAD, PERMISSION_SUPERSEDE_IGNORE_DELETE_FAIL, + TYPES_WITH_MULTIPLES, ) from nrlf.core.decorators import request_handler from nrlf.core.dynamodb.repository import DocumentPointer, DocumentPointerRepository @@ -255,6 +256,23 @@ def handler( logger.log(LogReference.PROCREATE999) return NRLResponse.RESOURCE_SUPERSEDED(resource_id=result.resource.id) + pointer_type = core_model.type + if pointer_type not in TYPES_WITH_MULTIPLES: + patient_number = core_model.nhs_number + pointer_custodian = core_model.custodian + existing_pointers_count = repository.count_by_nhs_number( + patient_number, [pointer_type] + ) + + if existing_pointers_count > 0: + logger.log( + LogReference.PROCREATE012, + pointer_type=pointer_type, + patient_number=patient_number, + existing_pointers_count=existing_pointers_count, + pointer_custodian=pointer_custodian, + ) + logger.log(LogReference.PROCREATE009, pointer_id=result.resource.id) repository.create(core_model) logger.log(LogReference.PROCREATE999) diff --git a/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py b/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py index 92242ea46..1532daee4 100644 --- a/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py +++ b/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py @@ -1699,3 +1699,52 @@ def test__set_create_time_fields_when_no_date_but_perms(): }, "date": test_time, } + + +@mock_aws +@mock_repository +def test_upsert_logs_for_unexpected_multi_pointer( + repository: DocumentPointerRepository, +): + doc_ref = load_document_reference("Y05868-736253002-Valid") + doc_pointer = DocumentPointer.from_document_reference(doc_ref) + repository.create(doc_pointer) + + event = create_test_api_gateway_event( + headers=create_headers(), + body=doc_ref.model_dump_json(exclude_none=True), + ) + + result = handler(event, create_mock_context()) + body = result.pop("body") + + assert result == { + "statusCode": "200", + "headers": { + "Location": "/producer/FHIR/R4/DocumentReference/Y05868-99999-99999-999999", + **default_response_headers(), + }, + "isBase64Encoded": False, + } + + parsed_body = json.loads(body) + + assert parsed_body == { + "resourceType": "OperationOutcome", + "issue": [ + { + "severity": "information", + "code": "informational", + "details": { + "coding": [ + { + "code": "RESOURCE_UPDATED", + "display": "Resource updated", + "system": "https://fhir.nhs.uk/ValueSet/NRL-ResponseCode", + } + ] + }, + "diagnostics": "The document has been updated", + } + ], + } diff --git a/api/producer/upsertDocumentReference/upsert_document_reference.py b/api/producer/upsertDocumentReference/upsert_document_reference.py index eb2f39c51..fed3974ef 100644 --- a/api/producer/upsertDocumentReference/upsert_document_reference.py +++ b/api/producer/upsertDocumentReference/upsert_document_reference.py @@ -2,6 +2,7 @@ from nrlf.core.constants import ( PERMISSION_AUDIT_DATES_FROM_PAYLOAD, PERMISSION_SUPERSEDE_IGNORE_DELETE_FAIL, + TYPES_WITH_MULTIPLES, ) from nrlf.core.decorators import request_handler from nrlf.core.dynamodb.repository import DocumentPointer, DocumentPointerRepository @@ -262,6 +263,23 @@ def handler( logger.log(LogReference.PROUPSERT999) return NRLResponse.RESOURCE_SUPERSEDED(resource_id=saved_model.id) + pointer_type = core_model.type + if pointer_type not in TYPES_WITH_MULTIPLES: + patient_number = core_model.nhs_number + pointer_custodian = core_model.custodian + existing_pointers_count = repository.count_by_nhs_number( + patient_number, [pointer_type] + ) + + if existing_pointers_count > 0: + logger.log( + LogReference.PROCREATE012, + pointer_type=pointer_type, + patient_number=patient_number, + existing_pointers_count=existing_pointers_count, + pointer_custodian=pointer_custodian, + ) + logger.log(LogReference.PROUPSERT009, pointer_id=result.resource.id) saved_model = repository.create(core_model) logger.log(LogReference.PROUPSERT999) diff --git a/layer/nrlf/core/constants.py b/layer/nrlf/core/constants.py index b5cc3bb9c..08f58e6e4 100644 --- a/layer/nrlf/core/constants.py +++ b/layer/nrlf/core/constants.py @@ -200,6 +200,14 @@ def coding_value(self): PointerTypes.SHARED_CARE_RECORD.value: Categories.RECORD_HEADINGS.value, } +# +# Pointer types that can have multiple pointers for a single patient +TYPES_WITH_MULTIPLES = [ + PointerTypes.MRA_UPPER_LIMB_ARTERY.value, + PointerTypes.MRI_AXILLA_BOTH.value, + PointerTypes.APPOINTMENT.value, +] + PRACTICE_SETTING_VALUE_SET_URL = ( "https://fhir.nhs.uk/England/ValueSet/England-PracticeSetting" ) diff --git a/layer/nrlf/core/log_references.py b/layer/nrlf/core/log_references.py index b8948fa0c..f68f72573 100644 --- a/layer/nrlf/core/log_references.py +++ b/layer/nrlf/core/log_references.py @@ -270,6 +270,9 @@ class LogReference(Enum): PROCREATE011 = _Reference( "INFO", "Preserved .date field when creating new document reference" ) + PROCREATE012 = _Reference( + "WARN", "Existing pointers found for patient during create operation" + ) PROCREATE999 = _Reference( "INFO", "Successfully completed producer createDocumentReference" ) @@ -329,6 +332,9 @@ class LogReference(Enum): PROUPSERT011 = _Reference( "INFO", "Preserved .date field when creating new document reference for upsert" ) + PROUPSERT012 = _Reference( + "WARN", "Existing pointers found for patient during upsert operation" + ) PROUPSERT999 = _Reference( "INFO", "Successfully completed producer upsertDocumentReference" ) diff --git a/layer/nrlf/core/logger.py b/layer/nrlf/core/logger.py index 72169bef7..a639b36ba 100644 --- a/layer/nrlf/core/logger.py +++ b/layer/nrlf/core/logger.py @@ -1,5 +1,6 @@ import os from datetime import datetime +from typing import Any from aws_lambda_powertools import Logger as PowertoolsLogger from aws_lambda_powertools.logging.formatter import LambdaPowertoolsFormatter @@ -9,7 +10,7 @@ class SplunkFormatter(LambdaPowertoolsFormatter): - def __init__(self, *args, **kwargs): + def __init__(self, *args: Any, **kwargs: Any) -> None: super().__init__(*args, **kwargs) self.splunk_index = os.getenv("SPLUNK_INDEX", "aws_recordlocator_dev") @@ -30,7 +31,7 @@ def serialize(self, log: LogRecord) -> str: class Logger(PowertoolsLogger): - def log(self, code: LogReference, **kwargs): + def log(self, code: LogReference, **kwargs: Any) -> None: kwargs["log_reference"] = code.name match code.value.level: case "DEBUG": @@ -45,6 +46,11 @@ def log(self, code: LogReference, **kwargs): self.critical(code.value.message, stacklevel=3, **kwargs) case "EXCEPTION": self.exception(code.value.message, **kwargs) + case _: + self.warning( + f"Unhandled log level: {code.value.level} - {code.value.message}", + **kwargs, + ) logger = Logger(logger_formatter=SplunkFormatter()) diff --git a/layer/nrlf/tests/events.py b/layer/nrlf/tests/events.py index c32d7cd31..aa39952be 100644 --- a/layer/nrlf/tests/events.py +++ b/layer/nrlf/tests/events.py @@ -1,5 +1,5 @@ import json -from typing import Dict, List, Optional +from typing import Any, Dict, List, Optional from unittest.mock import Mock @@ -46,7 +46,7 @@ def create_test_api_gateway_event( query_string_parameters: Optional[Dict[str, str]] = None, path_parameters: Optional[Dict[str, str]] = None, body: Optional[str] = None, -): +) -> Dict[str, Any]: return { "resource": "/", "path": "/", From 3e6ab45eeef7cf986b714234260ec0a30a0e5924 Mon Sep 17 00:00:00 2001 From: Matt Dean Date: Fri, 17 Oct 2025 10:31:01 +0100 Subject: [PATCH 2/5] [NRL-1631] Add unit tests for new logging --- .../create_document_reference.py | 10 +- .../tests/test_create_document_reference.py | 142 +++++++++++++++++- .../tests/test_upsert_document_reference.py | 105 ++++++++++++- .../upsert_document_reference.py | 11 +- .../Y05868-TestApp-12345678/Y05868.json | 1 + ...Y05868-736253002-Valid-with-master-id.json | 111 ++++++++++++++ .../Y05868-Appointment-Valid.json | 111 ++++++++++++++ 7 files changed, 474 insertions(+), 17 deletions(-) create mode 100644 tests/data/DocumentReference/Y05868-736253002-Valid-with-master-id.json create mode 100644 tests/data/DocumentReference/Y05868-Appointment-Valid.json diff --git a/api/producer/createDocumentReference/create_document_reference.py b/api/producer/createDocumentReference/create_document_reference.py index 7069d57c8..223086ae5 100644 --- a/api/producer/createDocumentReference/create_document_reference.py +++ b/api/producer/createDocumentReference/create_document_reference.py @@ -258,19 +258,21 @@ def handler( pointer_type = core_model.type if pointer_type not in TYPES_WITH_MULTIPLES: - patient_number = core_model.nhs_number + nhs_number = core_model.nhs_number pointer_custodian = core_model.custodian + pointer_master_identifier = core_model.master_identifier existing_pointers_count = repository.count_by_nhs_number( - patient_number, [pointer_type] + nhs_number, [pointer_type] ) if existing_pointers_count > 0: logger.log( LogReference.PROCREATE012, + new_pointer_master_id=pointer_master_identifier, pointer_type=pointer_type, - patient_number=patient_number, + nhs_number=nhs_number, existing_pointers_count=existing_pointers_count, - pointer_custodian=pointer_custodian, + custodian=pointer_custodian, ) logger.log(LogReference.PROCREATE009, pointer_id=result.resource.id) diff --git a/api/producer/createDocumentReference/tests/test_create_document_reference.py b/api/producer/createDocumentReference/tests/test_create_document_reference.py index 55bba64a7..353071c68 100644 --- a/api/producer/createDocumentReference/tests/test_create_document_reference.py +++ b/api/producer/createDocumentReference/tests/test_create_document_reference.py @@ -1,5 +1,5 @@ import json -from unittest.mock import patch +from unittest.mock import Mock, patch from freeze_uuid import freeze_uuid from freezegun import freeze_time @@ -1734,3 +1734,143 @@ def test__set_create_time_fields_when_no_date_but_perms(): }, "date": test_time, } + + +@mock_aws +@mock_repository +@freeze_uuid("00000000-0000-0000-0000-000000000001") +@patch("api.producer.createDocumentReference.create_document_reference.logger") +def test_create_logs_for_unexpected_multi_pointer( + mock_logger: Mock, + repository: DocumentPointerRepository, +): + doc_ref = load_document_reference("Y05868-736253002-Valid-with-master-id") + doc_pointer = DocumentPointer.from_document_reference(doc_ref) + repository.create(doc_pointer) + + event = create_test_api_gateway_event( + headers=create_headers(), + body=doc_ref.model_dump_json(exclude_none=True), + ) + + result = handler(event, create_mock_context()) + body = result.pop("body") + + assert result == { + "statusCode": "201", + "headers": { + "Location": "/producer/FHIR/R4/DocumentReference/Y05868-00000000-0000-0000-0000-000000000001", + **default_response_headers(), + }, + "isBase64Encoded": False, + } + + parsed_body = json.loads(body) + + assert parsed_body == { + "resourceType": "OperationOutcome", + "issue": [ + { + "severity": "information", + "code": "informational", + "details": { + "coding": [ + { + "code": "RESOURCE_CREATED", + "display": "Resource created", + "system": "https://fhir.nhs.uk/ValueSet/NRL-ResponseCode", + } + ] + }, + "diagnostics": "The document has been created", + } + ], + } + + assert any( + call[0][0].name == "PROCREATE012" for call in mock_logger.log.call_args_list + ) + + assert { + "existing_pointers_count": 1, + "nhs_number": ( + doc_ref.subject.identifier.value + if doc_ref.subject and doc_ref.subject.identifier + else None + ), + "pointer_type": ( + f"{doc_ref.type.coding[0].system}|{doc_ref.type.coding[0].code}" + if doc_ref.type and doc_ref.type.coding + else None + ), + "custodian": ( + doc_ref.custodian.identifier.value + if doc_ref.custodian and doc_ref.custodian.identifier + else None + ), + "new_pointer_master_id": ( + doc_ref.masterIdentifier.value if doc_ref.masterIdentifier else None + ), + } == [ + call[1:][0] + for call in mock_logger.log.call_args_list + if call[0][0].name == "PROCREATE012" + ][ + 0 + ] + + +@mock_aws +@mock_repository +@freeze_uuid("00000000-0000-0000-0000-000000000001") +@patch("api.producer.createDocumentReference.create_document_reference.logger") +def test_create_logs_for_expected_multi_pointer( + mock_logger: Mock, + repository: DocumentPointerRepository, +): + doc_ref = load_document_reference("Y05868-Appointment-Valid") + doc_pointer = DocumentPointer.from_document_reference(doc_ref) + repository.create(doc_pointer) + + event = create_test_api_gateway_event( + headers=create_headers(), + body=doc_ref.model_dump_json(exclude_none=True), + ) + + result = handler(event, create_mock_context()) + body = result.pop("body") + + assert result == { + "statusCode": "201", + "headers": { + "Location": "/producer/FHIR/R4/DocumentReference/Y05868-00000000-0000-0000-0000-000000000001", + **default_response_headers(), + }, + "isBase64Encoded": False, + } + + parsed_body = json.loads(body) + + assert parsed_body == { + "resourceType": "OperationOutcome", + "issue": [ + { + "severity": "information", + "code": "informational", + "details": { + "coding": [ + { + "code": "RESOURCE_CREATED", + "display": "Resource created", + "system": "https://fhir.nhs.uk/ValueSet/NRL-ResponseCode", + } + ] + }, + "diagnostics": "The document has been created", + } + ], + } + + assert not any( + call[0][0].name == "PROCREATE012" for call in mock_logger.log.call_args_list + ) diff --git a/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py b/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py index 1532daee4..5aa3ba079 100644 --- a/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py +++ b/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py @@ -1,5 +1,5 @@ import json -from unittest.mock import patch +from unittest.mock import Mock, patch from freezegun import freeze_time from moto import mock_aws @@ -1703,13 +1703,100 @@ def test__set_create_time_fields_when_no_date_but_perms(): @mock_aws @mock_repository +@patch("api.producer.upsertDocumentReference.upsert_document_reference.logger") def test_upsert_logs_for_unexpected_multi_pointer( + mock_logger: Mock, repository: DocumentPointerRepository, ): - doc_ref = load_document_reference("Y05868-736253002-Valid") + doc_ref = load_document_reference("Y05868-736253002-Valid-with-master-id") + doc_pointer = DocumentPointer.from_document_reference(doc_ref) + repository.create(doc_pointer) + + doc_ref.id = "Y05868-99999-99999-999999-02" + + event = create_test_api_gateway_event( + headers=create_headers(), + body=doc_ref.model_dump_json(exclude_none=True), + ) + + result = handler(event, create_mock_context()) + body = result.pop("body") + + assert result == { + "statusCode": "201", + "headers": { + "Location": "/producer/FHIR/R4/DocumentReference/Y05868-99999-99999-999999-02", + **default_response_headers(), + }, + "isBase64Encoded": False, + } + + parsed_body = json.loads(body) + + assert parsed_body == { + "resourceType": "OperationOutcome", + "issue": [ + { + "severity": "information", + "code": "informational", + "details": { + "coding": [ + { + "code": "RESOURCE_CREATED", + "display": "Resource created", + "system": "https://fhir.nhs.uk/ValueSet/NRL-ResponseCode", + } + ] + }, + "diagnostics": "The document has been created", + } + ], + } + + assert any( + call[0][0].name == "PROUPSERT012" for call in mock_logger.log.call_args_list + ) + + assert { + "existing_pointers_count": 1, + "nhs_number": ( + doc_ref.subject.identifier.value + if doc_ref.subject and doc_ref.subject.identifier + else None + ), + "pointer_type": ( + f"{doc_ref.type.coding[0].system}|{doc_ref.type.coding[0].code}" + if doc_ref.type and doc_ref.type.coding + else None + ), + "custodian": ( + doc_ref.custodian.identifier.value + if doc_ref.custodian and doc_ref.custodian.identifier + else None + ), + "new_pointer_id": doc_ref.id, + } == [ + call[1:][0] + for call in mock_logger.log.call_args_list + if call[0][0].name == "PROUPSERT012" + ][ + 0 + ] + + +@mock_aws +@mock_repository +@patch("api.producer.upsertDocumentReference.upsert_document_reference.logger") +def test_upsert_logs_for_expected_multi_pointer( + mock_logger: Mock, + repository: DocumentPointerRepository, +): + doc_ref = load_document_reference("Y05868-Appointment-Valid") doc_pointer = DocumentPointer.from_document_reference(doc_ref) repository.create(doc_pointer) + doc_ref.id = "Y05868-99999-99999-999999-02" + event = create_test_api_gateway_event( headers=create_headers(), body=doc_ref.model_dump_json(exclude_none=True), @@ -1719,9 +1806,9 @@ def test_upsert_logs_for_unexpected_multi_pointer( body = result.pop("body") assert result == { - "statusCode": "200", + "statusCode": "201", "headers": { - "Location": "/producer/FHIR/R4/DocumentReference/Y05868-99999-99999-999999", + "Location": "/producer/FHIR/R4/DocumentReference/Y05868-99999-99999-999999-02", **default_response_headers(), }, "isBase64Encoded": False, @@ -1738,13 +1825,17 @@ def test_upsert_logs_for_unexpected_multi_pointer( "details": { "coding": [ { - "code": "RESOURCE_UPDATED", - "display": "Resource updated", + "code": "RESOURCE_CREATED", + "display": "Resource created", "system": "https://fhir.nhs.uk/ValueSet/NRL-ResponseCode", } ] }, - "diagnostics": "The document has been updated", + "diagnostics": "The document has been created", } ], } + + assert not any( + call[0][0].name == "PROUPSERT012" for call in mock_logger.log.call_args_list + ) diff --git a/api/producer/upsertDocumentReference/upsert_document_reference.py b/api/producer/upsertDocumentReference/upsert_document_reference.py index fed3974ef..2018946c9 100644 --- a/api/producer/upsertDocumentReference/upsert_document_reference.py +++ b/api/producer/upsertDocumentReference/upsert_document_reference.py @@ -265,19 +265,20 @@ def handler( pointer_type = core_model.type if pointer_type not in TYPES_WITH_MULTIPLES: - patient_number = core_model.nhs_number + nhs_number = core_model.nhs_number pointer_custodian = core_model.custodian existing_pointers_count = repository.count_by_nhs_number( - patient_number, [pointer_type] + nhs_number, [pointer_type] ) if existing_pointers_count > 0: logger.log( - LogReference.PROCREATE012, + LogReference.PROUPSERT012, + new_pointer_id=core_model.id, pointer_type=pointer_type, - patient_number=patient_number, + nhs_number=nhs_number, existing_pointers_count=existing_pointers_count, - pointer_custodian=pointer_custodian, + custodian=pointer_custodian, ) logger.log(LogReference.PROUPSERT009, pointer_id=result.resource.id) diff --git a/layer/test_permissions/Y05868-TestApp-12345678/Y05868.json b/layer/test_permissions/Y05868-TestApp-12345678/Y05868.json index aa06c268c..a717f887e 100644 --- a/layer/test_permissions/Y05868-TestApp-12345678/Y05868.json +++ b/layer/test_permissions/Y05868-TestApp-12345678/Y05868.json @@ -3,5 +3,6 @@ "http://snomed.info/sct|736253002", "http://snomed.info/sct|1363501000000100", "http://snomed.info/sct|861421000000109", + "http://snomed.info/sct|749001000000101", "https://nicip.nhs.uk|MAULR" ] diff --git a/tests/data/DocumentReference/Y05868-736253002-Valid-with-master-id.json b/tests/data/DocumentReference/Y05868-736253002-Valid-with-master-id.json new file mode 100644 index 000000000..15d92f465 --- /dev/null +++ b/tests/data/DocumentReference/Y05868-736253002-Valid-with-master-id.json @@ -0,0 +1,111 @@ +{ + "resourceType": "DocumentReference", + "id": "Y05868-99999-99999-999999", + "status": "current", + "docStatus": "final", + "type": { + "coding": [ + { + "system": "http://snomed.info/sct", + "code": "736253002", + "display": "Mental health crisis plan" + } + ] + }, + "category": [ + { + "coding": [ + { + "system": "http://snomed.info/sct", + "code": "734163000", + "display": "Care plan" + } + ] + } + ], + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "6700028191" + } + }, + "author": [ + { + "identifier": { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "Y05868" + } + } + ], + "custodian": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "Y05868" + } + }, + "description": "Physical document mental health crisis plan", + "securityLabel": [ + { + "coding": [ + { + "system": "http://terminology.hl7.org/CodeSystem/v3-Confidentiality", + "code": "V", + "display": "very restricted" + } + ] + } + ], + "content": [ + { + "attachment": { + "contentType": "application/pdf", + "language": "en-US", + "url": "https://spine-proxy.national.ncrs.nhs.uk/https%3A%2F%2Fp1.nhs.uk%2FMentalhealthCrisisPlanReport.pdf", + "size": 3654, + "hash": "2jmj7l5rSw0yVb/vlWAYkK/YBwk=", + "title": "Mental health crisis plan report", + "creation": "2022-12-21T10:45:41+11:00" + }, + "format": { + "system": "https://fhir.nhs.uk/England/CodeSystem/England-NRLFormatCode", + "code": "urn:nhs-ic:unstructured", + "display": "Unstructured Document" + }, + "extension": [ + { + "url": "https://fhir.nhs.uk/England/StructureDefinition/Extension-England-ContentStability", + "valueCodeableConcept": { + "coding": [ + { + "system": "https://fhir.nhs.uk/England/CodeSystem/England-NRLContentStability", + "code": "static", + "display": "Static" + } + ] + } + } + ] + } + ], + "context": { + "practiceSetting": { + "coding": [ + { + "system": "http://snomed.info/sct", + "code": "788002001", + "display": "Adult mental health service" + } + ] + }, + "sourcePatientInfo": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "6700028191" + } + } + }, + "masterIdentifier": { + "system": "https://fhir.nhs.uk/Id/document-reference-master-id", + "value": "Y05868-736253002-0000000001" + } +} diff --git a/tests/data/DocumentReference/Y05868-Appointment-Valid.json b/tests/data/DocumentReference/Y05868-Appointment-Valid.json new file mode 100644 index 000000000..a0f7c1519 --- /dev/null +++ b/tests/data/DocumentReference/Y05868-Appointment-Valid.json @@ -0,0 +1,111 @@ +{ + "resourceType": "DocumentReference", + "id": "Y05868-99999-99999-999999", + "status": "current", + "docStatus": "final", + "type": { + "coding": [ + { + "system": "http://snomed.info/sct", + "code": "749001000000101", + "display": "Appointment" + } + ] + }, + "category": [ + { + "coding": [ + { + "system": "http://snomed.info/sct", + "code": "419891008", + "display": "Record artifact" + } + ] + } + ], + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "6700028191" + } + }, + "author": [ + { + "identifier": { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "Y05868" + } + } + ], + "custodian": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "Y05868" + } + }, + "description": "Physical document mental health crisis plan", + "securityLabel": [ + { + "coding": [ + { + "system": "http://terminology.hl7.org/CodeSystem/v3-Confidentiality", + "code": "V", + "display": "very restricted" + } + ] + } + ], + "content": [ + { + "attachment": { + "contentType": "application/pdf", + "language": "en-US", + "url": "https://spine-proxy.national.ncrs.nhs.uk/https%3A%2F%2Fp1.nhs.uk%2FMentalhealthCrisisPlanReport.pdf", + "size": 3654, + "hash": "2jmj7l5rSw0yVb/vlWAYkK/YBwk=", + "title": "Mental health crisis plan report", + "creation": "2022-12-21T10:45:41+11:00" + }, + "format": { + "system": "https://fhir.nhs.uk/England/CodeSystem/England-NRLFormatCode", + "code": "urn:nhs-ic:unstructured", + "display": "Unstructured Document" + }, + "extension": [ + { + "url": "https://fhir.nhs.uk/England/StructureDefinition/Extension-England-ContentStability", + "valueCodeableConcept": { + "coding": [ + { + "system": "https://fhir.nhs.uk/England/CodeSystem/England-NRLContentStability", + "code": "static", + "display": "Static" + } + ] + } + } + ] + } + ], + "context": { + "practiceSetting": { + "coding": [ + { + "system": "http://snomed.info/sct", + "code": "788002001", + "display": "Adult mental health service" + } + ] + }, + "sourcePatientInfo": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "6700028191" + } + } + }, + "masterIdentifier": { + "system": "https://fhir.nhs.uk/Id/document-reference-master-id", + "value": "Y05868-736253002-0000000001" + } +} From a636e7e8ed84244db1753c9ee0325bda43c0cbcc Mon Sep 17 00:00:00 2001 From: Matt Dean Date: Fri, 17 Oct 2025 10:33:27 +0100 Subject: [PATCH 3/5] [NRL-1631] Remove extra space in log message --- layer/nrlf/core/log_references.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/layer/nrlf/core/log_references.py b/layer/nrlf/core/log_references.py index f68f72573..eb4fbd33e 100644 --- a/layer/nrlf/core/log_references.py +++ b/layer/nrlf/core/log_references.py @@ -271,7 +271,7 @@ class LogReference(Enum): "INFO", "Preserved .date field when creating new document reference" ) PROCREATE012 = _Reference( - "WARN", "Existing pointers found for patient during create operation" + "WARN", "Existing pointers found for patient during create operation" ) PROCREATE999 = _Reference( "INFO", "Successfully completed producer createDocumentReference" From 5dce45b5586541a37986b6261b93b97f53a9e9cf Mon Sep 17 00:00:00 2001 From: Matt Dean Date: Fri, 17 Oct 2025 10:49:23 +0100 Subject: [PATCH 4/5] [NRL-1631] Fix tests for new Location header change --- .../tests/test_create_document_reference.py | 4 ++-- .../tests/test_upsert_document_reference.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/api/producer/createDocumentReference/tests/test_create_document_reference.py b/api/producer/createDocumentReference/tests/test_create_document_reference.py index 353071c68..fb55d5810 100644 --- a/api/producer/createDocumentReference/tests/test_create_document_reference.py +++ b/api/producer/createDocumentReference/tests/test_create_document_reference.py @@ -1759,7 +1759,7 @@ def test_create_logs_for_unexpected_multi_pointer( assert result == { "statusCode": "201", "headers": { - "Location": "/producer/FHIR/R4/DocumentReference/Y05868-00000000-0000-0000-0000-000000000001", + "Location": "/DocumentReference/Y05868-00000000-0000-0000-0000-000000000001", **default_response_headers(), }, "isBase64Encoded": False, @@ -1843,7 +1843,7 @@ def test_create_logs_for_expected_multi_pointer( assert result == { "statusCode": "201", "headers": { - "Location": "/producer/FHIR/R4/DocumentReference/Y05868-00000000-0000-0000-0000-000000000001", + "Location": "/DocumentReference/Y05868-00000000-0000-0000-0000-000000000001", **default_response_headers(), }, "isBase64Encoded": False, diff --git a/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py b/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py index 5aa3ba079..e251928dd 100644 --- a/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py +++ b/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py @@ -1725,7 +1725,7 @@ def test_upsert_logs_for_unexpected_multi_pointer( assert result == { "statusCode": "201", "headers": { - "Location": "/producer/FHIR/R4/DocumentReference/Y05868-99999-99999-999999-02", + "Location": "/DocumentReference/Y05868-99999-99999-999999-02", **default_response_headers(), }, "isBase64Encoded": False, @@ -1808,7 +1808,7 @@ def test_upsert_logs_for_expected_multi_pointer( assert result == { "statusCode": "201", "headers": { - "Location": "/producer/FHIR/R4/DocumentReference/Y05868-99999-99999-999999-02", + "Location": "/DocumentReference/Y05868-99999-99999-999999-02", **default_response_headers(), }, "isBase64Encoded": False, From c1da942b0586f5fe39df2d070882bc806725cc9b Mon Sep 17 00:00:00 2001 From: Matt Dean Date: Wed, 22 Oct 2025 08:58:47 +0100 Subject: [PATCH 5/5] [NRL-1631] Add masterid to multi-pointer warning log --- .../createDocumentReference/create_document_reference.py | 7 +++---- .../tests/test_create_document_reference.py | 1 + .../tests/test_upsert_document_reference.py | 3 +++ .../upsertDocumentReference/upsert_document_reference.py | 4 ++-- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/api/producer/createDocumentReference/create_document_reference.py b/api/producer/createDocumentReference/create_document_reference.py index 223086ae5..0e0c70021 100644 --- a/api/producer/createDocumentReference/create_document_reference.py +++ b/api/producer/createDocumentReference/create_document_reference.py @@ -259,8 +259,6 @@ def handler( pointer_type = core_model.type if pointer_type not in TYPES_WITH_MULTIPLES: nhs_number = core_model.nhs_number - pointer_custodian = core_model.custodian - pointer_master_identifier = core_model.master_identifier existing_pointers_count = repository.count_by_nhs_number( nhs_number, [pointer_type] ) @@ -268,11 +266,12 @@ def handler( if existing_pointers_count > 0: logger.log( LogReference.PROCREATE012, - new_pointer_master_id=pointer_master_identifier, + new_pointer_id=core_model.id, + new_pointer_master_id=core_model.master_identifier, pointer_type=pointer_type, nhs_number=nhs_number, + custodian=core_model.custodian, existing_pointers_count=existing_pointers_count, - custodian=pointer_custodian, ) logger.log(LogReference.PROCREATE009, pointer_id=result.resource.id) diff --git a/api/producer/createDocumentReference/tests/test_create_document_reference.py b/api/producer/createDocumentReference/tests/test_create_document_reference.py index fb55d5810..a702d78c5 100644 --- a/api/producer/createDocumentReference/tests/test_create_document_reference.py +++ b/api/producer/createDocumentReference/tests/test_create_document_reference.py @@ -1808,6 +1808,7 @@ def test_create_logs_for_unexpected_multi_pointer( if doc_ref.custodian and doc_ref.custodian.identifier else None ), + "new_pointer_id": "Y05868-00000000-0000-0000-0000-000000000001", "new_pointer_master_id": ( doc_ref.masterIdentifier.value if doc_ref.masterIdentifier else None ), diff --git a/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py b/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py index e251928dd..26280ed6e 100644 --- a/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py +++ b/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py @@ -1775,6 +1775,9 @@ def test_upsert_logs_for_unexpected_multi_pointer( else None ), "new_pointer_id": doc_ref.id, + "new_pointer_master_id": ( + doc_ref.masterIdentifier.value if doc_ref.masterIdentifier else None + ), } == [ call[1:][0] for call in mock_logger.log.call_args_list diff --git a/api/producer/upsertDocumentReference/upsert_document_reference.py b/api/producer/upsertDocumentReference/upsert_document_reference.py index 2018946c9..b57a1bf2b 100644 --- a/api/producer/upsertDocumentReference/upsert_document_reference.py +++ b/api/producer/upsertDocumentReference/upsert_document_reference.py @@ -266,7 +266,6 @@ def handler( pointer_type = core_model.type if pointer_type not in TYPES_WITH_MULTIPLES: nhs_number = core_model.nhs_number - pointer_custodian = core_model.custodian existing_pointers_count = repository.count_by_nhs_number( nhs_number, [pointer_type] ) @@ -275,10 +274,11 @@ def handler( logger.log( LogReference.PROUPSERT012, new_pointer_id=core_model.id, + new_pointer_master_id=core_model.master_identifier, pointer_type=pointer_type, nhs_number=nhs_number, + custodian=core_model.custodian, existing_pointers_count=existing_pointers_count, - custodian=pointer_custodian, ) logger.log(LogReference.PROUPSERT009, pointer_id=result.resource.id)