Skip to content

Commit 1941dba

Browse files
committed
VED-765: review based on changes
1 parent 8523b24 commit 1941dba

File tree

5 files changed

+89
-94
lines changed

5 files changed

+89
-94
lines changed

lambdas/id_sync/src/ieds_db_operations.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
from os_vars import get_ieds_table_name
33
from common.aws_dynamodb import get_dynamodb_table
44
from common.clients import logger, dynamodb_client
5-
from utils import log_status, BATCH_SIZE
5+
from utils import make_status, BATCH_SIZE
66
from exceptions.id_sync_exception import IdSyncException
77

88
ieds_table = None
@@ -21,10 +21,10 @@ def ieds_update_patient_id(old_id: str, new_id: str, items_to_update: list | Non
2121
"""Update the patient ID in the IEDS table."""
2222
logger.info(f"ieds_update_patient_id. Update patient ID from {old_id} to {new_id}")
2323
if not old_id or not new_id or not old_id.strip() or not new_id.strip():
24-
return log_status("Old ID and New ID cannot be empty", old_id, "error")
24+
return make_status("Old ID and New ID cannot be empty", old_id, "error")
2525

2626
if old_id == new_id:
27-
return log_status(f"No change in patient ID: {old_id}", old_id)
27+
return make_status(f"No change in patient ID: {old_id}", old_id)
2828

2929
try:
3030
logger.info(f"Updating patient ID in IEDS from {old_id} to {new_id}")
@@ -37,7 +37,7 @@ def ieds_update_patient_id(old_id: str, new_id: str, items_to_update: list | Non
3737

3838
if not items_to_update:
3939
logger.warning(f"No items found to update for patient ID: {old_id}")
40-
return log_status(f"No items found to update for patient ID: {old_id}", old_id)
40+
return make_status(f"No items found to update for patient ID: {old_id}", old_id)
4141

4242
logger.info(f"Items to update: {len(items_to_update)}")
4343

@@ -52,12 +52,12 @@ def ieds_update_patient_id(old_id: str, new_id: str, items_to_update: list | Non
5252
f"All batches complete. Total batches: {total_batches}, All successful: {all_batches_successful}")
5353

5454
if all_batches_successful:
55-
return log_status(
55+
return make_status(
5656
f"IEDS update, patient ID: {old_id}=>{new_id}. {len(items_to_update)} updated {total_batches}.",
5757
old_id,
5858
)
5959
else:
60-
return log_status(f"Failed to update some batches for patient ID: {old_id}", old_id, "error")
60+
return make_status(f"Failed to update some batches for patient ID: {old_id}", old_id, "error")
6161

6262
except Exception as e:
6363
logger.exception("Error updating patient ID")

lambdas/id_sync/src/record_processor.py

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
extract_patient_resource_from_item,
77
get_items_from_patient_id,
88
)
9-
from utils import log_status
9+
from utils import make_status
1010
import json
1111
import ast
1212

@@ -43,33 +43,35 @@ def process_nhs_number(nhs_number: str) -> Dict[str, Any]:
4343
new_nhs_number = pds_get_patient_id(nhs_number)
4444

4545
if not new_nhs_number:
46-
return log_status("No patient ID found for NHS number", nhs_number)
46+
return make_status("No patient ID found for NHS number", nhs_number)
4747

4848
if new_nhs_number == nhs_number:
49-
return log_status("No update required", nhs_number)
49+
return make_status("No update required", nhs_number)
50+
5051
logger.info("Update patient ID from %s to %s", nhs_number, new_nhs_number)
5152

5253
try:
53-
pds_details, ieds_details = fetch_demographic_details(nhs_number)
54+
# Fetch PDS Patient resource and IEDS resources for the old NHS number
55+
pds_patient_resource, ieds_resources = fetch_pds_and_ieds_resources(nhs_number)
5456
except Exception as e:
5557
logger.exception("process_nhs_number: failed to fetch demographic details: %s", e)
56-
return log_status(str(e), nhs_number, "error")
58+
return make_status(str(e), nhs_number, "error")
5759

58-
if not ieds_details:
60+
if not ieds_resources:
5961
logger.info("No IEDS records returned for NHS number: %s", nhs_number)
60-
return log_status(f"No records returned for ID: {nhs_number}", nhs_number)
62+
return make_status(f"No records returned for ID: {nhs_number}", nhs_number)
6163

6264
# Compare demographics from PDS to each IEDS item, keep only matching records
6365
matching_records, discarded_records = [], []
64-
for detail in ieds_details:
65-
if demographics_match(pds_details, detail):
66+
for detail in ieds_resources:
67+
if demographics_match(pds_patient_resource, detail):
6668
matching_records.append(detail)
6769
else:
6870
discarded_records.append(detail)
6971

7072
if not matching_records:
7173
logger.info("No records matched PDS demographics: %d", len(discarded_records))
72-
return log_status("No records matched PDS demographics; update skipped", nhs_number)
74+
return make_status("No records matched PDS demographics; update skipped", nhs_number)
7375

7476
response = ieds_update_patient_id(
7577
nhs_number, new_nhs_number, items_to_update=matching_records
@@ -81,7 +83,8 @@ def process_nhs_number(nhs_number: str) -> Dict[str, Any]:
8183
return response
8284

8385

84-
def fetch_demographic_details(nhs_number: str):
86+
# Function to fetch PDS Patient details and IEDS Immunisation records
87+
def fetch_pds_and_ieds_resources(nhs_number: str):
8588
try:
8689
pds = pds_get_patient_details(nhs_number)
8790
except Exception as e:

lambdas/id_sync/src/utils.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
from typing import Dict, Any
22

33

4-
def log_status(msg: str, nhs_number: str | None = None, status: str = "success") -> Dict[str, Any]:
4+
def make_status(msg: str, nhs_number: str | None = None, status: str = "success") -> Dict[str, Any]:
55
"""Return a simple status dict used by record processing for observability.
66
77
If `nhs_number` is None the key is omitted which keeps the output shape

lambdas/id_sync/tests/test_ieds_db_operations.py

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -539,3 +539,71 @@ def test_get_items_from_patient_id_no_records(self):
539539

540540
# Assert
541541
self.assertEqual(result, [])
542+
543+
544+
class TestIedsDbOperationsConditional(unittest.TestCase):
545+
def setUp(self):
546+
# Patch logger to suppress output
547+
self.logger_patcher = patch('ieds_db_operations.logger')
548+
self.mock_logger = self.logger_patcher.start()
549+
550+
# Patch get_ieds_table_name and get_ieds_table
551+
self.get_ieds_table_name_patcher = patch('ieds_db_operations.get_ieds_table_name')
552+
self.mock_get_ieds_table_name = self.get_ieds_table_name_patcher.start()
553+
self.mock_get_ieds_table_name.return_value = 'test-table'
554+
555+
self.get_ieds_table_patcher = patch('ieds_db_operations.get_ieds_table')
556+
self.mock_get_ieds_table = self.get_ieds_table_patcher.start()
557+
558+
# Patch dynamodb client
559+
self.dynamodb_client_patcher = patch('ieds_db_operations.dynamodb_client')
560+
self.mock_dynamodb_client = self.dynamodb_client_patcher.start()
561+
562+
def tearDown(self):
563+
patch.stopall()
564+
565+
def test_ieds_update_patient_id_empty_inputs(self):
566+
res = ieds_db_operations.ieds_update_patient_id('', '')
567+
self.assertEqual(res['status'], 'error')
568+
569+
def test_ieds_update_patient_id_same_ids(self):
570+
res = ieds_db_operations.ieds_update_patient_id('a', 'a')
571+
self.assertEqual(res['status'], 'success')
572+
573+
def test_ieds_update_with_items_to_update_uses_provided_list(self):
574+
items = [{'PK': 'Patient#1'}, {'PK': 'Patient#1#r2'}]
575+
# patch transact_write_items to return success
576+
self.mock_dynamodb_client.transact_write_items = MagicMock(
577+
return_value={'ResponseMetadata': {'HTTPStatusCode': 200}})
578+
579+
res = ieds_db_operations.ieds_update_patient_id('1', '2', items_to_update=items)
580+
self.assertEqual(res['status'], 'success')
581+
# ensure transact called at least once
582+
self.mock_dynamodb_client.transact_write_items.assert_called()
583+
584+
def test_ieds_update_batches_multiple_calls(self):
585+
# create 60 items to force 3 batches (25,25,10)
586+
items = [{'PK': f'Patient#old#{i}'} for i in range(60)]
587+
called = []
588+
589+
def fake_transact(TransactItems):
590+
called.append(len(TransactItems))
591+
return {'ResponseMetadata': {'HTTPStatusCode': 200}}
592+
593+
self.mock_dynamodb_client.transact_write_items = MagicMock(side_effect=fake_transact)
594+
595+
res = ieds_db_operations.ieds_update_patient_id('old', 'new', items_to_update=items)
596+
self.assertEqual(res['status'], 'success')
597+
# should have been called 3 times
598+
self.assertEqual(len(called), 3)
599+
self.assertEqual(called[0], 25)
600+
self.assertEqual(called[1], 25)
601+
self.assertEqual(called[2], 10)
602+
603+
def test_ieds_update_non_200_response(self):
604+
items = [{'PK': 'Patient#1'}]
605+
self.mock_dynamodb_client.transact_write_items = MagicMock(
606+
return_value={'ResponseMetadata': {'HTTPStatusCode': 500}})
607+
608+
res = ieds_db_operations.ieds_update_patient_id('1', '2', items_to_update=items)
609+
self.assertEqual(res['status'], 'error')

lambdas/id_sync/tests/test_ieds_db_operations_conditional.py

Lines changed: 0 additions & 76 deletions
This file was deleted.

0 commit comments

Comments
 (0)