Skip to content

Commit 0ef0e8f

Browse files
JamesW1-NHSJamesW1-NHS
committed
revert PK stuff from generic_utils
1 parent ed7ba9a commit 0ef0e8f

File tree

5 files changed

+62
-50
lines changed

5 files changed

+62
-50
lines changed

‎lambdas/backend/src/repository/fhir_repository.py‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@
1919
from common.models.utils.generic_utils import (
2020
get_contained_patient,
2121
get_nhs_number,
22-
make_immunization_pk,
23-
make_patient_pk,
2422
)
2523
from common.models.utils.validation_utils import (
2624
get_vaccine_type,
@@ -38,6 +36,14 @@ def create_table(table_name=None, endpoint_url=None, region_name="eu-west-2"):
3836
return db.Table(table_name)
3937

4038

39+
def _make_immunization_pk(_id: str):
40+
return f"Immunization#{_id}"
41+
42+
43+
def _make_patient_pk(_id: str):
44+
return f"Patient#{_id}"
45+
46+
4147
def _query_identifier(table, index, pk, identifier):
4248
queryresponse = table.query(IndexName=index, KeyConditionExpression=Key(pk).eq(identifier), Limit=1)
4349
if queryresponse.get("Count", 0) > 0:
@@ -58,10 +64,10 @@ class RecordAttributes:
5864
def __init__(self, imms: dict, patient: any):
5965
"""Create attributes that may be used in dynamodb table"""
6066
imms_id = imms["id"]
61-
self.pk = make_immunization_pk(imms_id)
67+
self.pk = _make_immunization_pk(imms_id)
6268
if patient or imms:
6369
nhs_number = get_nhs_number(imms)
64-
self.patient_pk = make_patient_pk(nhs_number)
70+
self.patient_pk = _make_patient_pk(nhs_number)
6571
self.patient = patient
6672
self.resource = imms
6773
self.timestamp = int(time.time())
@@ -99,7 +105,7 @@ def get_immunization_and_resource_meta_by_id(
99105
self, imms_id: str, include_deleted: bool = False
100106
) -> tuple[Optional[dict], Optional[ImmunizationRecordMetadata]]:
101107
"""Retrieves the immunization and resource metadata from the VEDS table"""
102-
response = self.table.get_item(Key={"PK": make_immunization_pk(imms_id)})
108+
response = self.table.get_item(Key={"PK": _make_immunization_pk(imms_id)})
103109
item = response.get("Item")
104110

105111
if not item:
@@ -228,7 +234,7 @@ def _perform_dynamo_update(
228234

229235
try:
230236
self.table.update_item(
231-
Key={"PK": make_immunization_pk(imms_id)},
237+
Key={"PK": _make_immunization_pk(imms_id)},
232238
UpdateExpression=update_exp,
233239
ExpressionAttributeNames={
234240
"#imms_resource": "Resource",
@@ -250,7 +256,7 @@ def delete_immunization(self, imms_id: str, supplier_system: str) -> None:
250256

251257
try:
252258
self.table.update_item(
253-
Key={"PK": make_immunization_pk(imms_id)},
259+
Key={"PK": _make_immunization_pk(imms_id)},
254260
UpdateExpression=(
255261
"SET DeletedAt = :timestamp, Operation = :operation, SupplierSystem = :supplier_system"
256262
),
@@ -260,7 +266,7 @@ def delete_immunization(self, imms_id: str, supplier_system: str) -> None:
260266
":supplier_system": supplier_system,
261267
},
262268
ConditionExpression=(
263-
Attr("PK").eq(make_immunization_pk(imms_id))
269+
Attr("PK").eq(_make_immunization_pk(imms_id))
264270
& (Attr("DeletedAt").not_exists() | Attr("DeletedAt").eq("reinstated"))
265271
),
266272
)
@@ -272,7 +278,7 @@ def delete_immunization(self, imms_id: str, supplier_system: str) -> None:
272278

273279
def find_immunizations(self, patient_identifier: str, vaccine_types: set):
274280
"""it should find all of the specified patient's Immunization events for all of the specified vaccine_types"""
275-
condition = Key("PatientPK").eq(make_patient_pk(patient_identifier))
281+
condition = Key("PatientPK").eq(_make_patient_pk(patient_identifier))
276282
is_not_deleted = Attr("DeletedAt").not_exists() | Attr("DeletedAt").eq("reinstated")
277283

278284
raw_items = self.get_all_items(condition, is_not_deleted)

‎lambdas/backend/tests/repository/test_fhir_repository.py‎

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,21 @@
1010

1111
from common.models.errors import ResourceNotFoundError
1212
from common.models.immunization_record_metadata import ImmunizationRecordMetadata
13-
from common.models.utils.generic_utils import make_immunization_pk, make_patient_pk
1413
from common.models.utils.validation_utils import get_vaccine_type
1514
from models.errors import UnhandledResponseError
1615
from repository.fhir_repository import ImmunizationRepository
1716
from test_common.testing_utils.generic_utils import update_target_disease_code
1817
from test_common.testing_utils.immunization_utils import VALID_NHS_NUMBER, create_covid_immunization_dict
1918

2019

20+
def _make_immunization_pk(_id):
21+
return f"Immunization#{_id}"
22+
23+
24+
def _make_patient_pk(_id):
25+
return f"Patient#{_id}"
26+
27+
2128
class TestFhirRepositoryBase(unittest.TestCase):
2229
"""Base class for all tests to set up common fixtures"""
2330

@@ -151,7 +158,7 @@ def test_get_immunization_by_id(self):
151158
self.assertEqual(resource_meta.resource_version, expected_version)
152159
self.assertEqual(resource_meta.is_deleted, False)
153160
self.assertEqual(resource_meta.is_reinstated, False)
154-
self.table.get_item.assert_called_once_with(Key={"PK": make_immunization_pk(imms_id)})
161+
self.table.get_item.assert_called_once_with(Key={"PK": _make_immunization_pk(imms_id)})
155162

156163
def test_get_immunization_by_id_returns_reinstated_records(self):
157164
"""it should find an Immunization by id, including reinstated records by default"""
@@ -175,7 +182,7 @@ def test_get_immunization_by_id_returns_reinstated_records(self):
175182
self.assertEqual(resource_meta.resource_version, expected_version)
176183
self.assertEqual(resource_meta.is_deleted, False)
177184
self.assertEqual(resource_meta.is_reinstated, True)
178-
self.table.get_item.assert_called_once_with(Key={"PK": make_immunization_pk(imms_id)})
185+
self.table.get_item.assert_called_once_with(Key={"PK": _make_immunization_pk(imms_id)})
179186

180187
def test_get_immunization_by_id_returns_deleted_records_when_flag_is_set(self):
181188
"""it should find an Immunization by id, including deleted records when the include_deleted flag is set True"""
@@ -201,7 +208,7 @@ def test_get_immunization_by_id_returns_deleted_records_when_flag_is_set(self):
201208
self.assertEqual(resource_meta.resource_version, expected_version)
202209
self.assertEqual(resource_meta.is_deleted, True)
203210
self.assertEqual(resource_meta.is_reinstated, False)
204-
self.table.get_item.assert_called_once_with(Key={"PK": make_immunization_pk(imms_id)})
211+
self.table.get_item.assert_called_once_with(Key={"PK": _make_immunization_pk(imms_id)})
205212

206213
def test_immunization_not_found(self):
207214
"""it should return None if Immunization doesn't exist"""
@@ -351,12 +358,12 @@ def test_update_immunisation_is_successful(self):
351358
patient_sk = f"{vaccine_type}#{imms_id}"
352359

353360
self.table.update_item.assert_called_once_with(
354-
Key={"PK": make_immunization_pk(imms_id)},
361+
Key={"PK": _make_immunization_pk(imms_id)},
355362
UpdateExpression=update_exp,
356363
ExpressionAttributeNames={"#imms_resource": "Resource"},
357364
ExpressionAttributeValues={
358365
":timestamp": ANY,
359-
":patient_pk": make_patient_pk(patient_id),
366+
":patient_pk": _make_patient_pk(patient_id),
360367
":patient_sk": patient_sk,
361368
":imms_resource_val": json.dumps(imms),
362369
":operation": "UPDATE",
@@ -390,12 +397,12 @@ def test_update_immunisation_is_successful_when_record_needs_to_be_reinstated(se
390397
patient_sk = f"{vaccine_type}#{imms_id}"
391398

392399
self.table.update_item.assert_called_once_with(
393-
Key={"PK": make_immunization_pk(imms_id)},
400+
Key={"PK": _make_immunization_pk(imms_id)},
394401
UpdateExpression=update_exp,
395402
ExpressionAttributeNames={"#imms_resource": "Resource"},
396403
ExpressionAttributeValues={
397404
":timestamp": ANY,
398-
":patient_pk": make_patient_pk(patient_id),
405+
":patient_pk": _make_patient_pk(patient_id),
399406
":patient_sk": patient_sk,
400407
":imms_resource_val": json.dumps(imms),
401408
":operation": "UPDATE",
@@ -454,7 +461,7 @@ def test_delete_immunization(self):
454461

455462
# Then
456463
self.table.update_item.assert_called_once_with(
457-
Key={"PK": make_immunization_pk(imms_id)},
464+
Key={"PK": _make_immunization_pk(imms_id)},
458465
UpdateExpression="SET DeletedAt = :timestamp, Operation = :operation, SupplierSystem = :supplier_system",
459466
ExpressionAttributeValues={
460467
":timestamp": now_epoch,
@@ -481,7 +488,7 @@ def test_multiple_delete_should_not_update_timestamp(self):
481488
Key=ANY,
482489
UpdateExpression=ANY,
483490
ExpressionAttributeValues=ANY,
484-
ConditionExpression=Attr("PK").eq(make_immunization_pk(imms_id))
491+
ConditionExpression=Attr("PK").eq(_make_immunization_pk(imms_id))
485492
& (Attr("DeletedAt").not_exists() | Attr("DeletedAt").eq("reinstated")),
486493
)
487494

@@ -524,7 +531,7 @@ def test_find_immunizations(self):
524531
dynamo_response = {"ResponseMetadata": {"HTTPStatusCode": 200}, "Items": []}
525532
self.table.query = MagicMock(return_value=dynamo_response)
526533

527-
condition = Key("PatientPK").eq(make_patient_pk(nhs_number))
534+
condition = Key("PatientPK").eq(_make_patient_pk(nhs_number))
528535

529536
# When
530537
_ = self.repository.find_immunizations(nhs_number, vaccine_types={"COVID"})
@@ -656,12 +663,12 @@ def run_update_immunization_test(self, imms_id, imms, updated_dose_quantity=None
656663
patient_sk = f"{vaccine_type}#{imms_id}"
657664

658665
self.table.update_item.assert_called_once_with(
659-
Key={"PK": make_immunization_pk(imms_id)},
666+
Key={"PK": _make_immunization_pk(imms_id)},
660667
UpdateExpression=update_exp,
661668
ExpressionAttributeNames={"#imms_resource": "Resource"},
662669
ExpressionAttributeValues={
663670
":timestamp": ANY,
664-
":patient_pk": make_patient_pk(patient_id),
671+
":patient_pk": _make_patient_pk(patient_id),
665672
":patient_sk": patient_sk,
666673
":imms_resource_val": json.dumps(imms, use_decimal=True),
667674
":operation": "UPDATE",

‎lambdas/recordforwarder/src/repository/fhir_batch_repository.py‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,7 @@
1515
ResourceNotFoundError,
1616
UnhandledResponseError,
1717
)
18-
from common.models.utils.generic_utils import (
19-
get_nhs_number,
20-
make_immunization_pk,
21-
make_patient_pk,
22-
)
18+
from common.models.utils.generic_utils import get_nhs_number
2319

2420

2521
def create_table(region_name="eu-west-2"):
@@ -28,6 +24,14 @@ def create_table(region_name="eu-west-2"):
2824
return dynamodb.Table(table_name)
2925

3026

27+
def _make_immunization_pk(_id: str):
28+
return f"Immunization#{_id}"
29+
30+
31+
def _make_patient_pk(_id: str):
32+
return f"Patient#{_id}"
33+
34+
3135
def _query_identifier(table, index, pk, identifier, is_present):
3236
retries = 0
3337
delay_milliseconds = 60
@@ -68,9 +72,9 @@ class RecordAttributes:
6872
def __init__(self, imms: dict, vax_type: str, supplier: str, version: int):
6973
"""Create attributes that may be used in dynamodb table"""
7074
imms_id = imms["id"]
71-
self.pk = make_immunization_pk(imms_id)
75+
self.pk = _make_immunization_pk(imms_id)
7276
nhs_number = get_nhs_number(imms)
73-
self.patient_pk = make_patient_pk(nhs_number)
77+
self.patient_pk = _make_patient_pk(nhs_number)
7478
self.resource = imms
7579
self.timestamp = int(time.time())
7680
self.vaccine_type = vax_type

‎lambdas/recordforwarder/tests/repository/test_fhir_batch_repository.py‎

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,16 @@
1414
ResourceNotFoundError,
1515
UnhandledResponseError,
1616
)
17-
from common.models.utils.generic_utils import make_immunization_pk
1817
from repository.fhir_batch_repository import ImmunizationBatchRepository, create_table
1918
from test_common.testing_utils.immunization_utils import create_covid_immunization_dict
2019

2120
imms_id = str(uuid4())
2221

2322

23+
def _make_immunization_pk(_id):
24+
return f"Immunization#{_id}"
25+
26+
2427
@mock_aws
2528
class TestImmunizationBatchRepository(unittest.TestCase):
2629
def setUp(self):
@@ -147,7 +150,7 @@ def test_update_immunization(self):
147150
"Count": 1,
148151
"Items": [
149152
{
150-
"PK": make_immunization_pk(imms_id),
153+
"PK": _make_immunization_pk(imms_id),
151154
"Resource": json.dumps(self.immunization),
152155
"Version": 1,
153156
}
@@ -161,7 +164,7 @@ def test_update_immunization(self):
161164
"Count": 1,
162165
"Items": [
163166
{
164-
"PK": make_immunization_pk(imms_id),
167+
"PK": _make_immunization_pk(imms_id),
165168
"Resource": json.dumps(self.immunization),
166169
"Version": 1,
167170
"DeletedAt": "20210101",
@@ -176,7 +179,7 @@ def test_update_immunization(self):
176179
"Count": 1,
177180
"Items": [
178181
{
179-
"PK": make_immunization_pk(imms_id),
182+
"PK": _make_immunization_pk(imms_id),
180183
"Resource": json.dumps(self.immunization),
181184
"Version": 1,
182185
"DeletedAt": "reinstated",
@@ -209,7 +212,7 @@ def test_update_immunization(self):
209212
expected_values.update(case["expected_extra_values"])
210213

211214
self.table.update_item.assert_called_with(
212-
Key={"PK": make_immunization_pk(imms_id)},
215+
Key={"PK": _make_immunization_pk(imms_id)},
213216
UpdateExpression=ANY,
214217
ExpressionAttributeNames={"#imms_resource": "Resource"},
215218
ExpressionAttributeValues=expected_values,
@@ -236,7 +239,7 @@ def test_update_should_catch_dynamo_error(self):
236239
"Count": 1,
237240
"Items": [
238241
{
239-
"PK": make_immunization_pk(imms_id),
242+
"PK": _make_immunization_pk(imms_id),
240243
"Resource": json.dumps(self.immunization),
241244
"Version": 1,
242245
}
@@ -262,7 +265,7 @@ def test_update_immunization_unhandled_error(self):
262265
"Count": 1,
263266
"Items": [
264267
{
265-
"PK": make_immunization_pk(imms_id),
268+
"PK": _make_immunization_pk(imms_id),
266269
"Resource": json.dumps(self.immunization),
267270
"Version": 1,
268271
}
@@ -288,7 +291,7 @@ def test_update_immunization_conditionalcheckfailedexception_error(self):
288291
"Count": 1,
289292
"Items": [
290293
{
291-
"PK": make_immunization_pk(imms_id),
294+
"PK": _make_immunization_pk(imms_id),
292295
"Resource": json.dumps(self.immunization),
293296
"Version": 1,
294297
}
@@ -307,7 +310,7 @@ def test_delete_immunization(self):
307310
"Count": 1,
308311
"Items": [
309312
{
310-
"PK": make_immunization_pk(imms_id),
313+
"PK": _make_immunization_pk(imms_id),
311314
"Resource": json.dumps(self.immunization),
312315
"Version": 1,
313316
}
@@ -319,7 +322,7 @@ def test_delete_immunization(self):
319322
self.immunization, "supplier", "vax-type", self.table, is_present
320323
)
321324
self.table.update_item.assert_called_with(
322-
Key={"PK": make_immunization_pk(imms_id)},
325+
Key={"PK": _make_immunization_pk(imms_id)},
323326
UpdateExpression="SET DeletedAt = :timestamp, Operation = :operation, SupplierSystem = :supplier_system",
324327
ExpressionAttributeValues={
325328
":timestamp": ANY,
@@ -349,7 +352,7 @@ def test_delete_should_catch_dynamo_error(self):
349352
"Count": 1,
350353
"Items": [
351354
{
352-
"PK": make_immunization_pk(imms_id),
355+
"PK": _make_immunization_pk(imms_id),
353356
"Resource": json.dumps(self.immunization),
354357
"Version": 1,
355358
}
@@ -375,7 +378,7 @@ def test_delete_immunization_unhandled_error(self):
375378
"Count": 1,
376379
"Items": [
377380
{
378-
"PK": make_immunization_pk(imms_id),
381+
"PK": _make_immunization_pk(imms_id),
379382
"Resource": json.dumps(self.immunization),
380383
"Version": 1,
381384
}
@@ -401,7 +404,7 @@ def test_delete_immunization_conditionalcheckfailedexception_error(self):
401404
"Count": 1,
402405
"Items": [
403406
{
404-
"PK": make_immunization_pk(imms_id),
407+
"PK": _make_immunization_pk(imms_id),
405408
"Resource": json.dumps(self.immunization),
406409
"Version": 1,
407410
}

‎lambdas/shared/src/common/models/utils/generic_utils.py‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,6 @@
2020
from common.models.constants import Constants
2121

2222

23-
def make_immunization_pk(id: str):
24-
return f"Immunization#{id}"
25-
26-
27-
def make_patient_pk(id: str):
28-
return f"Patient#{id}"
29-
30-
3123
def get_nhs_number(imms: dict):
3224
try:
3325
nhs_number = [x for x in imms["contained"] if x["resourceType"] == "Patient"][0]["identifier"][0]["value"]

0 commit comments

Comments
 (0)