Merge branch 'master' into VED-382-backend-configurable-disease-mapping

# Conflicts:
#	backend/poetry.lock
#	backend/src/fhir_controller.py
#	backend/src/fhir_repository.py
#	backend/src/models/utils/permission_checker.py
#	backend/tests/test_fhir_controller.py
#	backend/tests/test_fhir_service.py
#	terraform/endpoints.tf

Author: nhsdevws

README.md

@@ -39,8 +39,10 @@ See https://nhsd-confluence.digital.nhs.uk/display/APM/Glossary.
 | Folder                | Description |
 |------------------------|-------------|
 | `infra`                | Base infrastructure components. |
+| `infra_old`            | Old infra code used to create INT to mimic prod. |
 | `grafana`              | Terraform configuration for Grafana, built on top of core infra. |
 | `terraform`            | Core Terraform infrastructure code. This is run in each PR and sets up lambdas associated with the PR.|
+| `terraform_old`        | Old tf code used to create INT to mimic prod. |
 | `terraform_sandbox`    | Sandbox environment for testing infrastructure changes. |
 | `terraform_aws_backup` | Streamlined backup processing with AWS. |
 | `mesh-infra`           | Infrastructure setup for Imms batch MESH integration. |

backend/poetry.lock

@@ -431,9 +431,6 @@ def search_immunizations(self, aws_event: APIGatewayProxyEventV1) -> dict:
                 for vaccine_type in search_params.immunization_targets
                 if ApiOperationCode.SEARCH in expanded_permissions.get(vaccine_type.lower(), [])
             ]
-            # vax_type_perms = _expand_permissions(imms_vax_type_perms, ApiOperationCode.SEARCH)
-            # vax_type_perm = [ vaccine_type for vaccine_type in search_params.immunization_targets 
-            #                  if f"{vaccine_type.lower()}.{ApiOperationCode.SEARCH}" in vax_type_perms ]
             if not vax_type_perm:
                 raise UnauthorizedVaxError
         except UnauthorizedVaxError as unauthorized:
@@ -670,5 +667,5 @@ def create_response(status_code, body=None, headers=None):
     def _identify_supplier_system(aws_event):
         supplier_system = aws_event["headers"]["SupplierSystem"]
         if not supplier_system:
-            raise UnauthorizedSystemError("SupplierSystem header is missing or empty.")
+            return self.create_response(403, unauthorized.to_operation_outcome())
         return supplier_system

backend/src/fhir_controller.py

@@ -106,28 +106,23 @@ def get_immunization_by_identifier(
 
     def get_immunization_by_id(self, imms_id: str, imms_vax_type_perms: str) -> Optional[dict]:
         response = self.table.get_item(Key={"PK": _make_immunization_pk(imms_id)})
+        item = response.get("Item")
 
-        if "Item" in response:
-            resp = dict()
-            if "DeletedAt" in response["Item"]:
-                if response["Item"]["DeletedAt"] == "reinstated":
-                    vaccine_type = self._vaccine_type(response["Item"]["PatientSK"])
-                    if not validate_permissions(imms_vax_type_perms,ApiOperationCode.READ, [vaccine_type]):
-                        raise UnauthorizedVaxError()
-                    resp["Resource"] = json.loads(response["Item"]["Resource"])
-                    resp["Version"] = response["Item"]["Version"]
-                    return resp
-                else:
-                    return None
-            else:
-                vaccine_type = self._vaccine_type(response["Item"]["PatientSK"])
-                if not validate_permissions(imms_vax_type_perms,ApiOperationCode.READ, [vaccine_type]):
-                    raise UnauthorizedVaxError()
-                resp["Resource"] = json.loads(response["Item"]["Resource"])
-                resp["Version"] = response["Item"]["Version"]
-                return resp
-        else:
+        if not item:
             return None
+        if item.get("DeletedAt") and item["DeletedAt"] != "reinstated":
+            return None
+
+        # Get vaccine type + validate permissions
+        vaccine_type = self._vaccine_type(item["PatientSK"])
+        if not validate_permissions(imms_vax_type_perms, ApiOperationCode.READ, [vaccine_type]):
+            raise UnauthorizedVaxError()
+
+        # Build response
+        return {
+            "Resource": json.loads(item["Resource"]),
+            "Version": item["Version"]
+        }
 
     def get_immunization_by_id_all(self, imms_id: str, imms: dict) -> Optional[dict]:
         response = self.table.get_item(Key={"PK": _make_immunization_pk(imms_id)})
@@ -355,36 +350,39 @@ def _perform_dynamo_update(
                 )
 
     def delete_immunization(
-        self, imms_id: str, imms_vax_type_perms: str, supplier_system: str
-    ) -> dict:
+            self, imms_id: str, imms_vax_type_perms: str, supplier_system: str) -> dict:
         now_timestamp = int(time.time())
