From 0c8b05810ee3b8c3afd01955b01398971644f127 Mon Sep 17 00:00:00 2001 From: adamwhitingnhs Date: Thu, 12 Feb 2026 10:08:50 +0000 Subject: [PATCH 1/7] [PRMP-1271] Adjust ods pilot logic --- lambdas/services/create_document_reference_service.py | 5 +++-- lambdas/services/feature_flags_service.py | 5 ++--- lambdas/services/update_document_reference_service.py | 5 +++-- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/lambdas/services/create_document_reference_service.py b/lambdas/services/create_document_reference_service.py index 28d45813f0..7b6652dc36 100644 --- a/lambdas/services/create_document_reference_service.py +++ b/lambdas/services/create_document_reference_service.py @@ -177,7 +177,7 @@ def build_doc_ref_info( def check_if_user_ods_code_is_in_pilot(self, ods_code) -> bool: pilot_ods_codes = self.get_allowed_list_of_ods_codes_for_upload_pilot() - if ods_code in pilot_ods_codes: + if ods_code in pilot_ods_codes or pilot_ods_codes == []: return True else: raise OdsErrorException() @@ -315,6 +315,7 @@ def get_allowed_list_of_ods_codes_for_upload_pilot(self) -> list[str]: "Starting ssm request to retrieve allowed list of ODS codes for Upload Pilot" ) response = self.ssm_service.get_ssm_parameter(UPLOAD_PILOT_ODS_ALLOWED_LIST) - if not response: + if not response or response == ["ALLOW_ALL"]: logger.warning("No ODS codes found in allowed list for Upload Pilot") + return [] return response diff --git a/lambdas/services/feature_flags_service.py b/lambdas/services/feature_flags_service.py index dba5ef6715..3c11599c1c 100644 --- a/lambdas/services/feature_flags_service.py +++ b/lambdas/services/feature_flags_service.py @@ -133,8 +133,7 @@ def get_allowed_list_of_ods_codes_for_upload_pilot(self) -> list[str]: "Starting ssm request to retrieve allowed list of ODS codes for Upload Pilot" ) response = self.ssm_service.get_ssm_parameter(UPLOAD_PILOT_ODS_ALLOWED_LIST) - if not response: - logger.warning("No ODS codes found in allowed list for Upload Pilot") + if not response or response == ["ALLOW_ALL"]: return [] return response.split(",") @@ -150,7 +149,7 @@ def check_if_ods_code_is_in_pilot(self) -> bool: return False pilot_ods_codes = self.get_allowed_list_of_ods_codes_for_upload_pilot() - return ods_code in pilot_ods_codes + return ods_code in pilot_ods_codes or pilot_ods_codes == [] def validate_feature_flag(self, flag_name: str): flag_object = self.get_feature_flags_by_flag(flag_name) diff --git a/lambdas/services/update_document_reference_service.py b/lambdas/services/update_document_reference_service.py index ed0be0c954..709df475f7 100644 --- a/lambdas/services/update_document_reference_service.py +++ b/lambdas/services/update_document_reference_service.py @@ -170,7 +170,7 @@ def build_doc_ref_info( def check_if_ods_code_is_in_pilot(self, ods_code) -> bool: pilot_ods_codes = self.get_allowed_list_of_ods_codes_for_upload_pilot() - if ods_code in pilot_ods_codes: + if ods_code in pilot_ods_codes or pilot_ods_codes == []: return True else: raise OdsErrorException() @@ -213,6 +213,7 @@ def get_allowed_list_of_ods_codes_for_upload_pilot(self) -> list[str]: "Starting ssm request to retrieve allowed list of ODS codes for Upload Pilot" ) response = self.ssm_service.get_ssm_parameter(UPLOAD_PILOT_ODS_ALLOWED_LIST) - if not response: + if not response or response == ["ALLOW_ALL"]: logger.warning("No ODS codes found in allowed list for Upload Pilot") + return [] return response From 1d31300c89d33eb35b0f6bbbe5810bd4cc07196e Mon Sep 17 00:00:00 2001 From: Kamen Bachvarov Date: Thu, 12 Feb 2026 14:57:57 +0000 Subject: [PATCH 2/7] fix UT --- .../create_document_reference_service.py | 18 +++++++++--------- lambdas/services/feature_flags_service.py | 5 +++-- .../update_document_reference_service.py | 18 +++++++++--------- .../services/test_feature_flags_service.py | 2 +- 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/lambdas/services/create_document_reference_service.py b/lambdas/services/create_document_reference_service.py index 7b6652dc36..3b5cbebded 100644 --- a/lambdas/services/create_document_reference_service.py +++ b/lambdas/services/create_document_reference_service.py @@ -310,12 +310,12 @@ def remove_records_of_failed_upload( logger.info("Previous failed records are deleted.") - def get_allowed_list_of_ods_codes_for_upload_pilot(self) -> list[str]: - logger.info( - "Starting ssm request to retrieve allowed list of ODS codes for Upload Pilot" - ) - response = self.ssm_service.get_ssm_parameter(UPLOAD_PILOT_ODS_ALLOWED_LIST) - if not response or response == ["ALLOW_ALL"]: - logger.warning("No ODS codes found in allowed list for Upload Pilot") - return [] - return response + # def get_allowed_list_of_ods_codes_for_upload_pilot(self) -> list[str]: + # logger.info( + # "Starting ssm request to retrieve allowed list of ODS codes for Upload Pilot" + # ) + # response = self.ssm_service.get_ssm_parameter(UPLOAD_PILOT_ODS_ALLOWED_LIST) + # if not response or response == ["*"]: + # logger.warning("No ODS codes found in allowed list for Upload Pilot") + # return [] + # return response diff --git a/lambdas/services/feature_flags_service.py b/lambdas/services/feature_flags_service.py index 3c11599c1c..b59a7b015f 100644 --- a/lambdas/services/feature_flags_service.py +++ b/lambdas/services/feature_flags_service.py @@ -133,9 +133,10 @@ def get_allowed_list_of_ods_codes_for_upload_pilot(self) -> list[str]: "Starting ssm request to retrieve allowed list of ODS codes for Upload Pilot" ) response = self.ssm_service.get_ssm_parameter(UPLOAD_PILOT_ODS_ALLOWED_LIST) - if not response or response == ["ALLOW_ALL"]: + if not response or response == ["*"]: + logger.warning("No ODS codes found in allowed list for Upload Pilot") return [] - return response.split(",") + return response def check_if_ods_code_is_in_pilot(self) -> bool: ods_code = "" diff --git a/lambdas/services/update_document_reference_service.py b/lambdas/services/update_document_reference_service.py index 709df475f7..d276e380f8 100644 --- a/lambdas/services/update_document_reference_service.py +++ b/lambdas/services/update_document_reference_service.py @@ -208,12 +208,12 @@ def stop_if_upload_is_in_progress(self, nhs_number: str): ) raise DocumentRefException(423, LambdaError.UploadInProgressError) - def get_allowed_list_of_ods_codes_for_upload_pilot(self) -> list[str]: - logger.info( - "Starting ssm request to retrieve allowed list of ODS codes for Upload Pilot" - ) - response = self.ssm_service.get_ssm_parameter(UPLOAD_PILOT_ODS_ALLOWED_LIST) - if not response or response == ["ALLOW_ALL"]: - logger.warning("No ODS codes found in allowed list for Upload Pilot") - return [] - return response + # def get_allowed_list_of_ods_codes_for_upload_pilot(self) -> list[str]: + # logger.info( + # "Starting ssm request to retrieve allowed list of ODS codes for Upload Pilot" + # ) + # response = self.ssm_service.get_ssm_parameter(UPLOAD_PILOT_ODS_ALLOWED_LIST) + # if not response or response == ["*"]: + # logger.warning("No ODS codes found in allowed list for Upload Pilot") + # return [] + # return response diff --git a/lambdas/tests/unit/services/test_feature_flags_service.py b/lambdas/tests/unit/services/test_feature_flags_service.py index c04c9218cc..88b86d6127 100644 --- a/lambdas/tests/unit/services/test_feature_flags_service.py +++ b/lambdas/tests/unit/services/test_feature_flags_service.py @@ -202,7 +202,7 @@ def test_get_allowed_list_of_ods_codes_for_upload_pilot(mock_feature_flag_servic def test_get_allowed_list_of_ods_codes_for_upload_pilot_no_codes_found( mock_feature_flag_service, caplog ): - mock_feature_flag_service.ssm_service.get_ssm_parameter.return_value = [] + mock_feature_flag_service.ssm_service.get_ssm_parameter.return_value = ["*"] result = mock_feature_flag_service.get_allowed_list_of_ods_codes_for_upload_pilot() From 68cb55eaae67e3b71728488172db7cef97cba2e4 Mon Sep 17 00:00:00 2001 From: Kamen Bachvarov Date: Thu, 12 Feb 2026 15:53:51 +0000 Subject: [PATCH 3/7] refactor and fix UT --- .../create_document_reference_service.py | 15 +++----------- lambdas/services/feature_flags_service.py | 2 +- .../update_document_reference_service.py | 15 +++----------- .../test_create_document_reference_service.py | 20 +------------------ .../services/test_feature_flags_service.py | 2 +- .../test_update_document_reference_service.py | 11 +++++----- 6 files changed, 15 insertions(+), 50 deletions(-) diff --git a/lambdas/services/create_document_reference_service.py b/lambdas/services/create_document_reference_service.py index 3b5cbebded..e31234ce47 100644 --- a/lambdas/services/create_document_reference_service.py +++ b/lambdas/services/create_document_reference_service.py @@ -10,13 +10,13 @@ from models.fhir.R4.fhir_document_reference import Attachment, DocumentReferenceInfo from pydantic import ValidationError from services.base.ssm_service import SSMService +from services.feature_flags_service import FeatureFlagService from services.post_fhir_document_reference_service import ( PostFhirDocumentReferenceService, ) from utils import upload_file_configs from utils.audit_logging_setup import LoggingService from utils.common_query_filters import get_document_type_filter -from utils.constants.ssm import UPLOAD_PILOT_ODS_ALLOWED_LIST from utils.dynamo_query_filter_builder import DynamoQueryFilterBuilder from utils.exceptions import ( ConfigNotFoundException, @@ -49,6 +49,7 @@ class CreateDocumentReferenceService: def __init__(self): self.post_fhir_doc_ref_service = PostFhirDocumentReferenceService() self.ssm_service = SSMService() + self.feature_flag_service = FeatureFlagService() self.lg_dynamo_table = os.getenv("LLOYD_GEORGE_DYNAMODB_NAME") self.staging_bucket_name = os.getenv("STAGING_STORE_BUCKET_NAME") @@ -176,7 +177,7 @@ def build_doc_ref_info( return doc_ref_info def check_if_user_ods_code_is_in_pilot(self, ods_code) -> bool: - pilot_ods_codes = self.get_allowed_list_of_ods_codes_for_upload_pilot() + pilot_ods_codes = self.feature_flag_service.get_allowed_list_of_ods_codes_for_upload_pilot() if ods_code in pilot_ods_codes or pilot_ods_codes == []: return True else: @@ -309,13 +310,3 @@ def remove_records_of_failed_upload( ) logger.info("Previous failed records are deleted.") - - # def get_allowed_list_of_ods_codes_for_upload_pilot(self) -> list[str]: - # logger.info( - # "Starting ssm request to retrieve allowed list of ODS codes for Upload Pilot" - # ) - # response = self.ssm_service.get_ssm_parameter(UPLOAD_PILOT_ODS_ALLOWED_LIST) - # if not response or response == ["*"]: - # logger.warning("No ODS codes found in allowed list for Upload Pilot") - # return [] - # return response diff --git a/lambdas/services/feature_flags_service.py b/lambdas/services/feature_flags_service.py index b59a7b015f..37fc80b7b2 100644 --- a/lambdas/services/feature_flags_service.py +++ b/lambdas/services/feature_flags_service.py @@ -132,7 +132,7 @@ def get_allowed_list_of_ods_codes_for_upload_pilot(self) -> list[str]: logger.info( "Starting ssm request to retrieve allowed list of ODS codes for Upload Pilot" ) - response = self.ssm_service.get_ssm_parameter(UPLOAD_PILOT_ODS_ALLOWED_LIST) + response = self.ssm_service.get_ssm_parameter(UPLOAD_PILOT_ODS_ALLOWED_LIST).split(",") if not response or response == ["*"]: logger.warning("No ODS codes found in allowed list for Upload Pilot") return [] diff --git a/lambdas/services/update_document_reference_service.py b/lambdas/services/update_document_reference_service.py index d276e380f8..5cabf036e1 100644 --- a/lambdas/services/update_document_reference_service.py +++ b/lambdas/services/update_document_reference_service.py @@ -8,10 +8,10 @@ from pydantic import ValidationError from services.base.ssm_service import SSMService from services.document_service import DocumentService +from services.feature_flags_service import FeatureFlagService from services.put_fhir_document_reference_service import PutFhirDocumentReferenceService from utils.audit_logging_setup import LoggingService from utils.common_query_filters import CurrentStatusFile, NotDeleted -from utils.constants.ssm import UPLOAD_PILOT_ODS_ALLOWED_LIST from utils.dynamo_utils import DocTypeTableRouter from utils.exceptions import ( InvalidNhsNumberException, @@ -38,6 +38,7 @@ def __init__(self): self.fhir_doc_ref_service = PutFhirDocumentReferenceService() self.document_service = DocumentService() self.ssm_service = SSMService() + self.feature_flag_service = FeatureFlagService() self.doctype_table_router = DocTypeTableRouter() def update_document_reference_request( @@ -169,7 +170,7 @@ def build_doc_ref_info( return doc_ref_info def check_if_ods_code_is_in_pilot(self, ods_code) -> bool: - pilot_ods_codes = self.get_allowed_list_of_ods_codes_for_upload_pilot() + pilot_ods_codes = self.feature_flag_service.get_allowed_list_of_ods_codes_for_upload_pilot() if ods_code in pilot_ods_codes or pilot_ods_codes == []: return True else: @@ -207,13 +208,3 @@ def stop_if_upload_is_in_progress(self, nhs_number: str): {"Result": UPDATE_REFERENCE_FAILED_MESSAGE}, ) raise DocumentRefException(423, LambdaError.UploadInProgressError) - - # def get_allowed_list_of_ods_codes_for_upload_pilot(self) -> list[str]: - # logger.info( - # "Starting ssm request to retrieve allowed list of ODS codes for Upload Pilot" - # ) - # response = self.ssm_service.get_ssm_parameter(UPLOAD_PILOT_ODS_ALLOWED_LIST) - # if not response or response == ["*"]: - # logger.warning("No ODS codes found in allowed list for Upload Pilot") - # return [] - # return response diff --git a/lambdas/tests/unit/services/test_create_document_reference_service.py b/lambdas/tests/unit/services/test_create_document_reference_service.py index 8a1332e037..86af69654d 100644 --- a/lambdas/tests/unit/services/test_create_document_reference_service.py +++ b/lambdas/tests/unit/services/test_create_document_reference_service.py @@ -172,7 +172,7 @@ def mock_get_allowed_list_of_ods_codes_for_upload_pilot( mock_create_doc_ref_service, mocker ): return mocker.patch.object( - mock_create_doc_ref_service, "get_allowed_list_of_ods_codes_for_upload_pilot" + mock_create_doc_ref_service.feature_flag_service, "get_allowed_list_of_ods_codes_for_upload_pilot" ) @@ -633,24 +633,6 @@ def test_ods_code_not_in_pilot_raises_exception( assert exception.status_code == 404 assert exception.message == "ODS code does not match any of the allowed." - -def test_get_allowed_list_of_ods_codes_for_upload_pilot( - mock_fhir_doc_ref_base_service, mock_create_doc_ref_service, mock_ssm -): - mock_ssm.get_ssm_parameter.return_value = MOCK_ALLOWED_ODS_CODES_LIST_PILOT[ - "Parameter" - ]["Value"] - expected = "PI001,PI002,PI003" - - actual = ( - mock_create_doc_ref_service.get_allowed_list_of_ods_codes_for_upload_pilot() - ) - - mock_ssm.get_ssm_parameter.assert_called_once() - - assert actual == expected - - def test_patient_ods_does_not_match_user_ods_and_raises_exception( mock_fhir_doc_ref_base_service, mock_create_doc_ref_service, diff --git a/lambdas/tests/unit/services/test_feature_flags_service.py b/lambdas/tests/unit/services/test_feature_flags_service.py index 88b86d6127..1f4ab10247 100644 --- a/lambdas/tests/unit/services/test_feature_flags_service.py +++ b/lambdas/tests/unit/services/test_feature_flags_service.py @@ -202,7 +202,7 @@ def test_get_allowed_list_of_ods_codes_for_upload_pilot(mock_feature_flag_servic def test_get_allowed_list_of_ods_codes_for_upload_pilot_no_codes_found( mock_feature_flag_service, caplog ): - mock_feature_flag_service.ssm_service.get_ssm_parameter.return_value = ["*"] + mock_feature_flag_service.ssm_service.get_ssm_parameter.return_value = "*" result = mock_feature_flag_service.get_allowed_list_of_ods_codes_for_upload_pilot() diff --git a/lambdas/tests/unit/services/test_update_document_reference_service.py b/lambdas/tests/unit/services/test_update_document_reference_service.py index 1a09166a45..5e92415bc5 100644 --- a/lambdas/tests/unit/services/test_update_document_reference_service.py +++ b/lambdas/tests/unit/services/test_update_document_reference_service.py @@ -28,7 +28,7 @@ def mock_update_doc_ref_service(mocker): @pytest.fixture -def mock_fhir_doc_ref_base_service(mocker, setup_request_context): +def mock_fhir_doc_ref_base_service(mocker, setup_request_context, set_env): mock_document_service = mocker.patch( "services.fhir_document_reference_service_base.DocumentService" ) @@ -104,7 +104,7 @@ def mock_get_allowed_list_of_ods_codes_for_upload_pilot( mock_update_doc_ref_service, mocker ): return mocker.patch.object( - mock_update_doc_ref_service, "get_allowed_list_of_ods_codes_for_upload_pilot" + mock_update_doc_ref_service.feature_flag_service, "get_allowed_list_of_ods_codes_for_upload_pilot" ) @@ -134,8 +134,8 @@ def mock_fetch_documents_from_table(mocker, mock_update_doc_ref_service): def test_update_document_reference_request_with_lg_list_happy_path( - mock_update_doc_ref_service, mock_fhir_doc_ref_base_service, + mock_update_doc_ref_service, mock_getting_patient_info_from_pds, mock_stop_if_upload_is_in_progress, mock_get_allowed_list_of_ods_codes_for_upload_pilot, @@ -223,12 +223,12 @@ def test_nhs_number_not_found_raises_exception( # covers for number of files expected, non-pdf files, incorrect file name format, duplicate files def test_invalid_files_raises_exception( + mock_fhir_doc_ref_base_service, mock_update_doc_ref_service, mock_validate_files_for_access_and_store, mock_getting_patient_info_from_pds, mock_pds_patient, mock_get_allowed_list_of_ods_codes_for_upload_pilot, - mock_fhir_doc_ref_base_service, mock_process_fhir_document_reference, mock_stop_if_upload_is_in_progress, mock_fetch_documents_from_table, @@ -255,12 +255,12 @@ def test_invalid_files_raises_exception( @freeze_time("2023-10-30T10:25:00") def test_upload_already_in_progress_raises_exception( + mock_fhir_doc_ref_base_service, mock_update_doc_ref_service, mock_fetch_document_by_type, mock_get_allowed_list_of_ods_codes_for_upload_pilot, mock_getting_patient_info_from_pds, mock_pds_patient, - mock_fhir_doc_ref_base_service, mock_process_fhir_document_reference, mock_validate_files_for_access_and_store, mock_fetch_documents_from_table, @@ -287,6 +287,7 @@ def test_upload_already_in_progress_raises_exception( def test_fail_early_if_there_is_no_document_reference_to_update( + mock_fhir_doc_ref_base_service, mock_update_doc_ref_service, mock_fetch_documents_from_table, mock_process_fhir_document_reference, From 039cc1a659a62d1754d7bcc46846ffcfaaabfc2a Mon Sep 17 00:00:00 2001 From: Kamen Bachvarov Date: Thu, 12 Feb 2026 16:00:48 +0000 Subject: [PATCH 4/7] format --- .../create_document_reference_service.py | 47 ++++--- lambdas/services/feature_flags_service.py | 10 +- .../update_document_reference_service.py | 25 ++-- .../test_create_document_reference_service.py | 130 +++++++++--------- .../services/test_feature_flags_service.py | 27 ++-- .../test_update_document_reference_service.py | 43 +++--- 6 files changed, 148 insertions(+), 134 deletions(-) diff --git a/lambdas/services/create_document_reference_service.py b/lambdas/services/create_document_reference_service.py index e31234ce47..81cfc5c48c 100644 --- a/lambdas/services/create_document_reference_service.py +++ b/lambdas/services/create_document_reference_service.py @@ -56,7 +56,9 @@ def __init__(self): self.upload_sub_folder = "user_upload" def create_document_reference_request( - self, nhs_number: str, documents_list: list[dict] + self, + nhs_number: str, + documents_list: list[dict], ): upload_document_names = [] url_responses = {} @@ -74,18 +76,15 @@ def create_document_reference_request( for validated_doc in upload_request_documents: snomed_code = validated_doc.doc_type - config = upload_file_configs.get_config_by_snomed_code( - snomed_code - ) + config = upload_file_configs.get_config_by_snomed_code(snomed_code) if config.single_file_only: self.check_existing_records_and_remove_failed_upload( - nhs_number, - snomed_code + nhs_number, snomed_code, ) document_reference = self.create_document_reference( - nhs_number, user_ods_code, validated_doc, snomed_code + nhs_number, user_ods_code, validated_doc, snomed_code, ) self.validate_document_file_type(validated_doc, config) @@ -127,13 +126,15 @@ def create_document_reference_request( raise DocumentRefException(400, LambdaError.DocRefInvalidFiles) def validate_document_file_type(self, validated_doc, document_config): - if not is_file_type_allowed(validated_doc.file_name, document_config.accepted_file_types): + if not is_file_type_allowed( + validated_doc.file_name, document_config.accepted_file_types, + ): raise LGInvalidFilesException( - f"Unsupported file type for file: {validated_doc.file_name}" + f"Unsupported file type for file: {validated_doc.file_name}", ) def build_and_process_fhir_doc_ref( - self, nhs_number, user_ods_code, validated_doc, snomed_code, document_reference + self, nhs_number, user_ods_code, validated_doc, snomed_code, document_reference, ): doc_ref_info = self.build_doc_ref_info( validated_doc, @@ -143,11 +144,11 @@ def build_and_process_fhir_doc_ref( ) fhir_doc_ref = doc_ref_info.create_fhir_document_reference_object( - document_reference + document_reference, ) fhir_response = self.post_fhir_doc_ref_service.process_fhir_document_reference( - fhir_doc_ref.model_dump_json() + fhir_doc_ref.model_dump_json(), ) return fhir_response @@ -160,7 +161,7 @@ def validate_patient_user_ods_codes_match(self, user_ods_code, patient_ods_code) raise DocumentRefException(401, LambdaError.DocRefUnauthorizedOdsCode) def build_doc_ref_info( - self, validated_doc, nhs_number, snomed_code, user_ods_code + self, validated_doc, nhs_number, snomed_code, user_ods_code, ) -> DocumentReferenceInfo: attachment_details = Attachment( title=validated_doc.file_name, @@ -177,14 +178,16 @@ def build_doc_ref_info( return doc_ref_info def check_if_user_ods_code_is_in_pilot(self, ods_code) -> bool: - pilot_ods_codes = self.feature_flag_service.get_allowed_list_of_ods_codes_for_upload_pilot() + pilot_ods_codes = ( + self.feature_flag_service.get_allowed_list_of_ods_codes_for_upload_pilot() + ) if ods_code in pilot_ods_codes or pilot_ods_codes == []: return True else: raise OdsErrorException() def parse_documents_list( - self, document_list: list[dict] + self, document_list: list[dict], ) -> list[UploadRequestDocument]: upload_request_document_list = [] for document in document_list: @@ -251,7 +254,7 @@ def check_existing_records_and_remove_failed_upload( ) if not previous_records: logger.info( - "No record was found for this patient. Will continue to create doc ref." + "No record was found for this patient. Will continue to create doc ref.", ) return @@ -262,7 +265,7 @@ def check_existing_records_and_remove_failed_upload( def stop_if_upload_is_in_process(self, previous_records: list[DocumentReference]): if any( self.post_fhir_doc_ref_service.document_service.is_upload_in_process( - document + document, ) for document in previous_records ): @@ -277,7 +280,7 @@ def stop_if_all_records_uploaded(self, previous_records: list[DocumentReference] if all_records_uploaded: logger.info( "The patient already has a full set of record. " - "We should not be processing the new Lloyd George record upload." + "We should not be processing the new Lloyd George record upload.", ) logger.error( f"{LambdaError.DocRefRecordAlreadyInPlace.to_str()}", @@ -292,21 +295,21 @@ def remove_records_of_failed_upload( ): logger.info( "Found previous records of failed upload. " - "Will delete those records before creating new document references." + "Will delete those records before creating new document references.", ) logger.info("Deleting files from s3...") for record in failed_upload_records: s3_bucket_name, s3_file_key = record._parse_s3_location( - record.file_location + record.file_location, ) self.post_fhir_doc_ref_service.s3_service.delete_object( - s3_bucket_name, s3_file_key + s3_bucket_name, s3_file_key, ) logger.info("Deleting dynamodb record...") self.post_fhir_doc_ref_service.document_service.hard_delete_metadata_records( - table_name=table_name, document_references=failed_upload_records + table_name=table_name, document_references=failed_upload_records, ) logger.info("Previous failed records are deleted.") diff --git a/lambdas/services/feature_flags_service.py b/lambdas/services/feature_flags_service.py index 37fc80b7b2..67d92e3f38 100644 --- a/lambdas/services/feature_flags_service.py +++ b/lambdas/services/feature_flags_service.py @@ -130,9 +130,11 @@ def get_feature_flags_by_flag(self, flag: str): def get_allowed_list_of_ods_codes_for_upload_pilot(self) -> list[str]: logger.info( - "Starting ssm request to retrieve allowed list of ODS codes for Upload Pilot" + "Starting ssm request to retrieve allowed list of ODS codes for Upload Pilot", ) - response = self.ssm_service.get_ssm_parameter(UPLOAD_PILOT_ODS_ALLOWED_LIST).split(",") + response = self.ssm_service.get_ssm_parameter( + UPLOAD_PILOT_ODS_ALLOWED_LIST, + ).split(",") if not response or response == ["*"]: logger.warning("No ODS codes found in allowed list for Upload Pilot") return [] @@ -143,7 +145,7 @@ def check_if_ods_code_is_in_pilot(self) -> bool: if isinstance(request_context.authorization, dict): ods_code = request_context.authorization.get( - "selected_organisation", {} + "selected_organisation", {}, ).get("org_ods_code", "") if not ods_code: @@ -157,6 +159,6 @@ def validate_feature_flag(self, flag_name: str): if not flag_object.get(flag_name, False): logger.info( - f"Feature flag '{flag_name}' not enabled, event will not be processed" + f"Feature flag '{flag_name}' not enabled, event will not be processed", ) raise FeatureFlagsException(404, LambdaError.FeatureFlagDisabled) diff --git a/lambdas/services/update_document_reference_service.py b/lambdas/services/update_document_reference_service.py index 5cabf036e1..6f03407bb0 100644 --- a/lambdas/services/update_document_reference_service.py +++ b/lambdas/services/update_document_reference_service.py @@ -42,7 +42,10 @@ def __init__(self): self.doctype_table_router = DocTypeTableRouter() def update_document_reference_request( - self, nhs_number: str, document: dict, doc_ref_id: str + self, + nhs_number: str, + document: dict, + doc_ref_id: str, ): self.validate_doc_ref_exists(doc_ref_id) @@ -65,12 +68,12 @@ def update_document_reference_request( self.validate_user_patient_ods_match(patient_ods_code, user_ods_code) validate_files_for_access_and_store( - [update_request_document], pds_patient_details + [update_request_document], pds_patient_details, ) self.stop_if_upload_is_in_progress(nhs_number) fhir_response = self.build_and_process_fhir_doc_ref( - nhs_number, doc_ref_id, update_request_document, user_ods_code + nhs_number, doc_ref_id, update_request_document, user_ods_code, ) fhir_response_data = json.loads(fhir_response) @@ -98,12 +101,12 @@ def update_document_reference_request( raise DocumentRefException(400, LambdaError.DocRefInvalidFiles) def build_and_process_fhir_doc_ref( - self, nhs_number, doc_ref_id, update_request_document, user_ods_code + self, nhs_number, doc_ref_id, update_request_document, user_ods_code, ): snomed_code_type = self.get_snomed_code_from_doc(update_request_document) doc_ref_info = self.build_doc_ref_info( - nhs_number, update_request_document, snomed_code_type, user_ods_code + nhs_number, update_request_document, snomed_code_type, user_ods_code, ) logger.info(f"Updating document reference for client id: {doc_ref_id}") @@ -111,11 +114,11 @@ def build_and_process_fhir_doc_ref( validate_doc_version = update_request_document.version_id fhir_doc_ref = doc_ref_info.create_fhir_document_reference_object_basic( - doc_ref_id, validate_doc_version + doc_ref_id, validate_doc_version, ) fhir_response = self.fhir_doc_ref_service.process_fhir_document_reference( - fhir_doc_ref.model_dump_json() + fhir_doc_ref.model_dump_json(), ) return fhir_response @@ -154,7 +157,7 @@ def get_snomed_code_from_doc(self, update_request_document): return snomed_code_type def build_doc_ref_info( - self, nhs_number, update_request_document, snomed_code_type, user_ods_code + self, nhs_number, update_request_document, snomed_code_type, user_ods_code, ): attachment_details = Attachment( title=update_request_document.file_name, @@ -170,7 +173,9 @@ def build_doc_ref_info( return doc_ref_info def check_if_ods_code_is_in_pilot(self, ods_code) -> bool: - pilot_ods_codes = self.feature_flag_service.get_allowed_list_of_ods_codes_for_upload_pilot() + pilot_ods_codes = ( + self.feature_flag_service.get_allowed_list_of_ods_codes_for_upload_pilot() + ) if ods_code in pilot_ods_codes or pilot_ods_codes == []: return True else: @@ -179,7 +184,7 @@ def check_if_ods_code_is_in_pilot(self, ods_code) -> bool: def parse_document(self, document: dict) -> UploadRequestDocument: try: validated_doc: UploadRequestDocument = UploadRequestDocument.model_validate( - document + document, ) except ValidationError as e: logger.error( diff --git a/lambdas/tests/unit/services/test_create_document_reference_service.py b/lambdas/tests/unit/services/test_create_document_reference_service.py index 86af69654d..7ff84f1b7f 100644 --- a/lambdas/tests/unit/services/test_create_document_reference_service.py +++ b/lambdas/tests/unit/services/test_create_document_reference_service.py @@ -65,13 +65,13 @@ def mock_create_doc_ref_service(set_env, mocker): @pytest.fixture def mock_fhir_doc_ref_base_service(mocker, setup_request_context): mock_document_service = mocker.patch( - "services.fhir_document_reference_service_base.DocumentService" + "services.fhir_document_reference_service_base.DocumentService", ) mock_s3_service = mocker.patch( - "services.fhir_document_reference_service_base.S3Service" + "services.fhir_document_reference_service_base.S3Service", ) mock_dynamo_service = mocker.patch( - "services.fhir_document_reference_service_base.DynamoDBService" + "services.fhir_document_reference_service_base.DynamoDBService", ) service = FhirDocumentReferenceServiceBase() service.document_service = mock_document_service.return_value @@ -98,9 +98,9 @@ def mock_process_fhir_document_reference(mocker): return_value=json.dumps( { "content": [ - {"attachment": {"url": "https://test-bucket.s3.amazonaws.com/"}} - ] - } + {"attachment": {"url": "https://test-bucket.s3.amazonaws.com/"}}, + ], + }, ), ) @@ -119,13 +119,14 @@ def mock_create_document_reference(mock_create_doc_ref_service, mocker): @pytest.fixture() def mock_remove_records(mock_create_doc_ref_service, mocker): yield mocker.patch.object( - mock_create_doc_ref_service, "remove_records_of_failed_upload" + mock_create_doc_ref_service, + "remove_records_of_failed_upload", ) @pytest.fixture() def mock_check_existing_records_and_remove_failed_upload( - mock_create_doc_ref_service, mocker + mock_create_doc_ref_service, mocker, ): yield mocker.patch.object( mock_create_doc_ref_service, @@ -136,7 +137,7 @@ def mock_check_existing_records_and_remove_failed_upload( @pytest.fixture() def mock_check_for_duplicate_files(mocker): yield mocker.patch( - "services.create_document_reference_service.check_for_duplicate_files" + "services.create_document_reference_service.check_for_duplicate_files", ) @@ -150,7 +151,7 @@ def mock_getting_patient_info_from_pds(mocker, mock_pds_patient): @pytest.fixture def mock_fetch_available_document_references_by_type( - mocker, mock_fhir_doc_ref_base_service + mocker, mock_fhir_doc_ref_base_service, ): mock = mocker.patch.object( mock_fhir_doc_ref_base_service.document_service, @@ -169,10 +170,11 @@ def undo_mocking_for_is_upload_in_process(mock_fhir_doc_ref_base_service): @pytest.fixture def mock_get_allowed_list_of_ods_codes_for_upload_pilot( - mock_create_doc_ref_service, mocker + mock_create_doc_ref_service, mocker, ): return mocker.patch.object( - mock_create_doc_ref_service.feature_flag_service, "get_allowed_list_of_ods_codes_for_upload_pilot" + mock_create_doc_ref_service.feature_flag_service, + "get_allowed_list_of_ods_codes_for_upload_pilot", ) @@ -186,7 +188,7 @@ def test_create_document_reference_request_empty_list( ): with pytest.raises(DocumentRefException) as e: mock_create_doc_ref_service.create_document_reference_request( - TEST_NHS_NUMBER, [] + TEST_NHS_NUMBER, [], ) assert e.value == DocumentRefException(400, LambdaError.DocRefInvalidFiles) @@ -206,12 +208,12 @@ def test_create_document_reference_request_with_lg_list_happy_path( mock_check_for_duplicate_files, ): mock_get_allowed_list_of_ods_codes_for_upload_pilot.return_value = [ - TEST_CURRENT_GP_ODS + TEST_CURRENT_GP_ODS, ] mock_presigned_url_response = "https://test-bucket.s3.amazonaws.com/" url_references = mock_create_doc_ref_service.create_document_reference_request( - TEST_NHS_NUMBER, LG_FILE_LIST + TEST_NHS_NUMBER, LG_FILE_LIST, ) expected_response = { "uuid1": mock_presigned_url_response, @@ -221,7 +223,7 @@ def test_create_document_reference_request_with_lg_list_happy_path( assert url_references == expected_response mock_check_existing_records_and_remove_failed_upload.assert_called_with( - TEST_NHS_NUMBER, LG_FILE_LIST[0]["docType"] + TEST_NHS_NUMBER, LG_FILE_LIST[0]["docType"], ) mock_check_for_duplicate_files.assert_called_once() @@ -253,19 +255,19 @@ def test_create_document_reference_request_raise_error_when_invalid_lg( file_name=file["fileName"], doc_type=SupportedDocumentTypes.LG, document_snomed_code_type=SnomedCodes.LLOYD_GEORGE.value.code, - ) + ), ) side_effects.append(document_references[index]) mock_create_document_reference.side_effect = side_effects mock_check_for_duplicate_files.side_effect = LGInvalidFilesException("test") mock_get_allowed_list_of_ods_codes_for_upload_pilot.return_value = [ - TEST_CURRENT_GP_ODS + TEST_CURRENT_GP_ODS, ] with pytest.raises(DocumentRefException): mock_create_doc_ref_service.create_document_reference_request( - TEST_NHS_NUMBER, LG_FILE_LIST + TEST_NHS_NUMBER, LG_FILE_LIST, ) mock_create_document_reference.assert_has_calls( @@ -293,7 +295,7 @@ def test_create_document_reference_failed_to_parse_pds_response( with pytest.raises(Exception) as exc_info: mock_create_doc_ref_service.create_document_reference_request( - TEST_NHS_NUMBER, LG_FILE_LIST + TEST_NHS_NUMBER, LG_FILE_LIST, ) exception = exc_info.value @@ -315,7 +317,7 @@ def test_cdr_nhs_number_not_found_raises_search_patient_exception( with pytest.raises(Exception) as exc_info: mock_create_doc_ref_service.create_document_reference_request( - TEST_NHS_NUMBER, LG_FILE_LIST + TEST_NHS_NUMBER, LG_FILE_LIST, ) exception = exc_info.value @@ -337,12 +339,12 @@ def test_cdr_non_pdf_file_raises_exception( ): mock_check_for_duplicate_files.side_effect = LGInvalidFilesException mock_get_allowed_list_of_ods_codes_for_upload_pilot.return_value = [ - TEST_CURRENT_GP_ODS + TEST_CURRENT_GP_ODS, ] with pytest.raises(Exception) as exc_info: mock_create_doc_ref_service.create_document_reference_request( - TEST_NHS_NUMBER, LG_FILE_LIST + TEST_NHS_NUMBER, LG_FILE_LIST, ) exception = exc_info.value @@ -363,18 +365,18 @@ def test_create_document_reference_request_lg_upload_throw_lambda_error_if_uploa ): two_minutes_ago = 1698661380 # 2023-10-30T10:23:00 mock_records_upload_in_process = create_test_lloyd_george_doc_store_refs( - override={"uploaded": False, "uploading": True, "last_updated": two_minutes_ago} + override={"uploaded": False, "uploading": True, "last_updated": two_minutes_ago}, ) mock_fetch_available_document_references_by_type.return_value = ( mock_records_upload_in_process ) mock_get_allowed_list_of_ods_codes_for_upload_pilot.return_value = [ - TEST_CURRENT_GP_ODS + TEST_CURRENT_GP_ODS, ] with pytest.raises(DocumentRefException) as e: mock_create_doc_ref_service.create_document_reference_request( - TEST_NHS_NUMBER, LG_FILE_LIST + TEST_NHS_NUMBER, LG_FILE_LIST, ) assert e.value == DocumentRefException(423, LambdaError.UploadInProgressError) @@ -392,12 +394,12 @@ def test_create_document_reference_request_lg_upload_throw_lambda_error_if_got_a create_test_lloyd_george_doc_store_refs() ) mock_get_allowed_list_of_ods_codes_for_upload_pilot.return_value = [ - TEST_CURRENT_GP_ODS + TEST_CURRENT_GP_ODS, ] with pytest.raises(DocumentRefException) as e: mock_create_doc_ref_service.create_document_reference_request( - TEST_NHS_NUMBER, LG_FILE_LIST + TEST_NHS_NUMBER, LG_FILE_LIST, ) assert e.value == DocumentRefException(422, LambdaError.DocRefRecordAlreadyInPlace) @@ -411,7 +413,7 @@ def test_check_existing_records_remove_previous_failed_upload_and_continue( mocker, ): mock_doc_refs_of_failed_upload = create_test_lloyd_george_doc_store_refs( - override={"uploaded": False} + override={"uploaded": False}, ) mock_fetch_available_document_references_by_type.return_value = ( mock_doc_refs_of_failed_upload @@ -420,15 +422,15 @@ def test_check_existing_records_remove_previous_failed_upload_and_continue( mock_create_doc_ref_service.stop_if_upload_is_in_process = mocker.MagicMock() mock_create_doc_ref_service.check_existing_records_and_remove_failed_upload( - TEST_NHS_NUMBER, mock_doc_refs_of_failed_upload[0].document_snomed_code_type + TEST_NHS_NUMBER, mock_doc_refs_of_failed_upload[0].document_snomed_code_type, ) mock_remove_records.assert_called_with( - MOCK_LG_TABLE_NAME, mock_doc_refs_of_failed_upload + MOCK_LG_TABLE_NAME, mock_doc_refs_of_failed_upload, ) def test_parse_documents_list_for_valid_input( - mock_fhir_doc_ref_base_service, mock_create_doc_ref_service + mock_fhir_doc_ref_base_service, mock_create_doc_ref_service, ): mock_input = LG_FILE_LIST expected = PARSED_LG_FILE_LIST @@ -446,7 +448,7 @@ def test_parse_documents_list_raise_lambda_error_when_no_type( { "fileName": "test1.txt", "contentType": "text/plain", - } + }, ] with pytest.raises(DocumentRefException): @@ -462,7 +464,7 @@ def test_parse_documents_list_raise_lambda_error_when_doc_type_is_invalid( "fileName": "test1.txt", "contentType": "text/plain", "docType": "banana", - } + }, ] with pytest.raises(DocumentRefException): @@ -470,7 +472,7 @@ def test_parse_documents_list_raise_lambda_error_when_doc_type_is_invalid( def test_prepare_doc_object_lg_happy_path( - mocker, mock_fhir_doc_ref_base_service, mock_create_doc_ref_service + mocker, mock_fhir_doc_ref_base_service, mock_create_doc_ref_service, ): validated_document = UploadRequestDocument.model_validate(LG_FILE_LIST[0]) nhs_number = "1234567890" @@ -488,7 +490,7 @@ def test_prepare_doc_object_lg_happy_path( ) actual_document_reference = mock_create_doc_ref_service.create_document_reference( - nhs_number, current_gp_ods, validated_document, snomed_code_type="SNOMED" + nhs_number, current_gp_ods, validated_document, snomed_code_type="SNOMED", ) assert actual_document_reference == mocked_doc @@ -520,8 +522,7 @@ def test_check_existing_records_does_nothing_if_no_record_exist( assert ( mock_create_doc_ref_service.check_existing_records_and_remove_failed_upload( - TEST_NHS_NUMBER, - SupportedDocumentTypes.LG + TEST_NHS_NUMBER, SupportedDocumentTypes.LG, ) is None ) @@ -542,14 +543,13 @@ def test_check_existing_records_throw_error_if_upload_in_progress( "uploaded": False, "uploading": True, "last_updated": two_minutes_ago, - } + }, ) ) with pytest.raises(Exception) as e: mock_create_doc_ref_service.check_existing_records_and_remove_failed_upload( - TEST_NHS_NUMBER, - SupportedDocumentTypes.LG + TEST_NHS_NUMBER, SupportedDocumentTypes.LG, ) ex = e.value assert isinstance(ex, DocumentRefException) @@ -571,8 +571,7 @@ def test_check_existing_records_throw_error_if_got_a_full_set_of_uploaded_record with pytest.raises(Exception) as e: mock_create_doc_ref_service.check_existing_records_and_remove_failed_upload( - TEST_NHS_NUMBER, - SupportedDocumentTypes.LG + TEST_NHS_NUMBER, SupportedDocumentTypes.LG, ) ex = e.value @@ -584,10 +583,10 @@ def test_check_existing_records_throw_error_if_got_a_full_set_of_uploaded_record def test_remove_records_of_failed_upload( - mock_fhir_doc_ref_base_service, mock_create_doc_ref_service, mocker + mock_fhir_doc_ref_base_service, mock_create_doc_ref_service, mocker, ): mock_doc_refs_of_failed_upload = create_test_lloyd_george_doc_store_refs( - override={"uploaded": False} + override={"uploaded": False}, ) mock_create_doc_ref_service.post_fhir_doc_ref_service.s3_service = ( @@ -623,7 +622,7 @@ def test_ods_code_not_in_pilot_raises_exception( with pytest.raises(DocumentRefException) as exc_info: mock_create_doc_ref_service.create_document_reference_request( - TEST_NHS_NUMBER, LG_FILE_LIST + TEST_NHS_NUMBER, LG_FILE_LIST, ) mock_create_document_reference.assert_not_called() @@ -633,6 +632,7 @@ def test_ods_code_not_in_pilot_raises_exception( assert exception.status_code == 404 assert exception.message == "ODS code does not match any of the allowed." + def test_patient_ods_does_not_match_user_ods_and_raises_exception( mock_fhir_doc_ref_base_service, mock_create_doc_ref_service, @@ -642,7 +642,7 @@ def test_patient_ods_does_not_match_user_ods_and_raises_exception( with pytest.raises(DocumentRefException) as exc_info: mock_create_doc_ref_service.create_document_reference_request( - TEST_NHS_NUMBER, LG_FILE_LIST + TEST_NHS_NUMBER, LG_FILE_LIST, ) mock_create_document_reference.assert_not_called() @@ -665,49 +665,51 @@ def test_unable_to_find_config_raises_exception( mock_process_fhir_document_reference, ): mock_get_allowed_list_of_ods_codes_for_upload_pilot.return_value = [ - TEST_CURRENT_GP_ODS + TEST_CURRENT_GP_ODS, ] with pytest.raises(DocumentRefException) as exc_info: mock_create_doc_ref_service.create_document_reference_request( - TEST_NHS_NUMBER, ARF_FILE_LIST + TEST_NHS_NUMBER, ARF_FILE_LIST, ) exception = exc_info.value assert isinstance(exception, DocumentRefException) assert exception.status_code == 400 - assert ( - exception.message - == "Invalid files or id" - ) + assert exception.message == "Invalid files or id" mock_process_fhir_document_reference.assert_not_called() + def test_check_existing_records_fetches_previous_records_for_doc_type( mock_fhir_doc_ref_base_service, mock_create_doc_ref_service, mock_fetch_available_document_references_by_type, mock_remove_records, - mocker + mocker, ): doc_type = SupportedDocumentTypes.LG - expected_query_filter = NotDeleted & DynamoQueryFilterBuilder().add_condition( - DocumentReferenceMetadataFields.DOCUMENT_SNOMED_CODE_TYPE, - AttributeOperator.EQUAL, - doc_type - ).build() + expected_query_filter = ( + NotDeleted + & DynamoQueryFilterBuilder() + .add_condition( + DocumentReferenceMetadataFields.DOCUMENT_SNOMED_CODE_TYPE, + AttributeOperator.EQUAL, + doc_type, + ) + .build() + ) mocker.patch( - "services.create_document_reference_service.get_document_type_filter" + "services.create_document_reference_service.get_document_type_filter", ).return_value = expected_query_filter mock_create_doc_ref_service.check_existing_records_and_remove_failed_upload( - TEST_NHS_NUMBER, - doc_type + TEST_NHS_NUMBER, doc_type, ) mock_fetch_available_document_references_by_type.assert_called_with( nhs_number=TEST_NHS_NUMBER, doc_type=doc_type, - query_filter=expected_query_filter - ) \ No newline at end of file + query_filter=expected_query_filter, + ) diff --git a/lambdas/tests/unit/services/test_feature_flags_service.py b/lambdas/tests/unit/services/test_feature_flags_service.py index 1f4ab10247..20c99e3bbb 100644 --- a/lambdas/tests/unit/services/test_feature_flags_service.py +++ b/lambdas/tests/unit/services/test_feature_flags_service.py @@ -53,7 +53,7 @@ def mock_feature_flag_service(set_env, mocker, setup_request_context): def test_request_app_config_data_valid_response_returns_data( - mock_requests, mock_feature_flag_service + mock_requests, mock_feature_flag_service, ): mock_requests.get(test_url, json=success_200_all_response, status_code=200) @@ -64,7 +64,7 @@ def test_request_app_config_data_valid_response_returns_data( def test_request_app_config_data_invalid_json_raises_exception( - mock_requests, mock_feature_flag_service + mock_requests, mock_feature_flag_service, ): invalid_json = "invalid:" mock_requests.get(test_url, text=invalid_json, status_code=500) @@ -78,7 +78,7 @@ def test_request_app_config_data_invalid_json_raises_exception( def test_request_app_config_data_400_raises_not_found_exception( - mock_requests, mock_feature_flag_service + mock_requests, mock_feature_flag_service, ): mock_requests.get(test_url, json=err_response, status_code=400) @@ -91,7 +91,7 @@ def test_request_app_config_data_400_raises_not_found_exception( def test_request_app_config_data_catch_all_raises_failure_exception( - mock_requests, mock_feature_flag_service + mock_requests, mock_feature_flag_service, ): mock_requests.get(test_url, json=err_response, status_code=500) @@ -117,7 +117,7 @@ def test_get_feature_flags_returns_all_flags(mock_requests, mock_feature_flag_se def test_get_feature_flags_no_flags_returns_empty( - mock_requests, mock_feature_flag_service + mock_requests, mock_feature_flag_service, ): mock_requests.get(test_url, json=empty_response, status_code=200) mock_feature_flag_service.request_app_config_data.return_value = empty_response @@ -130,7 +130,7 @@ def test_get_feature_flags_no_flags_returns_empty( def test_get_feature_flags_invalid_raises_exception( - mock_requests, mock_feature_flag_service + mock_requests, mock_feature_flag_service, ): mock_requests.get(test_url, json=err_response, status_code=200) mock_feature_flag_service.request_app_config_data.return_value = err_response @@ -143,7 +143,7 @@ def test_get_feature_flags_invalid_raises_exception( def test_get_feature_flags_by_flag_returns_single_flag( - mock_requests, mock_feature_flag_service + mock_requests, mock_feature_flag_service, ): mock_requests.get(test_url, json=success_200_with_filter_reponse, status_code=200) mock_feature_flag_service.request_app_config_data.return_value = ( @@ -158,7 +158,7 @@ def test_get_feature_flags_by_flag_returns_single_flag( def test_get_feature_flags_by_flag_no_flag_raises_exception( - mock_requests, mock_feature_flag_service + mock_requests, mock_feature_flag_service, ): mock_requests.get(test_url, json=empty_response, status_code=200) mock_feature_flag_service.request_app_config_data.return_value = empty_response @@ -172,7 +172,8 @@ def test_get_feature_flags_by_flag_no_flag_raises_exception( def test_get_feature_flags_by_flag_invalid_raises_exception( - mock_requests, mock_feature_flag_service + mock_requests, + mock_feature_flag_service, ): mock_requests.get(test_url, json=err_response, status_code=200) mock_feature_flag_service.request_app_config_data.return_value = err_response @@ -195,12 +196,12 @@ def test_get_allowed_list_of_ods_codes_for_upload_pilot(mock_feature_flag_servic assert actual_codes == expected_codes mock_feature_flag_service.ssm_service.get_ssm_parameter.assert_called_with( - UPLOAD_PILOT_ODS_ALLOWED_LIST + UPLOAD_PILOT_ODS_ALLOWED_LIST, ) def test_get_allowed_list_of_ods_codes_for_upload_pilot_no_codes_found( - mock_feature_flag_service, caplog + mock_feature_flag_service, caplog, ): mock_feature_flag_service.ssm_service.get_ssm_parameter.return_value = "*" @@ -221,7 +222,7 @@ def test_get_allowed_list_of_ods_codes_for_upload_pilot_no_codes_found( ], ) def test_check_if_ods_code_is_in_pilot( - mocker, mock_feature_flag_service, auth_context, expected_result + mocker, mock_feature_flag_service, auth_context, expected_result, ): mock_context = mocker.MagicMock() mock_context.authorization = auth_context @@ -309,7 +310,7 @@ def test_get_feature_flags_by_flag_overwrites_upload_flag( def test_get_feature_flags_by_flag_for_non_upload_flag( - mocker, mock_feature_flag_service + mocker, mock_feature_flag_service, ): flag_name = "some_other_flag" mocker.patch.object(mock_feature_flag_service, "check_if_ods_code_is_in_pilot") diff --git a/lambdas/tests/unit/services/test_update_document_reference_service.py b/lambdas/tests/unit/services/test_update_document_reference_service.py index 5e92415bc5..d1a811d7c1 100644 --- a/lambdas/tests/unit/services/test_update_document_reference_service.py +++ b/lambdas/tests/unit/services/test_update_document_reference_service.py @@ -30,13 +30,13 @@ def mock_update_doc_ref_service(mocker): @pytest.fixture def mock_fhir_doc_ref_base_service(mocker, setup_request_context, set_env): mock_document_service = mocker.patch( - "services.fhir_document_reference_service_base.DocumentService" + "services.fhir_document_reference_service_base.DocumentService", ) mock_s3_service = mocker.patch( - "services.fhir_document_reference_service_base.S3Service" + "services.fhir_document_reference_service_base.S3Service", ) mock_dynamo_service = mocker.patch( - "services.fhir_document_reference_service_base.DynamoDBService" + "services.fhir_document_reference_service_base.DynamoDBService", ) service = FhirDocumentReferenceServiceBase() service.document_service = mock_document_service.return_value @@ -66,10 +66,10 @@ def mock_stop_if_upload_is_in_progress(mock_update_doc_ref_service, mocker): @pytest.fixture() def mock_validate_files_for_access_and_store( - mocker, mock_getting_patient_info_from_pds + mocker, mock_getting_patient_info_from_pds, ): yield mocker.patch( - "services.update_document_reference_service.validate_files_for_access_and_store" + "services.update_document_reference_service.validate_files_for_access_and_store", ) @@ -92,26 +92,27 @@ def mock_process_fhir_document_reference(mocker): return_value=json.dumps( { "content": [ - {"attachment": {"url": "https://test-bucket.s3.amazonaws.com/"}} - ] - } + {"attachment": {"url": "https://test-bucket.s3.amazonaws.com/"}}, + ], + }, ), ) @pytest.fixture def mock_get_allowed_list_of_ods_codes_for_upload_pilot( - mock_update_doc_ref_service, mocker + mock_update_doc_ref_service, mocker, ): return mocker.patch.object( - mock_update_doc_ref_service.feature_flag_service, "get_allowed_list_of_ods_codes_for_upload_pilot" + mock_update_doc_ref_service.feature_flag_service, + "get_allowed_list_of_ods_codes_for_upload_pilot", ) @pytest.fixture def mock_check_if_ods_code_is_in_pilot(mock_update_doc_ref_service, mocker): return mocker.patch.object( - mock_update_doc_ref_service, "check_if_ods_code_is_in_pilot" + mock_update_doc_ref_service, "check_if_ods_code_is_in_pilot", ) @@ -145,7 +146,7 @@ def test_update_document_reference_request_with_lg_list_happy_path( mock_fetch_documents_from_table, ): mock_get_allowed_list_of_ods_codes_for_upload_pilot.return_value = [ - TEST_CURRENT_GP_ODS + TEST_CURRENT_GP_ODS, ] mock_getting_patient_info_from_pds.return_value = mock_pds_patient mock_fetch_documents_from_table.return_value = create_test_doc_store_refs() @@ -157,7 +158,7 @@ def test_update_document_reference_request_with_lg_list_happy_path( ) url_references = mock_update_doc_ref_service.update_document_reference_request( - TEST_NHS_NUMBER, LG_FILE, TEST_UUID + TEST_NHS_NUMBER, LG_FILE, TEST_UUID, ) expected_response = {"uuid1": mock_presigned_url_response} @@ -183,7 +184,7 @@ def test_ods_code_not_in_pilot_raises_exception( with pytest.raises(DocumentRefException) as exc_info: mock_update_doc_ref_service.update_document_reference_request( - TEST_NHS_NUMBER, LG_FILE, TEST_UUID + TEST_NHS_NUMBER, LG_FILE, TEST_UUID, ) mock_process_fhir_document_reference.assert_not_called() @@ -209,7 +210,7 @@ def test_nhs_number_not_found_raises_exception( with pytest.raises(DocumentRefException) as exc_info: mock_update_doc_ref_service.update_document_reference_request( - TEST_NHS_NUMBER, LG_FILE, TEST_UUID + TEST_NHS_NUMBER, LG_FILE, TEST_UUID, ) exception = exc_info.value @@ -235,14 +236,14 @@ def test_invalid_files_raises_exception( ): mock_getting_patient_info_from_pds.return_value = mock_pds_patient mock_get_allowed_list_of_ods_codes_for_upload_pilot.return_value = [ - TEST_CURRENT_GP_ODS + TEST_CURRENT_GP_ODS, ] mock_validate_files_for_access_and_store.side_effect = LGInvalidFilesException mock_fetch_documents_from_table.return_value = create_test_doc_store_refs() with pytest.raises(DocumentRefException) as exc_info: mock_update_doc_ref_service.update_document_reference_request( - TEST_NHS_NUMBER, LG_FILE, TEST_UUID + TEST_NHS_NUMBER, LG_FILE, TEST_UUID, ) exception = exc_info.value @@ -267,17 +268,17 @@ def test_upload_already_in_progress_raises_exception( ): mock_getting_patient_info_from_pds.return_value = mock_pds_patient mock_get_allowed_list_of_ods_codes_for_upload_pilot.return_value = [ - TEST_CURRENT_GP_ODS + TEST_CURRENT_GP_ODS, ] mock_fetch_documents_from_table.return_value = create_test_doc_store_refs() two_minutes_ago = 1698661380 # 2023-10-30T10:23:00 mock_records_upload_in_process = create_test_lloyd_george_doc_store_refs( - override={"uploaded": False, "uploading": True, "last_updated": two_minutes_ago} + override={"uploaded": False, "uploading": True, "last_updated": two_minutes_ago}, ) mock_fetch_document_by_type.return_value = mock_records_upload_in_process with pytest.raises(DocumentRefException) as exc_info: mock_update_doc_ref_service.update_document_reference_request( - TEST_NHS_NUMBER, LG_FILE, TEST_UUID + TEST_NHS_NUMBER, LG_FILE, TEST_UUID, ) exception = exc_info.value @@ -296,7 +297,7 @@ def test_fail_early_if_there_is_no_document_reference_to_update( mock_fetch_documents_from_table.return_value = [] with pytest.raises(DocumentRefException) as exc_info: mock_update_doc_ref_service.update_document_reference_request( - TEST_NHS_NUMBER, LG_FILE, TEST_UUID + TEST_NHS_NUMBER, LG_FILE, TEST_UUID, ) exception = exc_info.value From 5c7c9e464bbfe73f480bcbd68a8fd845d0a119b9 Mon Sep 17 00:00:00 2001 From: Kamen Bachvarov Date: Thu, 12 Feb 2026 16:08:00 +0000 Subject: [PATCH 5/7] python lint --- .../test_create_document_reference_service.py | 85 +++++++++++++------ 1 file changed, 59 insertions(+), 26 deletions(-) diff --git a/lambdas/tests/unit/services/test_create_document_reference_service.py b/lambdas/tests/unit/services/test_create_document_reference_service.py index 7ff84f1b7f..3f8cd77ea8 100644 --- a/lambdas/tests/unit/services/test_create_document_reference_service.py +++ b/lambdas/tests/unit/services/test_create_document_reference_service.py @@ -126,7 +126,8 @@ def mock_remove_records(mock_create_doc_ref_service, mocker): @pytest.fixture() def mock_check_existing_records_and_remove_failed_upload( - mock_create_doc_ref_service, mocker, + mock_create_doc_ref_service, + mocker, ): yield mocker.patch.object( mock_create_doc_ref_service, @@ -151,7 +152,8 @@ def mock_getting_patient_info_from_pds(mocker, mock_pds_patient): @pytest.fixture def mock_fetch_available_document_references_by_type( - mocker, mock_fhir_doc_ref_base_service, + mocker, + mock_fhir_doc_ref_base_service, ): mock = mocker.patch.object( mock_fhir_doc_ref_base_service.document_service, @@ -170,7 +172,8 @@ def undo_mocking_for_is_upload_in_process(mock_fhir_doc_ref_base_service): @pytest.fixture def mock_get_allowed_list_of_ods_codes_for_upload_pilot( - mock_create_doc_ref_service, mocker, + mock_create_doc_ref_service, + mocker, ): return mocker.patch.object( mock_create_doc_ref_service.feature_flag_service, @@ -188,7 +191,8 @@ def test_create_document_reference_request_empty_list( ): with pytest.raises(DocumentRefException) as e: mock_create_doc_ref_service.create_document_reference_request( - TEST_NHS_NUMBER, [], + TEST_NHS_NUMBER, + [], ) assert e.value == DocumentRefException(400, LambdaError.DocRefInvalidFiles) @@ -213,7 +217,8 @@ def test_create_document_reference_request_with_lg_list_happy_path( mock_presigned_url_response = "https://test-bucket.s3.amazonaws.com/" url_references = mock_create_doc_ref_service.create_document_reference_request( - TEST_NHS_NUMBER, LG_FILE_LIST, + TEST_NHS_NUMBER, + LG_FILE_LIST, ) expected_response = { "uuid1": mock_presigned_url_response, @@ -223,7 +228,8 @@ def test_create_document_reference_request_with_lg_list_happy_path( assert url_references == expected_response mock_check_existing_records_and_remove_failed_upload.assert_called_with( - TEST_NHS_NUMBER, LG_FILE_LIST[0]["docType"], + TEST_NHS_NUMBER, + LG_FILE_LIST[0]["docType"], ) mock_check_for_duplicate_files.assert_called_once() @@ -267,7 +273,8 @@ def test_create_document_reference_request_raise_error_when_invalid_lg( with pytest.raises(DocumentRefException): mock_create_doc_ref_service.create_document_reference_request( - TEST_NHS_NUMBER, LG_FILE_LIST, + TEST_NHS_NUMBER, + LG_FILE_LIST, ) mock_create_document_reference.assert_has_calls( @@ -295,7 +302,8 @@ def test_create_document_reference_failed_to_parse_pds_response( with pytest.raises(Exception) as exc_info: mock_create_doc_ref_service.create_document_reference_request( - TEST_NHS_NUMBER, LG_FILE_LIST, + TEST_NHS_NUMBER, + LG_FILE_LIST, ) exception = exc_info.value @@ -317,7 +325,8 @@ def test_cdr_nhs_number_not_found_raises_search_patient_exception( with pytest.raises(Exception) as exc_info: mock_create_doc_ref_service.create_document_reference_request( - TEST_NHS_NUMBER, LG_FILE_LIST, + TEST_NHS_NUMBER, + LG_FILE_LIST, ) exception = exc_info.value @@ -344,7 +353,8 @@ def test_cdr_non_pdf_file_raises_exception( with pytest.raises(Exception) as exc_info: mock_create_doc_ref_service.create_document_reference_request( - TEST_NHS_NUMBER, LG_FILE_LIST, + TEST_NHS_NUMBER, + LG_FILE_LIST, ) exception = exc_info.value @@ -365,7 +375,11 @@ def test_create_document_reference_request_lg_upload_throw_lambda_error_if_uploa ): two_minutes_ago = 1698661380 # 2023-10-30T10:23:00 mock_records_upload_in_process = create_test_lloyd_george_doc_store_refs( - override={"uploaded": False, "uploading": True, "last_updated": two_minutes_ago}, + override={ + "uploaded": False, + "uploading": True, + "last_updated": two_minutes_ago, + }, ) mock_fetch_available_document_references_by_type.return_value = ( mock_records_upload_in_process @@ -376,7 +390,8 @@ def test_create_document_reference_request_lg_upload_throw_lambda_error_if_uploa with pytest.raises(DocumentRefException) as e: mock_create_doc_ref_service.create_document_reference_request( - TEST_NHS_NUMBER, LG_FILE_LIST, + TEST_NHS_NUMBER, + LG_FILE_LIST, ) assert e.value == DocumentRefException(423, LambdaError.UploadInProgressError) @@ -399,7 +414,8 @@ def test_create_document_reference_request_lg_upload_throw_lambda_error_if_got_a with pytest.raises(DocumentRefException) as e: mock_create_doc_ref_service.create_document_reference_request( - TEST_NHS_NUMBER, LG_FILE_LIST, + TEST_NHS_NUMBER, + LG_FILE_LIST, ) assert e.value == DocumentRefException(422, LambdaError.DocRefRecordAlreadyInPlace) @@ -422,15 +438,18 @@ def test_check_existing_records_remove_previous_failed_upload_and_continue( mock_create_doc_ref_service.stop_if_upload_is_in_process = mocker.MagicMock() mock_create_doc_ref_service.check_existing_records_and_remove_failed_upload( - TEST_NHS_NUMBER, mock_doc_refs_of_failed_upload[0].document_snomed_code_type, + TEST_NHS_NUMBER, + mock_doc_refs_of_failed_upload[0].document_snomed_code_type, ) mock_remove_records.assert_called_with( - MOCK_LG_TABLE_NAME, mock_doc_refs_of_failed_upload, + MOCK_LG_TABLE_NAME, + mock_doc_refs_of_failed_upload, ) def test_parse_documents_list_for_valid_input( - mock_fhir_doc_ref_base_service, mock_create_doc_ref_service, + mock_fhir_doc_ref_base_service, + mock_create_doc_ref_service, ): mock_input = LG_FILE_LIST expected = PARSED_LG_FILE_LIST @@ -472,7 +491,9 @@ def test_parse_documents_list_raise_lambda_error_when_doc_type_is_invalid( def test_prepare_doc_object_lg_happy_path( - mocker, mock_fhir_doc_ref_base_service, mock_create_doc_ref_service, + mocker, + mock_fhir_doc_ref_base_service, + mock_create_doc_ref_service, ): validated_document = UploadRequestDocument.model_validate(LG_FILE_LIST[0]) nhs_number = "1234567890" @@ -490,7 +511,10 @@ def test_prepare_doc_object_lg_happy_path( ) actual_document_reference = mock_create_doc_ref_service.create_document_reference( - nhs_number, current_gp_ods, validated_document, snomed_code_type="SNOMED", + nhs_number, + current_gp_ods, + validated_document, + snomed_code_type="SNOMED", ) assert actual_document_reference == mocked_doc @@ -522,7 +546,8 @@ def test_check_existing_records_does_nothing_if_no_record_exist( assert ( mock_create_doc_ref_service.check_existing_records_and_remove_failed_upload( - TEST_NHS_NUMBER, SupportedDocumentTypes.LG, + TEST_NHS_NUMBER, + SupportedDocumentTypes.LG, ) is None ) @@ -549,7 +574,8 @@ def test_check_existing_records_throw_error_if_upload_in_progress( with pytest.raises(Exception) as e: mock_create_doc_ref_service.check_existing_records_and_remove_failed_upload( - TEST_NHS_NUMBER, SupportedDocumentTypes.LG, + TEST_NHS_NUMBER, + SupportedDocumentTypes.LG, ) ex = e.value assert isinstance(ex, DocumentRefException) @@ -571,7 +597,8 @@ def test_check_existing_records_throw_error_if_got_a_full_set_of_uploaded_record with pytest.raises(Exception) as e: mock_create_doc_ref_service.check_existing_records_and_remove_failed_upload( - TEST_NHS_NUMBER, SupportedDocumentTypes.LG, + TEST_NHS_NUMBER, + SupportedDocumentTypes.LG, ) ex = e.value @@ -583,7 +610,9 @@ def test_check_existing_records_throw_error_if_got_a_full_set_of_uploaded_record def test_remove_records_of_failed_upload( - mock_fhir_doc_ref_base_service, mock_create_doc_ref_service, mocker, + mock_fhir_doc_ref_base_service, + mock_create_doc_ref_service, + mocker, ): mock_doc_refs_of_failed_upload = create_test_lloyd_george_doc_store_refs( override={"uploaded": False}, @@ -622,7 +651,8 @@ def test_ods_code_not_in_pilot_raises_exception( with pytest.raises(DocumentRefException) as exc_info: mock_create_doc_ref_service.create_document_reference_request( - TEST_NHS_NUMBER, LG_FILE_LIST, + TEST_NHS_NUMBER, + LG_FILE_LIST, ) mock_create_document_reference.assert_not_called() @@ -642,7 +672,8 @@ def test_patient_ods_does_not_match_user_ods_and_raises_exception( with pytest.raises(DocumentRefException) as exc_info: mock_create_doc_ref_service.create_document_reference_request( - TEST_NHS_NUMBER, LG_FILE_LIST, + TEST_NHS_NUMBER, + LG_FILE_LIST, ) mock_create_document_reference.assert_not_called() @@ -670,7 +701,8 @@ def test_unable_to_find_config_raises_exception( with pytest.raises(DocumentRefException) as exc_info: mock_create_doc_ref_service.create_document_reference_request( - TEST_NHS_NUMBER, ARF_FILE_LIST, + TEST_NHS_NUMBER, + ARF_FILE_LIST, ) exception = exc_info.value @@ -705,7 +737,8 @@ def test_check_existing_records_fetches_previous_records_for_doc_type( ).return_value = expected_query_filter mock_create_doc_ref_service.check_existing_records_and_remove_failed_upload( - TEST_NHS_NUMBER, doc_type, + TEST_NHS_NUMBER, + doc_type, ) mock_fetch_available_document_references_by_type.assert_called_with( From 6cd00bbe8f62e254b7b578b39227da07fa8da180 Mon Sep 17 00:00:00 2001 From: Kamen Bachvarov Date: Thu, 12 Feb 2026 16:11:38 +0000 Subject: [PATCH 6/7] tests format again --- .../services/test_feature_flags_service.py | 35 +++++++++++------ .../test_update_document_reference_service.py | 39 ++++++++++++++----- 2 files changed, 53 insertions(+), 21 deletions(-) diff --git a/lambdas/tests/unit/services/test_feature_flags_service.py b/lambdas/tests/unit/services/test_feature_flags_service.py index 20c99e3bbb..b2e98b47f1 100644 --- a/lambdas/tests/unit/services/test_feature_flags_service.py +++ b/lambdas/tests/unit/services/test_feature_flags_service.py @@ -53,7 +53,8 @@ def mock_feature_flag_service(set_env, mocker, setup_request_context): def test_request_app_config_data_valid_response_returns_data( - mock_requests, mock_feature_flag_service, + mock_requests, + mock_feature_flag_service, ): mock_requests.get(test_url, json=success_200_all_response, status_code=200) @@ -64,7 +65,8 @@ def test_request_app_config_data_valid_response_returns_data( def test_request_app_config_data_invalid_json_raises_exception( - mock_requests, mock_feature_flag_service, + mock_requests, + mock_feature_flag_service, ): invalid_json = "invalid:" mock_requests.get(test_url, text=invalid_json, status_code=500) @@ -78,7 +80,8 @@ def test_request_app_config_data_invalid_json_raises_exception( def test_request_app_config_data_400_raises_not_found_exception( - mock_requests, mock_feature_flag_service, + mock_requests, + mock_feature_flag_service, ): mock_requests.get(test_url, json=err_response, status_code=400) @@ -91,7 +94,8 @@ def test_request_app_config_data_400_raises_not_found_exception( def test_request_app_config_data_catch_all_raises_failure_exception( - mock_requests, mock_feature_flag_service, + mock_requests, + mock_feature_flag_service, ): mock_requests.get(test_url, json=err_response, status_code=500) @@ -117,7 +121,8 @@ def test_get_feature_flags_returns_all_flags(mock_requests, mock_feature_flag_se def test_get_feature_flags_no_flags_returns_empty( - mock_requests, mock_feature_flag_service, + mock_requests, + mock_feature_flag_service, ): mock_requests.get(test_url, json=empty_response, status_code=200) mock_feature_flag_service.request_app_config_data.return_value = empty_response @@ -130,7 +135,8 @@ def test_get_feature_flags_no_flags_returns_empty( def test_get_feature_flags_invalid_raises_exception( - mock_requests, mock_feature_flag_service, + mock_requests, + mock_feature_flag_service, ): mock_requests.get(test_url, json=err_response, status_code=200) mock_feature_flag_service.request_app_config_data.return_value = err_response @@ -143,7 +149,8 @@ def test_get_feature_flags_invalid_raises_exception( def test_get_feature_flags_by_flag_returns_single_flag( - mock_requests, mock_feature_flag_service, + mock_requests, + mock_feature_flag_service, ): mock_requests.get(test_url, json=success_200_with_filter_reponse, status_code=200) mock_feature_flag_service.request_app_config_data.return_value = ( @@ -158,7 +165,8 @@ def test_get_feature_flags_by_flag_returns_single_flag( def test_get_feature_flags_by_flag_no_flag_raises_exception( - mock_requests, mock_feature_flag_service, + mock_requests, + mock_feature_flag_service, ): mock_requests.get(test_url, json=empty_response, status_code=200) mock_feature_flag_service.request_app_config_data.return_value = empty_response @@ -201,7 +209,8 @@ def test_get_allowed_list_of_ods_codes_for_upload_pilot(mock_feature_flag_servic def test_get_allowed_list_of_ods_codes_for_upload_pilot_no_codes_found( - mock_feature_flag_service, caplog, + mock_feature_flag_service, + caplog, ): mock_feature_flag_service.ssm_service.get_ssm_parameter.return_value = "*" @@ -222,7 +231,10 @@ def test_get_allowed_list_of_ods_codes_for_upload_pilot_no_codes_found( ], ) def test_check_if_ods_code_is_in_pilot( - mocker, mock_feature_flag_service, auth_context, expected_result, + mocker, + mock_feature_flag_service, + auth_context, + expected_result, ): mock_context = mocker.MagicMock() mock_context.authorization = auth_context @@ -310,7 +322,8 @@ def test_get_feature_flags_by_flag_overwrites_upload_flag( def test_get_feature_flags_by_flag_for_non_upload_flag( - mocker, mock_feature_flag_service, + mocker, + mock_feature_flag_service, ): flag_name = "some_other_flag" mocker.patch.object(mock_feature_flag_service, "check_if_ods_code_is_in_pilot") diff --git a/lambdas/tests/unit/services/test_update_document_reference_service.py b/lambdas/tests/unit/services/test_update_document_reference_service.py index d1a811d7c1..05eedf133e 100644 --- a/lambdas/tests/unit/services/test_update_document_reference_service.py +++ b/lambdas/tests/unit/services/test_update_document_reference_service.py @@ -66,7 +66,8 @@ def mock_stop_if_upload_is_in_progress(mock_update_doc_ref_service, mocker): @pytest.fixture() def mock_validate_files_for_access_and_store( - mocker, mock_getting_patient_info_from_pds, + mocker, + mock_getting_patient_info_from_pds, ): yield mocker.patch( "services.update_document_reference_service.validate_files_for_access_and_store", @@ -101,7 +102,8 @@ def mock_process_fhir_document_reference(mocker): @pytest.fixture def mock_get_allowed_list_of_ods_codes_for_upload_pilot( - mock_update_doc_ref_service, mocker, + mock_update_doc_ref_service, + mocker, ): return mocker.patch.object( mock_update_doc_ref_service.feature_flag_service, @@ -112,7 +114,8 @@ def mock_get_allowed_list_of_ods_codes_for_upload_pilot( @pytest.fixture def mock_check_if_ods_code_is_in_pilot(mock_update_doc_ref_service, mocker): return mocker.patch.object( - mock_update_doc_ref_service, "check_if_ods_code_is_in_pilot", + mock_update_doc_ref_service, + "check_if_ods_code_is_in_pilot", ) @@ -158,7 +161,9 @@ def test_update_document_reference_request_with_lg_list_happy_path( ) url_references = mock_update_doc_ref_service.update_document_reference_request( - TEST_NHS_NUMBER, LG_FILE, TEST_UUID, + TEST_NHS_NUMBER, + LG_FILE, + TEST_UUID, ) expected_response = {"uuid1": mock_presigned_url_response} @@ -184,7 +189,9 @@ def test_ods_code_not_in_pilot_raises_exception( with pytest.raises(DocumentRefException) as exc_info: mock_update_doc_ref_service.update_document_reference_request( - TEST_NHS_NUMBER, LG_FILE, TEST_UUID, + TEST_NHS_NUMBER, + LG_FILE, + TEST_UUID, ) mock_process_fhir_document_reference.assert_not_called() @@ -210,7 +217,9 @@ def test_nhs_number_not_found_raises_exception( with pytest.raises(DocumentRefException) as exc_info: mock_update_doc_ref_service.update_document_reference_request( - TEST_NHS_NUMBER, LG_FILE, TEST_UUID, + TEST_NHS_NUMBER, + LG_FILE, + TEST_UUID, ) exception = exc_info.value @@ -243,7 +252,9 @@ def test_invalid_files_raises_exception( with pytest.raises(DocumentRefException) as exc_info: mock_update_doc_ref_service.update_document_reference_request( - TEST_NHS_NUMBER, LG_FILE, TEST_UUID, + TEST_NHS_NUMBER, + LG_FILE, + TEST_UUID, ) exception = exc_info.value @@ -273,12 +284,18 @@ def test_upload_already_in_progress_raises_exception( mock_fetch_documents_from_table.return_value = create_test_doc_store_refs() two_minutes_ago = 1698661380 # 2023-10-30T10:23:00 mock_records_upload_in_process = create_test_lloyd_george_doc_store_refs( - override={"uploaded": False, "uploading": True, "last_updated": two_minutes_ago}, + override={ + "uploaded": False, + "uploading": True, + "last_updated": two_minutes_ago, + }, ) mock_fetch_document_by_type.return_value = mock_records_upload_in_process with pytest.raises(DocumentRefException) as exc_info: mock_update_doc_ref_service.update_document_reference_request( - TEST_NHS_NUMBER, LG_FILE, TEST_UUID, + TEST_NHS_NUMBER, + LG_FILE, + TEST_UUID, ) exception = exc_info.value @@ -297,7 +314,9 @@ def test_fail_early_if_there_is_no_document_reference_to_update( mock_fetch_documents_from_table.return_value = [] with pytest.raises(DocumentRefException) as exc_info: mock_update_doc_ref_service.update_document_reference_request( - TEST_NHS_NUMBER, LG_FILE, TEST_UUID, + TEST_NHS_NUMBER, + LG_FILE, + TEST_UUID, ) exception = exc_info.value From 70fb063dd5a9f9b4f9d6e7cd45c381e98f04b3f5 Mon Sep 17 00:00:00 2001 From: Kamen Bachvarov Date: Thu, 12 Feb 2026 16:14:36 +0000 Subject: [PATCH 7/7] format services again --- .../create_document_reference_service.py | 33 ++++++++++++++----- lambdas/services/feature_flags_service.py | 3 +- .../update_document_reference_service.py | 28 ++++++++++++---- 3 files changed, 49 insertions(+), 15 deletions(-) diff --git a/lambdas/services/create_document_reference_service.py b/lambdas/services/create_document_reference_service.py index 81cfc5c48c..68e9137fe7 100644 --- a/lambdas/services/create_document_reference_service.py +++ b/lambdas/services/create_document_reference_service.py @@ -80,11 +80,15 @@ def create_document_reference_request( if config.single_file_only: self.check_existing_records_and_remove_failed_upload( - nhs_number, snomed_code, + nhs_number, + snomed_code, ) document_reference = self.create_document_reference( - nhs_number, user_ods_code, validated_doc, snomed_code, + nhs_number, + user_ods_code, + validated_doc, + snomed_code, ) self.validate_document_file_type(validated_doc, config) @@ -127,14 +131,20 @@ def create_document_reference_request( def validate_document_file_type(self, validated_doc, document_config): if not is_file_type_allowed( - validated_doc.file_name, document_config.accepted_file_types, + validated_doc.file_name, + document_config.accepted_file_types, ): raise LGInvalidFilesException( f"Unsupported file type for file: {validated_doc.file_name}", ) def build_and_process_fhir_doc_ref( - self, nhs_number, user_ods_code, validated_doc, snomed_code, document_reference, + self, + nhs_number, + user_ods_code, + validated_doc, + snomed_code, + document_reference, ): doc_ref_info = self.build_doc_ref_info( validated_doc, @@ -161,7 +171,11 @@ def validate_patient_user_ods_codes_match(self, user_ods_code, patient_ods_code) raise DocumentRefException(401, LambdaError.DocRefUnauthorizedOdsCode) def build_doc_ref_info( - self, validated_doc, nhs_number, snomed_code, user_ods_code, + self, + validated_doc, + nhs_number, + snomed_code, + user_ods_code, ) -> DocumentReferenceInfo: attachment_details = Attachment( title=validated_doc.file_name, @@ -187,7 +201,8 @@ def check_if_user_ods_code_is_in_pilot(self, ods_code) -> bool: raise OdsErrorException() def parse_documents_list( - self, document_list: list[dict], + self, + document_list: list[dict], ) -> list[UploadRequestDocument]: upload_request_document_list = [] for document in document_list: @@ -304,12 +319,14 @@ def remove_records_of_failed_upload( record.file_location, ) self.post_fhir_doc_ref_service.s3_service.delete_object( - s3_bucket_name, s3_file_key, + s3_bucket_name, + s3_file_key, ) logger.info("Deleting dynamodb record...") self.post_fhir_doc_ref_service.document_service.hard_delete_metadata_records( - table_name=table_name, document_references=failed_upload_records, + table_name=table_name, + document_references=failed_upload_records, ) logger.info("Previous failed records are deleted.") diff --git a/lambdas/services/feature_flags_service.py b/lambdas/services/feature_flags_service.py index 67d92e3f38..f592108d02 100644 --- a/lambdas/services/feature_flags_service.py +++ b/lambdas/services/feature_flags_service.py @@ -145,7 +145,8 @@ def check_if_ods_code_is_in_pilot(self) -> bool: if isinstance(request_context.authorization, dict): ods_code = request_context.authorization.get( - "selected_organisation", {}, + "selected_organisation", + {}, ).get("org_ods_code", "") if not ods_code: diff --git a/lambdas/services/update_document_reference_service.py b/lambdas/services/update_document_reference_service.py index 6f03407bb0..db30f36599 100644 --- a/lambdas/services/update_document_reference_service.py +++ b/lambdas/services/update_document_reference_service.py @@ -68,12 +68,16 @@ def update_document_reference_request( self.validate_user_patient_ods_match(patient_ods_code, user_ods_code) validate_files_for_access_and_store( - [update_request_document], pds_patient_details, + [update_request_document], + pds_patient_details, ) self.stop_if_upload_is_in_progress(nhs_number) fhir_response = self.build_and_process_fhir_doc_ref( - nhs_number, doc_ref_id, update_request_document, user_ods_code, + nhs_number, + doc_ref_id, + update_request_document, + user_ods_code, ) fhir_response_data = json.loads(fhir_response) @@ -101,12 +105,19 @@ def update_document_reference_request( raise DocumentRefException(400, LambdaError.DocRefInvalidFiles) def build_and_process_fhir_doc_ref( - self, nhs_number, doc_ref_id, update_request_document, user_ods_code, + self, + nhs_number, + doc_ref_id, + update_request_document, + user_ods_code, ): snomed_code_type = self.get_snomed_code_from_doc(update_request_document) doc_ref_info = self.build_doc_ref_info( - nhs_number, update_request_document, snomed_code_type, user_ods_code, + nhs_number, + update_request_document, + snomed_code_type, + user_ods_code, ) logger.info(f"Updating document reference for client id: {doc_ref_id}") @@ -114,7 +125,8 @@ def build_and_process_fhir_doc_ref( validate_doc_version = update_request_document.version_id fhir_doc_ref = doc_ref_info.create_fhir_document_reference_object_basic( - doc_ref_id, validate_doc_version, + doc_ref_id, + validate_doc_version, ) fhir_response = self.fhir_doc_ref_service.process_fhir_document_reference( @@ -157,7 +169,11 @@ def get_snomed_code_from_doc(self, update_request_document): return snomed_code_type def build_doc_ref_info( - self, nhs_number, update_request_document, snomed_code_type, user_ods_code, + self, + nhs_number, + update_request_document, + snomed_code_type, + user_ods_code, ): attachment_details = Attachment( title=update_request_document.file_name,