Skip to content

Commit e3807bd

Browse files
JamesW1-NHSJamesW1-NHS
authored
VED-850 Redundant code (#956)
* remove unused CsvImmunisationErrorModel * remove redundant UpdateOutcome * remove redundant UpdateOutcome II * dummy commit for github actions
1 parent 27a0f1e commit e3807bd

File tree

5 files changed

+24
-37
lines changed

5 files changed

+24
-37
lines changed

‎backend/src/controller/fhir_controller.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
from models.utils.generic_utils import check_keys_in_sources
3333
from parameter_parser import create_query_string, process_params, process_search_params
3434
from repository.fhir_repository import ImmunizationRepository, create_table
35-
from service.fhir_service import FhirService, UpdateOutcome, get_service_url
35+
from service.fhir_service import FhirService, get_service_url
3636

3737
IMMUNIZATION_ENV = os.getenv("IMMUNIZATION_ENV")
3838

@@ -297,7 +297,7 @@ def update_immunization(self, aws_event):
297297
diagnostics=resource["diagnostics"],
298298
)
299299
return create_response(400, json.dumps(exp_error))
300-
if outcome == UpdateOutcome.UPDATE:
300+
if outcome:
301301
return create_response(
302302
200, None, {"E-Tag": updated_version}
303303
) # include e-tag here, is it not included in the response resource

‎backend/src/models/failures.py‎

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

‎backend/src/service/fhir_service.py‎

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
import logging
33
import os
44
import uuid
5-
from enum import Enum
65
from typing import Optional, Union, cast
76
from uuid import uuid4
87