+        
         try:
-            resp = self.table.get_item(Key={"PK": _make_immunization_pk(imms_id)})
-
-            if "Item" in resp:
-                if "DeletedAt" in resp["Item"]:
-                    if resp["Item"]["DeletedAt"] == "reinstated":
-                        pass
-                vaccine_type = self._vaccine_type(resp["Item"]["PatientSK"])
+            item = self.table.get_item(Key={"PK": _make_immunization_pk(imms_id)}).get("Item")
+            if not item:
+                raise ResourceNotFoundError(resource_type="Immunization", resource_id=imms_id)
+            
+            if not item.get("DeletedAt") or item.get("DeletedAt") == "reinstated":
+                vaccine_type = self._vaccine_type(item["PatientSK"])
                 if not validate_permissions(imms_vax_type_perms, ApiOperationCode.DELETE, [vaccine_type]):
                     raise UnauthorizedVaxError()
 
+            # Proceed with delete update
             response = self.table.update_item(
                 Key={"PK": _make_immunization_pk(imms_id)},
-                UpdateExpression="SET DeletedAt = :timestamp, Operation = :operation, SupplierSystem = :supplier_system",
+                UpdateExpression=(
+                    "SET DeletedAt = :timestamp, Operation = :operation, SupplierSystem = :supplier_system"
+                ),
                 ExpressionAttributeValues={
                     ":timestamp": now_timestamp,
                     ":operation": "DELETE",
                     ":supplier_system": supplier_system,
                 },
                 ReturnValues="ALL_NEW",
-                ConditionExpression=Attr("PK").eq(_make_immunization_pk(imms_id))
-                & (Attr("DeletedAt").not_exists() | Attr("DeletedAt").eq("reinstated")),
+                ConditionExpression=(
+                    Attr("PK").eq(_make_immunization_pk(imms_id)) &
+                    (Attr("DeletedAt").not_exists() | Attr("DeletedAt").eq("reinstated"))
+                ),
             )
+            
             return self._handle_dynamo_response(response)
-
         except botocore.exceptions.ClientError as error:
-            # Either resource didn't exist or it has already been deleted. See ConditionExpression in the request
             if error.response["Error"]["Code"] == "ConditionalCheckFailedException":
                 raise ResourceNotFoundError(resource_type="Immunization", resource_id=imms_id)
             else:

backend/src/fhir_repository.py

