Refactored update journey

Author: dlzhry2nhs

backend/src/fhir_controller.py

@@ -268,7 +268,6 @@ def update_immunization(self, aws_event):
                     imms_id,
                     imms,
                     existing_resource_version,
-                    imms_vax_type_perms,
                     supplier_system
                 )
             # Validate if the imms resource to be updated is a logically deleted resource-end
@@ -323,15 +322,13 @@ def update_immunization(self, aws_event):
                         imms_id,
                         imms,
                         existing_resource_version,
-                        imms_vax_type_perms,
                         supplier_system
                     )
                 else:
                     outcome, resource, updated_version = self.fhir_service.update_immunization(
                         imms_id,
                         imms,
                         existing_resource_version,
-                        imms_vax_type_perms,
                         supplier_system
                     )
 

backend/src/fhir_repository.py

@@ -183,11 +183,9 @@ def update_immunization(
         immunization: dict,
         patient: any,
         existing_resource_version: int,
-        imms_vax_type_perms: list[str],
         supplier_system: str,
     ) -> tuple[dict, int]:
         attr = RecordAttributes(immunization, patient)
-        self._handle_permissions(imms_vax_type_perms, attr)
         update_exp = self._build_update_expression(is_reinstate=False)
 
         self._check_duplicate_identifier(attr)
@@ -208,12 +206,9 @@ def reinstate_immunization(
         immunization: dict,
         patient: any,
         existing_resource_version: int,
-        imms_vax_type_perms: list[str],
         supplier_system: str,
-
     ) -> tuple[dict, int]:
         attr = RecordAttributes(immunization, patient)
-        self._handle_permissions(imms_vax_type_perms, attr)
         update_exp = self._build_update_expression(is_reinstate=True)
 
         self._check_duplicate_identifier(attr)
@@ -234,11 +229,9 @@ def update_reinstated_immunization(
         immunization: dict,
         patient: any,
         existing_resource_version: int,
-        imms_vax_type_perms: list[str],
         supplier_system: str,
     ) -> tuple[dict, int]:
         attr = RecordAttributes(immunization, patient)
-        self._handle_permissions(imms_vax_type_perms, attr)
         update_exp = self._build_update_expression(is_reinstate=False)
 
         self._check_duplicate_identifier(attr)

backend/src/fhir_service.py

@@ -136,20 +136,25 @@ def update_immunization(
         imms_id: str,
         immunization: dict,
         existing_resource_version: int,
-        imms_vax_type_perms: list[str],
         supplier_system: str,
-    ) -> tuple[UpdateOutcome, Immunization, int]:
+    ) -> tuple[Optional[UpdateOutcome], Immunization | dict, Optional[int]]:
+        # TODO - raise ticket to refactor this and below 3. Should have one update method and call repo based on type
         immunization["id"] = imms_id
 
         patient = self._validate_patient(immunization)
         if "diagnostics" in patient:
-            return (None, patient, None)
+            return None, patient, None
+
+        vaccination_type = get_vaccine_type(immunization)
+
+        if not self.authoriser.authorise(supplier_system, ApiOperationCode.UPDATE, {vaccination_type}):
+            raise UnauthorizedVaxError()
+
         imms, updated_version = self.immunization_repo.update_immunization(
             imms_id,
             immunization,
             patient,
             existing_resource_version,
-            imms_vax_type_perms,
             supplier_system
         )
 
@@ -160,20 +165,24 @@ def reinstate_immunization(
         imms_id: str,
         immunization: dict,
         existing_resource_version: int,
-        imms_vax_type_perms: list[str],
         supplier_system: str,
-    ) -> tuple[UpdateOutcome, Immunization, int]:
+    ) -> tuple[Optional[UpdateOutcome], Immunization | dict, Optional[int]]:
         immunization["id"] = imms_id
         patient = self._validate_patient(immunization)
         if "diagnostics" in patient:
-            return (None, patient, None)
+            return None, patient, None
+
+        vaccination_type = get_vaccine_type(immunization)
+
+        if not self.authoriser.authorise(supplier_system, ApiOperationCode.UPDATE, {vaccination_type}):
+            raise UnauthorizedVaxError()
+
         imms, updated_version = self.immunization_repo.reinstate_immunization(
             imms_id,
             immunization,
             patient,
             existing_resource_version,
-            imms_vax_type_perms,
-            supplier_system,
+            supplier_system
         )
 
         return UpdateOutcome.UPDATE, Immunization.parse_obj(imms), updated_version
