[NDR-290] Allow incoming FHIR to exclude type and author. (#938)

Author: jameslinnell

lambdas/models/document_reference.py

@@ -99,7 +99,7 @@ class DocumentReference(BaseModel):
     )
 
     id: str = Field(..., alias=str(DocumentReferenceMetadataFields.ID.value))
-    author: str = Field(default=None)
+    author: str | None = None
     content_type: str = Field(default=DEFAULT_CONTENT_TYPE)
     created: str = Field(
         default_factory=lambda: datetime.now(timezone.utc).strftime(DATE_FORMAT)

lambdas/models/fhir/R4/fhir_document_reference.py

@@ -116,11 +116,11 @@ class DocumentReference(BaseModel):
         "unknown",
     ] = "final"
     status: Literal["current", "superseded", "entered-in-error"] = "current"
-    type: Optional[CodeableConcept]
+    type: Optional[CodeableConcept] = None
     category: Optional[List[CodeableConcept]] = None
     subject: Optional[Reference]
     date: Optional[str] = None
-    author: Optional[List[Reference]]
+    author: Optional[List[Reference]] = None
     authenticator: Optional[Reference] = None
     custodian: Optional[Reference] = None
     content: List[DocumentReferenceContent]

lambdas/services/post_fhir_document_reference_service.py

@@ -43,9 +43,10 @@ def process_fhir_document_reference(
                 fhir_document
             )
 
-            # Extract NHS number from the FHIR document
+            # Extract NHS number and author from the FHIR document
             try:
                 nhs_number = validated_fhir_doc.extract_nhs_number_from_fhir()
+                author = self._extract_author_from_fhir(validated_fhir_doc)
             except FhirDocumentReferenceException:
                 raise DocumentRefException(400, LambdaError.DocRefNoParse)
 
