Remove PID from id sync lambda

Author: dlzhry2nhs

.github/pull_request_template.md

@@ -19,6 +19,7 @@ Add any other relevant notes or explanations here. **Remove this line if you hav
 
 :information_source: This section is to be filled in by the **reviewer**.
 
-- [ ] I have reviewed the changes in this PR and they fill all or part of the acceptance criteria of the ticket, and the code is in a mergeable state.
+- [ ] I have reviewed the changes in this PR and they fill all of the acceptance criteria of the ticket.
 - [ ] If there were infrastructure, operational, or build changes, I have made sure there is sufficient evidence that the changes will work.
-- [ ] I have ensured the changelog has been updated by the submitter, if necessary.
+- [ ] If there were changes that are outside of the regular release processes e.g. account infrastructure to setup, manual setup for external API integrations, secrets to set, then I have checked that the developer has flagged this to the Tech Lead as release steps.
+- [ ] I have checked that no Personal Identifiable Data (PID) is logged as part of the changes.

lambdas/id_sync/src/id_sync.py

@@ -26,30 +26,21 @@ def handler(event_data: Dict[str, Any], _context) -> Dict[str, Any]:
         logger.info("id_sync processing event with %d records", len(records))
 
         results = []
-        nhs_numbers = []
         error_count = 0
 
         for record in records:
             result = process_record(record)
             results.append(result)
 
-            if "nhs_number" in result:
-                nhs_numbers.append(result["nhs_number"])
-
             if result.get("status") == "error":
                 error_count += 1
 
         if error_count > 0:
             raise IdSyncException(
                 message=f"Processed {len(records)} records with {error_count} errors",
-                nhs_numbers=nhs_numbers,
             )
 
-        response = {
-            "status": "success",
-            "message": f"Successfully processed {len(records)} records",
-            "nhs_numbers": nhs_numbers,
-        }
+        response = {"status": "success", "message": f"Successfully processed {len(records)} records"}
 
         logger.info("id_sync handler completed: %s", response)
         return response

lambdas/id_sync/src/ieds_db_operations.py

@@ -23,25 +23,22 @@ def get_ieds_table():
 
 def ieds_update_patient_id(old_id: str, new_id: str, items_to_update: list | None = None) -> dict:
     """Update the patient ID (new NHS number) in the IEDS table."""
-    logger.info(f"ieds_update_patient_id. Update patient ID from {old_id} to {new_id}")
     if not old_id or not new_id or not old_id.strip() or not new_id.strip():
-        return make_status("Old ID and New ID cannot be empty", old_id, "error")
+        return make_status("Old ID and New ID cannot be empty", status="error")
 
     if old_id == new_id:
-        return make_status(f"No change in patient ID: {old_id}", old_id)
+        return make_status("No change in patient ID")
 
     try:
-        logger.info(f"Updating patient ID in IEDS from {old_id} to {new_id}")
-
         if items_to_update is None:
             logger.info("Getting items to update in IEDS table...")
             items_to_update = get_items_from_patient_id(old_id)
         else:
             logger.info("Using provided items_to_update list, size=%d", len(items_to_update))
 
         if not items_to_update:
-            logger.warning(f"No items found to update for patient ID: {old_id}")
-            return make_status(f"No items found to update for patient ID: {old_id}", old_id)
+            logger.warning("No items found to update for patient NHS Number")
+            return make_status("No items found to update for patient ID")
 
         logger.info(f"Items to update: {len(items_to_update)}")
 
