Addition for the update journey - missed during refactor

Author: dlzhry2nhs

backend/src/fhir_controller.py

@@ -209,7 +209,7 @@ def update_immunization(self, aws_event):
                 )
                 return self.create_response(404, json.dumps(exp_error))
 
-            if "diagnostics" in existing_record and existing_record is not None:
+            if "diagnostics" in existing_record:
                 exp_error = create_operation_outcome(
                     resource_id=str(uuid.uuid4()),
                     severity=Severity.error,
@@ -222,14 +222,16 @@ def update_immunization(self, aws_event):
         # Validate if the imms resource does not exist - end
 
         existing_resource_version = int(existing_record["Version"])
+        existing_resource_vacc_type = existing_record["VaccineType"]
 
         try:
             # Validate if the imms resource to be updated is a logically deleted resource - start
-            if existing_record["DeletedAt"] == True:
+            if existing_record["DeletedAt"]:
                 outcome, resource, updated_version = self.fhir_service.reinstate_immunization(
                     imms_id,
                     imms,
                     existing_resource_version,
+                    existing_resource_vacc_type,
                     supplier_system
                 )
             # Validate if the imms resource to be updated is a logically deleted resource-end
@@ -284,13 +286,15 @@ def update_immunization(self, aws_event):
                         imms_id,
                         imms,
                         existing_resource_version,
+                        existing_resource_vacc_type,
                         supplier_system
                     )
                 else:
                     outcome, resource, updated_version = self.fhir_service.update_immunization(
                         imms_id,
                         imms,
                         existing_resource_version,
+                        existing_resource_vacc_type,
                         supplier_system
                     )
 

backend/src/fhir_service.py

@@ -139,6 +139,7 @@ def update_immunization(
         imms_id: str,
         immunization: dict,
         existing_resource_version: int,
+        existing_resource_vacc_type: str,
         supplier_system: str,
     ) -> 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
