VED-813-Fix NHS-Number-Change (#847)

Fix Bug NHS Number Change

Author: Akol125

lambdas/id_sync/src/id_sync.py

@@ -24,11 +24,21 @@ def handler(event_data: Dict[str, Any], _context) -> Dict[str, Any]:
 
         logger.info("id_sync processing event with %d records", len(records))
 
-        results = [process_record(record) for record in records]
-        nhs_numbers = [result["nhs_number"] for result in results]
-        error_count = sum(1 for result in results if result.get("status") == "error")
+        results = []
+        nhs_numbers = []
+        error_count = 0
 
-        if error_count:
+        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)
 

lambdas/id_sync/src/ieds_db_operations.py

@@ -2,6 +2,7 @@
 from os_vars import get_ieds_table_name
 from common.aws_dynamodb import get_dynamodb_table
 from common.clients import logger, dynamodb_client
+import json
 from utils import make_status
 from exceptions.id_sync_exception import IdSyncException
 
@@ -19,7 +20,7 @@ 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 in the IEDS table."""
+    """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")
@@ -133,13 +134,23 @@ def paginate_items_for_patient_pk(patient_pk: str) -> list:
 
 def extract_patient_resource_from_item(item: dict) -> dict | None:
     """
-    Extract a Patient resource dict from an IEDS database.
+    Extract a Patient resource from an IEDS database.
     """
     patient_resource = item.get("Resource", None)
+
+    if isinstance(patient_resource, str):
+        try:
+            patient_resource_parsed = json.loads(patient_resource)
+        except json.JSONDecodeError:
+            logger.error("Failed to decode patient_resource JSON string")
+            return None
+        patient_resource = patient_resource_parsed
+
     if not isinstance(patient_resource, dict):
         return None
 
-    for response in patient_resource.get("contained", []):
+    contained = patient_resource.get("contained") or []
+    for response in contained:
         if isinstance(response, dict) and response.get("resourceType") == "Patient":
             return response
 

lambdas/id_sync/src/pds_details.py