@@ -55,22 +52,18 @@ def ieds_update_patient_id(old_id: str, new_id: str, items_to_update: list | Non
         logger.info(f"All batches complete. Total batches: {total_batches}, All successful: {all_batches_successful}")
 
         if all_batches_successful:
-            return make_status(
-                f"IEDS update, patient ID: {old_id}=>{new_id}. {len(items_to_update)} updated {total_batches}.",
-                old_id,
-            )
+            return make_status(f"IEDS update. {len(items_to_update)} item(s) updated in {total_batches} batch(es).")
         else:
             return make_status(
-                f"Failed to update some batches for patient ID: {old_id}",
-                old_id,
-                "error",
+                "Failed to update some batches for patient ID",
+                status="error",
             )
 
     except Exception as e:
         logger.exception("Error updating patient ID")
         logger.info("Error details: %s", e)
         raise IdSyncException(
-            message=f"Error updating patient Id from :{old_id} to {new_id}",
+            message="Error updating patient ID",
             nhs_numbers=[old_id, new_id],
             exception=e,
         )
@@ -82,16 +75,15 @@ def get_items_from_patient_id(id: str) -> list:
     Delegates actual paging to the internal helper `_paginate_items_for_patient_pk`.
     Raises IdSyncException on error.
     """
-    logger.info("Getting items for patient id: %s", id)
     patient_pk = f"Patient#{id}"
     try:
         return paginate_items_for_patient_pk(patient_pk)
     except IdSyncException:
         raise
     except Exception as e:
-        logger.exception("Error querying items for patient PK: %s", patient_pk)
+        logger.exception("Error querying items for patient PK")
         raise IdSyncException(
-            message=f"Error querying items for patient PK: {patient_pk}",
+            message="Error querying items for patient PK",
             nhs_numbers=[patient_pk],
             exception=e,
         )

lambdas/id_sync/src/pds_details.py

@@ -3,6 +3,7 @@
 """
 
 import tempfile
+from typing import Optional
 
 from common.authentication import AppRestrictedAuth, Service
 from common.cache import Cache
@@ -18,8 +19,6 @@
 # Get Patient details from external service PDS using NHS number from MNS notification
 def pds_get_patient_details(nhs_number: str) -> dict:
     try:
-        logger.info(f"get patient details. nhs_number: {nhs_number}")
-
         cache = Cache(directory=safe_tmp_dir)
         authenticator = AppRestrictedAuth(
             service=Service.PDS,
@@ -31,26 +30,27 @@ def pds_get_patient_details(nhs_number: str) -> dict:
         patient = pds_service.get_patient_details(nhs_number)
         return patient
     except Exception as e:
-        msg = f"Error getting PDS patient details for {nhs_number}"
+        msg = "Error retrieving patient details from PDS"
         logger.exception(msg)
         raise IdSyncException(message=msg, exception=e)
 
 
 # Extract Patient identifier value from PDS patient details
-def pds_get_patient_id(nhs_number: str) -> str:
+def pds_get_patient_id(nhs_number: str) -> Optional[str]:
     """
     Get PDS patient ID from NHS number.
     :param nhs_number: NHS number of the patient
     :return: PDS patient ID
     """
     try:
         patient_details = pds_get_patient_details(nhs_number)
+
         if not patient_details:
             return None
 
         return patient_details["identifier"][0]["value"]
 
     except Exception as e:
-        msg = f"Error getting PDS patient ID for {nhs_number}"
+        msg = "Error retrieving patient details from PDS"
         logger.exception(msg)
         raise IdSyncException(message=msg, exception=e)

lambdas/id_sync/src/record_processor.py

@@ -1,4 +1,3 @@
-import ast
 import json
 from typing import Any, Dict
 
@@ -13,75 +12,66 @@
 
 
 def process_record(event_record: Dict[str, Any]) -> Dict[str, Any]:
-    logger.info("process_record. Processing record: %s", event_record)
+    logger.info("Processing record with SQS messageId: %s", event_record.get("messageId"))
     body_text = event_record.get("body", "")
 
-    # convert body to json (try JSON first, then fall back to Python literal)
-    if isinstance(body_text, str):
-        try:
-            body = json.loads(body_text)
-        except json.JSONDecodeError:
-            try:
-                body = ast.literal_eval(body_text)
-            except (ValueError, SyntaxError):
-                logger.error("Failed to parse body: %s", body_text)
-                return {"status": "error", "message": "Invalid body format"}
-    else:
-        body = body_text
+    try:
+        body = json.loads(body_text)
+    except json.JSONDecodeError:
+        logger.error("Failed to parse body: %s", body_text)
+        return {"status": "error", "message": "Invalid body format"}
 
     nhs_number = body.get("subject")
-    logger.info("process record NHS number: %s", nhs_number)
-    if nhs_number:
-        return process_nhs_number(nhs_number)
+    logger.info("Processing MNS event with id: %s", body.get("id"))
+
+    if not nhs_number:
+        logger.info("No NHS number found in event record")
+        return {"status": "error", "message": "No NHS number found in event record"}
 
-    logger.info("No NHS number found in event record")
-    return {"status": "error", "message": "No NHS number found in event record"}
+    return process_nhs_number(nhs_number)
 
 
 def process_nhs_number(nhs_number: str) -> Dict[str, Any]:
-    # get patient details from PDS
     new_nhs_number = pds_get_patient_id(nhs_number)
 
     if not new_nhs_number:
-        return make_status("No patient ID found for NHS number", nhs_number)
+        return make_status("No patient ID found for NHS number")
 
     if new_nhs_number == nhs_number:
-        return make_status("No update required", nhs_number)
+        return make_status("No update required")
 
-    logger.info("Update patient ID from %s to %s", nhs_number, new_nhs_number)
+    logger.info("NHS Number has changed. Performing updates on relevant IEDS records")
 
     try:
         # Fetch PDS Patient resource and IEDS resources for the old NHS number
         pds_patient_resource, ieds_resources = fetch_pds_and_ieds_resources(nhs_number)
     except Exception as e:
         logger.exception("process_nhs_number: failed to fetch demographic details: %s", e)
-        return make_status(str(e), nhs_number, "error")
+        return make_status(str(e), status="error")
 
     logger.info(
         "Fetched IEDS resources. IEDS count: %d",
         len(ieds_resources) if ieds_resources else 0,
     )
 
     if not ieds_resources:
-        logger.info("No IEDS records returned for NHS number: %s", nhs_number)
-        return make_status(f"No records returned for ID: {nhs_number}", nhs_number)
+        logger.info("No IEDS records returned for NHS number")
+        return make_status("No records returned for NHS Number")
 
     # Compare demographics from PDS to each IEDS item, keep only matching records
     matching_records = []
     discarded_count = 0
     for detail in ieds_resources:
-        logger.info("Processing IEDS record: %s", detail)
         if demographics_match(pds_patient_resource, detail):
             matching_records.append(detail)
         else:
             discarded_count += 1
 
     if not matching_records:
         logger.info("No records matched PDS demographics: %d", discarded_count)
-        return make_status("No records matched PDS demographics; update skipped", nhs_number)
+        return make_status("No records matched PDS demographics; update skipped")
 
     response = ieds_update_patient_id(nhs_number, new_nhs_number, items_to_update=matching_records)
-    response["nhs_number"] = nhs_number
     # add counts for observability
     response["matched"] = len(matching_records)
     response["discarded"] = discarded_count
@@ -90,23 +80,16 @@ def process_nhs_number(nhs_number: str) -> Dict[str, Any]:
 
 # Function to fetch PDS Patient details and IEDS Immunisation records.
 def fetch_pds_and_ieds_resources(nhs_number: str):
-    logger.info("fetch_pds_and_ieds_resources: fetching for %s", nhs_number)
     try:
         pds = pds_get_patient_details(nhs_number)
     except Exception as e:
-        logger.exception(
-            "fetch_pds_and_ieds_resources: failed to fetch PDS details for %s",
-            nhs_number,
-        )
+        logger.exception("fetch_pds_and_ieds_resources: failed to fetch PDS details")
         raise RuntimeError("Failed to fetch PDS details") from e
 
     try:
         ieds = get_items_from_patient_id(nhs_number)
     except Exception as e:
-        logger.exception(
-            "fetch_pds_and_ieds_resources: failed to fetch IEDS items for %s",
-            nhs_number,
-        )
+        logger.exception("fetch_pds_and_ieds_resources: failed to fetch IEDS items")
         raise RuntimeError("Failed to fetch IEDS items") from e
 
     return pds, ieds
@@ -147,13 +130,7 @@ def normalize_strings(item: Any) -> str | None:
         # Retrieve patient resource from PDS
         pds_name = normalize_strings(extract_normalized_name_from_patient(pds_details))
         pds_gender = normalize_strings(pds_details.get("gender"))
-        pds_birth = normalize_strings(pds_details.get("birthDate"))
-        logger.debug(
-            "demographics_match: demographics match for name=%s, gender=%s, birthDate=%s",
-            pds_name,
-            pds_gender,
-            pds_birth,
-        )
+        pds_dob = normalize_strings(pds_details.get("birthDate"))
 
         # Retrieve patient resource from IEDS item
         patient = extract_patient_resource_from_item(ieds_item)
@@ -164,24 +141,24 @@ def normalize_strings(item: Any) -> str | None:
         # normalize patient fields from IEDS
         ieds_name = normalize_strings(extract_normalized_name_from_patient(patient))
         ieds_gender = normalize_strings(patient.get("gender"))
-        ieds_birth = normalize_strings(patient.get("birthDate"))
+        ieds_dob = normalize_strings(patient.get("birthDate"))
 
         # All required fields must be present
-        if not all([pds_name, pds_gender, pds_birth, ieds_name, ieds_gender, ieds_birth]):
+        if not all([pds_name, pds_gender, pds_dob, ieds_name, ieds_gender, ieds_dob]):
             logger.debug("demographics_match: missing required demographics")
             return False
 
         # Compare fields
-        if pds_birth != ieds_birth:
-            logger.debug("demographics_match: birthDate mismatch %s != %s", pds_birth, ieds_birth)
+        if pds_dob != ieds_dob:
+            logger.debug("demographics_match: birthDate mismatch between the patient resources")
             return False
 
         if pds_gender != ieds_gender:
-            logger.debug("demographics_match: gender mismatch %s != %s", pds_gender, ieds_gender)
+            logger.debug("demographics_match: gender mismatch between the patient resources")
             return False
 
         if pds_name != ieds_name:
-            logger.debug("demographics_match: name mismatch %s != %s", pds_name, ieds_name)
+            logger.debug("demographics_match: name mismatch between the patient resources")
             return False
 
         return True

lambdas/id_sync/src/utils.py

@@ -1,13 +1,6 @@
 from typing import Any, Dict
 
 
-def make_status(msg: str, nhs_number: str | None = None, status: str = "success") -> Dict[str, Any]:
-    """Return a simple status dict used by record processing for observability.
-
-    If `nhs_number` is None the key is omitted which keeps the output shape
-    compatible with callers that expect only a status/message.
-    """
-    result = {"status": status, "message": msg}
-    if nhs_number is not None:
-        result["nhs_number"] = nhs_number
-    return result
+def make_status(msg: str, status: str = "success") -> Dict[str, Any]:
+    """Return a simple status dict used by record processing for observability."""
+    return {"status": status, "message": msg}