@@ -150,7 +151,9 @@ def update_immunization(
 
         vaccination_type = get_vaccine_type(immunization)
 
-        if not self.authoriser.authorise(supplier_system, ApiOperationCode.UPDATE, {vaccination_type}):
+        # If the user is updating the vaccination_type, they must have permissions for both the new and old type
+        if not self.authoriser.authorise(supplier_system, ApiOperationCode.UPDATE,
+                                         {vaccination_type, existing_resource_vacc_type}):
             raise UnauthorizedVaxError()
 
         imms, updated_version = self.immunization_repo.update_immunization(
@@ -168,6 +171,7 @@ def reinstate_immunization(
         imms_id: str,
         immunization: dict,
         existing_resource_version: int,
+        existing_resource_vacc_type: str,
         supplier_system: str,
     ) -> tuple[Optional[UpdateOutcome], Immunization | dict, Optional[int]]:
         immunization["id"] = imms_id
@@ -177,7 +181,8 @@ def reinstate_immunization(
 
         vaccination_type = get_vaccine_type(immunization)
 
-        if not self.authoriser.authorise(supplier_system, ApiOperationCode.UPDATE, {vaccination_type}):
+        if not self.authoriser.authorise(supplier_system, ApiOperationCode.UPDATE,
+                                         {vaccination_type, existing_resource_vacc_type}):
             raise UnauthorizedVaxError()
 
         imms, updated_version = self.immunization_repo.reinstate_immunization(
@@ -195,6 +200,7 @@ def update_reinstated_immunization(
         imms_id: str,
         immunization: dict,
         existing_resource_version: int,
+        existing_resource_vacc_type: str,
         supplier_system: str,
     ) -> tuple[Optional[UpdateOutcome], Immunization | dict, Optional[int]]:
         immunization["id"] = imms_id
@@ -204,7 +210,8 @@ def update_reinstated_immunization(
 
         vaccination_type = get_vaccine_type(immunization)
 
-        if not self.authoriser.authorise(supplier_system, ApiOperationCode.UPDATE, {vaccination_type}):
+        if not self.authoriser.authorise(supplier_system, ApiOperationCode.UPDATE,
+                                         {vaccination_type, existing_resource_vacc_type}):
             raise UnauthorizedVaxError()
 
         imms, updated_version = self.immunization_repo.update_reinstated_immunization(

backend/tests/test_fhir_controller.py

@@ -889,7 +889,7 @@ def test_update_immunization(self):
         response = self.controller.update_immunization(aws_event)
 
         self.service.update_immunization.assert_called_once_with(
-            imms_id, json.loads(imms), 1, "Test"
+            imms_id, json.loads(imms), 1, "COVID19", "Test"
         )
         self.assertEqual(response["statusCode"], 200)
         self.assertEqual(response["headers"]["E-Tag"], 2)
@@ -1041,7 +1041,7 @@ def test_update_deletedat_immunization_with_version(self):
         response = self.controller.update_immunization(aws_event)
 
         self.service.reinstate_immunization.assert_called_once_with(
-            imms_id, json.loads(imms), 1, "Test"
+            imms_id, json.loads(imms), 1, "COVID19", "Test"
         )
         self.assertEqual(response["statusCode"], 200)
         self.assertEqual(response["headers"]["E-Tag"], 2)
@@ -1067,7 +1067,7 @@ def test_update_deletedat_immunization_without_version(self):
         response = self.controller.update_immunization(aws_event)
 
         self.service.reinstate_immunization.assert_called_once_with(
-            imms_id, json.loads(imms), 1, "Test"
+            imms_id, json.loads(imms), 1, "COVID19", "Test"
         )
         self.assertEqual(response["statusCode"], 200)
         self.assertEqual(response["headers"]["E-Tag"],  2)
@@ -1269,7 +1269,7 @@ def test_update_immunization_when_reinstated_true(self):
         response = self.controller.update_immunization(aws_event)
 
         self.service.update_reinstated_immunization.assert_called_once_with(
-            imms_id, json.loads(imms), 1, "Test"
+            imms_id, json.loads(imms), 1, "COVID19", "Test"
         )
         self.assertEqual(response["statusCode"], 200)
         self.assertEqual(response["headers"]["E-Tag"], int("3"))

backend/tests/test_fhir_service.py

@@ -564,18 +564,20 @@ def test_update_immunization(self):
         self.imms_repo.update_immunization.return_value = (
             create_covid_19_immunization_dict(imms_id), 2
         )
+        self.mock_redis_client.hget.return_value = "COVID19"
         self.authoriser.authorise.return_value = True
 
         nhs_number = VALID_NHS_NUMBER
         req_imms = create_covid_19_immunization_dict(imms_id, nhs_number)
         req_patient = get_contained_patient(req_imms)
 
         # When
-        outcome, _, _ = self.fhir_service.update_immunization(imms_id, req_imms, 1, "Test")
+        outcome, _, _ = self.fhir_service.update_immunization(imms_id, req_imms, 1, "COVID19", "Test")
 
         # Then
         self.assertEqual(outcome, UpdateOutcome.UPDATE)
         self.imms_repo.update_immunization.assert_called_once_with(imms_id, req_imms, req_patient, 1, "Test")
+        self.authoriser.authorise.assert_called_once_with("Test", ApiOperationCode.UPDATE, {"COVID19"})
 
     def test_id_not_present(self):
         """it should populate id in the message if it is not present"""
@@ -587,7 +589,7 @@ def test_id_not_present(self):
         del req_imms["id"]
 
         # When
-        self.fhir_service.update_immunization(req_imms_id, req_imms, 1, "Test")
+        self.fhir_service.update_immunization(req_imms_id, req_imms, 1, "COVID19", "Test")
 
         # Then
         passed_imms = self.imms_repo.update_immunization.call_args.args[1]
@@ -601,7 +603,7 @@ def test_patient_error(self):
 
         with self.assertRaises(InvalidPatientId) as e:
             # When
-            self.fhir_service.update_immunization(imms_id, bad_patient_imms, 1, "Test")
+            self.fhir_service.update_immunization(imms_id, bad_patient_imms, 1, "COVID19", "Test")
 
         # Then
         self.assertEqual(e.exception.patient_identifier, invalid_nhs_number)
@@ -615,7 +617,7 @@ def test_patient_error_invalid_nhs_number(self):
 
         with self.assertRaises(InvalidPatientId) as e:
             # When
-            self.fhir_service.update_immunization(imms_id, bad_patient_imms, 1, "Test")
+            self.fhir_service.update_immunization(imms_id, bad_patient_imms, 1, "COVID19", "Test")
 
         # Then
         self.assertEqual(e.exception.patient_identifier, invalid_nhs_number)
@@ -631,7 +633,7 @@ def test_reinstate_immunization_returns_updated_version(self):
         self.imms_repo.reinstate_immunization.return_value = (req_imms, 5)
 
         outcome, resource, version = self.fhir_service.reinstate_immunization(
-            imms_id, req_imms, 1, "Test"
+            imms_id, req_imms, 1, "COVID19", "Test"
         )
 
         self.assertEqual(outcome, UpdateOutcome.UPDATE)
@@ -646,7 +648,7 @@ def test_reinstate_immunization_raises_exception_when_missing_authz(self):
         self.mock_redis_client.hget.return_value = "FLU"
 
         with self.assertRaises(UnauthorizedVaxError):
-            self.fhir_service.reinstate_immunization(imms_id, req_imms, 1, "Test")
+            self.fhir_service.reinstate_immunization(imms_id, req_imms, 1, "FLU", "Test")
 
         self.authoriser.authorise.assert_called_once_with("Test", ApiOperationCode.UPDATE, {"FLU"})
 
@@ -659,7 +661,7 @@ def test_update_reinstated_immunization_returns_updated_version(self):
         self.imms_repo.update_reinstated_immunization.return_value = (req_imms, 9)
 
         outcome, resource, version = self.fhir_service.update_reinstated_immunization(
-            imms_id, req_imms, 1, "Test"
+            imms_id, req_imms, 1, "COVID19", "Test"
         )
 
         self.assertEqual(outcome, UpdateOutcome.UPDATE)
@@ -672,7 +674,7 @@ def test_reinstate_immunization_with_diagnostics(self):
         self.fhir_service._validate_patient = MagicMock(return_value={"diagnostics": "invalid patient"})
 
         outcome, resource, version = self.fhir_service.reinstate_immunization(
-            imms_id, req_imms, 1, "Test"
+            imms_id, req_imms, 1, "COVID19", "Test"
         )
 
         self.assertIsNone(outcome)
@@ -687,7 +689,7 @@ def test_update_reinstated_immunization_with_diagnostics(self):
         self.fhir_service._validate_patient = MagicMock(return_value={"diagnostics": "invalid patient"})
 
         outcome, resource, version = self.fhir_service.update_reinstated_immunization(
-            imms_id, req_imms, 1, "Test"
+            imms_id, req_imms, 1, "COVID19", "Test"
         )
 
         self.assertIsNone(outcome)