@@ -63,6 +64,7 @@ def process_fhir_document_reference(
             # Create a document reference model
             document_reference = self._create_document_reference(
                 nhs_number,
+                author,
                 doc_type,
                 validated_fhir_doc,
                 patient_details.general_practice_ods,
@@ -82,6 +84,23 @@ def process_fhir_document_reference(
             logger.error(f"AWS client error: {str(e)}")
             raise DocumentRefException(500, LambdaError.InternalServerError)
 
+    def _extract_author_from_fhir(self, fhir_doc: FhirDocumentReference) -> str | None:
+        authors = getattr(fhir_doc, "author", None)
+
+        if not authors:
+            return None
+
+        try:
+            identifier = authors[0].identifier
+            if not identifier or identifier.value is None:
+                logger.error("FHIR document validation error: author identifier value")
+                raise DocumentRefException(400, LambdaError.DocRefNoParse)
+
+            return identifier.value
+        except (AttributeError, IndexError) as e:
+            logger.error(f"FHIR document validation error: {str(e)}")
+            raise DocumentRefException(400, LambdaError.DocRefNoParse)
+
     def _determine_document_type(
         self, fhir_doc: FhirDocumentReference, common_name: MtlsCommonNames | None
     ) -> SnomedCode:
@@ -109,6 +128,7 @@ def _determine_document_type(
     def _create_document_reference(
         self,
         nhs_number: str,
+        author: str | None,
         doc_type: SnomedCode,
         fhir_doc: FhirDocumentReference,
         current_gp_ods: str,
@@ -133,7 +153,7 @@ def _create_document_reference(
             current_gp_ods=current_gp_ods,
             custodian=custodian,
             s3_bucket_name=self.staging_bucket_name,
-            author=fhir_doc.author[0].identifier.value,
+            author=author,
             content_type=fhir_doc.content[0].attachment.contentType,
             file_name=fhir_doc.content[0].attachment.title,
             document_snomed_code_type=doc_type.code,

…pi/fhir/test_upload_document_fhir_api.py …test_upload_document_fhir_api_failure.pylambdas/tests/e2e/api/fhir/test_upload_document_fhir_api.py renamed to lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_failure.py lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api.py renamed to lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_failure.py

@@ -1,4 +1,5 @@
 import base64
+import json
 import logging
 import os
 
@@ -36,44 +37,6 @@ def retrieve_document_with_retry(doc_id, condition):
     return fetch_with_retry_mtls(session, retrieve_url, headers, condition)
 
 
-def test_create_document_base64(test_data):
-    record = {
-        "ods": "H81109",
-        "nhs_number": "9912003071",
-    }
-
-    sample_pdf_path = os.path.join(os.path.dirname(__file__), "files", "dummy.pdf")
-    with open(sample_pdf_path, "rb") as f:
-        record["data"] = base64.b64encode(f.read()).decode("utf-8")
-    payload = pdm_data_helper.create_upload_payload(record)
-
-    raw_upload_response = upload_document(payload)
-    assert raw_upload_response.status_code == 200
-    record["id"] = raw_upload_response.json()["id"].split("~")[1]
-    test_data.append(record)
-
-    # Validate attachment URL
-    upload_response = raw_upload_response.json()
-    attachment_url = upload_response["content"][0]["attachment"]["url"]
-    assert (
-        f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{PDM_SNOMED}~"
-        in attachment_url
-    )
-
-    def condition(response_json):
-        logging.info(response_json)
-        return response_json["content"][0]["attachment"].get("data", False)
-
-    raw_retrieve_response = retrieve_document_with_retry(
-        upload_response["id"], condition
-    )
-    retrieve_response = raw_retrieve_response.json()
-
-    # Validate base64 data
-    base64_data = retrieve_response["content"][0]["attachment"]["data"]
-    assert base64.b64decode(base64_data, validate=True)
-
-
 def test_create_document_presign_fails():
     record = {
         "ods": "H81109",
@@ -187,7 +150,11 @@ def test_forbidden_with_invalid_cert(temp_cert_and_key):
     assert body["message"] == "Forbidden"
 
 
-def test_create_document_saves_raw(test_data):
+@pytest.mark.parametrize(
+    "author_payload",
+    [({}), {"identifier": {"system": "https://fhir.nhs.uk/Id/ods-organization-code"}}],
+)
+def test_create_document_with_invalid_author_returns_error(test_data, author_payload):
     record = {
         "ods": "H81109",
         "nhs_number": "9912003071",
@@ -196,15 +163,14 @@ def test_create_document_saves_raw(test_data):
     sample_pdf_path = os.path.join(os.path.dirname(__file__), "files", "dummy.pdf")
     with open(sample_pdf_path, "rb") as f:
         record["data"] = base64.b64encode(f.read()).decode("utf-8")
-    payload = pdm_data_helper.create_upload_payload(record)
+    payload = pdm_data_helper.create_upload_payload(record=record, return_json=True)
+    payload["author"][0] = author_payload
+    payload = json.dumps(payload)
 
     raw_upload_response = upload_document(payload)
-    assert raw_upload_response.status_code == 200
-    record["id"] = raw_upload_response.json()["id"].split("~")[1]
-    test_data.append(record)
-
-    doc_ref = pdm_data_helper.retrieve_document_reference(record=record)
-    assert "Item" in doc_ref
-    assert "RawRequest" in doc_ref["Item"]
-    assert doc_ref["Item"]["RawRequest"]
-    assert doc_ref["Item"]["RawRequest"] == payload
+    response_json = raw_upload_response.json()
+    assert raw_upload_response.status_code == 400
+    assert response_json["resourceType"] == "OperationOutcome"
+    assert (
+        response_json["issue"][0]["details"]["coding"][0]["code"] == "VALIDATION_ERROR"
+    )

lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_success.py

@@ -0,0 +1,128 @@
+import base64
+import logging
+import os
+
+import pytest
+import requests
+
+from lambdas.tests.e2e.api.fhir.conftest import (
+    MTLS_ENDPOINT,
+    create_mtls_session,
+    fetch_with_retry_mtls,
+)
+from lambdas.tests.e2e.conftest import APIM_ENDPOINT, PDM_SNOMED
+from lambdas.tests.e2e.helpers.data_helper import PdmDataHelper
+
+pdm_data_helper = PdmDataHelper()
+
+
+def upload_document(payload):
+    """Helper to upload DocumentReference."""
+    url = f"https://{MTLS_ENDPOINT}/DocumentReference"
+    headers = {
+        "X-Correlation-Id": "1234",
+    }
+    session = create_mtls_session()
+    return session.post(url, headers=headers, data=payload)
+
+
+def retrieve_document_with_retry(doc_id, condition):
+    """Poll until condition is met on DocumentReference retrieval."""
+    retrieve_url = f"https://{MTLS_ENDPOINT}/DocumentReference/{doc_id}"
+    headers = {
+        "X-Correlation-Id": "1234",
+    }
+    session = create_mtls_session()
+    return fetch_with_retry_mtls(session, retrieve_url, headers, condition)
+
+
+def test_create_document_base64(test_data):
+    record = {
+        "ods": "H81109",
+        "nhs_number": "9912003071",
+    }
+
+    sample_pdf_path = os.path.join(os.path.dirname(__file__), "files", "dummy.pdf")
+    with open(sample_pdf_path, "rb") as f:
+        record["data"] = base64.b64encode(f.read()).decode("utf-8")
+    payload = pdm_data_helper.create_upload_payload(record)
+
+    raw_upload_response = upload_document(payload)
+    assert raw_upload_response.status_code == 200
+    record["id"] = raw_upload_response.json()["id"].split("~")[1]
+    test_data.append(record)
+
+    # Validate attachment URL
+    upload_response = raw_upload_response.json()
+    attachment_url = upload_response["content"][0]["attachment"]["url"]
+    assert (
+        f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{PDM_SNOMED}~"
+        in attachment_url
+    )
+
+    def condition(response_json):
+        logging.info(response_json)
+        return response_json["content"][0]["attachment"].get("data", False)
+
+    raw_retrieve_response = retrieve_document_with_retry(
+        upload_response["id"], condition
+    )
+    retrieve_response = raw_retrieve_response.json()
+
+    # Validate base64 data
+    base64_data = retrieve_response["content"][0]["attachment"]["data"]
+    assert base64.b64decode(base64_data, validate=True)
+
+
+def test_create_document_saves_raw(test_data):
+    record = {
+        "ods": "H81109",
+        "nhs_number": "9912003071",
+    }
+
+    sample_pdf_path = os.path.join(os.path.dirname(__file__), "files", "dummy.pdf")
+    with open(sample_pdf_path, "rb") as f:
+        record["data"] = base64.b64encode(f.read()).decode("utf-8")
+    payload = pdm_data_helper.create_upload_payload(record)
+
+    raw_upload_response = upload_document(payload)
+    assert raw_upload_response.status_code == 200
+    record["id"] = raw_upload_response.json()["id"].split("~")[1]
+    test_data.append(record)
+
+    doc_ref = pdm_data_helper.retrieve_document_reference(record=record)
+    assert "Item" in doc_ref
+    assert "RawRequest" in doc_ref["Item"]
+    assert "Author" in doc_ref["Item"]
+    assert doc_ref["Item"]["RawRequest"]
+    assert doc_ref["Item"]["RawRequest"] == payload
+
+
+def test_create_document_without_author_or_type(test_data):
+    record = {
+        "ods": "H81109",
+        "nhs_number": "9912003071",
+    }
+
+    sample_pdf_path = os.path.join(os.path.dirname(__file__), "files", "dummy.pdf")
+    with open(sample_pdf_path, "rb") as f:
+        record["data"] = base64.b64encode(f.read()).decode("utf-8")
+    payload = pdm_data_helper.create_upload_payload(
+        record=record, exclude=["type", "author"]
+    )
+
+    for field in ["type", "author"]:
+        assert field not in payload
+    raw_upload_response = upload_document(payload)
+    assert raw_upload_response.status_code == 200
+    record["id"] = raw_upload_response.json()["id"].split("~")[1]
+    test_data.append(record)
+
+    doc_ref = pdm_data_helper.retrieve_document_reference(record=record)
+    assert "Item" in doc_ref
+    assert "RawRequest" in doc_ref["Item"]
+    assert "Author" not in doc_ref["Item"]
+    assert doc_ref["Item"]["RawRequest"]
+    assert doc_ref["Item"]["RawRequest"] == payload
+    for field in ["type", "author"]:
+        assert field not in doc_ref["Item"]["RawRequest"]

lambdas/tests/e2e/helpers/data_helper.py

@@ -93,7 +93,7 @@ def retrieve_document_reference(self, record):
             table_name=self.dynamo_table, key={"ID": record["id"]}
         )
 
-    def create_upload_payload(self, record):
+    def create_upload_payload(self, record, exclude=[], return_json=False):
         """Helper to build DocumentReference payload."""
         payload = {
             "resourceType": "DocumentReference",
@@ -137,8 +137,16 @@ def create_upload_payload(self, record):
                 }
             ],
         }
+
+        for field in exclude:
+            payload.pop(field, None)
+
         if "data" in record:
             payload["content"][0]["attachment"]["data"] = record["data"]
+
+        if return_json:
+            return payload
+
         return json.dumps(payload)
 
     def tidyup(self, record):