@@ -183,19 +192,23 @@ def update_reinstated_immunization(
         imms_id: str,
         immunization: dict,
         existing_resource_version: int,
-        imms_vax_type_perms: list[str],
         supplier_system: str,
-    ) -> tuple[UpdateOutcome, Immunization, int]:
+    ) -> tuple[Optional[UpdateOutcome], Immunization | dict, Optional[int]]:
         immunization["id"] = imms_id
         patient = self._validate_patient(immunization)
         if "diagnostics" in patient:
-            return (None, patient, None)
+            return None, patient, None
+
+        vaccination_type = get_vaccine_type(immunization)
+
+        if not self.authoriser.authorise(supplier_system, ApiOperationCode.UPDATE, {vaccination_type}):
+            raise UnauthorizedVaxError()
+
         imms, updated_version = self.immunization_repo.update_reinstated_immunization(
             imms_id,
             immunization,
             patient,
             existing_resource_version,
-            imms_vax_type_perms,
             supplier_system,
         )
 

backend/tests/test_fhir_controller.py

@@ -1073,7 +1073,7 @@ def test_update_immunization(self,mock_get_permissions):
         response = self.controller.update_immunization(aws_event)
 
         self.service.update_immunization.assert_called_once_with(
-            imms_id, json.loads(imms), 1, ["COVID19.CRUD"], "Test"
+            imms_id, json.loads(imms), 1, "Test"
         )
         mock_get_permissions.assert_called_once_with("Test")
         self.assertEqual(response["statusCode"], 200)