@@ -43,7 +43,6 @@ def pds_get_patient_id(nhs_number: str) -> str:
     :return: PDS patient ID
     """
     try:
-        logger.info(f"get_pds_patient_id. nhs_number: {nhs_number}")
         patient_details = pds_get_patient_details(nhs_number)
         if not patient_details:
             return None

lambdas/id_sync/src/record_processor.py

@@ -57,6 +57,8 @@ def process_nhs_number(nhs_number: str) -> Dict[str, Any]:
         logger.exception("process_nhs_number: failed to fetch demographic details: %s", e)
         return make_status(str(e), nhs_number, "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)
@@ -65,6 +67,7 @@ def process_nhs_number(nhs_number: str) -> Dict[str, Any]:
     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:
@@ -84,16 +87,21 @@ def process_nhs_number(nhs_number: str) -> Dict[str, Any]:
     return response
 
 
-# Function to fetch PDS Patient details and IEDS Immunisation records
+# 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)
         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)
         raise RuntimeError("Failed to fetch IEDS items") from e
+
     return pds, ieds
 
 
@@ -132,11 +140,13 @@ def normalize_strings(item: Any) -> str | None:
         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)
 
         # Retrieve patient resource from IEDS item
         patient = extract_patient_resource_from_item(ieds_item)
         if not patient:
-            logger.debug("demographics_match: no patient resource in IEDS table item")
+            logger.info("demographics_match: no patient resource in IEDS table item")
             return False
 
         # normalize patient fields from IEDS

lambdas/id_sync/tests/test_id_sync.py

@@ -235,29 +235,30 @@ def test_handler_process_record_exception(self):
         self.assertEqual(exception.message, "Error processing id_sync event")
 
     def test_handler_process_record_missing_nhs_number(self):
-        """Test handler when process_record returns incomplete data"""
+        """Test handler when process_record returns error and missing NHS number"""
+
         # Setup mocks
         mock_event = MagicMock()
         mock_event.records = [MagicMock()]
         self.mock_aws_lambda_event.return_value = mock_event
 
-        # Missing "nhs_number" in response
+        # Return result without 'nhs_number' but with an 'error' status
         self.mock_process_record.return_value = {
-            "status": "success"
-            # Missing "nhs_number"
+            "status": "error",
+            "message": "Missing NHS number"
+            # No 'nhs_number'
         }
 
-        # Call handler
+        # Call handler and expect exception
         with self.assertRaises(IdSyncException) as exception_context:
             handler(self.single_sqs_event, None)
 
         exception = exception_context.exception
 
-        # convert exception payload to json
         self.assertIsInstance(exception, IdSyncException)
-        self.assertEqual(exception.nhs_numbers, None)
-        self.assertEqual(exception.message, "Error processing id_sync event")
-        self.mock_logger.exception.assert_called_once_with("Error processing id_sync event")
+        self.assertEqual(exception.nhs_numbers, [])
+        self.assertEqual(exception.message, "Processed 1 records with 1 errors")
+        self.mock_logger.exception.assert_called_once_with(f"id_sync error: {exception.message}")
 
     def test_handler_context_parameter_ignored(self):
         """Test that context parameter is properly ignored"""

lambdas/id_sync/tests/test_ieds_db_operations.py

@@ -1,10 +1,51 @@
 import unittest
+
+from ieds_db_operations import extract_patient_resource_from_item
 from unittest.mock import patch, MagicMock
 from exceptions.id_sync_exception import IdSyncException
-
 import ieds_db_operations
 
 
+class TestExtractPatientResourceFromItem(unittest.TestCase):
+
+    def test_extract_from_dict_with_contained_patient(self):
+        item = {
+            "Resource": {
+                "resourceType": "Immunization",
+                "contained": [
+                    {"resourceType": "Patient", "id": "P1", "name": [{"family": "Doe"}]}
+                ],
+            }
+        }
+
+        patient = extract_patient_resource_from_item(item)
+        self.assertIsNotNone(patient)
+        self.assertIsInstance(patient, dict)
+        self.assertEqual(patient.get("resourceType"), "Patient")
+        self.assertEqual(patient.get("id"), "P1")
+
+    def test_extract_from_json_string(self):
+        resource_json = '{"resourceType": "Immunization", "contained": [{"resourceType": "Patient", "id": "P2"}]}'
+        item = {"Resource": resource_json}
+
+        patient = extract_patient_resource_from_item(item)
+        self.assertIsNotNone(patient)
+        self.assertEqual(patient.get("id"), "P2")
+
+    def test_malformed_json_string_returns_none(self):
+        # A malformed JSON string should not raise, but return None
+        item = {"Resource": "{not: valid json}"}
+        self.assertIsNone(extract_patient_resource_from_item(item))
+
+    def test_non_dict_resource_returns_none(self):
+        item = {"Resource": 12345}
+        self.assertIsNone(extract_patient_resource_from_item(item))
+
+    def test_missing_resource_returns_none(self):
+        item = {}
+        self.assertIsNone(extract_patient_resource_from_item(item))
+
+
 class TestIedsDbOperations(unittest.TestCase):
     """Base test class for IEDS database operations"""
 

lambdas/shared/src/common/clients.py

@@ -2,9 +2,9 @@
 import logging
 from boto3 import client as boto3_client, resource as boto3_resource
 
-logging.basicConfig(level="INFO")
+logging.basicConfig(level=logging.INFO)
 logger = logging.getLogger()
-logger.setLevel("INFO")
+logger.setLevel(logging.INFO)
 
 STREAM_NAME = os.getenv("SPLUNK_FIREHOSE_NAME", "firehose-name-not-defined")
 CONFIG_BUCKET_NAME = os.getenv("CONFIG_BUCKET_NAME", "variconfig-bucketable-not-defined")

lambdas/shared/tests/test_common/test_clients.py

@@ -1,63 +1,61 @@
 import unittest
-from unittest.mock import patch
-import common.clients as clients
+from unittest.mock import patch, MagicMock
+import logging
 import importlib
+import common.clients as clients
 
 
 class TestClients(unittest.TestCase):
-
     BUCKET_NAME = "default-bucket"
     AWS_REGION = "eu-west-2"
 
     def setUp(self):
-        self.boto3_client_patch = patch("boto3.client")
+        # Patch boto3.client
+        self.boto3_client_patch = patch("boto3.client", autospec=True)
         self.mock_boto3_client = self.boto3_client_patch.start()
-        self.logging_patch = patch("logging.getLogger")
-        self.mock_logging = self.logging_patch.start()
-        self.logger_info_patcher = patch("logging.Logger.info")
-        self.mock_logger_info = self.logger_info_patcher.start()
-        self.getenv_patch = patch("os.getenv")
+        self.addCleanup(self.boto3_client_patch.stop)
+
+        # Patch logging.getLogger
+        self.logging_patch = patch("logging.getLogger", autospec=True)
+        self.mock_getLogger = self.logging_patch.start()
+        self.addCleanup(self.logging_patch.stop)
+
+        # Patch os.getenv
+        self.getenv_patch = patch("os.getenv", autospec=True)
         self.mock_getenv = self.getenv_patch.start()
+        self.addCleanup(self.getenv_patch.stop)
+
+        # Set environment variable mock return values
         self.mock_getenv.side_effect = lambda key, default=None: {
             "CONFIG_BUCKET_NAME": self.BUCKET_NAME,
             "AWS_REGION": self.AWS_REGION,
         }.get(key, default)
 
-        self.mock_boto3_client.return_value = self.mock_boto3_client
-        self.mock_boto3_client.return_value.send_message = {}
-
-    def tearDown(self):
-        patch.stopall()
+        # Simulate logger instance and patch setLevel
+        self.mock_logger_instance = MagicMock()
+        self.mock_getLogger.return_value = self.mock_logger_instance
 
-    def test_os_environ(self):
-        # Test if environment variables are set correctly
+        # Reload the module under test to apply patches
         importlib.reload(clients)
+
+    def test_env_variables_loaded(self):
+        """Test that environment variables are loaded correctly"""
         self.assertEqual(clients.CONFIG_BUCKET_NAME, self.BUCKET_NAME)
         self.assertEqual(clients.REGION_NAME, self.AWS_REGION)
 
-    def test_boto3_client(self):
-        ''' Test boto3 client is created with correct parameters '''
-        importlib.reload(clients)
+    def test_boto3_client_created_for_s3(self):
+        """Test that S3 boto3 client is created with correct region"""
         self.mock_boto3_client.assert_any_call("s3", region_name=self.AWS_REGION)
 
-    def test_firehose_client(self):
-        ''' Test firehose client is created with correct parameters '''
-        importlib.reload(clients)
+    def test_boto3_client_created_for_firehose(self):
+        """Test that Firehose boto3 client is created with correct region"""
         self.mock_boto3_client.assert_any_call("firehose", region_name=self.AWS_REGION)
 
-    def test_logging_setup(self):
-        ''' Test logging is set up correctly '''
-        importlib.reload(clients)
-        self.assertTrue(hasattr(clients, 'logger'))
+    def test_logger_is_initialized(self):
+        """Test that a logger instance is initialized"""
+        self.mock_getLogger.assert_called_once_with()
+        self.assertTrue(hasattr(clients, "logger"))
 
-    def test_logging_configuration(self):
-        ''' Test logging configuration '''
-        importlib.reload(clients)
-        clients.logger.setLevel.assert_called_once_with("INFO")
-
-    def test_logging_initialization(self):
-        ''' Test logging initialization '''
-        importlib.reload(clients)
-        self.mock_logging.assert_called_once_with()
-        self.assertTrue(hasattr(clients, 'logger'))
-        clients.logger.setLevel.assert_any_call("INFO")
+    def test_logger_set_level(self):
+        """Test that logger level is set to INFO"""
+        self.mock_logger_instance.setLevel.assert_called_once_with(logging.INFO)