@@ -23,7 +23,7 @@ def _expand_permissions(permissions: list[str]) -> dict[str, list[ApiOperationCo
 def validate_permissions(permissions: list[str], operation: ApiOperationCode, vaccine_types: list[str]):
     expanded_permissions = _expand_permissions(permissions)
     print(f"operation: {operation}, expanded_permissions: {expanded_permissions}, vaccine_types: {vaccine_types}")
-    return all([
+    return all(
         operation in expanded_permissions.get(vaccine_type.lower(), [])
         for vaccine_type in vaccine_types
-    ])
+    )

backend/src/models/utils/permission_checker.py

@@ -32,35 +32,17 @@
 from parameter_parser import patient_identifier_system, process_search_params
 from tests.utils.generic_utils import load_json_data
 from tests.utils.values_for_tests import ValidValues
-from utils.mock_redis import MOCK_REDIS_V2D_RESPONSE
 
-class TestFhirControllerBase(unittest.TestCase):
-    """Base class for all tests to set up common fixtures"""
+"test"
 
+
+class TestFhirController(unittest.TestCase):
     def setUp(self):
-        super().setUp()
-        self.redis_patcher = patch("parameter_parser.redis_client")
-        self.mock_redis_client = self.redis_patcher.start()
-        self.logger_info_patcher = patch("logging.Logger.info")
-        self.mock_logger_info = self.logger_info_patcher.start()
-
-    def tearDown(self):
-        self.redis_patcher.stop()
-        self.logger_info_patcher.stop()
-        super().tearDown()
-
-class TestFhirController(TestFhirControllerBase):
-    def setUp(self):
-        super().setUp()
         self.service = create_autospec(FhirService)
         self.repository = create_autospec(ImmunizationRepository)
         self.authorizer = create_autospec(Authorization)
         self.controller = FhirController(self.authorizer, self.service)
 
-    def tearDown(self):
-        self.redis_patcher.stop()
-        super().tearDown()
-
     def test_create_response(self):
         """it should return application/fhir+json with correct status code"""
         body = {"message": "a body"}
@@ -151,7 +133,7 @@ def test_get_imms_by_identifer_header_missing(self):
         response = self.controller.get_immunization_by_identifier(lambda_event)
 
         self.assertEqual(response["statusCode"], 403)
-
+    
     @patch("fhir_controller.get_supplier_permissions")
     def test_not_found_for_identifier(self, mock_get_permissions):
         """it should return not-found OperationOutcome if it doesn't exist"""
@@ -183,7 +165,7 @@ def test_not_found_for_identifier(self, mock_get_permissions):
 
         imms = identifier.replace("|", "#")
         # When
-
+        
         response = self.controller.get_immunization_by_identifier(lambda_event)
 
         # Then
@@ -218,7 +200,7 @@ def test_get_imms_by_identifer_patient_identifier_and_element_present(self, mock
         self.assertEqual(response["statusCode"], 400)
         body = json.loads(response["body"])
         self.assertEqual(body["resourceType"], "OperationOutcome")
-
+    
     @patch("fhir_controller.get_supplier_permissions")
     def test_get_imms_by_identifer_both_body_and_query_params_present(self, mock_get_supplier_permissions):
         """it should return Immunization Id if it exists"""
@@ -440,7 +422,7 @@ def test_validate_immunization_identifier_having_whitespace(self,mock_get_suppli
         self.assertEqual(response["statusCode"], 400)
         outcome = json.loads(response["body"])
         self.assertEqual(outcome["resourceType"], "OperationOutcome")
-
+    
     @patch("fhir_controller.get_supplier_permissions")
     def test_validate_imms_id_invalid_vaccinetype(self, mock_get_permissions):
         """it should validate lambda's Immunization id"""
@@ -756,7 +738,7 @@ def test_validate_immunization_identifier_having_whitespace(self,mock_get_suppli
         self.assertEqual(response["statusCode"], 400)
         outcome = json.loads(response["body"])
         self.assertEqual(outcome["resourceType"], "OperationOutcome")
-
+   
     @patch("fhir_controller.get_supplier_permissions")
     def test_validate_imms_id_invalid_vaccinetype(self, mock_get_permissions):
         """it should validate lambda's Immunization id"""
@@ -838,7 +820,7 @@ def test_get_imms_by_id_unauthorised_vax_error(self,mock_permissions):
         # Then
         mock_permissions.assert_called_once_with("test")
         self.assertEqual(response["statusCode"], 403)
-
+    
     @patch("fhir_controller.get_supplier_permissions")
     def test_get_imms_by_id_no_vax_permission(self, mock_permissions):
         """it should return Immunization Id if it exists"""
@@ -1160,7 +1142,7 @@ def test_update_immunization_UnauthorizedVaxError(self, mock_get_supplier_permis
         response = self.controller.update_immunization(aws_event)
         mock_get_supplier_permissions.assert_called_once_with("Test")
         self.assertEqual(response["statusCode"], 403)
-
+    
     @patch("fhir_controller.get_supplier_permissions")
     def test_update_immunization_UnauthorizedVaxError_check_for_non_batch(self, mock_get_supplier_permissions):
         """it should not update the Immunization record"""
@@ -1591,7 +1573,7 @@ def test_immunization_exception_not_found(self, mock_get_permissions):
         body = json.loads(response["body"])
         self.assertEqual(body["resourceType"], "OperationOutcome")
         self.assertEqual(body["issue"][0]["code"], "not-found")
-
+    
     @patch("fhir_controller.get_supplier_permissions")
     def test_immunization_unhandled_error(self, mock_get_supplier_permissions):
         """it should return server-error OperationOutcome if service throws UnhandledResponseError"""
@@ -1614,9 +1596,8 @@ def test_immunization_unhandled_error(self, mock_get_supplier_permissions):
         self.assertEqual(body["resourceType"], "OperationOutcome")
         self.assertEqual(body["issue"][0]["code"], "exception")
 
-class TestSearchImmunizations(TestFhirControllerBase):
+class TestSearchImmunizations(unittest.TestCase):
     def setUp(self):
-        super().setUp()
         self.service = create_autospec(FhirService)
         self.authorizer = create_autospec(Authorization)
         self.controller = FhirController(self.authorizer, self.service)
@@ -1626,15 +1607,11 @@ def setUp(self):
         self.date_to_key = "-date.to"
         self.nhs_number_valid_value = "9000000009"
         self.patient_identifier_valid_value = f"{patient_identifier_system}|{self.nhs_number_valid_value}"
-        self.mock_redis_client.hkeys.return_value = MOCK_REDIS_V2D_RESPONSE
-
-    def tearDown(self):
-        return super().tearDown()
 
     @patch("fhir_controller.get_supplier_permissions")
     def test_get_search_immunizations(self, mock_get_supplier_permissions):
         """it should search based on patient_identifier and immunization_target"""
-
+        
         mock_get_supplier_permissions.return_value = ["COVID19.S"]
         search_result = Bundle.construct()
         self.service.search_immunizations.return_value = search_result
@@ -1818,7 +1795,7 @@ def test_post_search_immunizations(self,mock_get_supplier_permissions):
             "headers": {"Content-Type": "application/x-www-form-urlencoded", "SupplierSystem": "Test"},
             "body": base64_encoded_body,
         }
-
+        
         # When
         response = self.controller.search_immunizations(lambda_event)
         # Then
@@ -1929,7 +1906,6 @@ def test_post_search_immunizations_for_unauthorized_vaccine_type_search_403(self
 
     @patch("fhir_controller.process_search_params", wraps=process_search_params)
     def test_uses_parameter_parser(self, process_search_params: Mock):
-        self.mock_redis_client.hkeys.return_value = MOCK_REDIS_V2D_RESPONSE
         lambda_event = {
             "multiValueQueryStringParameters": {
                 self.patient_identifier_key: ["https://fhir.nhs.uk/Id/nhs-number|9000000009"],

backend/tests/test_fhir_controller.py

@@ -576,6 +576,7 @@ def test_unauthorised_vax_delete(self):
                     "Resource": json.dumps({"foo": "bar"}),
                     "Version": 1,
                     "PatientSK": "FLU#2516525251",
+                    "DeletedAt": "reinstated"
                 }
             }
         )