@@ -1270,7 +1270,7 @@ def test_update_deletedat_immunization_with_version(self, mock_get_supplier_perm
         response = self.controller.update_immunization(aws_event)
 
         self.service.reinstate_immunization.assert_called_once_with(
-            imms_id, json.loads(imms), 1, ["COVID19.CRUDS"], "Test"
+            imms_id, json.loads(imms), 1, "Test"
         )
         self.assertEqual(response["statusCode"], 200)
         self.assertEqual(response["headers"]["E-Tag"], 2)
@@ -1298,7 +1298,7 @@ def test_update_deletedat_immunization_without_version(self, mock_get_supplier_p
         response = self.controller.update_immunization(aws_event)
 
         self.service.reinstate_immunization.assert_called_once_with(
-            imms_id, json.loads(imms), 1, ["COVID19.CRUD"], "Test"
+            imms_id, json.loads(imms), 1, "Test"
         )
         mock_get_supplier_permissions.assert_called_once_with("Test")
         self.assertEqual(response["statusCode"], 200)
@@ -1515,7 +1515,7 @@ def test_update_immunization_when_reinstated_true(self, mock_get_permissions):
         response = self.controller.update_immunization(aws_event)
 
         self.service.update_reinstated_immunization.assert_called_once_with(
-            imms_id, json.loads(imms), 1, ["COVID19.CRUD"], "Test"
+            imms_id, json.loads(imms), 1, "Test"
         )
         self.assertEqual(response["statusCode"], 200)
         self.assertEqual(response["headers"]["E-Tag"], int("3"))

backend/tests/test_fhir_repository.py

@@ -320,7 +320,6 @@ def setUp(self):
     def test_update1(self):
         """it should update record by replacing both Immunization and Patient"""
 
-        self.mock_redis_client.hget.return_value = "COVID19"
         imms_id = "an-imms-id"
         imms = create_covid_19_immunization_dict(imms_id)
         imms["patient"] = self.patient
@@ -339,7 +338,7 @@ def test_update1(self):
             # When
 
             act_resource, updated_version = self.repository.update_immunization(
-                imms_id, imms, self.patient, 1, ["COVID19.CRUD"], "Test"
+                imms_id, imms, self.patient, 1, "Test"
             )
 
         # Then
@@ -375,7 +374,6 @@ def test_update1(self):
     def test_update_throws_error_when_response_can_not_be_handled(self):
         """it should throw UnhandledResponse when the response from dynamodb can't be handled"""
 
-        self.mock_redis_client.hget.return_value = "COVID19"
         imms_id = "an-id"
         imms = create_covid_19_immunization_dict(imms_id)
         imms["patient"] = self.patient
@@ -388,15 +386,14 @@ def test_update_throws_error_when_response_can_not_be_handled(self):
         with self.assertRaises(UnhandledResponseError) as e:
             # When
 
-            self.repository.update_immunization(imms_id, imms, self.patient, 1, ["COVID19.CRUD"], "Test")
+            self.repository.update_immunization(imms_id, imms, self.patient, 1, "Test")
 
         # Then
         self.assertDictEqual(e.exception.response, response)
 
     def test_update_throws_error_when_identifier_already_in_dynamodb(self):
         """it should throw IdentifierDuplicationError when trying to update an immunization with an identfier that is already stored"""
 
-        self.mock_redis_client.hget.return_value = "COVID19"
         imms_id = "an-id"
         imms = create_covid_19_immunization_dict(imms_id)
         imms["patient"] = self.patient
@@ -406,13 +403,12 @@ def test_update_throws_error_when_identifier_already_in_dynamodb(self):
         with self.assertRaises(IdentifierDuplicationError) as e:
             # When
 
-            self.repository.update_immunization(imms_id, imms, self.patient, 1, ["COVID19.CRUD"], "Test")
+            self.repository.update_immunization(imms_id, imms, self.patient, 1, "Test")
 
         self.assertEqual(str(e.exception), f"The provided identifier: {identifier} is duplicated")
 
     def test_reinstate_immunization_success(self):
         """it should reinstate an immunization successfully"""
-        self.mock_redis_client.hget.return_value = "COVID19"
         imms_id = "reinstate-id"
         imms = create_covid_19_immunization_dict(imms_id)
         imms["patient"] = self.patient
@@ -426,15 +422,14 @@ def test_reinstate_immunization_success(self):
 
         with patch("time.time", return_value=123456):
             result, version = self.repository.reinstate_immunization(
-                imms_id, imms, self.patient, 1, ["COVID19.CRUD"], "Test"
+                imms_id, imms, self.patient, 1, "Test"
             )
 
         self.assertEqual(result, resource)
         self.assertEqual(version, 2)
 
     def test_update_reinstated_immunization_success(self):
         """it should update a reinstated immunization successfully"""
-        self.mock_redis_client.hget.return_value = "COVID19"
         imms_id = "reinstated-id"
         imms = create_covid_19_immunization_dict(imms_id)
         imms["patient"] = self.patient
@@ -448,7 +443,7 @@ def test_update_reinstated_immunization_success(self):
 
         with patch("time.time", return_value=123456):
             result, version = self.repository.update_reinstated_immunization(
-                imms_id, imms, self.patient, 1, ["COVID19.CRUD"], "Test"
+                imms_id, imms, self.patient, 1, "Test"
             )
 
         self.assertEqual(result, resource)
@@ -761,7 +756,7 @@ def run_update_immunization_test(self, imms_id, imms, resource, updated_dose_qua
         with patch("time.time") as mock_time:
             mock_time.return_value = now_epoch
             act_resource, act_version = self.repository.update_immunization(
-                imms_id, imms, self.patient, 1, ["COVID19.CRUD"], "Test"
+                imms_id, imms, self.patient, 1, "Test"
             )
         self.assertDictEqual(act_resource, resource)
         self.assertEqual(act_version, 2)
@@ -801,7 +796,6 @@ def run_update_immunization_test(self, imms_id, imms, resource, updated_dose_qua
 
     def test_decimal_on_update(self):
         """it should update record when replacing doseQuantity and keep decimal precision"""
-        self.mock_redis_client.hget.return_value = "COVID19"
         imms_id = "an-imms-id"
         imms = create_covid_19_immunization_dict(imms_id)
         imms["doseQuantity"] = 1.5556
@@ -813,7 +807,6 @@ def test_decimal_on_update(self):
 
     def test_decimal_on_update_patient(self):
         """it should update record by replacing both Immunization and Patient and dosequantity"""
-        self.mock_redis_client.hget.return_value = "COVID19"
         imms_id = "an-imms-id"
         imms = create_covid_19_immunization_dict(imms_id)
         imms["doseQuantity"] = 1.590