Remove duplicated PDS call

dlzhry2nhs · dlzhry2nhs · commit 4767e9b25113 · 2025-11-04T17:23:00.000Z
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/pds_details.py b/lambdas/id_sync/src/pds_details.py
@@ -3,7 +3,6 @@
 """
 
 import tempfile
-from typing import Optional
 
 from common.authentication import AppRestrictedAuth, Service
 from common.cache import Cache
@@ -35,22 +34,7 @@ def pds_get_patient_details(nhs_number: str) -> dict:
         raise IdSyncException(message=msg, exception=e)
 
 
-# Extract Patient identifier value from PDS patient details
-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 = "Error retrieving patient details from PDS"
-        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
@@ -2,12 +2,13 @@
 from typing import Any, Dict
 
 from common.clients import logger
+from exceptions.id_sync_exception import IdSyncException
 from ieds_db_operations import (
     extract_patient_resource_from_item,
     get_items_from_patient_id,
     ieds_update_patient_id,
 )
-from pds_details import pds_get_patient_details, pds_get_patient_id
+from pds_details import get_nhs_number_from_pds_resource, pds_get_patient_details
 from utils import make_status
 
 
@@ -32,21 +33,26 @@ def process_record(event_record: Dict[str, Any]) -> Dict[str, Any]:
 
 
 def process_nhs_number(nhs_number: str) -> Dict[str, Any]:
-    new_nhs_number = pds_get_patient_id(nhs_number)
+    try:
+        pds_patient_resource = pds_get_patient_details(nhs_number)
+    except IdSyncException as e:
+        return make_status(str(e), status="error")
 
-    if not new_nhs_number:
+    if not pds_patient_resource:
         return make_status("No patient ID found for NHS number")
 
+    new_nhs_number = get_nhs_number_from_pds_resource(pds_patient_resource)
+
     if new_nhs_number == nhs_number:
         return make_status("No update required")
 
     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)
+        # Fetch the IEDS resources for the old NHS number
+        ieds_resources = fetch_ieds_resources(nhs_number)
     except Exception as e:
-        logger.exception("process_nhs_number: failed to fetch demographic details: %s", e)
+        logger.exception("process_nhs_number: failed to fetch ieds resources: %s", e)
         return make_status(str(e), status="error")
 
     logger.info(
@@ -78,21 +84,15 @@ def process_nhs_number(nhs_number: str) -> Dict[str, Any]:
     return response
 
 
-# Function to fetch PDS Patient details and IEDS Immunisation records.
-def fetch_pds_and_ieds_resources(nhs_number: str):
-    try:
-        pds = pds_get_patient_details(nhs_number)
-    except Exception as e:
-        logger.exception("fetch_pds_and_ieds_resources: failed to fetch PDS details")
-        raise RuntimeError("Failed to fetch PDS details") from e
-
+def fetch_ieds_resources(nhs_number: str) -> list[dict]:
+    """Retrieves IEDS records by patient NHS Number"""
     try:
-        ieds = get_items_from_patient_id(nhs_number)
+        ieds_records = get_items_from_patient_id(nhs_number)
     except Exception as e:
         logger.exception("fetch_pds_and_ieds_resources: failed to fetch IEDS items")
         raise RuntimeError("Failed to fetch IEDS items") from e
 
-    return pds, ieds
+    return ieds_records
 
 
 def extract_normalized_name_from_patient(patient: dict) -> str | None:
diff --git a/lambdas/id_sync/tests/test_pds_details.py b/lambdas/id_sync/tests/test_pds_details.py
@@ -2,7 +2,7 @@
 from unittest.mock import MagicMock, patch
 
 from exceptions.id_sync_exception import IdSyncException
-from pds_details import pds_get_patient_details, pds_get_patient_id
+from pds_details import get_nhs_number_from_pds_resource, pds_get_patient_details
 
 
 class TestGetPdsPatientDetails(unittest.TestCase):
@@ -214,53 +214,17 @@ def test_pds_get_patient_details(self):
         self.assertEqual(result, mock_pds_response)
         self.mock_pds_service_instance.get_patient_details.assert_called_once_with(test_nhs_number)
 
-
-# test pds_get_patient_id function
-class TestGetPdsPatientId(unittest.TestCase):
-    def setUp(self):
-        """Set up test fixtures and mocks"""
-        self.test_nhs_number = "9912003888"
-
-        # Patch all external dependencies
-        self.logger_patcher = patch("pds_details.logger")
-        self.mock_logger = self.logger_patcher.start()
-
-        self.pds_get_patient_details_patcher = patch("pds_details.pds_get_patient_details")
-        self.mock_pds_get_patient_details = self.pds_get_patient_details_patcher.start()
-
-    def tearDown(self):
-        """Clean up patches"""
-        patch.stopall()
-
-    def test_pds_get_patient_id_success(self):
-        """Test successful retrieval of PDS patient ID"""
-        # Arrange
-        expected_id = "1234567890"
-        self.mock_pds_get_patient_details.return_value = {"identifier": [{"value": expected_id}]}
-
-        # Act
-        result = pds_get_patient_id(self.test_nhs_number)
-
-        # Assert
-        self.assertEqual(result, expected_id)
-
-    def test_pds_get_patient_id_empty_identifier_array(self):
-        """Test when identifier array is empty"""
-        # Arrange
-        patient_data_empty_identifier = {
-            "identifier": [],  # Empty array
-            "name": "John Doe",
+    def test_get_nhs_number_from_pds_resource(self):
+        """Test that the NHS Number is retrieved from a full PDS patient resource."""
+        mock_pds_resource = {
+            "identifier": [
+                {
+                    "system": "https://fhir.nhs.uk/Id/nhs-number",
+                    "value": "123456789012",
+                }
+            ]
         }
-        self.mock_pds_get_patient_details.return_value = patient_data_empty_identifier
 
-        # Act
-        with self.assertRaises(IdSyncException) as context:
-            pds_get_patient_id(self.test_nhs_number)
+        result = get_nhs_number_from_pds_resource(mock_pds_resource)
 
-        # Assert
-        exception = context.exception
-        self.assertEqual(
-            exception.message,
-            "Error retrieving patient details from PDS",
-        )
-        self.assertEqual(exception.nhs_numbers, None)
+        self.assertEqual(result, "123456789012")
diff --git a/lambdas/id_sync/tests/test_record_processor.py b/lambdas/id_sync/tests/test_record_processor.py
