comments addressed

pythonpath

Author: nhsdevws

.github/workflows/sonarcloud.yml

@@ -59,6 +59,8 @@ jobs:
       - name: Run unittest with recordforwarder-coverage
         working-directory: backend
         id: recordforwarder
+        env:
+          PYTHONPATH: ${{ github.workspace }}/backend/src:${{ github.workspace }}/backend/tests
         continue-on-error: true
         run: |
           poetry env use 3.11

.gitignore

@@ -29,3 +29,4 @@ openapi.json
 !**/.vscode/settings.json.default
 
 devtools/volume/
+backend/tests/.coverage

backend/src/constants.py

@@ -23,6 +23,3 @@ class Urls:
 
 
 GENERIC_SERVER_ERROR_DIAGNOSTICS_MESSAGE = "Unable to process request. Issue may be transient."
-
-VACCINE_TYPE_TO_DISEASES_HASH_KEY = "vacc_to_diseases"
-DISEASES_TO_VACCINE_TYPE_HASH_KEY = "diseases_to_vacc"

backend/src/models/constants.py

@@ -42,4 +42,4 @@ class Constants:
 
     SUPPLIER_PERMISSIONS_KEY = "supplier_permissions"
     VACCINE_TYPE_TO_DISEASES_HASH_KEY = "vacc_to_diseases"
-    DISEASES_TO_VACCINE_TYPE_HASH_KEY = "diseases_to_vacc"
+    DISEASES_TO_VACCINE_TYPE_HASH_KEY = "diseases_to_vacc"

backend/src/models/utils/validation_utils.py

@@ -1,15 +1,15 @@
 """Utils for backend folder"""
 
 import json
-import inspect
 
 from typing import Union
 from .generic_utils import create_diagnostics_error
 from base_utils.base_utils import obtain_field_location
 from models.obtain_field_value import ObtainFieldValue
 from models.field_names import FieldNames
 from models.errors import MandatoryError
-from constants import Urls, DISEASES_TO_VACCINE_TYPE_HASH_KEY
+from constants import Urls
+from models.constants import Constants
 from clients import redis_client
 
 