@@ -70,11 +69,6 @@ def get_service_url(
7069
return f"https://{subdomain}api.service.nhs.uk/{service_base_path}"
7170

7271

73-
class UpdateOutcome(Enum):
74-
UPDATE = 0
75-
CREATE = 1
76-
77-
7872
class FhirService:
7973
def __init__(
8074
self,
@@ -168,13 +162,13 @@ def update_immunization(
168162
existing_resource_version: int,
169163
existing_resource_vacc_type: str,
170164
supplier_system: str,
171-
) -> tuple[Optional[UpdateOutcome], Immunization | dict, Optional[int]]:
165+
) -> tuple[bool, Immunization | dict, Optional[int]]:
172166
# VED-747 - TODO - this and the below 2 methods are duplicated. We should streamline the update journey
173167
immunization["id"] = imms_id
174168

175169
patient = self._validate_patient(immunization)
176170
if "diagnostics" in patient:
177-
return None, patient, None
171+
return False, patient, None
178172

179173
vaccination_type = get_vaccine_type(immunization)
180174

@@ -191,7 +185,7 @@ def update_immunization(
191185
imms_id, immunization, patient, existing_resource_version, supplier_system
192186
)
193187

194-
return UpdateOutcome.UPDATE, Immunization.parse_obj(imms), updated_version
188+
return True, Immunization.parse_obj(imms), updated_version
195189

196190
def reinstate_immunization(
197191
self,
@@ -200,11 +194,11 @@ def reinstate_immunization(
200194
existing_resource_version: int,
201195
existing_resource_vacc_type: str,
202196
supplier_system: str,
203-
) -> tuple[Optional[UpdateOutcome], Immunization | dict, Optional[int]]:
197+
) -> tuple[bool, Immunization | dict, Optional[int]]:
204198
immunization["id"] = imms_id
205199
patient = self._validate_patient(immunization)
206200
if "diagnostics" in patient:
207-
return None, patient, None
201+
return False, patient, None
208202

209203
vaccination_type = get_vaccine_type(immunization)
210204

@@ -219,7 +213,7 @@ def reinstate_immunization(
219213
imms_id, immunization, patient, existing_resource_version, supplier_system
220214
)
221215

222-
return UpdateOutcome.UPDATE, Immunization.parse_obj(imms), updated_version
216+
return True, Immunization.parse_obj(imms), updated_version
223217

224218
def update_reinstated_immunization(
225219
self,
@@ -228,11 +222,11 @@ def update_reinstated_immunization(
228222
existing_resource_version: int,
229223
existing_resource_vacc_type: str,
230224
supplier_system: str,
231-
) -> tuple[Optional[UpdateOutcome], Immunization | dict, Optional[int]]:
225+
) -> tuple[bool, Immunization | dict, Optional[int]]:
232226
immunization["id"] = imms_id
233227
patient = self._validate_patient(immunization)
234228
if "diagnostics" in patient:
235-
return None, patient, None
229+
return False, patient, None
236230

237231
vaccination_type = get_vaccine_type(immunization)
238232

@@ -251,7 +245,7 @@ def update_reinstated_immunization(
251245
supplier_system,
252246
)
253247

254-
return UpdateOutcome.UPDATE, Immunization.parse_obj(imms), updated_version
248+
return True, Immunization.parse_obj(imms), updated_version
255249

256250
def delete_immunization(self, imms_id: str, supplier_system: str) -> None:
257251
"""

‎backend/tests/controller/test_fhir_controller.py‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
)
2323
from parameter_parser import patient_identifier_system, process_search_params
2424
from repository.fhir_repository import ImmunizationRepository
25-
from service.fhir_service import FhirService, UpdateOutcome
25+
from service.fhir_service import FhirService
2626
from testing_utils.generic_utils import load_json_data
2727
from testing_utils.immunization_utils import create_covid_immunization
2828

@@ -951,7 +951,7 @@ def test_update_immunization(self):
951951
"pathParameters": {"id": imms_id},
952952
}
953953
self.service.update_immunization.return_value = (
954-
UpdateOutcome.UPDATE,
954+
True,
955955
"value doesn't matter",
956956
2,
957957
)
@@ -1055,7 +1055,7 @@ def test_update_immunization_for_batch_existing_record_is_none(self):
10551055
"pathParameters": {"id": imms_id},
10561056
}
10571057
self.service.update_immunization.return_value = (
1058-
UpdateOutcome.UPDATE,
1058+
True,
10591059
"value doesn't matter",
10601060
)
10611061
self.service.get_immunization_by_id_all.return_value = None
@@ -1104,7 +1104,7 @@ def test_update_deletedat_immunization_with_version(self):
11041104
"body": imms,
11051105
"pathParameters": {"id": imms_id},
11061106
}
1107-
self.service.reinstate_immunization.return_value = UpdateOutcome.UPDATE, {}, 2
1107+
self.service.reinstate_immunization.return_value = True, {}, 2
11081108
self.service.get_immunization_by_id_all.return_value = {
11091109
"resource": "new_value",
11101110
"Version": 1,
@@ -1128,7 +1128,7 @@ def test_update_deletedat_immunization_without_version(self):
11281128
"body": imms,
11291129
"pathParameters": {"id": imms_id},
11301130
}
1131-
self.service.reinstate_immunization.return_value = UpdateOutcome.UPDATE, {}, 2
1131+
self.service.reinstate_immunization.return_value = True, {}, 2
11321132
self.service.get_immunization_by_id_all.return_value = {
11331133
"resource": "new_value",
11341134
"Version": 1,
@@ -1311,7 +1311,7 @@ def test_update_immunization_when_reinstated_true(self):
13111311
"pathParameters": {"id": imms_id},
13121312
}
13131313
self.service.update_reinstated_immunization.return_value = (
1314-
UpdateOutcome.UPDATE,
1314+
True,
13151315
{},
13161316
3,
13171317
)
@@ -1359,7 +1359,7 @@ def test_update_reinstated_immunization_with_diagnostics_error(self):
13591359
"VaccineType": "COVID",
13601360
}
13611361
self.service.reinstate_immunization.return_value = (
1362-
None,
1362+
False,
13631363
{"diagnostics": "Patient NHS number has been superseded"},
13641364
None,
13651365
)

‎backend/tests/service/test_fhir_service.py‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
from models.fhir_immunization import ImmunizationValidator
2727
from models.utils.generic_utils import get_contained_patient
2828
from repository.fhir_repository import ImmunizationRepository
29-
from service.fhir_service import FhirService, UpdateOutcome, get_service_url
29+
from service.fhir_service import FhirService, get_service_url
3030
from testing_utils.generic_utils import load_json_data
3131
from testing_utils.immunization_utils import (
3232
VALID_NHS_NUMBER,
@@ -634,7 +634,7 @@ def test_update_immunization(self):
634634
outcome, _, _ = self.fhir_service.update_immunization(imms_id, req_imms, 1, "COVID", "Test")
635635

636636
# Then
637-
self.assertEqual(outcome, UpdateOutcome.UPDATE)
637+
self.assertTrue(outcome)
638638
self.imms_repo.update_immunization.assert_called_once_with(imms_id, req_imms, req_patient, 1, "Test")
639639
self.authoriser.authorise.assert_called_once_with("Test", ApiOperationCode.UPDATE, {"COVID"})
640640

@@ -696,7 +696,7 @@ def test_reinstate_immunization_returns_updated_version(self):
696696

697697
outcome, resource, version = self.fhir_service.reinstate_immunization(imms_id, req_imms, 1, "COVID", "Test")
698698

699-
self.assertEqual(outcome, UpdateOutcome.UPDATE)
699+
self.assertTrue(outcome)
700700
self.assertEqual(version, 5)
701701

702702
def test_reinstate_immunization_raises_exception_when_missing_authz(self):
@@ -724,7 +724,7 @@ def test_update_reinstated_immunization_returns_updated_version(self):
724724
imms_id, req_imms, 1, "COVID", "Test"
725725
)
726726

727-
self.assertEqual(outcome, UpdateOutcome.UPDATE)
727+
self.assertTrue(outcome)
728728
self.assertEqual(version, 9)
729729

730730
def test_reinstate_immunization_with_diagnostics(self):
@@ -735,7 +735,7 @@ def test_reinstate_immunization_with_diagnostics(self):
735735

736736
outcome, resource, version = self.fhir_service.reinstate_immunization(imms_id, req_imms, 1, "COVID", "Test")
737737

738-
self.assertIsNone(outcome)
738+
self.assertFalse(outcome)
739739
self.assertEqual(resource, {"diagnostics": "invalid patient"})
740740
self.assertIsNone(version)
741741
self.imms_repo.reinstate_immunization.assert_not_called()
@@ -750,7 +750,7 @@ def test_update_reinstated_immunization_with_diagnostics(self):
750750
imms_id, req_imms, 1, "COVID", "Test"
751751
)
752752

753-
self.assertIsNone(outcome)
753+
self.assertFalse(outcome)
754754
self.assertEqual(resource, {"diagnostics": "invalid patient"})
755755
self.assertIsNone(version)
756756
self.imms_repo.update_reinstated_immunization.assert_not_called()

0 commit comments

Comments
 (0)