VED-897 Remove PID from id sync lambda (#968)

dlzhry2nhs · web-flow · commit 5926e92f9f0e · 2025-11-06T00:56:08.000Z
diff --git a/.github/pull_request_template.md b/.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.
diff --git a/infrastructure/instance/endpoints.tf b/infrastructure/instance/endpoints.tf
@@ -28,7 +28,6 @@ locals {
     "IMMUNIZATION_BASE_PATH" = strcontains(var.sub_environment, "pr-") ? "immunisation-fhir-api/FHIR/R4-${var.sub_environment}" : "immunisation-fhir-api/FHIR/R4"
     # except for prod and ref, any other env uses PDS int environment
     "PDS_ENV"              = var.pds_environment
-    "PDS_CHECK_ENABLED"    = tostring(var.pds_check_enabled)
     "SPLUNK_FIREHOSE_NAME" = module.splunk.firehose_stream_name
     "SQS_QUEUE_URL"        = "https://sqs.eu-west-2.amazonaws.com/${var.immunisation_account_id}/${local.short_prefix}-ack-metadata-queue.fifo"
     "REDIS_HOST"           = data.aws_elasticache_cluster.existing_redis.cache_nodes[0].address
diff --git a/infrastructure/instance/environments/dev/int/variables.tfvars b/infrastructure/instance/environments/dev/int/variables.tfvars
@@ -3,6 +3,5 @@ immunisation_account_id           = "345594581768"
 dspp_core_account_id              = "603871901111"
 pds_environment                   = "int"
 batch_error_notifications_enabled = true
-pds_check_enabled                 = false
 create_mesh_processor             = true
 has_sub_environment_scope         = true
diff --git a/infrastructure/instance/environments/dev/internal-dev/variables.tfvars b/infrastructure/instance/environments/dev/internal-dev/variables.tfvars
@@ -3,6 +3,5 @@ immunisation_account_id           = "345594581768"
 dspp_core_account_id              = "603871901111"
 pds_environment                   = "int"
 batch_error_notifications_enabled = true
-pds_check_enabled                 = true
 create_mesh_processor             = false
 has_sub_environment_scope         = true
diff --git a/infrastructure/instance/environments/dev/pr/variables.tfvars b/infrastructure/instance/environments/dev/pr/variables.tfvars
@@ -3,6 +3,5 @@ immunisation_account_id           = "345594581768"
 dspp_core_account_id              = "603871901111"
 pds_environment                   = "int"
 batch_error_notifications_enabled = false
-pds_check_enabled                 = true
 create_mesh_processor             = false
 has_sub_environment_scope         = true
diff --git a/infrastructure/instance/environments/dev/ref/variables.tfvars b/infrastructure/instance/environments/dev/ref/variables.tfvars
@@ -3,6 +3,5 @@ immunisation_account_id           = "345594581768"
 dspp_core_account_id              = "603871901111"
 pds_environment                   = "ref"
 batch_error_notifications_enabled = true
-pds_check_enabled                 = true
 create_mesh_processor             = false
 has_sub_environment_scope         = true
diff --git a/infrastructure/instance/environments/preprod/int-blue/variables.tfvars b/infrastructure/instance/environments/preprod/int-blue/variables.tfvars
@@ -3,6 +3,5 @@ immunisation_account_id           = "084828561157"
 dspp_core_account_id              = "603871901111"
 pds_environment                   = "int"
 batch_error_notifications_enabled = true
-pds_check_enabled                 = false
 create_mesh_processor             = false
 has_sub_environment_scope         = false
diff --git a/infrastructure/instance/environments/preprod/int-green/variables.tfvars b/infrastructure/instance/environments/preprod/int-green/variables.tfvars
@@ -3,6 +3,5 @@ immunisation_account_id           = "084828561157"
 dspp_core_account_id              = "603871901111"
 pds_environment                   = "int"
 batch_error_notifications_enabled = true
-pds_check_enabled                 = false
 create_mesh_processor             = false
 has_sub_environment_scope         = false
diff --git a/infrastructure/instance/environments/prod/blue/variables.tfvars b/infrastructure/instance/environments/prod/blue/variables.tfvars
@@ -3,6 +3,5 @@ immunisation_account_id           = "664418956997"
 dspp_core_account_id              = "232116723729"
 pds_environment                   = "prod"
 batch_error_notifications_enabled = true
-pds_check_enabled                 = true
 create_mesh_processor             = true
 has_sub_environment_scope         = false
diff --git a/infrastructure/instance/environments/prod/green/variables.tfvars b/infrastructure/instance/environments/prod/green/variables.tfvars
@@ -3,6 +3,5 @@ immunisation_account_id           = "664418956997"
 dspp_core_account_id              = "232116723729"
 pds_environment                   = "prod"
 batch_error_notifications_enabled = true
-pds_check_enabled                 = true
 create_mesh_processor             = true
 has_sub_environment_scope         = false
diff --git a/infrastructure/instance/variables.tf b/infrastructure/instance/variables.tf
@@ -34,10 +34,6 @@ variable "pds_environment" {
   default = "int"
 }
 
-variable "pds_check_enabled" {
-  default = true
-}
-
 # Remember to switch off in PR envs after testing
 variable "batch_error_notifications_enabled" {
   default     = true
diff --git a/lambdas/id_sync/src/exceptions/id_sync_exception.py b/lambdas/id_sync/src/exceptions/id_sync_exception.py
@@ -1,8 +1,6 @@
 class IdSyncException(Exception):
     """Custom exception for ID Sync errors."""
 
-    def __init__(self, message: str, nhs_numbers: list = None, exception=None):
+    def __init__(self, message: str):
         self.message = message
-        self.nhs_numbers = nhs_numbers
-        self.inner_exception = exception
         super().__init__(message)
diff --git a/lambdas/id_sync/src/id_sync.py b/lambdas/id_sync/src/id_sync.py
@@ -25,31 +25,20 @@ 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
diff --git a/lambdas/id_sync/src/ieds_db_operations.py b/lambdas/id_sync/src/ieds_db_operations.py
@@ -21,30 +21,15 @@ def get_ieds_table():
     return ieds_table
 
 
-def ieds_update_patient_id(old_id: str, new_id: str, items_to_update: list | None = None) -> dict:
+def ieds_update_patient_id(old_id: str, new_id: str, items_to_update: list) -> 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")
+    if not items_to_update:
+        logger.info("No items found to update for patient NHS Number")
+        return make_status("No items found to update for patient ID")
 
-    if old_id == new_id:
-        return make_status(f"No change in patient ID: {old_id}", old_id)
+    logger.info(f"Items to update: {len(items_to_update)}")
 
     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.info(f"Items to update: {len(items_to_update)}")
-
         # Build transact items and execute them in batches via helpers to keep
         # the top-level function easy to read and test.
         transact_items = build_transact_items(old_id, new_id, items_to_update)
@@ -55,25 +40,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}",
-            nhs_numbers=[old_id, new_id],
-            exception=e,
-        )
+            message="Error updating patient ID",
+        ) from e
 
 
 def get_items_from_patient_id(id: str) -> list:
@@ -82,18 +60,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)
+    except Exception:
+        logger.exception("Error querying items for patient PK")
         raise IdSyncException(
-            message=f"Error querying items for patient PK: {patient_pk}",
-            nhs_numbers=[patient_pk],
-            exception=e,
+            message="Error querying items for patient PK",
         )
 
 
@@ -119,8 +94,6 @@ def paginate_items_for_patient_pk(patient_pk: str) -> list:
             logger.exception("Unexpected DynamoDB response: missing 'Items'")
             raise IdSyncException(
                 message="No Items in DynamoDB response",
-                nhs_numbers=[patient_pk],
-                exception=response,
             )
 
         items = response.get("Items", [])
@@ -131,7 +104,7 @@ def paginate_items_for_patient_pk(patient_pk: str) -> list:
             break
 
     if not all_items:
-        logger.warning("No items found for patient PK: %s", patient_pk)
+        logger.info("No items found for patient PK in VEDS database")
         return []
 
     return all_items
diff --git a/lambdas/id_sync/src/pds_details.py b/lambdas/id_sync/src/pds_details.py
@@ -18,8 +18,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 +29,12 @@ 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:
-    """
-    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
+        raise IdSyncException(message=msg) from e
 
-        return patient_details["identifier"][0]["value"]
 
-    except Exception as e:
-        msg = f"Error getting PDS patient ID for {nhs_number}"
-        logger.exception(msg)
-        raise IdSyncException(message=msg, exception=e)
+def get_nhs_number_from_pds_resource(pds_resource: dict) -> str:
+    """Simple helper to get the NHS Number from a PDS Resource. No handling as this is a mandatory field in the PDS
+    response. Must only use where we have ensured an object has been returned."""
+    return pds_resource["identifier"][0]["value"]
diff --git a/lambdas/id_sync/src/record_processor.py b/lambdas/id_sync/src/record_processor.py
diff --git a/lambdas/id_sync/src/utils.py b/lambdas/id_sync/src/utils.py
diff --git a/lambdas/id_sync/tests/test_id_sync.py b/lambdas/id_sync/tests/test_id_sync.py
diff --git a/lambdas/id_sync/tests/test_ieds_db_operations.py b/lambdas/id_sync/tests/test_ieds_db_operations.py
diff --git a/lambdas/id_sync/tests/test_pds_details.py b/lambdas/id_sync/tests/test_pds_details.py
diff --git a/lambdas/id_sync/tests/test_record_processor.py b/lambdas/id_sync/tests/test_record_processor.py
diff --git a/lambdas/shared/src/common/pds_service.py b/lambdas/shared/src/common/pds_service.py