@@ -53,7 +53,7 @@ def convert_disease_codes_to_vaccine_type(disease_codes_input: list) -> Union[st
     otherwise raises a value error
     """
     key = ":".join(sorted(disease_codes_input))
-    vaccine_type = redis_client.hget(DISEASES_TO_VACCINE_TYPE_HASH_KEY, key)
+    vaccine_type = redis_client.hget(Constants.DISEASES_TO_VACCINE_TYPE_HASH_KEY, key)
 
     if not vaccine_type:
         raise ValueError(

backend/src/parameter_parser.py

@@ -8,7 +8,7 @@
 
 from clients import redis_client
 from models.errors import ParameterException
-from constants import VACCINE_TYPE_TO_DISEASES_HASH_KEY
+from models.constants import Constants
 
 ParamValue = list[str]
 ParamContainer = dict[str, ParamValue]
@@ -109,8 +109,8 @@ def process_search_params(params: ParamContainer) -> SearchParams:
     if len(vaccine_types) < 1:
         raise ParameterException(f"Search parameter {immunization_target_key} must have one or more values.")
 
-    valid_vaccine_types = redis_client.hkeys(VACCINE_TYPE_TO_DISEASES_HASH_KEY)
-    if any([x not in valid_vaccine_types for x in vaccine_types]):
+    valid_vaccine_types = redis_client.hkeys(Constants.VACCINE_TYPE_TO_DISEASES_HASH_KEY)
+    if any(x not in valid_vaccine_types for x in vaccine_types):
         raise ParameterException(
             f"immunization-target must be one or more of the following: {', '.join(valid_vaccine_types)}")
 

backend/tests/.coverage

@@ -33,16 +33,33 @@
 from tests.utils.generic_utils import load_json_data
 from tests.utils.values_for_tests import ValidValues
 
-"test"
+class TestFhirControllerBase(unittest.TestCase):
+    """Base class for all tests to set up common fixtures"""
 
-
-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"}
@@ -133,7 +150,6 @@ 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"""
@@ -165,7 +181,6 @@ def test_not_found_for_identifier(self, mock_get_permissions):
 
         imms = identifier.replace("|", "#")
         # When
-        
         response = self.controller.get_immunization_by_identifier(lambda_event)
 
         # Then
@@ -200,7 +215,6 @@ 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"""
@@ -422,7 +436,6 @@ 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"""
@@ -738,7 +751,6 @@ 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"""
@@ -820,7 +832,6 @@ 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"""
@@ -1142,7 +1153,6 @@ 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"""
@@ -1573,7 +1583,6 @@ 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"""
@@ -1596,8 +1605,22 @@ def test_immunization_unhandled_error(self, mock_get_supplier_permissions):
         self.assertEqual(body["resourceType"], "OperationOutcome")
         self.assertEqual(body["issue"][0]["code"], "exception")
 
-class TestSearchImmunizations(unittest.TestCase):
+class TestSearchImmunizations(TestFhirControllerBase):
+    MOCK_REDIS_V2D_RESPONSE = {
+        "PERTUSSIS": "[{\"code\": \"27836007\", \"term\": \"Pertussis (disorder)\"}]",
+        "RSV": "[{\"code\": \"55735004\", \"term\": \"Respiratory syncytial virus infection (disorder)\"}]",
+        "3in1": "[{\"code\": \"398102009\", \"term\": \"Acute poliomyelitis\"}, {\"code\": \"397430003\", \"term\": \"Diphtheria caused by Corynebacterium diphtheriae\"}, {\"code\": \"76902006\", \"term\": \"Tetanus (disorder)\"}]",
+        "MMR": "[{\"code\": \"14189004\", \"term\": \"Measles (disorder)\"}, {\"code\": \"36989005\", \"term\": \"Mumps (disorder)\"}, {\"code\": \"36653000\", \"term\": \"Rubella (disorder)\"}]",
+        "HPV": "[{\"code\": \"240532009\", \"term\": \"Human papillomavirus infection\"}]",
+        "MMRV": "[{\"code\": \"14189004\", \"term\": \"Measles (disorder)\"}, {\"code\": \"36989005\", \"term\": \"Mumps (disorder)\"}, {\"code\": \"36653000\", \"term\": \"Rubella (disorder)\"}, {\"code\": \"38907003\", \"term\": \"Varicella (disorder)\"}]",
+        "PCV13": "[{\"code\": \"16814004\", \"term\": \"Pneumococcal infectious disease\"}]",
+        "SHINGLES": "[{\"code\": \"4740000\", \"term\": \"Herpes zoster\"}]",
+        "COVID19": "[{\"code\": \"840539006\", \"term\": \"Disease caused by severe acute respiratory syndrome coronavirus 2\"}]",
+        "FLU": "[{\"code\": \"6142004\", \"term\": \"Influenza caused by seasonal influenza virus (disorder)\"}]",
+        "MENACWY": "[{\"code\": \"23511006\", \"term\": \"Meningococcal infectious disease\"}]"
+    }
     def setUp(self):
+        super().setUp()
         self.service = create_autospec(FhirService)
         self.authorizer = create_autospec(Authorization)
         self.controller = FhirController(self.authorizer, self.service)
@@ -1607,11 +1630,14 @@ 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 = self.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
@@ -1795,7 +1821,6 @@ 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
@@ -1906,6 +1931,7 @@ 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 = self.MOCK_REDIS_V2D_RESPONSE
         lambda_event = {
             "multiValueQueryStringParameters": {
                 self.patient_identifier_key: ["https://fhir.nhs.uk/Id/nhs-number|9000000009"],

backend/tests/sample_data/permissions_config.py

@@ -3,6 +3,7 @@
 import datetime
 import unittest
 from copy import deepcopy
+from unittest.mock import create_autospec, patch
 from unittest import skip
 from unittest.mock import create_autospec
 from decimal import Decimal
@@ -23,10 +24,25 @@
     create_covid_19_immunization_dict_no_id,
     VALID_NHS_NUMBER,
 )
-from .utils.generic_utils import load_json_data
-from src.constants import NHS_NUMBER_USED_IN_SAMPLE_DATA
+from utils.generic_utils import load_json_data
+from constants import NHS_NUMBER_USED_IN_SAMPLE_DATA
+
+class TestFhirServiceBase(unittest.TestCase):
+    """Base class for all tests to set up common fixtures"""
+
+    def setUp(self):
+        super().setUp()
+        self.redis_patcher = patch("models.utils.validation_utils.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()
+
 
-"test"
 class TestServiceUrl(unittest.TestCase):
     def test_get_service_url(self):
         """it should create service url"""
@@ -51,15 +67,20 @@ def test_get_service_url(self):
         self.assertEqual(url, f"https://internal-dev.api.service.nhs.uk/{base_path}")
 
 
-class TestGetImmunizationByAll(unittest.TestCase):
+class TestGetImmunizationByAll(TestFhirServiceBase):
     """Tests for FhirService.get_immunization_by_id"""
 
     def setUp(self):
+        super().setUp()
         self.imms_repo = create_autospec(ImmunizationRepository)
         self.pds_service = create_autospec(PdsService)
         self.validator = create_autospec(ImmunizationValidator)
         self.fhir_service = FhirService(self.imms_repo, self.pds_service, self.validator)
 
+    def tearDown(self):
+        super().tearDown()
+        patch.stopall()
+
     def test_get_immunization_by_id_by_all(self):
         """it should find an Immunization by id"""
         imms_id = "an-id"
@@ -119,7 +140,8 @@ def test_pre_validation_failed(self):
         self.assertEqual(error.exception.message, expected_msg)
         self.imms_repo.update_immunization.assert_not_called()
 
-    def test_post_validation_failed(self):
+    def test_post_validation_failed_get_by_all(self):
+        self.mock_redis_client.hget.side_effect = [None, 'COVID-19']
         valid_imms = create_covid_19_immunization_dict("an-id", VALID_NHS_NUMBER)
 
         bad_target_disease_imms = deepcopy(valid_imms)
@@ -150,14 +172,19 @@ def test_post_validation_failed(self):
         self.imms_repo.get_immunization_by_id_all.assert_not_called()
 
 
-class TestGetImmunization(unittest.TestCase):
+class TestGetImmunization(TestFhirServiceBase):
     """Tests for FhirService.get_immunization_by_id"""
 
     def setUp(self):
+        super().setUp()
         self.imms_repo = create_autospec(ImmunizationRepository)
         self.pds_service = create_autospec(PdsService)
         self.validator = create_autospec(ImmunizationValidator)
         self.fhir_service = FhirService(self.imms_repo, self.pds_service, self.validator)
+        self.logger_info_patcher = patch("logging.Logger.info")
+        self.mock_logger_info = self.logger_info_patcher.start()
+    def tearDown(self):
+        patch.stopall()
 
     def test_get_immunization_by_id(self):
         """it should find an Immunization by id"""
@@ -234,7 +261,8 @@ def test_pre_validation_failed(self):
         self.imms_repo.update_immunization.assert_not_called()
         self.pds_service.get_patient_details.assert_not_called()
 
-    def test_post_validation_failed(self):
+    def test_post_validation_failed_get(self):
+        self.mock_redis_client.hget.side_effect = [None, 'COVID-19']
         valid_imms = create_covid_19_immunization_dict("an-id", VALID_NHS_NUMBER)
 
         bad_target_disease_imms = deepcopy(valid_imms)
@@ -309,10 +337,11 @@ def test_immunization_not_found(self):
         self.assertEqual(act_imms["entry"], [])
 
 
-class TestCreateImmunization(unittest.TestCase):
+class TestCreateImmunization(TestFhirServiceBase):
     """Tests for FhirService.create_immunization"""
 
     def setUp(self):
+        super().setUp()
         self.imms_repo = create_autospec(ImmunizationRepository)
         self.pds_service = create_autospec(PdsService)
         self.validator = create_autospec(ImmunizationValidator)
@@ -323,6 +352,10 @@ def setUp(self):
             ImmunizationValidator(add_post_validators=False),
         )
 
+    def tearDown(self):
+        patch.stopall()
+        super().tearDown()
+
     def test_create_immunization(self):
         """it should create Immunization and validate it"""
         imms_id = "an-id"
@@ -382,7 +415,7 @@ def test_pre_validation_failed(self):
 
     def test_post_validation_failed(self):
         """it should throw exception if Immunization is not valid"""
-
+        self.mock_redis_client.hget.side_effect = [None, 'COVID-19']
         valid_imms = create_covid_19_immunization_dict_no_id(VALID_NHS_NUMBER)
 
         bad_target_disease_imms = deepcopy(valid_imms)