From 15260d99b306cc8bd2b951c6fcf76d0a6f3ebbc3 Mon Sep 17 00:00:00 2001 From: NogaNHS Date: Tue, 21 Jan 2025 15:50:17 +0000 Subject: [PATCH 01/13] [PRMP-1376] Add validation score, and changes to validation to return score --- lambdas/enums/validation_score.py | 16 + lambdas/models/pds_models.py | 22 +- lambdas/services/bulk_upload_service.py | 28 +- lambdas/services/edge_presign_service.py | 4 +- .../test_cases_for_patient_name_matching.py | 2 +- lambdas/tests/unit/models/test_pds_models.py | 32 +- .../unit/services/test_bulk_upload_service.py | 27 +- .../unit/utils/test_lloyd_george_validator.py | 400 ++++++++++++------ lambdas/utils/lloyd_george_validator.py | 117 +++-- lambdas/utils/unicode_utils.py | 7 + 10 files changed, 439 insertions(+), 216 deletions(-) create mode 100644 lambdas/enums/validation_score.py diff --git a/lambdas/enums/validation_score.py b/lambdas/enums/validation_score.py new file mode 100644 index 000000000..7fa659aa0 --- /dev/null +++ b/lambdas/enums/validation_score.py @@ -0,0 +1,16 @@ +from enum import Enum + +from pydantic import BaseModel + + +class ValidationScore(Enum): + NO_MATCH = 0 + PARTIAL_MATCH = 1 + MIXED_FULL_MATCH = 2 + FULL_MATCH = 3 + + +class ValidationResult(BaseModel): + given_name_match: list[str] = [] + family_name_match: str = "" + score: ValidationScore = ValidationScore.NO_MATCH diff --git a/lambdas/models/pds_models.py b/lambdas/models/pds_models.py index 74e20f1e2..81abdcb75 100644 --- a/lambdas/models/pds_models.py +++ b/lambdas/models/pds_models.py @@ -116,26 +116,32 @@ def get_usual_name(self) -> Optional[Name]: if entry.use.lower() == "usual": return entry - def get_most_recent_name(self) -> Optional[Name]: - active_names = [name for name in self.name if name.is_currently_in_use()] + def get_names_by_start_date(self) -> [Name]: + active_names = [name for name in self.name] if not active_names: return None sorted_by_start_date_desc = sorted( - active_names, key=lambda name: name.period.start, reverse=True + active_names, + key=lambda name: ( + (1, name.period.start, name.use.lower() == "usual") + if name.period and name.period.start is not None + else (0, 0, name.use.lower() == "usual") + ), + reverse=True, ) - return sorted_by_start_date_desc[0] + return sorted_by_start_date_desc def get_current_family_name_and_given_name(self) -> Tuple[str, list[str]]: - current_name = self.get_most_recent_name() or self.get_usual_name() - if not current_name: + ordered_names = self.get_names_by_start_date() + if not ordered_names: logger.warning( "The patient does not have a currently active name or a usual name." ) return "", [""] - given_name = current_name.given - family_name = current_name.family + given_name = ordered_names[0].given + family_name = ordered_names[0].family if not given_name or given_name == [""]: logger.warning("The given name of patient is empty.") diff --git a/lambdas/services/bulk_upload_service.py b/lambdas/services/bulk_upload_service.py index bc3c2a5e1..84cee1774 100644 --- a/lambdas/services/bulk_upload_service.py +++ b/lambdas/services/bulk_upload_service.py @@ -8,6 +8,7 @@ from enums.patient_ods_inactive_status import PatientOdsInactiveStatus from enums.snomed_codes import SnomedCodes from enums.upload_status import UploadStatus +from enums.validation_score import ValidationScore from enums.virus_scan_result import VirusScanResult from models.fhir.R4.nrl_fhir_document_reference import Attachment from models.nhs_document_reference import NHSDocumentReference @@ -133,13 +134,34 @@ def handle_sqs_message(self, message: dict): patient_ods_code = ( pds_patient_details.get_ods_code_or_inactive_status_for_gp() ) - is_name_validation_based_on_historic_name = ( - validate_filename_with_patient_details(file_names, pds_patient_details) - ) + ( + name_validation_score, + is_dob_valid, + is_name_validation_based_on_historic_name, + ) = validate_filename_with_patient_details(file_names, pds_patient_details) if is_name_validation_based_on_historic_name: accepted_reason = self.concatenate_acceptance_reason( accepted_reason, "Patient matched on historical name" ) + validation_messages = { + ValidationScore.PARTIAL_MATCH: { + True: "Patient matched on partial match 2/3", + False: "", + }, + ValidationScore.MIXED_FULL_MATCH: { + True: "Patient matched on mixed match 3/3", + False: "Patient matched on mixed match 2/3", + }, + ValidationScore.FULL_MATCH: { + True: "Patient matched on full match 3/3", + False: "Patient matched on full match 2/3", + }, + } + if name_validation_score in validation_messages: + message = validation_messages[name_validation_score][is_dob_valid] + accepted_reason = self.concatenate_acceptance_reason( + accepted_reason, message + ) if not allowed_to_ingest_ods_code(patient_ods_code): raise LGInvalidFilesException("Patient not registered at your practice") patient_death_notification_status = ( diff --git a/lambdas/services/edge_presign_service.py b/lambdas/services/edge_presign_service.py index cab057619..28858a9a5 100644 --- a/lambdas/services/edge_presign_service.py +++ b/lambdas/services/edge_presign_service.py @@ -25,7 +25,7 @@ def use_presign(self, request_values: dict): domain_name: str = request_values["domain_name"] presign_string: str = f"{uri}?{querystring}" - encoded_presign_string: str = presign_string.encode("utf-8") + encoded_presign_string = presign_string.encode("utf-8") presign_credentials_hash: str = hashlib.md5(encoded_presign_string).hexdigest() self.attempt_presign_ingestion( @@ -70,7 +70,7 @@ def filter_request_values(request: dict) -> dict: uri: str = request["uri"] querystring: str = request["querystring"] headers: dict = request["headers"] - origin: str = request.get("origin", {}) + origin: dict = request.get("origin", {}) domain_name: str = origin["s3"]["domainName"] except KeyError as e: logger.error(f"Missing request component: {str(e)}") diff --git a/lambdas/tests/unit/helpers/data/pds/test_cases_for_patient_name_matching.py b/lambdas/tests/unit/helpers/data/pds/test_cases_for_patient_name_matching.py index 0d8ee3817..bbbb55e67 100644 --- a/lambdas/tests/unit/helpers/data/pds/test_cases_for_patient_name_matching.py +++ b/lambdas/tests/unit/helpers/data/pds/test_cases_for_patient_name_matching.py @@ -18,9 +18,9 @@ class PdsNameMatchingTestCase(NamedTuple): "Jane Bob Smith Anderson", "Jane Smith Anderson", "Jane B Smith Anderson", + "Bob Smith Anderson", ], "reject": [ - "Bob Smith Anderson", "Jane Smith", "Jane Anderson", "Jane Anderson Smith", diff --git a/lambdas/tests/unit/models/test_pds_models.py b/lambdas/tests/unit/models/test_pds_models.py index a200830b1..9f7357349 100644 --- a/lambdas/tests/unit/models/test_pds_models.py +++ b/lambdas/tests/unit/models/test_pds_models.py @@ -253,6 +253,19 @@ def test_get_current_family_name_and_given_name_return_the_first_usual_name_if_a assert actual == expected_given_name +def test_get_current_family_name_and_given_name_return_the_first_most_recent_name(): + name_1 = build_test_name(use="temp", start="1990-01-01", end=None, given=["Jones"]) + name_2 = build_test_name(use="usual", start=None, end=None, given=["Jane"]) + name_3 = build_test_name(use="usual", start=None, end=None, given=["Bob"]) + + test_patient = build_test_patient_with_names([name_1, name_2, name_3]) + + expected_given_name = ["Jones"] + actual = test_patient.get_patient_details(test_patient.id).given_name + + assert actual == expected_given_name + + def test_get_current_family_name_and_given_name_logs_a_warning_if_no_current_name_or_usual_name_found( caplog, ): @@ -330,7 +343,7 @@ def test_name_is_currently_in_use_return_false_for_nickname_or_old_name(): @freeze_time("2024-01-01") -def test_get_most_recent_name_return_the_name_with_most_recent_start_date(): +def test_get_names_by_start_date_return_ordered_list_by_start_date(): name_1 = build_test_name(start="1990-01-01", end=None, given=["Jane"]) name_2 = build_test_name(start="2010-02-14", end=None, given=["Jones"]) name_3 = build_test_name(start="2000-03-25", end=None, given=["Bob"]) @@ -341,13 +354,22 @@ def test_get_most_recent_name_return_the_name_with_most_recent_start_date(): use="nickname", start="2023-01-01", end=None, given=["Janie"] ) future_name = build_test_name(start="2047-01-01", end=None, given=["Neo Jane"]) + name_no_date = build_test_name(start=None, end=None, given=["Jane"]) test_patient = build_test_patient_with_names( - [name_1, name_2, name_3, expired_name, nickname, future_name] + [name_1, name_2, name_3, expired_name, nickname, future_name, name_no_date] ) - expected = name_2 - actual = test_patient.get_most_recent_name() + expected = [ + future_name, + nickname, + expired_name, + name_2, + name_3, + name_1, + name_no_date, + ] + actual = test_patient.get_names_by_start_date() assert actual == expected @@ -356,7 +378,7 @@ def test_get_most_recent_name_return_the_name_with_most_recent_start_date(): def test_get_most_recent_name_return_none_if_no_active_name_found(): test_patient = build_test_patient_with_names([]) - assert test_patient.get_most_recent_name() is None + assert test_patient.get_names_by_start_date() is None def test_get_death_notification_status_return_the_death_notification_status(): diff --git a/lambdas/tests/unit/services/test_bulk_upload_service.py b/lambdas/tests/unit/services/test_bulk_upload_service.py index 3fe0d2425..d9df0f882 100644 --- a/lambdas/tests/unit/services/test_bulk_upload_service.py +++ b/lambdas/tests/unit/services/test_bulk_upload_service.py @@ -7,6 +7,7 @@ from enums.patient_ods_inactive_status import PatientOdsInactiveStatus from enums.snomed_codes import SnomedCodes from enums.upload_status import UploadStatus +from enums.validation_score import ValidationScore from enums.virus_scan_result import SCAN_RESULT_TAG_KEY, VirusScanResult from freezegun import freeze_time from models.fhir.R4.nrl_fhir_document_reference import Attachment @@ -76,7 +77,8 @@ def mock_check_virus_result(mocker): @pytest.fixture def mock_validate_files(mocker): - yield mocker.patch("services.bulk_upload_service.validate_lg_file_names") + validate_files = mocker.patch("services.bulk_upload_service.validate_lg_file_names") + yield validate_files @pytest.fixture @@ -122,7 +124,8 @@ def mock_pds_service_patient_restricted(mocker): @pytest.fixture def mock_pds_validation(mocker): yield mocker.patch( - "services.bulk_upload_service.validate_filename_with_patient_details" + "services.bulk_upload_service.validate_filename_with_patient_details", + return_value=(ValidationScore.FULL_MATCH, True, True), ) @@ -510,7 +513,7 @@ def test_handle_sqs_message_calls_report_upload_successful_when_patient_is_forma mock_remove_ingested_file_from_source_bucket = ( repo_under_test.s3_repository.remove_ingested_file_from_source_bucket ) - mock_pds_validation.return_value = False + mock_pds_validation.return_value = (ValidationScore.FULL_MATCH, True, False) mock_put_staging_metadata_back_to_queue = ( repo_under_test.sqs_repository.put_staging_metadata_back_to_queue ) @@ -525,7 +528,7 @@ def test_handle_sqs_message_calls_report_upload_successful_when_patient_is_forma mock_report_upload.assert_called_with( TEST_STAGING_METADATA, UploadStatus.COMPLETE, - "Patient is deceased - FORMAL", + "Patient matched on full match 3/3, Patient is deceased - FORMAL", PatientOdsInactiveStatus.DECEASED, ) @@ -544,7 +547,7 @@ def test_handle_sqs_message_calls_report_upload_successful_when_patient_is_infor mock_create_lg_records_and_copy_files = mocker.patch.object( BulkUploadService, "create_lg_records_and_copy_files" ) - mock_pds_validation.return_value = True + mock_pds_validation.return_value = (ValidationScore.MIXED_FULL_MATCH, True, True) mock_remove_ingested_file_from_source_bucket = ( repo_under_test.s3_repository.remove_ingested_file_from_source_bucket ) @@ -562,7 +565,7 @@ def test_handle_sqs_message_calls_report_upload_successful_when_patient_is_infor mock_report_upload.assert_called_with( TEST_STAGING_METADATA, UploadStatus.COMPLETE, - "Patient matched on historical name, Patient is deceased - INFORMAL", + "Patient matched on historical name, Patient matched on mixed match 3/3, Patient is deceased - INFORMAL", "Y12345", ) @@ -581,7 +584,7 @@ def test_handle_sqs_message_calls_report_upload_successful_when_patient_has_hist mock_create_lg_records_and_copy_files = mocker.patch.object( BulkUploadService, "create_lg_records_and_copy_files" ) - mock_pds_validation.return_value = True + mock_pds_validation.return_value = (ValidationScore.MIXED_FULL_MATCH, True, True) mock_remove_ingested_file_from_source_bucket = ( repo_under_test.s3_repository.remove_ingested_file_from_source_bucket ) @@ -599,7 +602,7 @@ def test_handle_sqs_message_calls_report_upload_successful_when_patient_has_hist mock_report_upload.assert_called_with( TEST_STAGING_METADATA, UploadStatus.COMPLETE, - "Patient matched on historical name, PDS record is restricted", + "Patient matched on historical name, Patient matched on mixed match 3/3, PDS record is restricted", "REST", ) @@ -618,7 +621,7 @@ def test_handle_sqs_message_calls_report_upload_successful_when_patient_is_infor mock_create_lg_records_and_copy_files = mocker.patch.object( BulkUploadService, "create_lg_records_and_copy_files" ) - mock_pds_validation.return_value = False + mock_pds_validation.return_value = (ValidationScore.PARTIAL_MATCH, True, False) mock_remove_ingested_file_from_source_bucket = ( repo_under_test.s3_repository.remove_ingested_file_from_source_bucket ) @@ -636,7 +639,7 @@ def test_handle_sqs_message_calls_report_upload_successful_when_patient_is_infor mock_report_upload.assert_called_with( TEST_STAGING_METADATA, UploadStatus.COMPLETE, - "Patient is deceased - INFORMAL", + "Patient matched on partial match 2/3, Patient is deceased - INFORMAL", "Y12345", ) @@ -979,7 +982,7 @@ def test_handle_sqs_message_happy_path_historical_name( repo_under_test.s3_repository, "remove_ingested_file_from_source_bucket" ) mocker.patch.object(repo_under_test.s3_repository, "check_virus_result") - mock_pds_validation.return_value = True + mock_pds_validation.return_value = (ValidationScore.FULL_MATCH, True, True) repo_under_test.handle_sqs_message(message=TEST_SQS_MESSAGE) @@ -990,7 +993,7 @@ def test_handle_sqs_message_happy_path_historical_name( mock_report_upload_complete.assert_called_with( TEST_STAGING_METADATA, UploadStatus.COMPLETE, - "Patient matched on historical name", + "Patient matched on historical name, Patient matched on full match 3/3", "Y12345", ) mock_remove_ingested_file_from_source_bucket.assert_called() diff --git a/lambdas/tests/unit/utils/test_lloyd_george_validator.py b/lambdas/tests/unit/utils/test_lloyd_george_validator.py index 9019ae3b0..e2b2f4dbb 100644 --- a/lambdas/tests/unit/utils/test_lloyd_george_validator.py +++ b/lambdas/tests/unit/utils/test_lloyd_george_validator.py @@ -1,6 +1,7 @@ import pytest from botocore.exceptions import ClientError from enums.supported_document_types import SupportedDocumentTypes +from enums.validation_score import ValidationResult, ValidationScore from models.pds_models import Patient from requests import Response from services.base.ssm_service import SSMService @@ -14,6 +15,10 @@ from tests.unit.helpers.data.pds.pds_patient_response import ( PDS_PATIENT_WITH_MIDDLE_NAME, ) +from tests.unit.helpers.data.pds.test_cases_for_date_logic import ( + build_test_name, + build_test_patient_with_names, +) from tests.unit.helpers.data.pds.test_cases_for_patient_name_matching import ( TEST_CASES_FOR_FAMILY_NAME_WITH_HYPHEN, TEST_CASES_FOR_TWO_WORDS_FAMILY_NAME, @@ -30,6 +35,7 @@ from utils.lloyd_george_validator import ( LGInvalidFilesException, allowed_to_ingest_ods_code, + calculate_validation_score, check_for_duplicate_files, check_for_file_names_agrees_with_each_other, check_for_number_of_files_match_expected, @@ -46,10 +52,26 @@ validate_lg_files, validate_patient_date_of_birth, validate_patient_name, - validate_patient_name_using_full_name_history, ) +@pytest.fixture +def mock_get_ssm_parameter(mocker): + return mocker.patch.object(SSMService, "get_ssm_parameter") + + +@pytest.fixture +def mock_pds_call(mocker): + yield mocker.patch("services.mock_pds_service.MockPdsApiService.pds_request") + + +@pytest.fixture +def mock_fetch_available_document_references_by_type(mocker): + yield mocker.patch.object( + DocumentService, "fetch_available_document_references_by_type" + ) + + def test_catching_error_when_file_type_not_pdf(): with pytest.raises(LGInvalidFilesException): file_type = "image/png" @@ -268,24 +290,18 @@ def test_validate_name_with_correct_name(mocker, mock_pds_patient): mock_validate_name = mocker.patch( "utils.lloyd_george_validator.validate_patient_name" ) - - with expect_not_to_raise(LGInvalidFilesException): - validate_patient_name_using_full_name_history( - lg_file_patient_name, mock_pds_patient - ) - assert mock_validate_name.call_count == 1 - - -def test_validate_name_with_file_missing_middle_name(mocker): - lg_file_patient_name = "Jane Smith" - patient = Patient.model_validate(PDS_PATIENT_WITH_MIDDLE_NAME) - mock_validate_name = mocker.patch( - "utils.lloyd_george_validator.validate_patient_name" + mock_validate_name.return_value = ValidationResult( + score=ValidationScore.FULL_MATCH, + given_name_match=["Jane"], + family_name_match="Smith", + ) + actual_score, actual_is_name_validation_based_on_historic_name = ( + calculate_validation_score(lg_file_patient_name, mock_pds_patient) ) - with expect_not_to_raise(LGInvalidFilesException): - validate_patient_name_using_full_name_history(lg_file_patient_name, patient) assert mock_validate_name.call_count == 1 + assert actual_is_name_validation_based_on_historic_name is False + assert actual_score == ValidationScore.FULL_MATCH def test_validate_name_with_additional_middle_name_in_file_mismatching_pds(mocker): @@ -294,13 +310,18 @@ def test_validate_name_with_additional_middle_name_in_file_mismatching_pds(mocke "utils.lloyd_george_validator.validate_patient_name" ) patient = Patient.model_validate(PDS_PATIENT_WITH_MIDDLE_NAME) + mock_validate_name.return_value = ValidationResult( + score=ValidationScore.FULL_MATCH, + given_name_match=["Jane"], + family_name_match="Smith", + ) + actual_score, actual_is_name_validation_based_on_historic_name = ( + calculate_validation_score(lg_file_patient_name, patient) + ) - with expect_not_to_raise(LGInvalidFilesException): - actual_is_name_validation_based_on_historic_name = ( - validate_patient_name_using_full_name_history(lg_file_patient_name, patient) - ) assert mock_validate_name.call_count == 1 assert actual_is_name_validation_based_on_historic_name is False + assert actual_score == ValidationScore.FULL_MATCH def test_validate_name_with_additional_middle_name_in_file_but_none_in_pds( @@ -310,41 +331,47 @@ def test_validate_name_with_additional_middle_name_in_file_but_none_in_pds( mock_validate_name = mocker.patch( "utils.lloyd_george_validator.validate_patient_name" ) - with expect_not_to_raise(LGInvalidFilesException): - actual_is_name_validation_based_on_historic_name = ( - validate_patient_name_using_full_name_history( - lg_file_patient_name, mock_pds_patient - ) - ) + mock_validate_name.return_value = ValidationResult( + score=ValidationScore.FULL_MATCH, + given_name_match=["Jane"], + family_name_match="Smith", + ) + actual_score, actual_is_name_validation_based_on_historic_name = ( + calculate_validation_score(lg_file_patient_name, mock_pds_patient) + ) + assert mock_validate_name.call_count == 1 assert actual_is_name_validation_based_on_historic_name is False + assert actual_score == ValidationScore.FULL_MATCH def test_validate_name_with_wrong_first_name(mocker, mock_pds_patient): lg_file_patient_name = "John Smith" - mock_validate_name = mocker.patch( - "utils.lloyd_george_validator.validate_patient_name" - ) - mock_validate_name.return_value = False - with pytest.raises(LGInvalidFilesException): - validate_patient_name_using_full_name_history( - lg_file_patient_name, mock_pds_patient - ) - assert mock_validate_name.call_count == 3 + actual_response = validate_patient_name( + lg_file_patient_name, + mock_pds_patient.name[0].given, + mock_pds_patient.name[0].family, + ) + assert actual_response == ValidationResult( + score=ValidationScore.PARTIAL_MATCH, + given_name_match=[], + family_name_match="Smith", + ) def test_validate_name_with_wrong_family_name(mocker, mock_pds_patient): lg_file_patient_name = "Jane Johnson" - mock_validate_name = mocker.patch( - "utils.lloyd_george_validator.validate_patient_name" + actual_response = validate_patient_name( + lg_file_patient_name, + mock_pds_patient.name[0].given, + mock_pds_patient.name[0].family, + ) + assert actual_response == ValidationResult( + score=ValidationScore.PARTIAL_MATCH, + given_name_match=["Jane"], + family_name_match="", ) - mock_validate_name.return_value = False - with pytest.raises(LGInvalidFilesException): - validate_patient_name_using_full_name_history( - lg_file_patient_name, mock_pds_patient - ) - assert mock_validate_name.call_count == 3 def test_validate_name_with_historical_name(mocker, mock_pds_patient): @@ -352,15 +379,20 @@ def test_validate_name_with_historical_name(mocker, mock_pds_patient): mock_validate_name = mocker.patch( "utils.lloyd_george_validator.validate_patient_name" ) - mock_validate_name.side_effect = [False, True] - with expect_not_to_raise(LGInvalidFilesException): - actual_is_name_validation_based_on_historic_name = ( - validate_patient_name_using_full_name_history( - lg_file_patient_name, mock_pds_patient - ) - ) + mock_validate_name.side_effect = [ + ValidationResult( + score=ValidationScore.NO_MATCH, + ), + ValidationResult( + score=ValidationScore.FULL_MATCH, + ), + ] + actual_score, actual_is_validate_on_historic = calculate_validation_score( + lg_file_patient_name, mock_pds_patient + ) + assert actual_score == ValidationScore.FULL_MATCH assert mock_validate_name.call_count == 2 - assert actual_is_name_validation_based_on_historic_name is True + assert actual_is_validate_on_historic is True def test_validate_name_without_given_name(mocker, mock_pds_patient): @@ -369,13 +401,12 @@ def test_validate_name_without_given_name(mocker, mock_pds_patient): mock_validate_name = mocker.patch( "utils.lloyd_george_validator.validate_patient_name" ) - - with expect_not_to_raise(LGInvalidFilesException): - actual_is_validate_on_historic = validate_patient_name_using_full_name_history( - lg_file_patient_name, mock_pds_patient - ) + actual_score, actual_is_validate_on_historic = calculate_validation_score( + lg_file_patient_name, mock_pds_patient + ) + assert actual_score == ValidationScore.NO_MATCH assert actual_is_validate_on_historic is False - assert mock_validate_name.call_count == 1 + assert mock_validate_name.call_count == 2 @pytest.mark.parametrize( @@ -387,19 +418,15 @@ def test_validate_patient_name_with_two_words_family_name( patient_name_in_file_name: str, should_accept_name: bool, ): + actual_score, actual_is_validate_on_historic = calculate_validation_score( + patient_name_in_file_name, patient_details + ) if should_accept_name: - with expect_not_to_raise(LGInvalidFilesException): - actual_is_validate_on_historic = ( - validate_patient_name_using_full_name_history( - patient_name_in_file_name, patient_details - ) - ) - assert actual_is_validate_on_historic is False + assert actual_is_validate_on_historic is False + assert actual_score == ValidationScore.FULL_MATCH else: - with pytest.raises(LGInvalidFilesException): - validate_patient_name_using_full_name_history( - patient_name_in_file_name, patient_details - ) + assert actual_is_validate_on_historic is False + assert actual_score == ValidationScore.PARTIAL_MATCH @pytest.mark.parametrize( @@ -411,19 +438,15 @@ def test_validate_patient_name_with_family_name_with_hyphen( patient_name_in_file_name: str, should_accept_name: bool, ): + actual_score, actual_is_validate_on_historic = calculate_validation_score( + patient_name_in_file_name, patient_details + ) if should_accept_name: - with expect_not_to_raise(LGInvalidFilesException): - actual_is_validate_on_historic = ( - validate_patient_name_using_full_name_history( - patient_name_in_file_name, patient_details - ) - ) - assert actual_is_validate_on_historic is False + assert actual_is_validate_on_historic is False + assert actual_score == ValidationScore.FULL_MATCH else: - with pytest.raises(LGInvalidFilesException): - validate_patient_name_using_full_name_history( - patient_name_in_file_name, patient_details - ) + assert actual_is_validate_on_historic is False + assert actual_score == ValidationScore.PARTIAL_MATCH @pytest.mark.parametrize( @@ -435,16 +458,15 @@ def test_validate_patient_name_with_two_words_given_name( patient_name_in_file_name: str, should_accept_name: bool, ): + actual_score, actual_is_validate_on_historic = calculate_validation_score( + patient_name_in_file_name, patient_details + ) if should_accept_name: - with expect_not_to_raise(LGInvalidFilesException): - validate_patient_name_using_full_name_history( - patient_name_in_file_name, patient_details - ) + assert actual_is_validate_on_historic is False + assert actual_score == ValidationScore.FULL_MATCH else: - with pytest.raises(LGInvalidFilesException): - validate_patient_name_using_full_name_history( - patient_name_in_file_name, patient_details - ) + assert actual_is_validate_on_historic is False + assert actual_score == ValidationScore.PARTIAL_MATCH @pytest.mark.parametrize( @@ -456,16 +478,15 @@ def test_validate_patient_name_with_two_words_family_name_and_given_name( patient_name_in_file_name: str, should_accept_name: bool, ): + actual_score, actual_is_validate_on_historic = calculate_validation_score( + patient_name_in_file_name, patient_details + ) if should_accept_name: - with expect_not_to_raise(LGInvalidFilesException): - validate_patient_name_using_full_name_history( - patient_name_in_file_name, patient_details - ) + assert actual_is_validate_on_historic is False + assert actual_score == ValidationScore.FULL_MATCH else: - with pytest.raises(LGInvalidFilesException): - validate_patient_name_using_full_name_history( - patient_name_in_file_name, patient_details - ) + assert actual_is_validate_on_historic is False + assert actual_score == ValidationScore.PARTIAL_MATCH def test_missing_middle_name_names_with_pds_service(): @@ -733,70 +754,173 @@ def test_mismatch_nhs_in_validate_lg_file(mocker, mock_pds_patient): @pytest.mark.parametrize( - ["file_patient_name", "first_name_from_pds", "family_name_from_pds"], + ["file_patient_name", "first_name_from_pds", "family_name_from_pds", "result"], [ - ["Jim Stevens", "Jane", "Smith"], - ["Jane Smith Anderson", "Jane", "Smith"], - ["Bob Smith Anderson", "Jane", "Smith"], - ["Jane B Smith Anderson", "Jane", "Smith"], - ["Jane Bob Anderson", "Jane", "Smith"], - ["Jane Bob Smith", "Jane Bob", "Smith Anderson"], - ["Jane Smith", "Jane Bob", "Smith"], - ["Jane B Smith", "Jane Bob", "Smith"], - ["Jane-Bob Smith", "Jane Bob", "Smith"], - ["Jane Smith", "Jane Bob", "Smith"], - ["Jane Smith Anderson", "Jane", "Smith-Anderson"], - ["Jane Smith", "Jane", "Smith-Anderson"], - ["Jane Anderson", "Jane", "Smith-Anderson"], - ["Jane Bob Smith", "Jane Bob", "Smith-Anderson"], - ["Bob Smith Anderson", "Jane", "Smith Anderson"], - ["Jane Smith", "Jane", "Smith Anderson"], - ["Jane Anderson", "Jane", "Smith Anderson"], - ["Jane Anderson Smith", "Jane", "Smith Anderson"], + ("Jim Stevens", ["Jane"], "Smith", ValidationScore.NO_MATCH), + ["Jane Smith Anderson", ["Jane"], "Smith", ValidationScore.FULL_MATCH], + ["Bob Smith Anderson", ["Jane"], "Smith", ValidationScore.PARTIAL_MATCH], + ["Bob Smith Anderson", ["Jane", "Bob"], "Smith", ValidationScore.FULL_MATCH], + ["Jane B Smith Anderson", ["Jane"], "Smith", ValidationScore.FULL_MATCH], + ["Jane Bob Anderson", ["Jane"], "Smith", ValidationScore.PARTIAL_MATCH], + [ + "Jane Bob Smith", + ["Jane Bob"], + "Smith Anderson", + ValidationScore.PARTIAL_MATCH, + ], + ["Jane Smith", ["Jane Bob"], "Smith", ValidationScore.PARTIAL_MATCH], + ["Jane B Smith", ["Jane Bob"], "Smith", ValidationScore.PARTIAL_MATCH], + ["Jane-Bob Smith", ["Jane Bob"], "Smith", ValidationScore.PARTIAL_MATCH], + ["Jane Smith", ["Jane Bob"], "Smith", ValidationScore.PARTIAL_MATCH], + [ + "Jane Smith Anderson", + ["Jane"], + "Smith-Anderson", + ValidationScore.PARTIAL_MATCH, + ], + ["Jane Smith", ["Jane"], "Smith-Anderson", ValidationScore.PARTIAL_MATCH], + ["Jane Anderson", ["Jane"], "Smith-Anderson", ValidationScore.PARTIAL_MATCH], + [ + "Jane Bob Smith", + ["Jane Bob"], + "Smith-Anderson", + ValidationScore.PARTIAL_MATCH, + ], + [ + "Bob Smith Anderson", + ["Jane"], + "Smith Anderson", + ValidationScore.PARTIAL_MATCH, + ], + ["Jane Smith", ["Jane"], "Smith Anderson", ValidationScore.PARTIAL_MATCH], + ["Jane Anderson", ["Jane"], "Smith Anderson", ValidationScore.PARTIAL_MATCH], + [ + "Jane Anderson Smith", + ["Jane"], + "Smith Anderson", + ValidationScore.PARTIAL_MATCH, + ], ], ) def test_validate_patient_name_return_false( - file_patient_name, first_name_from_pds, family_name_from_pds + file_patient_name, first_name_from_pds, family_name_from_pds, result ): actual = validate_patient_name( file_patient_name, first_name_from_pds, family_name_from_pds ) - assert actual is False + assert actual.score == result @pytest.mark.parametrize( - ["file_patient_name", "first_name_from_pds", "family_name_from_pds"], + ["file_patient_name", "first_name_from_pds", "family_name_from_pds", "expected"], [ - ["Jane Smith", "Jane", "Smith"], - ["Jane Bob Smith Anderson", "Jane", "Smith Anderson"], - ["Jane Smith Anderson", "Jane", "Smith Anderson"], - ["Jane B Smith Anderson", "Jane", "Smith Anderson"], - ["Jane Smith-Anderson", "Jane", "Smith-Anderson"], - ["Jane Bob Smith Anderson", "Jane Bob", "Smith Anderson"], - ["Jane Bob Smith", "Jane Bob", "Smith"], + ( + "Jane Smith", + ["Jane"], + "Smith", + ValidationResult( + score=ValidationScore.FULL_MATCH, + given_name_match=["Jane"], + family_name_match="Smith", + ), + ), + ( + "Jane Bob Smith Anderson", + ["Jane"], + "Smith Anderson", + ValidationResult( + score=ValidationScore.FULL_MATCH, + given_name_match=["Jane"], + family_name_match="Smith Anderson", + ), + ), + ( + "Jane Smith Anderson", + ["Jane"], + "Smith Anderson", + ValidationResult( + score=ValidationScore.FULL_MATCH, + given_name_match=["Jane"], + family_name_match="Smith Anderson", + ), + ), + ( + "Jane B Smith Anderson", + ["Jane"], + "Smith Anderson", + ValidationResult( + score=ValidationScore.FULL_MATCH, + given_name_match=["Jane"], + family_name_match="Smith Anderson", + ), + ), + ( + "Jane Smith-Anderson", + ["Jane"], + "Smith-Anderson", + ValidationResult( + score=ValidationScore.FULL_MATCH, + given_name_match=["Jane"], + family_name_match="Smith-Anderson", + ), + ), + ( + "Jane Bob Smith Anderson", + ["Jane Bob"], + "Smith Anderson", + ValidationResult( + score=ValidationScore.FULL_MATCH, + given_name_match=["Jane Bob"], + family_name_match="Smith Anderson", + ), + ), + ( + "Jane Bob Smith", + ["Jane Bob"], + "Smith", + ValidationResult( + score=ValidationScore.FULL_MATCH, + given_name_match=["Jane Bob"], + family_name_match="Smith", + ), + ), ], ) def test_validate_patient_name_return_true( - file_patient_name, first_name_from_pds, family_name_from_pds + file_patient_name, first_name_from_pds, family_name_from_pds, expected ): actual = validate_patient_name( file_patient_name, first_name_from_pds, family_name_from_pds ) - assert actual is True - - -@pytest.fixture -def mock_get_ssm_parameter(mocker): - return mocker.patch.object(SSMService, "get_ssm_parameter") + assert actual == expected -@pytest.fixture -def mock_pds_call(mocker): - yield mocker.patch("services.mock_pds_service.MockPdsApiService.pds_request") +@pytest.mark.parametrize( + ["file_patient_name", "score"], + [ + ("Jane Smith", ValidationScore.FULL_MATCH), + ("Smith Jane", ValidationScore.FULL_MATCH), + ("Jane Bob Smith Moody", ValidationScore.FULL_MATCH), + ("Jane Smith Moody", ValidationScore.FULL_MATCH), + ("Jane B Smith Moody", ValidationScore.FULL_MATCH), + ("Jane Smith-Moody", ValidationScore.FULL_MATCH), + ("Jane Bob Smith Moody", ValidationScore.FULL_MATCH), + ("Jane Bob Smith", ValidationScore.FULL_MATCH), + ("Bob Smith", ValidationScore.PARTIAL_MATCH), + ("Bob Jane", ValidationScore.FULL_MATCH), + ("Alastor Moody", ValidationScore.NO_MATCH), + ("Jones Bob", ValidationScore.MIXED_FULL_MATCH), + ], +) +def test_calculate_validation_score(file_patient_name, score): + name_1 = build_test_name(start="1990-01-01", end=None, given=["Jane"]) + name_2 = build_test_name(start="1995-01-01", end=None, given=["Jane"], family="Bob") + name_3 = build_test_name(use="temp", start=None, end=None, given=["Jones"]) + test_patient = build_test_patient_with_names([name_1, name_2, name_3]) -@pytest.fixture -def mock_fetch_available_document_references_by_type(mocker): - yield mocker.patch.object( - DocumentService, "fetch_available_document_references_by_type" + actual_result, historical = calculate_validation_score( + file_patient_name, test_patient ) + + assert actual_result == score diff --git a/lambdas/utils/lloyd_george_validator.py b/lambdas/utils/lloyd_george_validator.py index d44ff154c..3fa353372 100644 --- a/lambdas/utils/lloyd_george_validator.py +++ b/lambdas/utils/lloyd_george_validator.py @@ -4,9 +4,9 @@ import pydantic import requests -from botocore.exceptions import ClientError from enums.pds_ssm_parameters import SSMParameter from enums.supported_document_types import SupportedDocumentTypes +from enums.validation_score import ValidationResult, ValidationScore from models.nhs_document_reference import NHSDocumentReference from models.pds_models import Patient from requests import HTTPError @@ -18,11 +18,7 @@ PatientRecordAlreadyExistException, PdsTooManyRequestsException, ) -from utils.unicode_utils import ( - REGEX_PATIENT_NAME_PATTERN, - name_ends_with, - name_starts_with, -) +from utils.unicode_utils import REGEX_PATIENT_NAME_PATTERN, name_contains_in from utils.utilities import get_pds_service logger = LoggingService(__name__) @@ -144,60 +140,87 @@ def check_for_file_names_agrees_with_each_other(file_name_list: list[str]): def validate_filename_with_patient_details( file_name_list: list[str], patient_details: Patient -): +) -> (ValidationScore, bool, bool): try: file_name_info = extract_info_from_filename(file_name_list[0]) file_patient_name = file_name_info["patient_name"] file_date_of_birth = file_name_info["date_of_birth"] - validate_patient_date_of_birth(file_date_of_birth, patient_details) - is_name_validation_based_on_historic_name = ( - validate_patient_name_using_full_name_history( - file_patient_name, patient_details - ) + name_validation_score, historical_match = calculate_validation_score( + file_patient_name, patient_details + ) + if name_validation_score == ValidationScore.NO_MATCH: + raise LGInvalidFilesException("Patient name does not match our records") + is_dob_valid = validate_patient_date_of_birth( + file_date_of_birth, patient_details ) - return is_name_validation_based_on_historic_name - except (ClientError, ValueError) as e: + if not is_dob_valid and name_validation_score == ValidationScore.PARTIAL_MATCH: + raise LGInvalidFilesException("Patient name does not match our records 1/3") + return name_validation_score, is_dob_valid, historical_match + + except (ValueError, KeyError) as e: logger.error(e) raise LGInvalidFilesException(e) -def validate_patient_name( - file_patient_name: str, first_name_in_pds: str, family_name_in_pds: str -): - logger.info("Verifying patient name against the record in PDS...") - - first_name_matches = name_starts_with(file_patient_name, first_name_in_pds) - family_name_matches = name_ends_with(file_patient_name, family_name_in_pds) - - if not (first_name_matches and family_name_matches): - return False - return True - - -def validate_patient_name_using_full_name_history( - file_patient_name: str, pds_patient_details: Patient -): - usual_family_name_in_pds, usual_first_name_in_pds = ( - pds_patient_details.get_current_family_name_and_given_name() - ) - - if validate_patient_name( - file_patient_name, usual_first_name_in_pds[0], usual_family_name_in_pds - ): - return False +def calculate_validation_score( + file_patient_name: str, patient_details: Patient +) -> (ValidationScore, bool): + matched_on_given_name = set() + matched_on_family_name = set() + historical_match = False + ordered_names = patient_details.get_names_by_start_date() + for index, name in enumerate(ordered_names): + first_name_in_pds = name.given + family_name_in_pds = name.family + result = validate_patient_name( + file_patient_name, first_name_in_pds, family_name_in_pds + ) + if result.score == ValidationScore.FULL_MATCH: + historical_match = index != 0 + return result.score, historical_match + elif result.score == ValidationScore.PARTIAL_MATCH: + historical_match = index != 0 + matched_on_given_name.update(result.given_name_match) + matched_on_family_name.update(result.family_name_match) logger.info( - "Failed to validate patient name using usual name, trying to validate using name history" + "Failed to find full match on patient name, trying to validate using name history" ) + if matched_on_given_name and matched_on_family_name: + return ValidationScore.MIXED_FULL_MATCH, historical_match + elif matched_on_given_name or matched_on_family_name: + return ValidationScore.PARTIAL_MATCH, historical_match + return ValidationScore.NO_MATCH, False + - for name in pds_patient_details.name: - historic_first_name_in_pds: str = name.given[0] - historic_family_name_in_pds = name.family - if validate_patient_name( - file_patient_name, historic_first_name_in_pds, historic_family_name_in_pds - ): - return True +def validate_patient_name( + file_patient_name: str, first_name_in_pds: list[str], family_name_in_pds: str +) -> ValidationResult: + logger.info("Verifying patient name against the record in PDS...") + given_name_matches = [ + first_name + for first_name in first_name_in_pds + if name_contains_in(file_patient_name, first_name) + ] + family_name_matches = name_contains_in(file_patient_name, family_name_in_pds) - raise LGInvalidFilesException("Patient name does not match our records") + if given_name_matches and family_name_matches: + return ValidationResult( + score=ValidationScore.FULL_MATCH, + given_name_match=given_name_matches, + family_name_match=family_name_in_pds, + ) + elif given_name_matches: + return ValidationResult( + score=ValidationScore.PARTIAL_MATCH, + given_name_match=given_name_matches, + ) + elif family_name_matches: + return ValidationResult( + score=ValidationScore.PARTIAL_MATCH, family_name_match=family_name_in_pds + ) + return ValidationResult( + score=ValidationScore.NO_MATCH, + ) def validate_patient_date_of_birth(file_date_of_birth, pds_patient_details): diff --git a/lambdas/utils/unicode_utils.py b/lambdas/utils/unicode_utils.py index eadc5a660..0634ad853 100644 --- a/lambdas/utils/unicode_utils.py +++ b/lambdas/utils/unicode_utils.py @@ -58,6 +58,13 @@ def name_ends_with(full_name: str, partial_name: str) -> bool: return folded_full_name.endswith(folded_partial_name) +def name_contains_in(full_name: str, partial_name: str) -> bool: + folded_full_name = convert_to_nfd_form(full_name).casefold() + folded_partial_name = convert_to_nfd_form(partial_name).casefold() + + return folded_partial_name in folded_full_name + + def convert_to_nfc_form(input_str: str) -> str: """ Convert a string to the NFC normalization form From 5f4e7eba2e0413e6441e5b05a050ef7eacfb8020 Mon Sep 17 00:00:00 2001 From: NogaNHS Date: Tue, 21 Jan 2025 16:48:21 +0000 Subject: [PATCH 02/13] [PRMP-1376] Change to validate on match regardless of family/given name --- lambdas/enums/validation_score.py | 3 +- .../unit/utils/test_lloyd_george_validator.py | 32 +++++++------------ lambdas/utils/lloyd_george_validator.py | 28 +++++++--------- 3 files changed, 23 insertions(+), 40 deletions(-) diff --git a/lambdas/enums/validation_score.py b/lambdas/enums/validation_score.py index 7fa659aa0..e6648e0f4 100644 --- a/lambdas/enums/validation_score.py +++ b/lambdas/enums/validation_score.py @@ -11,6 +11,5 @@ class ValidationScore(Enum): class ValidationResult(BaseModel): - given_name_match: list[str] = [] - family_name_match: str = "" + name_match: list[str] = [] score: ValidationScore = ValidationScore.NO_MATCH diff --git a/lambdas/tests/unit/utils/test_lloyd_george_validator.py b/lambdas/tests/unit/utils/test_lloyd_george_validator.py index e2b2f4dbb..fa80b3567 100644 --- a/lambdas/tests/unit/utils/test_lloyd_george_validator.py +++ b/lambdas/tests/unit/utils/test_lloyd_george_validator.py @@ -333,8 +333,7 @@ def test_validate_name_with_additional_middle_name_in_file_but_none_in_pds( ) mock_validate_name.return_value = ValidationResult( score=ValidationScore.FULL_MATCH, - given_name_match=["Jane"], - family_name_match="Smith", + name_match=["Jane", "Smith"], ) actual_score, actual_is_name_validation_based_on_historic_name = ( calculate_validation_score(lg_file_patient_name, mock_pds_patient) @@ -355,8 +354,7 @@ def test_validate_name_with_wrong_first_name(mocker, mock_pds_patient): ) assert actual_response == ValidationResult( score=ValidationScore.PARTIAL_MATCH, - given_name_match=[], - family_name_match="Smith", + name_match=["Smith"], ) @@ -369,8 +367,7 @@ def test_validate_name_with_wrong_family_name(mocker, mock_pds_patient): ) assert actual_response == ValidationResult( score=ValidationScore.PARTIAL_MATCH, - given_name_match=["Jane"], - family_name_match="", + name_match=["Jane"], ) @@ -820,8 +817,7 @@ def test_validate_patient_name_return_false( "Smith", ValidationResult( score=ValidationScore.FULL_MATCH, - given_name_match=["Jane"], - family_name_match="Smith", + name_match=["Jane", "Smith"], ), ), ( @@ -830,8 +826,7 @@ def test_validate_patient_name_return_false( "Smith Anderson", ValidationResult( score=ValidationScore.FULL_MATCH, - given_name_match=["Jane"], - family_name_match="Smith Anderson", + name_match=["Jane", "Smith Anderson"], ), ), ( @@ -840,8 +835,7 @@ def test_validate_patient_name_return_false( "Smith Anderson", ValidationResult( score=ValidationScore.FULL_MATCH, - given_name_match=["Jane"], - family_name_match="Smith Anderson", + name_match=["Jane", "Smith Anderson"], ), ), ( @@ -850,8 +844,7 @@ def test_validate_patient_name_return_false( "Smith Anderson", ValidationResult( score=ValidationScore.FULL_MATCH, - given_name_match=["Jane"], - family_name_match="Smith Anderson", + name_match=["Jane", "Smith Anderson"], ), ), ( @@ -860,8 +853,7 @@ def test_validate_patient_name_return_false( "Smith-Anderson", ValidationResult( score=ValidationScore.FULL_MATCH, - given_name_match=["Jane"], - family_name_match="Smith-Anderson", + name_match=["Jane", "Smith-Anderson"], ), ), ( @@ -870,8 +862,7 @@ def test_validate_patient_name_return_false( "Smith Anderson", ValidationResult( score=ValidationScore.FULL_MATCH, - given_name_match=["Jane Bob"], - family_name_match="Smith Anderson", + name_match=["Jane Bob", "Smith Anderson"], ), ), ( @@ -880,8 +871,7 @@ def test_validate_patient_name_return_false( "Smith", ValidationResult( score=ValidationScore.FULL_MATCH, - given_name_match=["Jane Bob"], - family_name_match="Smith", + name_match=["Jane Bob", "Smith"], ), ), ], @@ -906,7 +896,7 @@ def test_validate_patient_name_return_true( ("Jane Smith-Moody", ValidationScore.FULL_MATCH), ("Jane Bob Smith Moody", ValidationScore.FULL_MATCH), ("Jane Bob Smith", ValidationScore.FULL_MATCH), - ("Bob Smith", ValidationScore.PARTIAL_MATCH), + ("Bob Smith", ValidationScore.MIXED_FULL_MATCH), ("Bob Jane", ValidationScore.FULL_MATCH), ("Alastor Moody", ValidationScore.NO_MATCH), ("Jones Bob", ValidationScore.MIXED_FULL_MATCH), diff --git a/lambdas/utils/lloyd_george_validator.py b/lambdas/utils/lloyd_george_validator.py index 3fa353372..600faed69 100644 --- a/lambdas/utils/lloyd_george_validator.py +++ b/lambdas/utils/lloyd_george_validator.py @@ -165,8 +165,7 @@ def validate_filename_with_patient_details( def calculate_validation_score( file_patient_name: str, patient_details: Patient ) -> (ValidationScore, bool): - matched_on_given_name = set() - matched_on_family_name = set() + matched_on = set() historical_match = False ordered_names = patient_details.get_names_by_start_date() for index, name in enumerate(ordered_names): @@ -180,14 +179,13 @@ def calculate_validation_score( return result.score, historical_match elif result.score == ValidationScore.PARTIAL_MATCH: historical_match = index != 0 - matched_on_given_name.update(result.given_name_match) - matched_on_family_name.update(result.family_name_match) + matched_on.update(result.name_match) logger.info( "Failed to find full match on patient name, trying to validate using name history" ) - if matched_on_given_name and matched_on_family_name: + if len(matched_on) > 1: return ValidationScore.MIXED_FULL_MATCH, historical_match - elif matched_on_given_name or matched_on_family_name: + elif len(matched_on) == 1: return ValidationScore.PARTIAL_MATCH, historical_match return ValidationScore.NO_MATCH, False @@ -196,27 +194,23 @@ def validate_patient_name( file_patient_name: str, first_name_in_pds: list[str], family_name_in_pds: str ) -> ValidationResult: logger.info("Verifying patient name against the record in PDS...") - given_name_matches = [ + name_match = [ first_name for first_name in first_name_in_pds if name_contains_in(file_patient_name, first_name) ] - family_name_matches = name_contains_in(file_patient_name, family_name_in_pds) + if name_contains_in(file_patient_name, family_name_in_pds): + name_match.append(family_name_in_pds) - if given_name_matches and family_name_matches: + if len(name_match) > 1: return ValidationResult( score=ValidationScore.FULL_MATCH, - given_name_match=given_name_matches, - family_name_match=family_name_in_pds, + name_match=name_match, ) - elif given_name_matches: + elif len(name_match) == 1: return ValidationResult( score=ValidationScore.PARTIAL_MATCH, - given_name_match=given_name_matches, - ) - elif family_name_matches: - return ValidationResult( - score=ValidationScore.PARTIAL_MATCH, family_name_match=family_name_in_pds + name_match=name_match, ) return ValidationResult( score=ValidationScore.NO_MATCH, From 640130f731a3cf55d8bc0e5bc62460a439665834 Mon Sep 17 00:00:00 2001 From: NogaNHS Date: Mon, 27 Jan 2025 09:50:59 +0000 Subject: [PATCH 03/13] [PRMP-1367] feature flag change, and unit tests --- lambdas/enums/feature_flags.py | 1 + lambdas/enums/validation_score.py | 4 +- lambdas/handlers/bulk_upload_handler.py | 14 +- lambdas/models/pds_models.py | 6 +- lambdas/services/bulk_upload_service.py | 49 +-- .../pds_patient_9000000003_H85686_gp.json | 6 +- lambdas/tests/unit/handlers/conftest.py | 9 + .../unit/handlers/test_bulk_upload_handler.py | 12 +- .../test_cases_for_patient_name_matching.py | 7 + lambdas/tests/unit/models/test_pds_models.py | 10 +- .../unit/services/test_bulk_upload_service.py | 79 ++-- .../unit/utils/test_lloyd_george_validator.py | 413 ++++++++++++++---- lambdas/utils/lloyd_george_validator.py | 158 +++++-- 13 files changed, 572 insertions(+), 196 deletions(-) diff --git a/lambdas/enums/feature_flags.py b/lambdas/enums/feature_flags.py index 1d6b6ae18..898e5c20c 100755 --- a/lambdas/enums/feature_flags.py +++ b/lambdas/enums/feature_flags.py @@ -7,3 +7,4 @@ class FeatureFlags(Enum): UPLOAD_LAMBDA_ENABLED = "uploadLambdaEnabled" UPLOAD_ARF_WORKFLOW_ENABLED = "uploadArfWorkflowEnabled" USE_SMARTCARD_AUTH = "useSmartcardAuth" + LLOYD_GEORGE_VALIDATION_STRICT_MODE = "lloydGeorgeValidationStrictMode" diff --git a/lambdas/enums/validation_score.py b/lambdas/enums/validation_score.py index e6648e0f4..c20986b9a 100644 --- a/lambdas/enums/validation_score.py +++ b/lambdas/enums/validation_score.py @@ -1,4 +1,5 @@ from enum import Enum +from typing import Optional from pydantic import BaseModel @@ -11,5 +12,6 @@ class ValidationScore(Enum): class ValidationResult(BaseModel): - name_match: list[str] = [] + given_name_match: list[str] = [] + family_name_match: Optional[str] = None score: ValidationScore = ValidationScore.NO_MATCH diff --git a/lambdas/handlers/bulk_upload_handler.py b/lambdas/handlers/bulk_upload_handler.py index dc51de17c..874be63d9 100644 --- a/lambdas/handlers/bulk_upload_handler.py +++ b/lambdas/handlers/bulk_upload_handler.py @@ -1,4 +1,6 @@ +from enums.feature_flags import FeatureFlags from services.bulk_upload_service import BulkUploadService +from services.feature_flags_service import FeatureFlagService from utils.audit_logging_setup import LoggingService from utils.decorators.handle_lambda_exceptions import handle_lambda_exceptions from utils.decorators.override_error_check import override_error_check @@ -14,6 +16,16 @@ @handle_lambda_exceptions def lambda_handler(event, _context): logger.info("Received event. Starting bulk upload process") + feature_flag_service = FeatureFlagService() + validation_strict_mode_flag_object = feature_flag_service.get_feature_flags_by_flag( + FeatureFlags.LLOYD_GEORGE_VALIDATION_STRICT_MODE.value + ) + validation_strict_mode = validation_strict_mode_flag_object[ + FeatureFlags.LLOYD_GEORGE_VALIDATION_STRICT_MODE.value + ] + + if validation_strict_mode: + logger.info("Lloyd George validation strict mose is enabled") if "Records" not in event or len(event["Records"]) < 1: http_status_code = 400 @@ -25,7 +37,7 @@ def lambda_handler(event, _context): status_code=http_status_code, body=response_body, methods="GET" ).create_api_gateway_response() - bulk_upload_service = BulkUploadService() + bulk_upload_service = BulkUploadService(strict_mode=validation_strict_mode) try: bulk_upload_service.process_message_queue(event["Records"]) diff --git a/lambdas/models/pds_models.py b/lambdas/models/pds_models.py index 81abdcb75..7ae13181c 100644 --- a/lambdas/models/pds_models.py +++ b/lambdas/models/pds_models.py @@ -117,12 +117,8 @@ def get_usual_name(self) -> Optional[Name]: return entry def get_names_by_start_date(self) -> [Name]: - active_names = [name for name in self.name] - if not active_names: - return None - sorted_by_start_date_desc = sorted( - active_names, + self.name, key=lambda name: ( (1, name.period.start, name.use.lower() == "usual") if name.period and name.period.start is not None diff --git a/lambdas/services/bulk_upload_service.py b/lambdas/services/bulk_upload_service.py index 84cee1774..ab1261ed6 100644 --- a/lambdas/services/bulk_upload_service.py +++ b/lambdas/services/bulk_upload_service.py @@ -8,7 +8,6 @@ from enums.patient_ods_inactive_status import PatientOdsInactiveStatus from enums.snomed_codes import SnomedCodes from enums.upload_status import UploadStatus -from enums.validation_score import ValidationScore from enums.virus_scan_result import VirusScanResult from models.fhir.R4.nrl_fhir_document_reference import Attachment from models.nhs_document_reference import NHSDocumentReference @@ -35,7 +34,8 @@ LGInvalidFilesException, allowed_to_ingest_ods_code, getting_patient_info_from_pds, - validate_filename_with_patient_details, + validate_filename_with_patient_details_lenient, + validate_filename_with_patient_details_strict, validate_lg_file_names, ) from utils.request_context import request_context @@ -49,11 +49,11 @@ class BulkUploadService: - def __init__(self): + def __init__(self, strict_mode): self.dynamo_repository = BulkUploadDynamoRepository() self.sqs_repository = BulkUploadSqsRepository() self.s3_repository = BulkUploadS3Repository() - + self.strict_mode = strict_mode self.pdf_content_type = "application/pdf" self.unhandled_messages = [] self.file_path_cache = {} @@ -134,34 +134,27 @@ def handle_sqs_message(self, message: dict): patient_ods_code = ( pds_patient_details.get_ods_code_or_inactive_status_for_gp() ) - ( - name_validation_score, - is_dob_valid, - is_name_validation_based_on_historic_name, - ) = validate_filename_with_patient_details(file_names, pds_patient_details) - if is_name_validation_based_on_historic_name: + if not self.strict_mode: + ( + name_validation_accepted_reason, + is_name_validation_based_on_historic_name, + ) = validate_filename_with_patient_details_lenient( + file_names, pds_patient_details + ) accepted_reason = self.concatenate_acceptance_reason( - accepted_reason, "Patient matched on historical name" + accepted_reason, name_validation_accepted_reason ) - validation_messages = { - ValidationScore.PARTIAL_MATCH: { - True: "Patient matched on partial match 2/3", - False: "", - }, - ValidationScore.MIXED_FULL_MATCH: { - True: "Patient matched on mixed match 3/3", - False: "Patient matched on mixed match 2/3", - }, - ValidationScore.FULL_MATCH: { - True: "Patient matched on full match 3/3", - False: "Patient matched on full match 2/3", - }, - } - if name_validation_score in validation_messages: - message = validation_messages[name_validation_score][is_dob_valid] + else: + is_name_validation_based_on_historic_name = ( + validate_filename_with_patient_details_strict( + file_names, pds_patient_details + ) + ) + if is_name_validation_based_on_historic_name: accepted_reason = self.concatenate_acceptance_reason( - accepted_reason, message + accepted_reason, "Patient matched on historical name" ) + if not allowed_to_ingest_ods_code(patient_ods_code): raise LGInvalidFilesException("Patient not registered at your practice") patient_death_notification_status = ( diff --git a/lambdas/services/mock_data/pds_patient_9000000003_H85686_gp.json b/lambdas/services/mock_data/pds_patient_9000000003_H85686_gp.json index e0f65ef4f..99abc0a28 100644 --- a/lambdas/services/mock_data/pds_patient_9000000003_H85686_gp.json +++ b/lambdas/services/mock_data/pds_patient_9000000003_H85686_gp.json @@ -38,7 +38,7 @@ "use": "usual", "period": { "start": "2020-01-01", - "end": "2025-12-31" + "end": "2035-12-31" }, "given": [ "Jane" @@ -55,8 +55,8 @@ "id": "1234", "use": "other", "period": { - "start": "2020-01-01", - "end": "2025-12-31" + "start": "2010-01-01", + "end": "2019-12-31" }, "given": [ "Jim" diff --git a/lambdas/tests/unit/handlers/conftest.py b/lambdas/tests/unit/handlers/conftest.py index d5fde1ef9..a9a82cfc4 100755 --- a/lambdas/tests/unit/handlers/conftest.py +++ b/lambdas/tests/unit/handlers/conftest.py @@ -111,3 +111,12 @@ def mock_upload_lambda_disabled(mocker): "uploadLambdaEnabled": False } yield mock_upload_lambda_feature_flag + + +@pytest.fixture +def mock_validation_strict_disabled(mocker): + mock_function = mocker.patch.object(FeatureFlagService, "get_feature_flags_by_flag") + mock_upload_lambda_feature_flag = mock_function.return_value = { + "lloydGeorgeValidationStrictMode": False + } + yield mock_upload_lambda_feature_flag diff --git a/lambdas/tests/unit/handlers/test_bulk_upload_handler.py b/lambdas/tests/unit/handlers/test_bulk_upload_handler.py index e0b51c408..9a6419600 100644 --- a/lambdas/tests/unit/handlers/test_bulk_upload_handler.py +++ b/lambdas/tests/unit/handlers/test_bulk_upload_handler.py @@ -15,7 +15,9 @@ def mock_service(mocker): yield mocker.patch.object(BulkUploadService, "process_message_queue") -def test_can_process_event_with_one_message(mock_service, context, set_env): +def test_can_process_event_with_one_message( + mock_service, context, set_env, mock_validation_strict_disabled +): expected = ApiGatewayResponse( 200, "Finished processing all 1 messages", "GET" ).create_api_gateway_response() @@ -24,7 +26,9 @@ def test_can_process_event_with_one_message(mock_service, context, set_env): assert expected == actual -def test_can_process_event_with_several_messages(mock_service, context, set_env): +def test_can_process_event_with_several_messages( + mock_service, context, set_env, mock_validation_strict_disabled +): expected = ApiGatewayResponse( 200, "Finished processing all 3 messages", "GET" ).create_api_gateway_response() @@ -34,7 +38,7 @@ def test_can_process_event_with_several_messages(mock_service, context, set_env) def test_receive_correct_response_when_service_returns_error( - mock_service, context, set_env + mock_service, context, set_env, mock_validation_strict_disabled ): expected = ApiGatewayResponse( 500, "Bulk upload failed with error: ", "GET" @@ -46,7 +50,7 @@ def test_receive_correct_response_when_service_returns_error( def test_receive_correct_response_when_no_records_in_event( - mock_service, context, set_env + mock_service, context, set_env, mock_validation_strict_disabled ): expected = ApiGatewayResponse( 400, diff --git a/lambdas/tests/unit/helpers/data/pds/test_cases_for_patient_name_matching.py b/lambdas/tests/unit/helpers/data/pds/test_cases_for_patient_name_matching.py index bbbb55e67..559162f8e 100644 --- a/lambdas/tests/unit/helpers/data/pds/test_cases_for_patient_name_matching.py +++ b/lambdas/tests/unit/helpers/data/pds/test_cases_for_patient_name_matching.py @@ -27,6 +27,13 @@ class PdsNameMatchingTestCase(NamedTuple): ], } +TEST_CASES_FOR_TWO_WORDS_FAMILY_NAME_STRICT = copy.deepcopy( + TEST_CASES_FOR_TWO_WORDS_FAMILY_NAME +) +TEST_CASES_FOR_TWO_WORDS_FAMILY_NAME_STRICT["reject"].append( + TEST_CASES_FOR_TWO_WORDS_FAMILY_NAME_STRICT["accept"].pop(3) +) + TEST_CASES_FOR_FAMILY_NAME_WITH_HYPHEN = { "pds_name": {"family": "Smith-Anderson", "given": ["Jane"]}, "accept": ["Jane Smith-Anderson"], diff --git a/lambdas/tests/unit/models/test_pds_models.py b/lambdas/tests/unit/models/test_pds_models.py index 9f7357349..66254fe7c 100644 --- a/lambdas/tests/unit/models/test_pds_models.py +++ b/lambdas/tests/unit/models/test_pds_models.py @@ -374,11 +374,11 @@ def test_get_names_by_start_date_return_ordered_list_by_start_date(): assert actual == expected -@freeze_time("2024-01-01") -def test_get_most_recent_name_return_none_if_no_active_name_found(): - test_patient = build_test_patient_with_names([]) - - assert test_patient.get_names_by_start_date() is None +# @freeze_time("2024-01-01") +# def test_get_most_recent_name_return_none_if_no_active_name_found(): +# test_patient = build_test_patient_with_names([]) +# +# assert test_patient.get_names_by_start_date() is None def test_get_death_notification_status_return_the_death_notification_status(): diff --git a/lambdas/tests/unit/services/test_bulk_upload_service.py b/lambdas/tests/unit/services/test_bulk_upload_service.py index d9df0f882..0e1cc1478 100644 --- a/lambdas/tests/unit/services/test_bulk_upload_service.py +++ b/lambdas/tests/unit/services/test_bulk_upload_service.py @@ -7,7 +7,6 @@ from enums.patient_ods_inactive_status import PatientOdsInactiveStatus from enums.snomed_codes import SnomedCodes from enums.upload_status import UploadStatus -from enums.validation_score import ValidationScore from enums.virus_scan_result import SCAN_RESULT_TAG_KEY, VirusScanResult from freezegun import freeze_time from models.fhir.R4.nrl_fhir_document_reference import Attachment @@ -63,7 +62,7 @@ @pytest.fixture def repo_under_test(set_env, mocker): - service = BulkUploadService() + service = BulkUploadService(strict_mode=True) mocker.patch.object(service, "dynamo_repository") mocker.patch.object(service, "sqs_repository") mocker.patch.object(service, "s3_repository") @@ -77,8 +76,7 @@ def mock_check_virus_result(mocker): @pytest.fixture def mock_validate_files(mocker): - validate_files = mocker.patch("services.bulk_upload_service.validate_lg_file_names") - yield validate_files + yield mocker.patch("services.bulk_upload_service.validate_lg_file_names") @pytest.fixture @@ -122,10 +120,17 @@ def mock_pds_service_patient_restricted(mocker): @pytest.fixture -def mock_pds_validation(mocker): +def mock_pds_validation_lenient(mocker): yield mocker.patch( - "services.bulk_upload_service.validate_filename_with_patient_details", - return_value=(ValidationScore.FULL_MATCH, True, True), + "services.bulk_upload_service.validate_filename_with_patient_details_lenient", + return_value=("test string", True), + ) + + +@pytest.fixture +def mock_pds_validation_strict(mocker): + yield mocker.patch( + "services.bulk_upload_service.validate_filename_with_patient_details_strict", ) @@ -153,7 +158,7 @@ def build_resolved_file_names_cache( def test_lambda_handler_process_each_sqs_message_one_by_one( set_env, mock_handle_sqs_message ): - service = BulkUploadService() + service = BulkUploadService(True) service.process_message_queue(TEST_SQS_MESSAGES_AS_LIST) @@ -171,7 +176,7 @@ def test_lambda_handler_continue_process_next_message_after_handled_error( InvalidMessageException, None, ] - service = BulkUploadService() + service = BulkUploadService(True) service.process_message_queue(TEST_SQS_MESSAGES_AS_LIST) assert mock_handle_sqs_message.call_count == len(TEST_SQS_MESSAGES_AS_LIST) @@ -188,7 +193,7 @@ def test_lambda_handler_handle_pds_too_many_requests_exception( expected_handled_messages = TEST_SQS_10_MESSAGES_AS_LIST[0:6] expected_unhandled_message = TEST_SQS_10_MESSAGES_AS_LIST[6:] - service = BulkUploadService() + service = BulkUploadService(True) with pytest.raises(BulkUploadException): service.process_message_queue(TEST_SQS_10_MESSAGES_AS_LIST) @@ -208,7 +213,7 @@ def test_handle_sqs_message_happy_path( repo_under_test, mock_validate_files, mock_pds_service, - mock_pds_validation, + mock_pds_validation_strict, mock_ods_validation, ): TEST_STAGING_METADATA.retries = 0 @@ -240,7 +245,7 @@ def test_handle_sqs_message_happy_path_single_file( repo_under_test, mock_validate_files, mock_pds_service, - mock_pds_validation, + mock_pds_validation_strict, mock_ods_validation, ): TEST_STAGING_METADATA.retries = 0 @@ -337,7 +342,7 @@ def test_handle_sqs_message_happy_path_with_non_ascii_filenames( mock_validate_files, patient_name_on_s3, patient_name_in_metadata_file, - mock_pds_validation, + mock_pds_validation_strict, mock_pds_service, mock_ods_validation, ): @@ -363,7 +368,7 @@ def test_handle_sqs_message_calls_report_upload_failure_when_patient_record_alre mock_uuid, mock_validate_files, mock_pds_service, - mock_pds_validation, + mock_pds_validation_strict, ): TEST_STAGING_METADATA.retries = 0 @@ -398,7 +403,7 @@ def test_handle_sqs_message_calls_report_upload_failure_when_lg_file_name_invali mock_uuid, mock_validate_files, mock_pds_service, - mock_pds_validation, + mock_pds_validation_strict, ): TEST_STAGING_METADATA.retries = 0 mock_create_lg_records_and_copy_files = mocker.patch.object( @@ -436,7 +441,7 @@ def test_handle_sqs_message_report_failure_when_document_is_infected( mock_validate_files, mock_check_virus_result, mock_pds_service, - mock_pds_validation, + mock_pds_validation_strict, mock_ods_validation, ): TEST_STAGING_METADATA.retries = 0 @@ -474,7 +479,7 @@ def test_handle_sqs_message_report_failure_when_document_not_exist( mock_validate_files, mock_check_virus_result, mock_pds_service, - mock_pds_validation, + mock_pds_validation_strict, mock_ods_validation, ): TEST_STAGING_METADATA.retries = 0 @@ -504,7 +509,7 @@ def test_handle_sqs_message_calls_report_upload_successful_when_patient_is_forma mock_validate_files, mock_check_virus_result, mock_pds_service_patient_deceased_formal, - mock_pds_validation, + mock_pds_validation_strict, mock_ods_validation, ): mock_create_lg_records_and_copy_files = mocker.patch.object( @@ -513,7 +518,7 @@ def test_handle_sqs_message_calls_report_upload_successful_when_patient_is_forma mock_remove_ingested_file_from_source_bucket = ( repo_under_test.s3_repository.remove_ingested_file_from_source_bucket ) - mock_pds_validation.return_value = (ValidationScore.FULL_MATCH, True, False) + mock_pds_validation_strict.return_value = False mock_put_staging_metadata_back_to_queue = ( repo_under_test.sqs_repository.put_staging_metadata_back_to_queue ) @@ -528,7 +533,7 @@ def test_handle_sqs_message_calls_report_upload_successful_when_patient_is_forma mock_report_upload.assert_called_with( TEST_STAGING_METADATA, UploadStatus.COMPLETE, - "Patient matched on full match 3/3, Patient is deceased - FORMAL", + "Patient is deceased - FORMAL", PatientOdsInactiveStatus.DECEASED, ) @@ -541,13 +546,13 @@ def test_handle_sqs_message_calls_report_upload_successful_when_patient_is_infor mock_validate_files, mock_check_virus_result, mock_pds_service_patient_deceased_informal, - mock_pds_validation, + mock_pds_validation_strict, mock_ods_validation, ): mock_create_lg_records_and_copy_files = mocker.patch.object( BulkUploadService, "create_lg_records_and_copy_files" ) - mock_pds_validation.return_value = (ValidationScore.MIXED_FULL_MATCH, True, True) + mock_pds_validation_strict.return_value = True mock_remove_ingested_file_from_source_bucket = ( repo_under_test.s3_repository.remove_ingested_file_from_source_bucket ) @@ -565,7 +570,7 @@ def test_handle_sqs_message_calls_report_upload_successful_when_patient_is_infor mock_report_upload.assert_called_with( TEST_STAGING_METADATA, UploadStatus.COMPLETE, - "Patient matched on historical name, Patient matched on mixed match 3/3, Patient is deceased - INFORMAL", + "Patient matched on historical name, Patient is deceased - INFORMAL", "Y12345", ) @@ -578,13 +583,13 @@ def test_handle_sqs_message_calls_report_upload_successful_when_patient_has_hist mock_validate_files, mock_check_virus_result, mock_pds_service_patient_restricted, - mock_pds_validation, + mock_pds_validation_strict, mock_ods_validation, ): mock_create_lg_records_and_copy_files = mocker.patch.object( BulkUploadService, "create_lg_records_and_copy_files" ) - mock_pds_validation.return_value = (ValidationScore.MIXED_FULL_MATCH, True, True) + mock_pds_validation_strict.return_value = True mock_remove_ingested_file_from_source_bucket = ( repo_under_test.s3_repository.remove_ingested_file_from_source_bucket ) @@ -602,7 +607,7 @@ def test_handle_sqs_message_calls_report_upload_successful_when_patient_has_hist mock_report_upload.assert_called_with( TEST_STAGING_METADATA, UploadStatus.COMPLETE, - "Patient matched on historical name, Patient matched on mixed match 3/3, PDS record is restricted", + "Patient matched on historical name, PDS record is restricted", "REST", ) @@ -615,13 +620,13 @@ def test_handle_sqs_message_calls_report_upload_successful_when_patient_is_infor mock_validate_files, mock_check_virus_result, mock_pds_service_patient_deceased_informal, - mock_pds_validation, + mock_pds_validation_strict, mock_ods_validation, ): mock_create_lg_records_and_copy_files = mocker.patch.object( BulkUploadService, "create_lg_records_and_copy_files" ) - mock_pds_validation.return_value = (ValidationScore.PARTIAL_MATCH, True, False) + mock_pds_validation_strict.return_value = False mock_remove_ingested_file_from_source_bucket = ( repo_under_test.s3_repository.remove_ingested_file_from_source_bucket ) @@ -639,7 +644,7 @@ def test_handle_sqs_message_calls_report_upload_successful_when_patient_is_infor mock_report_upload.assert_called_with( TEST_STAGING_METADATA, UploadStatus.COMPLETE, - "Patient matched on partial match 2/3, Patient is deceased - INFORMAL", + "Patient is deceased - INFORMAL", "Y12345", ) @@ -652,7 +657,7 @@ def test_handle_sqs_message_put_staging_metadata_back_to_queue_when_virus_scan_r mock_validate_files, mock_check_virus_result, mock_pds_service, - mock_pds_validation, + mock_pds_validation_strict, mock_ods_validation, ): TEST_STAGING_METADATA.retries = 0 @@ -690,7 +695,7 @@ def test_handle_sqs_message_rollback_transaction_when_validation_pass_but_file_t mock_check_virus_result, mock_validate_files, mock_pds_service, - mock_pds_validation, + mock_pds_validation_strict, mock_ods_validation, ): TEST_STAGING_METADATA.retries = 0 @@ -893,7 +898,7 @@ def test_raise_client_error_from_ssm_with_pds_service( repo_under_test, mock_validate_files, mock_pds_service, - mock_pds_validation, + mock_pds_validation_strict, ): mock_client_error = ClientError( {"Error": {"Code": "500", "Message": "test error"}}, "testing" @@ -910,7 +915,7 @@ def test_mismatch_ods_with_pds_service( mock_uuid, mock_validate_files, mock_pds_service, - mock_pds_validation, + mock_pds_validation_strict, ): mock_ods_validation.return_value = False @@ -932,7 +937,7 @@ def test_create_lg_records_and_copy_files_client_error( mock_check_virus_result, mock_validate_files, mock_pds_service, - mock_pds_validation, + mock_pds_validation_strict, mock_ods_validation, ): TEST_STAGING_METADATA.retries = 0 @@ -968,7 +973,7 @@ def test_handle_sqs_message_happy_path_historical_name( repo_under_test, mock_validate_files, mock_pds_service, - mock_pds_validation, + mock_pds_validation_strict, mock_ods_validation, ): TEST_STAGING_METADATA.retries = 0 @@ -982,7 +987,7 @@ def test_handle_sqs_message_happy_path_historical_name( repo_under_test.s3_repository, "remove_ingested_file_from_source_bucket" ) mocker.patch.object(repo_under_test.s3_repository, "check_virus_result") - mock_pds_validation.return_value = (ValidationScore.FULL_MATCH, True, True) + mock_pds_validation_strict.return_value = True repo_under_test.handle_sqs_message(message=TEST_SQS_MESSAGE) @@ -993,7 +998,7 @@ def test_handle_sqs_message_happy_path_historical_name( mock_report_upload_complete.assert_called_with( TEST_STAGING_METADATA, UploadStatus.COMPLETE, - "Patient matched on historical name, Patient matched on full match 3/3", + "Patient matched on historical name", "Y12345", ) mock_remove_ingested_file_from_source_bucket.assert_called() diff --git a/lambdas/tests/unit/utils/test_lloyd_george_validator.py b/lambdas/tests/unit/utils/test_lloyd_george_validator.py index fa80b3567..239a746be 100644 --- a/lambdas/tests/unit/utils/test_lloyd_george_validator.py +++ b/lambdas/tests/unit/utils/test_lloyd_george_validator.py @@ -23,6 +23,7 @@ TEST_CASES_FOR_FAMILY_NAME_WITH_HYPHEN, TEST_CASES_FOR_TWO_WORDS_FAMILY_NAME, TEST_CASES_FOR_TWO_WORDS_FAMILY_NAME_AND_GIVEN_NAME, + TEST_CASES_FOR_TWO_WORDS_FAMILY_NAME_STRICT, TEST_CASES_FOR_TWO_WORDS_GIVEN_NAME, load_test_cases, ) @@ -46,12 +47,14 @@ getting_patient_info_from_pds, parse_pds_response, validate_file_name, - validate_filename_with_patient_details, + validate_filename_with_patient_details_lenient, + validate_filename_with_patient_details_strict, validate_lg_file_names, validate_lg_file_type, validate_lg_files, validate_patient_date_of_birth, - validate_patient_name, + validate_patient_name_lenient, + validate_patient_name_using_full_name_history, ) @@ -256,7 +259,10 @@ def test_validate_nhs_id_with_pds_service(mock_pds_patient): "2of2_Lloyd_George_Record_[Jane Smith]_[9000000009]_[22-10-2010].pdf", ] with expect_not_to_raise(LGInvalidFilesException): - validate_filename_with_patient_details(lg_file_list, mock_pds_patient) + validate_filename_with_patient_details_lenient(lg_file_list, mock_pds_patient) + + with expect_not_to_raise(LGInvalidFilesException): + validate_filename_with_patient_details_strict(lg_file_list, mock_pds_patient) def test_mismatch_nhs_id(mocker): @@ -269,33 +275,42 @@ def test_mismatch_nhs_id(mocker): mocker.patch( "utils.lloyd_george_validator.check_for_number_of_files_match_expected" ) - mocker.patch("utils.lloyd_george_validator.validate_file_name") with pytest.raises(LGInvalidFilesException): validate_lg_file_names(lg_file_list, "9000000009") -def test_mismatch_name_with_pds_service(mock_pds_patient): +def test_mismatch_name_with_pds_service_strict(mock_pds_patient): lg_file_list = [ "1of2_Lloyd_George_Record_[Jake Plain Smith]_[9000000009]_[22-10-2010].pdf", "2of2_Lloyd_George_Record_[Jake Plain Smith]_[9000000009]_[22-10-2010].pdf", ] with pytest.raises(LGInvalidFilesException): - validate_filename_with_patient_details(lg_file_list, mock_pds_patient) + validate_filename_with_patient_details_strict(lg_file_list, mock_pds_patient) + + +def test_mismatch_name_with_pds_service_lenient(mock_pds_patient): + lg_file_list = [ + "1of2_Lloyd_George_Record_[Jake Plain Moody]_[9000000009]_[22-10-2010].pdf", + "2of2_Lloyd_George_Record_[Jake Plain Moody]_[9000000009]_[22-10-2010].pdf", + ] + + with pytest.raises(LGInvalidFilesException): + validate_filename_with_patient_details_lenient(lg_file_list, mock_pds_patient) -def test_validate_name_with_correct_name(mocker, mock_pds_patient): +def test_validate_name_with_correct_name_lenient(mocker, mock_pds_patient): lg_file_patient_name = "Jane Smith" mock_validate_name = mocker.patch( - "utils.lloyd_george_validator.validate_patient_name" + "utils.lloyd_george_validator.validate_patient_name_lenient" ) mock_validate_name.return_value = ValidationResult( score=ValidationScore.FULL_MATCH, given_name_match=["Jane"], family_name_match="Smith", ) - actual_score, actual_is_name_validation_based_on_historic_name = ( + actual_score, actual_is_name_validation_based_on_historic_name, result_message = ( calculate_validation_score(lg_file_patient_name, mock_pds_patient) ) @@ -304,10 +319,52 @@ def test_validate_name_with_correct_name(mocker, mock_pds_patient): assert actual_score == ValidationScore.FULL_MATCH -def test_validate_name_with_additional_middle_name_in_file_mismatching_pds(mocker): +def test_validate_name_with_correct_name_strict(mocker, mock_pds_patient): + lg_file_patient_name = "Jane Smith" + mock_validate_name = mocker.patch( + "utils.lloyd_george_validator.validate_patient_name_strict" + ) + with expect_not_to_raise(LGInvalidFilesException): + validate_patient_name_using_full_name_history( + lg_file_patient_name, mock_pds_patient + ) + assert mock_validate_name.call_count == 1 + + +def test_validate_name_with_file_missing_middle_name(mocker): + lg_file_patient_name = "Jane Smith" + patient = Patient.model_validate(PDS_PATIENT_WITH_MIDDLE_NAME) + mock_validate_name = mocker.patch( + "utils.lloyd_george_validator.validate_patient_name_strict" + ) + with expect_not_to_raise(LGInvalidFilesException): + validate_patient_name_using_full_name_history(lg_file_patient_name, patient) + + assert mock_validate_name.call_count == 1 + + +def test_validate_name_with_additional_middle_name_in_file_mismatching_pds_strict( + mocker, +): + lg_file_patient_name = "Jane David Smith" + mock_validate_name = mocker.patch( + "utils.lloyd_george_validator.validate_patient_name_strict" + ) + patient = Patient.model_validate(PDS_PATIENT_WITH_MIDDLE_NAME) + with expect_not_to_raise(LGInvalidFilesException): + actual_is_name_validation_based_on_historic_name = ( + validate_patient_name_using_full_name_history(lg_file_patient_name, patient) + ) + assert mock_validate_name.call_count == 1 + assert actual_is_name_validation_based_on_historic_name is False + + +def test_validate_name_with_additional_middle_name_in_file_mismatching_pds_lenient( + mocker, +): lg_file_patient_name = "Jane David Smith" mock_validate_name = mocker.patch( - "utils.lloyd_george_validator.validate_patient_name" + "utils.lloyd_george_validator.validate_patient_name_lenient" ) patient = Patient.model_validate(PDS_PATIENT_WITH_MIDDLE_NAME) mock_validate_name.return_value = ValidationResult( @@ -315,7 +372,7 @@ def test_validate_name_with_additional_middle_name_in_file_mismatching_pds(mocke given_name_match=["Jane"], family_name_match="Smith", ) - actual_score, actual_is_name_validation_based_on_historic_name = ( + actual_score, actual_is_name_validation_based_on_historic_name, result_message = ( calculate_validation_score(lg_file_patient_name, patient) ) @@ -324,18 +381,37 @@ def test_validate_name_with_additional_middle_name_in_file_mismatching_pds(mocke assert actual_score == ValidationScore.FULL_MATCH +def test_validate_name_with_additional_middle_name_in_file_but_none_in_pds_strict( + mock_pds_patient, mocker +): + lg_file_patient_name = "Jane David Smith" + mock_validate_name = mocker.patch( + "utils.lloyd_george_validator.validate_patient_name_strict" + ) + + actual_is_name_validation_based_on_historic_name = ( + validate_patient_name_using_full_name_history( + lg_file_patient_name, mock_pds_patient + ) + ) + + assert mock_validate_name.call_count == 1 + assert actual_is_name_validation_based_on_historic_name is False + + def test_validate_name_with_additional_middle_name_in_file_but_none_in_pds( mock_pds_patient, mocker ): lg_file_patient_name = "Jane David Smith" mock_validate_name = mocker.patch( - "utils.lloyd_george_validator.validate_patient_name" + "utils.lloyd_george_validator.validate_patient_name_lenient" ) mock_validate_name.return_value = ValidationResult( score=ValidationScore.FULL_MATCH, - name_match=["Jane", "Smith"], + given_name_match=["Jane"], + family_name_match="Smith", ) - actual_score, actual_is_name_validation_based_on_historic_name = ( + actual_score, actual_is_name_validation_based_on_historic_name, result_message = ( calculate_validation_score(lg_file_patient_name, mock_pds_patient) ) @@ -344,37 +420,80 @@ def test_validate_name_with_additional_middle_name_in_file_but_none_in_pds( assert actual_score == ValidationScore.FULL_MATCH -def test_validate_name_with_wrong_first_name(mocker, mock_pds_patient): +def test_validate_name_with_wrong_first_name_strict(mocker, mock_pds_patient): + lg_file_patient_name = "John Smith" + mock_validate_name = mocker.patch( + "utils.lloyd_george_validator.validate_patient_name_strict" + ) + mock_validate_name.return_value = False + with pytest.raises(LGInvalidFilesException): + validate_patient_name_using_full_name_history( + lg_file_patient_name, mock_pds_patient + ) + assert mock_validate_name.call_count == 3 + + +def test_validate_name_with_wrong_first_name_lenient(mock_pds_patient): lg_file_patient_name = "John Smith" - actual_response = validate_patient_name( + actual_response = validate_patient_name_lenient( lg_file_patient_name, mock_pds_patient.name[0].given, mock_pds_patient.name[0].family, ) assert actual_response == ValidationResult( score=ValidationScore.PARTIAL_MATCH, - name_match=["Smith"], + family_name_match="Smith", + ) + + +def test_validate_name_with_wrong_family_name_strict(mocker, mock_pds_patient): + lg_file_patient_name = "John Johnson" + mock_validate_name = mocker.patch( + "utils.lloyd_george_validator.validate_patient_name_strict" ) + mock_validate_name.return_value = False + with pytest.raises(LGInvalidFilesException): + validate_patient_name_using_full_name_history( + lg_file_patient_name, mock_pds_patient + ) + assert mock_validate_name.call_count == 3 -def test_validate_name_with_wrong_family_name(mocker, mock_pds_patient): +def test_validate_name_with_wrong_family_name_lenient(mock_pds_patient): lg_file_patient_name = "Jane Johnson" - actual_response = validate_patient_name( + actual_response = validate_patient_name_lenient( lg_file_patient_name, mock_pds_patient.name[0].given, mock_pds_patient.name[0].family, ) assert actual_response == ValidationResult( score=ValidationScore.PARTIAL_MATCH, - name_match=["Jane"], + given_name_match=["Jane"], ) -def test_validate_name_with_historical_name(mocker, mock_pds_patient): +def test_validate_name_with_historical_name_strict(mocker, mock_pds_patient): lg_file_patient_name = "Jim Stevens" mock_validate_name = mocker.patch( - "utils.lloyd_george_validator.validate_patient_name" + "utils.lloyd_george_validator.validate_patient_name_strict" + ) + mock_validate_name.side_effect = [False, True] + with expect_not_to_raise(LGInvalidFilesException): + actual_is_name_validation_based_on_historic_name = ( + validate_patient_name_using_full_name_history( + lg_file_patient_name, mock_pds_patient + ) + ) + + assert mock_validate_name.call_count == 2 + assert actual_is_name_validation_based_on_historic_name is True + + +def test_validate_name_with_historical_name_lenient(mocker, mock_pds_patient): + lg_file_patient_name = "Jim Stevens" + mock_validate_name = mocker.patch( + "utils.lloyd_george_validator.validate_patient_name_lenient" ) mock_validate_name.side_effect = [ ValidationResult( @@ -384,39 +503,79 @@ def test_validate_name_with_historical_name(mocker, mock_pds_patient): score=ValidationScore.FULL_MATCH, ), ] - actual_score, actual_is_validate_on_historic = calculate_validation_score( - lg_file_patient_name, mock_pds_patient + actual_score, actual_is_validate_on_historic, result_message = ( + calculate_validation_score(lg_file_patient_name, mock_pds_patient) ) assert actual_score == ValidationScore.FULL_MATCH assert mock_validate_name.call_count == 2 assert actual_is_validate_on_historic is True -def test_validate_name_without_given_name(mocker, mock_pds_patient): +def test_validate_name_without_given_name_strict(mocker, mock_pds_patient): lg_file_patient_name = "Jane Smith" mock_pds_patient.name[0].given = [""] mock_validate_name = mocker.patch( - "utils.lloyd_george_validator.validate_patient_name" + "utils.lloyd_george_validator.validate_patient_name_strict" ) - actual_score, actual_is_validate_on_historic = calculate_validation_score( - lg_file_patient_name, mock_pds_patient + + with expect_not_to_raise(LGInvalidFilesException): + actual_is_validate_on_historic = validate_patient_name_using_full_name_history( + lg_file_patient_name, mock_pds_patient + ) + + assert actual_is_validate_on_historic is False + assert mock_validate_name.call_count == 1 + + +def test_validate_name_without_given_name_lenient(mocker, mock_pds_patient): + lg_file_patient_name = "Jane Smith" + mock_pds_patient.name[0].given = [""] + mock_validate_name = mocker.patch( + "utils.lloyd_george_validator.validate_patient_name_lenient" + ) + actual_score, actual_is_validate_on_historic, result_message = ( + calculate_validation_score(lg_file_patient_name, mock_pds_patient) ) assert actual_score == ValidationScore.NO_MATCH assert actual_is_validate_on_historic is False assert mock_validate_name.call_count == 2 +@pytest.mark.parametrize( + ["patient_details", "patient_name_in_file_name", "should_accept_name"], + load_test_cases(TEST_CASES_FOR_TWO_WORDS_FAMILY_NAME_STRICT), +) +def test_validate_patient_name_with_two_words_family_name_strict( + patient_details: Patient, + patient_name_in_file_name: str, + should_accept_name: bool, +): + if should_accept_name: + with expect_not_to_raise(LGInvalidFilesException): + actual_is_validate_on_historic = ( + validate_patient_name_using_full_name_history( + patient_name_in_file_name, patient_details + ) + ) + assert actual_is_validate_on_historic is False + else: + with pytest.raises(LGInvalidFilesException): + validate_patient_name_using_full_name_history( + patient_name_in_file_name, patient_details + ) + + @pytest.mark.parametrize( ["patient_details", "patient_name_in_file_name", "should_accept_name"], load_test_cases(TEST_CASES_FOR_TWO_WORDS_FAMILY_NAME), ) -def test_validate_patient_name_with_two_words_family_name( +def test_validate_patient_name_with_two_words_family_name_lenient( patient_details: Patient, patient_name_in_file_name: str, should_accept_name: bool, ): - actual_score, actual_is_validate_on_historic = calculate_validation_score( - patient_name_in_file_name, patient_details + actual_score, actual_is_validate_on_historic, result_message = ( + calculate_validation_score(patient_name_in_file_name, patient_details) ) if should_accept_name: assert actual_is_validate_on_historic is False @@ -430,13 +589,37 @@ def test_validate_patient_name_with_two_words_family_name( ["patient_details", "patient_name_in_file_name", "should_accept_name"], load_test_cases(TEST_CASES_FOR_FAMILY_NAME_WITH_HYPHEN), ) -def test_validate_patient_name_with_family_name_with_hyphen( +def test_validate_patient_name_with_family_name_with_hyphen_strict( patient_details: Patient, patient_name_in_file_name: str, should_accept_name: bool, ): - actual_score, actual_is_validate_on_historic = calculate_validation_score( - patient_name_in_file_name, patient_details + if should_accept_name: + with expect_not_to_raise(LGInvalidFilesException): + actual_is_validate_on_historic = ( + validate_patient_name_using_full_name_history( + patient_name_in_file_name, patient_details + ) + ) + assert actual_is_validate_on_historic is False + else: + with pytest.raises(LGInvalidFilesException): + validate_patient_name_using_full_name_history( + patient_name_in_file_name, patient_details + ) + + +@pytest.mark.parametrize( + ["patient_details", "patient_name_in_file_name", "should_accept_name"], + load_test_cases(TEST_CASES_FOR_FAMILY_NAME_WITH_HYPHEN), +) +def test_validate_patient_name_with_family_name_with_hyphen_lenient( + patient_details: Patient, + patient_name_in_file_name: str, + should_accept_name: bool, +): + actual_score, actual_is_validate_on_historic, result_message = ( + calculate_validation_score(patient_name_in_file_name, patient_details) ) if should_accept_name: assert actual_is_validate_on_historic is False @@ -450,13 +633,34 @@ def test_validate_patient_name_with_family_name_with_hyphen( ["patient_details", "patient_name_in_file_name", "should_accept_name"], load_test_cases(TEST_CASES_FOR_TWO_WORDS_GIVEN_NAME), ) -def test_validate_patient_name_with_two_words_given_name( +def test_validate_patient_name_with_two_words_given_name_strict( + patient_details: Patient, + patient_name_in_file_name: str, + should_accept_name: bool, +): + if should_accept_name: + with expect_not_to_raise(LGInvalidFilesException): + validate_patient_name_using_full_name_history( + patient_name_in_file_name, patient_details + ) + else: + with pytest.raises(LGInvalidFilesException): + validate_patient_name_using_full_name_history( + patient_name_in_file_name, patient_details + ) + + +@pytest.mark.parametrize( + ["patient_details", "patient_name_in_file_name", "should_accept_name"], + load_test_cases(TEST_CASES_FOR_TWO_WORDS_GIVEN_NAME), +) +def test_validate_patient_name_with_two_words_given_name_lenient( patient_details: Patient, patient_name_in_file_name: str, should_accept_name: bool, ): - actual_score, actual_is_validate_on_historic = calculate_validation_score( - patient_name_in_file_name, patient_details + actual_score, actual_is_validate_on_historic, result_message = ( + calculate_validation_score(patient_name_in_file_name, patient_details) ) if should_accept_name: assert actual_is_validate_on_historic is False @@ -470,13 +674,34 @@ def test_validate_patient_name_with_two_words_given_name( ["patient_details", "patient_name_in_file_name", "should_accept_name"], load_test_cases(TEST_CASES_FOR_TWO_WORDS_FAMILY_NAME_AND_GIVEN_NAME), ) -def test_validate_patient_name_with_two_words_family_name_and_given_name( +def test_validate_patient_name_with_two_words_family_name_and_given_name_strict( + patient_details: Patient, + patient_name_in_file_name: str, + should_accept_name: bool, +): + if should_accept_name: + with expect_not_to_raise(LGInvalidFilesException): + validate_patient_name_using_full_name_history( + patient_name_in_file_name, patient_details + ) + else: + with pytest.raises(LGInvalidFilesException): + validate_patient_name_using_full_name_history( + patient_name_in_file_name, patient_details + ) + + +@pytest.mark.parametrize( + ["patient_details", "patient_name_in_file_name", "should_accept_name"], + load_test_cases(TEST_CASES_FOR_TWO_WORDS_FAMILY_NAME_AND_GIVEN_NAME), +) +def test_validate_patient_name_with_two_words_family_name_and_given_name_lenient( patient_details: Patient, patient_name_in_file_name: str, should_accept_name: bool, ): - actual_score, actual_is_validate_on_historic = calculate_validation_score( - patient_name_in_file_name, patient_details + actual_score, actual_is_validate_on_historic, result_message = ( + calculate_validation_score(patient_name_in_file_name, patient_details) ) if should_accept_name: assert actual_is_validate_on_historic is False @@ -494,17 +719,19 @@ def test_missing_middle_name_names_with_pds_service(): patient = Patient.model_validate(PDS_PATIENT_WITH_MIDDLE_NAME) with expect_not_to_raise(LGInvalidFilesException): - validate_filename_with_patient_details(lg_file_list, patient) + validate_filename_with_patient_details_lenient(lg_file_list, patient) + validate_filename_with_patient_details_strict(lg_file_list, patient) -def test_mismatch_dob_with_pds_service(mock_pds_patient): +def test_mismatch_dob_and_name_with_pds_service(mock_pds_patient): lg_file_list = [ - "1of2_Lloyd_George_Record_[Jane Plain Smith]_[9000000009]_[14-01-2000].pdf", - "2of2_Lloyd_George_Record_[Jane Plain Smith]_[9000000009]_[14-01-2000].pdf", + "1of2_Lloyd_George_Record_[Jake Plain Smith]_[9000000009]_[14-01-2000].pdf", + "2of2_Lloyd_George_Record_[Jake Plain Smith]_[9000000009]_[14-01-2000].pdf", ] with pytest.raises(LGInvalidFilesException): - validate_filename_with_patient_details(lg_file_list, mock_pds_patient) + validate_filename_with_patient_details_lenient(lg_file_list, mock_pds_patient) + validate_filename_with_patient_details_strict(lg_file_list, mock_pds_patient) def test_validate_date_of_birth_when_mismatch_dob_with_pds_service( @@ -512,21 +739,22 @@ def test_validate_date_of_birth_when_mismatch_dob_with_pds_service( ): file_date_of_birth = "14-01-2000" - with pytest.raises(LGInvalidFilesException): - validate_patient_date_of_birth(file_date_of_birth, mock_pds_patient) + actual = validate_patient_date_of_birth(file_date_of_birth, mock_pds_patient) + + assert actual is False def test_validate_date_of_birth_valid_with_pds_service(mock_pds_patient): file_date_of_birth = "22-10-2010" - with expect_not_to_raise(LGInvalidFilesException): - validate_patient_date_of_birth(file_date_of_birth, mock_pds_patient) + actual = validate_patient_date_of_birth(file_date_of_birth, mock_pds_patient) + assert actual is True def test_validate_date_of_birth_none_with_pds_service(mock_pds_patient): file_date_of_birth = "22-10-2010" mock_pds_patient.birth_date = None - with pytest.raises(LGInvalidFilesException): - validate_patient_date_of_birth(file_date_of_birth, mock_pds_patient) + actual = validate_patient_date_of_birth(file_date_of_birth, mock_pds_patient) + assert actual is False def test_patient_not_found_with_pds_service(mock_pds_call): @@ -802,7 +1030,7 @@ def test_mismatch_nhs_in_validate_lg_file(mocker, mock_pds_patient): def test_validate_patient_name_return_false( file_patient_name, first_name_from_pds, family_name_from_pds, result ): - actual = validate_patient_name( + actual = validate_patient_name_lenient( file_patient_name, first_name_from_pds, family_name_from_pds ) assert actual.score == result @@ -817,16 +1045,18 @@ def test_validate_patient_name_return_false( "Smith", ValidationResult( score=ValidationScore.FULL_MATCH, - name_match=["Jane", "Smith"], + given_name_match=["Jane"], + family_name_match="Smith", ), ), ( "Jane Bob Smith Anderson", ["Jane"], - "Smith Anderson", + "Smith", ValidationResult( score=ValidationScore.FULL_MATCH, - name_match=["Jane", "Smith Anderson"], + given_name_match=["Jane"], + family_name_match="Smith", ), ), ( @@ -835,7 +1065,8 @@ def test_validate_patient_name_return_false( "Smith Anderson", ValidationResult( score=ValidationScore.FULL_MATCH, - name_match=["Jane", "Smith Anderson"], + given_name_match=["Jane"], + family_name_match="Smith Anderson", ), ), ( @@ -844,7 +1075,8 @@ def test_validate_patient_name_return_false( "Smith Anderson", ValidationResult( score=ValidationScore.FULL_MATCH, - name_match=["Jane", "Smith Anderson"], + given_name_match=["Jane"], + family_name_match="Smith Anderson", ), ), ( @@ -853,7 +1085,8 @@ def test_validate_patient_name_return_false( "Smith-Anderson", ValidationResult( score=ValidationScore.FULL_MATCH, - name_match=["Jane", "Smith-Anderson"], + given_name_match=["Jane"], + family_name_match="Smith-Anderson", ), ), ( @@ -862,7 +1095,8 @@ def test_validate_patient_name_return_false( "Smith Anderson", ValidationResult( score=ValidationScore.FULL_MATCH, - name_match=["Jane Bob", "Smith Anderson"], + given_name_match=["Jane Bob"], + family_name_match="Smith Anderson", ), ), ( @@ -871,7 +1105,25 @@ def test_validate_patient_name_return_false( "Smith", ValidationResult( score=ValidationScore.FULL_MATCH, - name_match=["Jane Bob", "Smith"], + given_name_match=["Jane Bob"], + family_name_match="Smith", + ), + ), + ( + "Jane Smith", + ["Bob"], + "Smith", + ValidationResult( + score=ValidationScore.PARTIAL_MATCH, + family_name_match="Smith", + ), + ), + ( + "Jane Smith", + ["Bob"], + "Dylan", + ValidationResult( + score=ValidationScore.NO_MATCH, ), ), ], @@ -879,38 +1131,41 @@ def test_validate_patient_name_return_false( def test_validate_patient_name_return_true( file_patient_name, first_name_from_pds, family_name_from_pds, expected ): - actual = validate_patient_name( + actual = validate_patient_name_lenient( file_patient_name, first_name_from_pds, family_name_from_pds ) assert actual == expected @pytest.mark.parametrize( - ["file_patient_name", "score"], + ["file_patient_name", "expected_score", "expected_historical_match"], [ - ("Jane Smith", ValidationScore.FULL_MATCH), - ("Smith Jane", ValidationScore.FULL_MATCH), - ("Jane Bob Smith Moody", ValidationScore.FULL_MATCH), - ("Jane Smith Moody", ValidationScore.FULL_MATCH), - ("Jane B Smith Moody", ValidationScore.FULL_MATCH), - ("Jane Smith-Moody", ValidationScore.FULL_MATCH), - ("Jane Bob Smith Moody", ValidationScore.FULL_MATCH), - ("Jane Bob Smith", ValidationScore.FULL_MATCH), - ("Bob Smith", ValidationScore.MIXED_FULL_MATCH), - ("Bob Jane", ValidationScore.FULL_MATCH), - ("Alastor Moody", ValidationScore.NO_MATCH), - ("Jones Bob", ValidationScore.MIXED_FULL_MATCH), + ("Jane Smith", ValidationScore.FULL_MATCH, True), + ("Smith Jane", ValidationScore.FULL_MATCH, True), + ("Jane Bob Smith Moody", ValidationScore.FULL_MATCH, False), + ("Jane Smith Moody", ValidationScore.FULL_MATCH, True), + ("Jane B Smith Moody", ValidationScore.FULL_MATCH, True), + ("Jane Smith-Moody", ValidationScore.FULL_MATCH, True), + ("Jane Bob Smith Moody", ValidationScore.FULL_MATCH, False), + ("Jane Bob Smith", ValidationScore.FULL_MATCH, False), + ("Bob Smith", ValidationScore.MIXED_FULL_MATCH, True), + ("Bob Jane", ValidationScore.FULL_MATCH, False), + ("Alastor Moody", ValidationScore.NO_MATCH, False), + ("Jones Bob", ValidationScore.MIXED_FULL_MATCH, True), + ("Jones Jane", ValidationScore.MIXED_FULL_MATCH, True), ], ) -def test_calculate_validation_score(file_patient_name, score): +def test_calculate_validation_score( + file_patient_name, expected_score, expected_historical_match +): name_1 = build_test_name(start="1990-01-01", end=None, given=["Jane"]) name_2 = build_test_name(start="1995-01-01", end=None, given=["Jane"], family="Bob") name_3 = build_test_name(use="temp", start=None, end=None, given=["Jones"]) test_patient = build_test_patient_with_names([name_1, name_2, name_3]) - actual_result, historical = calculate_validation_score( + actual_result, historical, _ = calculate_validation_score( file_patient_name, test_patient ) - - assert actual_result == score + assert historical == expected_historical_match + assert actual_result == expected_score diff --git a/lambdas/utils/lloyd_george_validator.py b/lambdas/utils/lloyd_george_validator.py index 600faed69..8a6412552 100644 --- a/lambdas/utils/lloyd_george_validator.py +++ b/lambdas/utils/lloyd_george_validator.py @@ -18,7 +18,12 @@ PatientRecordAlreadyExistException, PdsTooManyRequestsException, ) -from utils.unicode_utils import REGEX_PATIENT_NAME_PATTERN, name_contains_in +from utils.unicode_utils import ( + REGEX_PATIENT_NAME_PATTERN, + name_contains_in, + name_ends_with, + name_starts_with, +) from utils.utilities import get_pds_service logger = LoggingService(__name__) @@ -91,7 +96,7 @@ def validate_lg_files( files_name_list.append(doc.file_name) check_for_duplicate_files(files_name_list) - validate_filename_with_patient_details(files_name_list, pds_patient_details) + validate_filename_with_patient_details_strict(files_name_list, pds_patient_details) def validate_lg_file_names(file_name_list: list[str], nhs_number: str): @@ -138,15 +143,77 @@ def check_for_file_names_agrees_with_each_other(file_name_list: list[str]): raise LGInvalidFilesException("File names does not match with each other") -def validate_filename_with_patient_details( +def validate_filename_with_patient_details_strict( + file_name_list: list[str], patient_details: Patient +): + try: + file_name_info = extract_info_from_filename(file_name_list[0]) + file_patient_name = file_name_info["patient_name"] + file_date_of_birth = file_name_info["date_of_birth"] + is_dob_valid = validate_patient_date_of_birth( + file_date_of_birth, patient_details + ) + if not is_dob_valid: + raise LGInvalidFilesException("Patient DoB does not match our records") + is_name_validation_based_on_historic_name = ( + validate_patient_name_using_full_name_history( + file_patient_name, patient_details + ) + ) + return is_name_validation_based_on_historic_name + except (ValueError, KeyError) as e: + logger.error(e) + raise LGInvalidFilesException(e) + + +def validate_patient_name_strict( + file_patient_name: str, first_name_in_pds: str, family_name_in_pds: str +): + logger.info("Verifying patient name against the record in PDS...") + + first_name_matches = name_starts_with(file_patient_name, first_name_in_pds) + family_name_matches = name_ends_with(file_patient_name, family_name_in_pds) + + if not (first_name_matches and family_name_matches): + return False + return True + + +def validate_patient_name_using_full_name_history( + file_patient_name: str, pds_patient_details: Patient +): + usual_family_name_in_pds, usual_first_name_in_pds = ( + pds_patient_details.get_current_family_name_and_given_name() + ) + + if validate_patient_name_strict( + file_patient_name, usual_first_name_in_pds[0], usual_family_name_in_pds + ): + return False + logger.info( + "Failed to validate patient name using usual name, trying to validate using name history" + ) + + for name in pds_patient_details.name: + historic_first_name_in_pds: str = name.given[0] + historic_family_name_in_pds = name.family + if validate_patient_name_strict( + file_patient_name, historic_first_name_in_pds, historic_family_name_in_pds + ): + return True + + raise LGInvalidFilesException("Patient name does not match our records") + + +def validate_filename_with_patient_details_lenient( file_name_list: list[str], patient_details: Patient -) -> (ValidationScore, bool, bool): +) -> (str, bool): try: file_name_info = extract_info_from_filename(file_name_list[0]) file_patient_name = file_name_info["patient_name"] file_date_of_birth = file_name_info["date_of_birth"] - name_validation_score, historical_match = calculate_validation_score( - file_patient_name, patient_details + name_validation_score, historical_match, result_message = ( + calculate_validation_score(file_patient_name, patient_details) ) if name_validation_score == ValidationScore.NO_MATCH: raise LGInvalidFilesException("Patient name does not match our records") @@ -155,7 +222,22 @@ def validate_filename_with_patient_details( ) if not is_dob_valid and name_validation_score == ValidationScore.PARTIAL_MATCH: raise LGInvalidFilesException("Patient name does not match our records 1/3") - return name_validation_score, is_dob_valid, historical_match + validation_messages = { + ValidationScore.PARTIAL_MATCH: { + True: f"Patient matched on partial match 2/3, {result_message}", + False: "", + }, + ValidationScore.MIXED_FULL_MATCH: { + True: f"Patient matched on mixed match 3/3, {result_message}", + False: f"Patient matched on mixed match 2/3, {result_message}", + }, + ValidationScore.FULL_MATCH: { + True: f"Patient matched on full match 3/3, {result_message}", + False: f"Patient matched on full match 2/3, {result_message}", + }, + } + acceptance_message = validation_messages[name_validation_score][is_dob_valid] + return acceptance_message, historical_match except (ValueError, KeyError) as e: logger.error(e) @@ -164,53 +246,65 @@ def validate_filename_with_patient_details( def calculate_validation_score( file_patient_name: str, patient_details: Patient -) -> (ValidationScore, bool): - matched_on = set() +) -> (ValidationScore, bool, str): + matched_on_given_name = set() + matched_on_family_name = set() historical_match = False ordered_names = patient_details.get_names_by_start_date() for index, name in enumerate(ordered_names): first_name_in_pds = name.given family_name_in_pds = name.family - result = validate_patient_name( + result = validate_patient_name_lenient( file_patient_name, first_name_in_pds, family_name_in_pds ) if result.score == ValidationScore.FULL_MATCH: + result_message = f"matched on {len(matched_on_family_name)} family_name and {len(matched_on_given_name)} given name" historical_match = index != 0 - return result.score, historical_match + return result.score, historical_match, result_message elif result.score == ValidationScore.PARTIAL_MATCH: historical_match = index != 0 - matched_on.update(result.name_match) - logger.info( - "Failed to find full match on patient name, trying to validate using name history" - ) - if len(matched_on) > 1: - return ValidationScore.MIXED_FULL_MATCH, historical_match - elif len(matched_on) == 1: - return ValidationScore.PARTIAL_MATCH, historical_match - return ValidationScore.NO_MATCH, False + matched_on_given_name.update(result.given_name_match) + ( + matched_on_family_name.add(result.family_name_match) + if result.family_name_match + else None + ) + logger.info( + "Failed to find full match on patient name, trying to validate using name history" + ) + result_message = f"matched on {len(matched_on_family_name)} family_name and {len(matched_on_given_name)} given name" + if len(matched_on_given_name) + len(matched_on_family_name) > 1: + return ValidationScore.MIXED_FULL_MATCH, historical_match, result_message + elif matched_on_given_name or matched_on_family_name: + return ValidationScore.PARTIAL_MATCH, historical_match, result_message + return ValidationScore.NO_MATCH, False, result_message -def validate_patient_name( +def validate_patient_name_lenient( file_patient_name: str, first_name_in_pds: list[str], family_name_in_pds: str ) -> ValidationResult: logger.info("Verifying patient name against the record in PDS...") - name_match = [ + given_name_matches = [ first_name for first_name in first_name_in_pds if name_contains_in(file_patient_name, first_name) ] - if name_contains_in(file_patient_name, family_name_in_pds): - name_match.append(family_name_in_pds) + family_name_matches = name_contains_in(file_patient_name, family_name_in_pds) - if len(name_match) > 1: + if given_name_matches and family_name_matches: return ValidationResult( score=ValidationScore.FULL_MATCH, - name_match=name_match, + given_name_match=given_name_matches, + family_name_match=family_name_in_pds, ) - elif len(name_match) == 1: + elif given_name_matches: return ValidationResult( score=ValidationScore.PARTIAL_MATCH, - name_match=name_match, + given_name_match=given_name_matches, + ) + elif family_name_matches: + return ValidationResult( + score=ValidationScore.PARTIAL_MATCH, family_name_match=family_name_in_pds ) return ValidationResult( score=ValidationScore.NO_MATCH, @@ -219,11 +313,9 @@ def validate_patient_name( def validate_patient_date_of_birth(file_date_of_birth, pds_patient_details): date_of_birth = datetime.datetime.strptime(file_date_of_birth, "%d-%m-%Y").date() - if ( - not pds_patient_details.birth_date - or pds_patient_details.birth_date != date_of_birth - ): - raise LGInvalidFilesException("Patient DoB does not match our records") + if pds_patient_details.birth_date: + return pds_patient_details.birth_date == date_of_birth + return False def getting_patient_info_from_pds(nhs_number: str) -> Patient: From 91a3b589698e1f23007caa87a2b4fc09b2f02e96 Mon Sep 17 00:00:00 2001 From: NogaNHS Date: Mon, 27 Jan 2025 10:34:31 +0000 Subject: [PATCH 04/13] [PRMP-1367] add missing unit tests --- .../unit/utils/test_lloyd_george_validator.py | 54 ++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/lambdas/tests/unit/utils/test_lloyd_george_validator.py b/lambdas/tests/unit/utils/test_lloyd_george_validator.py index 239a746be..2ed641f84 100644 --- a/lambdas/tests/unit/utils/test_lloyd_george_validator.py +++ b/lambdas/tests/unit/utils/test_lloyd_george_validator.py @@ -54,7 +54,7 @@ validate_lg_files, validate_patient_date_of_birth, validate_patient_name_lenient, - validate_patient_name_using_full_name_history, + validate_patient_name_using_full_name_history, validate_patient_name_strict, ) @@ -978,6 +978,58 @@ def test_mismatch_nhs_in_validate_lg_file(mocker, mock_pds_patient): ) +@pytest.mark.parametrize( + ["file_patient_name", "first_name_from_pds", "family_name_from_pds"], + [ + ["Jim Stevens", "Jane", "Smith"], + ["Jane Smith Anderson", "Jane", "Smith"], + ["Bob Smith Anderson", "Jane", "Smith"], + ["Jane B Smith Anderson", "Jane", "Smith"], + ["Jane Bob Anderson", "Jane", "Smith"], + ["Jane Bob Smith", "Jane Bob", "Smith Anderson"], + ["Jane Smith", "Jane Bob", "Smith"], + ["Jane B Smith", "Jane Bob", "Smith"], + ["Jane-Bob Smith", "Jane Bob", "Smith"], + ["Jane Smith", "Jane Bob", "Smith"], + ["Jane Smith Anderson", "Jane", "Smith-Anderson"], + ["Jane Smith", "Jane", "Smith-Anderson"], + ["Jane Anderson", "Jane", "Smith-Anderson"], + ["Jane Bob Smith", "Jane Bob", "Smith-Anderson"], + ["Bob Smith Anderson", "Jane", "Smith Anderson"], + ["Jane Smith", "Jane", "Smith Anderson"], + ["Jane Anderson", "Jane", "Smith Anderson"], + ["Jane Anderson Smith", "Jane", "Smith Anderson"], + ], +) +def test_validate_patient_name_return_false( + file_patient_name, first_name_from_pds, family_name_from_pds +): + actual = validate_patient_name_strict( + file_patient_name, first_name_from_pds, family_name_from_pds + ) + assert actual is False + + +@pytest.mark.parametrize( + ["file_patient_name", "first_name_from_pds", "family_name_from_pds"], + [ + ["Jane Smith", "Jane", "Smith"], + ["Jane Bob Smith Anderson", "Jane", "Smith Anderson"], + ["Jane Smith Anderson", "Jane", "Smith Anderson"], + ["Jane B Smith Anderson", "Jane", "Smith Anderson"], + ["Jane Smith-Anderson", "Jane", "Smith-Anderson"], + ["Jane Bob Smith Anderson", "Jane Bob", "Smith Anderson"], + ["Jane Bob Smith", "Jane Bob", "Smith"], + ], +) +def test_validate_patient_name_return_true( + file_patient_name, first_name_from_pds, family_name_from_pds +): + actual = validate_patient_name_strict( + file_patient_name, first_name_from_pds, family_name_from_pds + ) + assert actual is True + @pytest.mark.parametrize( ["file_patient_name", "first_name_from_pds", "family_name_from_pds", "result"], [ From 820642bfb4eb740ff6c8e6865338391764b7400d Mon Sep 17 00:00:00 2001 From: NogaNHS Date: Mon, 27 Jan 2025 10:39:46 +0000 Subject: [PATCH 05/13] [PRMP-1367] change unit tests name --- lambdas/tests/unit/utils/test_lloyd_george_validator.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lambdas/tests/unit/utils/test_lloyd_george_validator.py b/lambdas/tests/unit/utils/test_lloyd_george_validator.py index 2ed641f84..188bcfeac 100644 --- a/lambdas/tests/unit/utils/test_lloyd_george_validator.py +++ b/lambdas/tests/unit/utils/test_lloyd_george_validator.py @@ -54,7 +54,8 @@ validate_lg_files, validate_patient_date_of_birth, validate_patient_name_lenient, - validate_patient_name_using_full_name_history, validate_patient_name_strict, + validate_patient_name_strict, + validate_patient_name_using_full_name_history, ) @@ -1030,6 +1031,7 @@ def test_validate_patient_name_return_true( ) assert actual is True + @pytest.mark.parametrize( ["file_patient_name", "first_name_from_pds", "family_name_from_pds", "result"], [ @@ -1079,7 +1081,7 @@ def test_validate_patient_name_return_true( ], ], ) -def test_validate_patient_name_return_false( +def test_validate_patient_name_lenient_return_false( file_patient_name, first_name_from_pds, family_name_from_pds, result ): actual = validate_patient_name_lenient( @@ -1180,7 +1182,7 @@ def test_validate_patient_name_return_false( ), ], ) -def test_validate_patient_name_return_true( +def test_validate_patient_name_lenient_return_true( file_patient_name, first_name_from_pds, family_name_from_pds, expected ): actual = validate_patient_name_lenient( From ac70aeaa325f40f1a4f93ab3fae70c5986429c08 Mon Sep 17 00:00:00 2001 From: NogaNHS Date: Mon, 27 Jan 2025 11:32:49 +0000 Subject: [PATCH 06/13] [PRMP-1367] add checks in unit tests --- lambdas/tests/unit/models/test_pds_models.py | 7 ----- .../unit/utils/test_lloyd_george_validator.py | 30 +++++++++++++++++++ lambdas/utils/lloyd_george_validator.py | 4 +-- 3 files changed, 32 insertions(+), 9 deletions(-) diff --git a/lambdas/tests/unit/models/test_pds_models.py b/lambdas/tests/unit/models/test_pds_models.py index 66254fe7c..092b759cb 100644 --- a/lambdas/tests/unit/models/test_pds_models.py +++ b/lambdas/tests/unit/models/test_pds_models.py @@ -374,13 +374,6 @@ def test_get_names_by_start_date_return_ordered_list_by_start_date(): assert actual == expected -# @freeze_time("2024-01-01") -# def test_get_most_recent_name_return_none_if_no_active_name_found(): -# test_patient = build_test_patient_with_names([]) -# -# assert test_patient.get_names_by_start_date() is None - - def test_get_death_notification_status_return_the_death_notification_status(): test_patient = create_patient(PDS_PATIENT_DECEASED_FORMAL) expected = DeathNotificationStatus.FORMAL diff --git a/lambdas/tests/unit/utils/test_lloyd_george_validator.py b/lambdas/tests/unit/utils/test_lloyd_george_validator.py index 188bcfeac..d17213f57 100644 --- a/lambdas/tests/unit/utils/test_lloyd_george_validator.py +++ b/lambdas/tests/unit/utils/test_lloyd_george_validator.py @@ -306,6 +306,7 @@ def test_validate_name_with_correct_name_lenient(mocker, mock_pds_patient): mock_validate_name = mocker.patch( "utils.lloyd_george_validator.validate_patient_name_lenient" ) + expected_message = "matched on 1 family_name and 1 given name" mock_validate_name.return_value = ValidationResult( score=ValidationScore.FULL_MATCH, given_name_match=["Jane"], @@ -315,6 +316,7 @@ def test_validate_name_with_correct_name_lenient(mocker, mock_pds_patient): calculate_validation_score(lg_file_patient_name, mock_pds_patient) ) + assert expected_message == result_message assert mock_validate_name.call_count == 1 assert actual_is_name_validation_based_on_historic_name is False assert actual_score == ValidationScore.FULL_MATCH @@ -367,6 +369,8 @@ def test_validate_name_with_additional_middle_name_in_file_mismatching_pds_lenie mock_validate_name = mocker.patch( "utils.lloyd_george_validator.validate_patient_name_lenient" ) + expected_message = "matched on 1 family_name and 1 given name" + patient = Patient.model_validate(PDS_PATIENT_WITH_MIDDLE_NAME) mock_validate_name.return_value = ValidationResult( score=ValidationScore.FULL_MATCH, @@ -377,6 +381,7 @@ def test_validate_name_with_additional_middle_name_in_file_mismatching_pds_lenie calculate_validation_score(lg_file_patient_name, patient) ) + assert expected_message == result_message assert mock_validate_name.call_count == 1 assert actual_is_name_validation_based_on_historic_name is False assert actual_score == ValidationScore.FULL_MATCH @@ -407,6 +412,9 @@ def test_validate_name_with_additional_middle_name_in_file_but_none_in_pds( mock_validate_name = mocker.patch( "utils.lloyd_george_validator.validate_patient_name_lenient" ) + + expected_message = "matched on 1 family_name and 1 given name" + mock_validate_name.return_value = ValidationResult( score=ValidationScore.FULL_MATCH, given_name_match=["Jane"], @@ -416,6 +424,7 @@ def test_validate_name_with_additional_middle_name_in_file_but_none_in_pds( calculate_validation_score(lg_file_patient_name, mock_pds_patient) ) + assert expected_message == result_message assert mock_validate_name.call_count == 1 assert actual_is_name_validation_based_on_historic_name is False assert actual_score == ValidationScore.FULL_MATCH @@ -496,17 +505,23 @@ def test_validate_name_with_historical_name_lenient(mocker, mock_pds_patient): mock_validate_name = mocker.patch( "utils.lloyd_george_validator.validate_patient_name_lenient" ) + expected_message = "matched on 1 family_name and 1 given name" + mock_validate_name.side_effect = [ ValidationResult( score=ValidationScore.NO_MATCH, ), ValidationResult( score=ValidationScore.FULL_MATCH, + given_name_match=["Jim"], + family_name_match="Stevens", ), ] actual_score, actual_is_validate_on_historic, result_message = ( calculate_validation_score(lg_file_patient_name, mock_pds_patient) ) + + assert result_message == expected_message assert actual_score == ValidationScore.FULL_MATCH assert mock_validate_name.call_count == 2 assert actual_is_validate_on_historic is True @@ -531,12 +546,16 @@ def test_validate_name_without_given_name_strict(mocker, mock_pds_patient): def test_validate_name_without_given_name_lenient(mocker, mock_pds_patient): lg_file_patient_name = "Jane Smith" mock_pds_patient.name[0].given = [""] + expected_message = "No match found" + mock_validate_name = mocker.patch( "utils.lloyd_george_validator.validate_patient_name_lenient" ) actual_score, actual_is_validate_on_historic, result_message = ( calculate_validation_score(lg_file_patient_name, mock_pds_patient) ) + + assert result_message == expected_message assert actual_score == ValidationScore.NO_MATCH assert actual_is_validate_on_historic is False assert mock_validate_name.call_count == 2 @@ -575,6 +594,7 @@ def test_validate_patient_name_with_two_words_family_name_lenient( patient_name_in_file_name: str, should_accept_name: bool, ): + actual_score, actual_is_validate_on_historic, result_message = ( calculate_validation_score(patient_name_in_file_name, patient_details) ) @@ -623,9 +643,13 @@ def test_validate_patient_name_with_family_name_with_hyphen_lenient( calculate_validation_score(patient_name_in_file_name, patient_details) ) if should_accept_name: + expected_message = "matched on 1 family_name and 1 given name" + assert result_message == expected_message assert actual_is_validate_on_historic is False assert actual_score == ValidationScore.FULL_MATCH else: + expected_message = "matched on 0 family_name and 1 given name" + assert result_message == expected_message assert actual_is_validate_on_historic is False assert actual_score == ValidationScore.PARTIAL_MATCH @@ -664,9 +688,13 @@ def test_validate_patient_name_with_two_words_given_name_lenient( calculate_validation_score(patient_name_in_file_name, patient_details) ) if should_accept_name: + expected_message = "matched on 1 family_name and 1 given name" + assert result_message == expected_message assert actual_is_validate_on_historic is False assert actual_score == ValidationScore.FULL_MATCH else: + expected_message = "matched on 1 family_name and 0 given name" + assert result_message == expected_message assert actual_is_validate_on_historic is False assert actual_score == ValidationScore.PARTIAL_MATCH @@ -705,6 +733,8 @@ def test_validate_patient_name_with_two_words_family_name_and_given_name_lenient calculate_validation_score(patient_name_in_file_name, patient_details) ) if should_accept_name: + expected_message = "matched on 1 family_name and 1 given name" + assert result_message == expected_message assert actual_is_validate_on_historic is False assert actual_score == ValidationScore.FULL_MATCH else: diff --git a/lambdas/utils/lloyd_george_validator.py b/lambdas/utils/lloyd_george_validator.py index 8a6412552..d19935b3c 100644 --- a/lambdas/utils/lloyd_george_validator.py +++ b/lambdas/utils/lloyd_george_validator.py @@ -258,7 +258,7 @@ def calculate_validation_score( file_patient_name, first_name_in_pds, family_name_in_pds ) if result.score == ValidationScore.FULL_MATCH: - result_message = f"matched on {len(matched_on_family_name)} family_name and {len(matched_on_given_name)} given name" + result_message = f"matched on {1 if bool(result.family_name_match) else 0} family_name and {len(result.given_name_match)} given name" historical_match = index != 0 return result.score, historical_match, result_message elif result.score == ValidationScore.PARTIAL_MATCH: @@ -277,7 +277,7 @@ def calculate_validation_score( return ValidationScore.MIXED_FULL_MATCH, historical_match, result_message elif matched_on_given_name or matched_on_family_name: return ValidationScore.PARTIAL_MATCH, historical_match, result_message - return ValidationScore.NO_MATCH, False, result_message + return ValidationScore.NO_MATCH, False, "No match found" def validate_patient_name_lenient( From 0521723f42f9d307fdef125e0609722f00241e48 Mon Sep 17 00:00:00 2001 From: NogaNHS Date: Mon, 27 Jan 2025 11:42:56 +0000 Subject: [PATCH 07/13] [PRMP-1367] add unit test in bulk upload service --- .../unit/services/test_bulk_upload_service.py | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/lambdas/tests/unit/services/test_bulk_upload_service.py b/lambdas/tests/unit/services/test_bulk_upload_service.py index 0e1cc1478..7dd674c24 100644 --- a/lambdas/tests/unit/services/test_bulk_upload_service.py +++ b/lambdas/tests/unit/services/test_bulk_upload_service.py @@ -233,6 +233,7 @@ def test_handle_sqs_message_happy_path( mock_create_lg_records_and_copy_files.assert_called_with( TEST_STAGING_METADATA, TEST_CURRENT_GP_ODS ) + mock_pds_validation_strict.assert_called() mock_report_upload_complete.assert_called() mock_remove_ingested_file_from_source_bucket.assert_called() repo_under_test.sqs_repository.send_message_to_nrl_fifo.assert_not_called() @@ -1003,6 +1004,41 @@ def test_handle_sqs_message_happy_path_historical_name( ) mock_remove_ingested_file_from_source_bucket.assert_called() +def test_handle_sqs_message_lenient_mode_happy_path( + set_env, + mocker, + mock_uuid, + mock_validate_files, + mock_pds_service, + mock_pds_validation_lenient, + mock_pds_validation_strict, + mock_ods_validation, +): + TEST_STAGING_METADATA.retries = 0 + service = BulkUploadService(strict_mode=False) + mocker.patch.object(service, "dynamo_repository") + mocker.patch.object(service, "sqs_repository") + mocker.patch.object(service, "s3_repository") + mock_create_lg_records_and_copy_files = mocker.patch.object( + BulkUploadService, "create_lg_records_and_copy_files" + ) + mock_report_upload_complete = mocker.patch.object( + service.dynamo_repository, "write_report_upload_to_dynamo" + ) + mock_remove_ingested_file_from_source_bucket = mocker.patch.object( + service.s3_repository, "remove_ingested_file_from_source_bucket" + ) + mocker.patch.object(service.s3_repository, "check_virus_result") + + service.handle_sqs_message(message=TEST_SQS_MESSAGE) + mock_create_lg_records_and_copy_files.assert_called_with( + TEST_STAGING_METADATA, TEST_CURRENT_GP_ODS + ) + mock_pds_validation_lenient.assert_called() + mock_pds_validation_strict.assert_not_called() + mock_report_upload_complete.assert_called() + mock_remove_ingested_file_from_source_bucket.assert_called() + service.sqs_repository.send_message_to_nrl_fifo.assert_not_called() def test_concatenate_acceptance_reason(repo_under_test): accepted_reason = None From 9b8deeb9947247661d35bc7d981ee35d1b210bd5 Mon Sep 17 00:00:00 2001 From: NogaNHS Date: Mon, 27 Jan 2025 11:46:39 +0000 Subject: [PATCH 08/13] [PRMP-1367] resolve soner cloud issue --- lambdas/tests/unit/utils/test_lloyd_george_validator.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lambdas/tests/unit/utils/test_lloyd_george_validator.py b/lambdas/tests/unit/utils/test_lloyd_george_validator.py index d17213f57..0e6f75dff 100644 --- a/lambdas/tests/unit/utils/test_lloyd_george_validator.py +++ b/lambdas/tests/unit/utils/test_lloyd_george_validator.py @@ -602,6 +602,8 @@ def test_validate_patient_name_with_two_words_family_name_lenient( assert actual_is_validate_on_historic is False assert actual_score == ValidationScore.FULL_MATCH else: + expected_message = "matched on 0 family_name and 1 given name" + assert result_message == expected_message assert actual_is_validate_on_historic is False assert actual_score == ValidationScore.PARTIAL_MATCH From de147af576c25e96fa65cd9c8c345057ccda8fc9 Mon Sep 17 00:00:00 2001 From: NogaNHS Date: Tue, 28 Jan 2025 12:14:23 +0000 Subject: [PATCH 09/13] [PRMP-1367] fix issue when a name is a empty string --- lambdas/models/pds_models.py | 4 ++-- ...000104_M85143_gp_given_name_with_whitespace.json | 1 + lambdas/tests/unit/models/test_pds_models.py | 2 +- .../tests/unit/utils/test_lloyd_george_validator.py | 13 ++++++++++++- lambdas/utils/lloyd_george_validator.py | 4 ++-- 5 files changed, 18 insertions(+), 6 deletions(-) diff --git a/lambdas/models/pds_models.py b/lambdas/models/pds_models.py index 7ae13181c..2ebf4d9c1 100644 --- a/lambdas/models/pds_models.py +++ b/lambdas/models/pds_models.py @@ -28,7 +28,7 @@ class Address(BaseModel): class Name(BaseModel): use: str period: Optional[Period] = None - given: list[str] = [""] + given: Optional[list[str]] = [] family: str def is_currently_in_use(self) -> bool: @@ -75,7 +75,7 @@ class Extension(BaseModel): class PatientDetails(BaseModel): model_config = conf - given_name: list[str] = [""] + given_name: Optional[list[str]] = None family_name: str = "" birth_date: Optional[date] = None postal_code: str = "" diff --git a/lambdas/services/mock_data/pds_patient_9000000104_M85143_gp_given_name_with_whitespace.json b/lambdas/services/mock_data/pds_patient_9000000104_M85143_gp_given_name_with_whitespace.json index 870ed128e..43d913533 100644 --- a/lambdas/services/mock_data/pds_patient_9000000104_M85143_gp_given_name_with_whitespace.json +++ b/lambdas/services/mock_data/pds_patient_9000000104_M85143_gp_given_name_with_whitespace.json @@ -53,6 +53,7 @@ "end": "2025-12-31" }, "family": "Stevens", + "given": [""], "prefix": ["Mr"], "suffix": ["MBE"] } diff --git a/lambdas/tests/unit/models/test_pds_models.py b/lambdas/tests/unit/models/test_pds_models.py index 092b759cb..2daf8ee82 100644 --- a/lambdas/tests/unit/models/test_pds_models.py +++ b/lambdas/tests/unit/models/test_pds_models.py @@ -208,7 +208,7 @@ def test_patient_without_given_name_in_current_name_logs_a_warning_and_process_s ): patient = create_patient(PDS_PATIENT_NO_GIVEN_NAME_IN_CURRENT_NAME) - expected = EXPECTED_PARSED_PATIENT_BASE_CASE.model_copy(update={"given_name": [""]}) + expected = EXPECTED_PARSED_PATIENT_BASE_CASE.model_copy(update={"given_name": []}) result = patient.get_patient_details(patient.id) assert expected == result diff --git a/lambdas/tests/unit/utils/test_lloyd_george_validator.py b/lambdas/tests/unit/utils/test_lloyd_george_validator.py index 0e6f75dff..4b7320c4d 100644 --- a/lambdas/tests/unit/utils/test_lloyd_george_validator.py +++ b/lambdas/tests/unit/utils/test_lloyd_george_validator.py @@ -1195,6 +1195,15 @@ def test_validate_patient_name_lenient_return_false( family_name_match="Smith", ), ), + ( + "Jane Bob Smith", + ["Jane Bob"], + "Anderson", + ValidationResult( + score=ValidationScore.PARTIAL_MATCH, + given_name_match=["Jane Bob"], + ), + ), ( "Jane Smith", ["Bob"], @@ -1239,6 +1248,7 @@ def test_validate_patient_name_lenient_return_true( ("Alastor Moody", ValidationScore.NO_MATCH, False), ("Jones Bob", ValidationScore.MIXED_FULL_MATCH, True), ("Jones Jane", ValidationScore.MIXED_FULL_MATCH, True), + ("Paul Anderson", ValidationScore.PARTIAL_MATCH, True), ], ) def test_calculate_validation_score( @@ -1247,8 +1257,9 @@ def test_calculate_validation_score( name_1 = build_test_name(start="1990-01-01", end=None, given=["Jane"]) name_2 = build_test_name(start="1995-01-01", end=None, given=["Jane"], family="Bob") name_3 = build_test_name(use="temp", start=None, end=None, given=["Jones"]) + name_4 = build_test_name(use="usual", start="1995-01-01", end=None, given=["Paul Anderson"]) - test_patient = build_test_patient_with_names([name_1, name_2, name_3]) + test_patient = build_test_patient_with_names([name_1, name_2, name_3, name_4]) actual_result, historical, _ = calculate_validation_score( file_patient_name, test_patient diff --git a/lambdas/utils/lloyd_george_validator.py b/lambdas/utils/lloyd_george_validator.py index d19935b3c..4a0b65b10 100644 --- a/lambdas/utils/lloyd_george_validator.py +++ b/lambdas/utils/lloyd_george_validator.py @@ -287,9 +287,9 @@ def validate_patient_name_lenient( given_name_matches = [ first_name for first_name in first_name_in_pds - if name_contains_in(file_patient_name, first_name) + if first_name and name_contains_in(file_patient_name, first_name) ] - family_name_matches = name_contains_in(file_patient_name, family_name_in_pds) + family_name_matches = name_contains_in(file_patient_name, family_name_in_pds) if family_name_in_pds else None if given_name_matches and family_name_matches: return ValidationResult( From ab5ff830354ea983695571fac82320479e352eea Mon Sep 17 00:00:00 2001 From: NogaNHS Date: Tue, 28 Jan 2025 12:16:50 +0000 Subject: [PATCH 10/13] [PRMP-1367] PR changes --- lambdas/handlers/bulk_upload_handler.py | 2 +- .../unit/services/test_bulk_upload_service.py | 2 ++ .../unit/utils/test_lloyd_george_validator.py | 18 ++++++++++-------- lambdas/utils/lloyd_george_validator.py | 6 +++++- 4 files changed, 18 insertions(+), 10 deletions(-) diff --git a/lambdas/handlers/bulk_upload_handler.py b/lambdas/handlers/bulk_upload_handler.py index 874be63d9..a561a46de 100644 --- a/lambdas/handlers/bulk_upload_handler.py +++ b/lambdas/handlers/bulk_upload_handler.py @@ -25,7 +25,7 @@ def lambda_handler(event, _context): ] if validation_strict_mode: - logger.info("Lloyd George validation strict mose is enabled") + logger.info("Lloyd George validation strict mode is enabled") if "Records" not in event or len(event["Records"]) < 1: http_status_code = 400 diff --git a/lambdas/tests/unit/services/test_bulk_upload_service.py b/lambdas/tests/unit/services/test_bulk_upload_service.py index 7dd674c24..6951b4bd6 100644 --- a/lambdas/tests/unit/services/test_bulk_upload_service.py +++ b/lambdas/tests/unit/services/test_bulk_upload_service.py @@ -1004,6 +1004,7 @@ def test_handle_sqs_message_happy_path_historical_name( ) mock_remove_ingested_file_from_source_bucket.assert_called() + def test_handle_sqs_message_lenient_mode_happy_path( set_env, mocker, @@ -1040,6 +1041,7 @@ def test_handle_sqs_message_lenient_mode_happy_path( mock_remove_ingested_file_from_source_bucket.assert_called() service.sqs_repository.send_message_to_nrl_fifo.assert_not_called() + def test_concatenate_acceptance_reason(repo_under_test): accepted_reason = None test_reason = "test_reason_1" diff --git a/lambdas/tests/unit/utils/test_lloyd_george_validator.py b/lambdas/tests/unit/utils/test_lloyd_george_validator.py index 4b7320c4d..11fe4e4ce 100644 --- a/lambdas/tests/unit/utils/test_lloyd_george_validator.py +++ b/lambdas/tests/unit/utils/test_lloyd_george_validator.py @@ -1196,13 +1196,13 @@ def test_validate_patient_name_lenient_return_false( ), ), ( - "Jane Bob Smith", - ["Jane Bob"], - "Anderson", - ValidationResult( - score=ValidationScore.PARTIAL_MATCH, - given_name_match=["Jane Bob"], - ), + "Jane Bob Smith", + ["Jane Bob"], + "Anderson", + ValidationResult( + score=ValidationScore.PARTIAL_MATCH, + given_name_match=["Jane Bob"], + ), ), ( "Jane Smith", @@ -1257,7 +1257,9 @@ def test_calculate_validation_score( name_1 = build_test_name(start="1990-01-01", end=None, given=["Jane"]) name_2 = build_test_name(start="1995-01-01", end=None, given=["Jane"], family="Bob") name_3 = build_test_name(use="temp", start=None, end=None, given=["Jones"]) - name_4 = build_test_name(use="usual", start="1995-01-01", end=None, given=["Paul Anderson"]) + name_4 = build_test_name( + use="usual", start="1995-01-01", end=None, given=["Paul Anderson"] + ) test_patient = build_test_patient_with_names([name_1, name_2, name_3, name_4]) diff --git a/lambdas/utils/lloyd_george_validator.py b/lambdas/utils/lloyd_george_validator.py index 4a0b65b10..c68cec234 100644 --- a/lambdas/utils/lloyd_george_validator.py +++ b/lambdas/utils/lloyd_george_validator.py @@ -289,7 +289,11 @@ def validate_patient_name_lenient( for first_name in first_name_in_pds if first_name and name_contains_in(file_patient_name, first_name) ] - family_name_matches = name_contains_in(file_patient_name, family_name_in_pds) if family_name_in_pds else None + family_name_matches = ( + name_contains_in(file_patient_name, family_name_in_pds) + if family_name_in_pds + else None + ) if given_name_matches and family_name_matches: return ValidationResult( From 7c8aea4870efb2555e5f518c76ce22d5f48cb714 Mon Sep 17 00:00:00 2001 From: NogaNHS Date: Wed, 29 Jan 2025 11:22:22 +0000 Subject: [PATCH 11/13] [PRMP-1367] fix for empty given name bug --- lambdas/models/pds_models.py | 2 +- .../test_cases_for_patient_name_matching.py | 12 ++++++++++ .../unit/utils/test_lloyd_george_validator.py | 23 +++++++++++++++++++ lambdas/utils/lloyd_george_validator.py | 10 ++++++-- 4 files changed, 44 insertions(+), 3 deletions(-) diff --git a/lambdas/models/pds_models.py b/lambdas/models/pds_models.py index 2ebf4d9c1..92fb440da 100644 --- a/lambdas/models/pds_models.py +++ b/lambdas/models/pds_models.py @@ -75,7 +75,7 @@ class Extension(BaseModel): class PatientDetails(BaseModel): model_config = conf - given_name: Optional[list[str]] = None + given_name: Optional[list[str]] = [] family_name: str = "" birth_date: Optional[date] = None postal_code: str = "" diff --git a/lambdas/tests/unit/helpers/data/pds/test_cases_for_patient_name_matching.py b/lambdas/tests/unit/helpers/data/pds/test_cases_for_patient_name_matching.py index 559162f8e..fb6d68109 100644 --- a/lambdas/tests/unit/helpers/data/pds/test_cases_for_patient_name_matching.py +++ b/lambdas/tests/unit/helpers/data/pds/test_cases_for_patient_name_matching.py @@ -46,6 +46,18 @@ class PdsNameMatchingTestCase(NamedTuple): "reject": ["Jane Smith", "Jane B Smith", "Jane-Bob Smith", "Bob Smith"], } +TEST_CASES_FOR_EMPTY_GIVEN_NAME = { + "pds_name": {"family": "Smith", "given": []}, + "accept": [], + "reject": [ + "Jane Smith", + "Jane B Smith", + "Jane-Bob Smith", + "Bob Smith", + "Jane Bob Smith", + ], +} + TEST_CASES_FOR_TWO_WORDS_FAMILY_NAME_AND_GIVEN_NAME = { "pds_name": {"family": "Smith Anderson", "given": ["Jane Bob"]}, "accept": ["Jane Bob Smith Anderson"], diff --git a/lambdas/tests/unit/utils/test_lloyd_george_validator.py b/lambdas/tests/unit/utils/test_lloyd_george_validator.py index 11fe4e4ce..001b17d6d 100644 --- a/lambdas/tests/unit/utils/test_lloyd_george_validator.py +++ b/lambdas/tests/unit/utils/test_lloyd_george_validator.py @@ -20,6 +20,7 @@ build_test_patient_with_names, ) from tests.unit.helpers.data.pds.test_cases_for_patient_name_matching import ( + TEST_CASES_FOR_EMPTY_GIVEN_NAME, TEST_CASES_FOR_FAMILY_NAME_WITH_HYPHEN, TEST_CASES_FOR_TWO_WORDS_FAMILY_NAME, TEST_CASES_FOR_TWO_WORDS_FAMILY_NAME_AND_GIVEN_NAME, @@ -722,6 +723,27 @@ def test_validate_patient_name_with_two_words_family_name_and_given_name_strict( ) +@pytest.mark.parametrize( + ["patient_details", "patient_name_in_file_name", "should_accept_name"], + load_test_cases(TEST_CASES_FOR_EMPTY_GIVEN_NAME), +) +def test_validate_patient_name_with_family_name_and_empty_given_name_strict( + patient_details: Patient, + patient_name_in_file_name: str, + should_accept_name: bool, +): + if should_accept_name: + with expect_not_to_raise(LGInvalidFilesException): + validate_patient_name_using_full_name_history( + patient_name_in_file_name, patient_details + ) + else: + with pytest.raises(LGInvalidFilesException): + validate_patient_name_using_full_name_history( + patient_name_in_file_name, patient_details + ) + + @pytest.mark.parametrize( ["patient_details", "patient_name_in_file_name", "should_accept_name"], load_test_cases(TEST_CASES_FOR_TWO_WORDS_FAMILY_NAME_AND_GIVEN_NAME), @@ -1053,6 +1075,7 @@ def test_validate_patient_name_return_false( ["Jane Smith-Anderson", "Jane", "Smith-Anderson"], ["Jane Bob Smith Anderson", "Jane Bob", "Smith Anderson"], ["Jane Bob Smith", "Jane Bob", "Smith"], + ["Jane Bob Smith", "Jane Bob", ""], ], ) def test_validate_patient_name_return_true( diff --git a/lambdas/utils/lloyd_george_validator.py b/lambdas/utils/lloyd_george_validator.py index c68cec234..43b1e6ab4 100644 --- a/lambdas/utils/lloyd_george_validator.py +++ b/lambdas/utils/lloyd_george_validator.py @@ -186,8 +186,12 @@ def validate_patient_name_using_full_name_history( pds_patient_details.get_current_family_name_and_given_name() ) - if validate_patient_name_strict( - file_patient_name, usual_first_name_in_pds[0], usual_family_name_in_pds + if ( + usual_first_name_in_pds + and usual_family_name_in_pds + and validate_patient_name_strict( + file_patient_name, usual_first_name_in_pds[0], usual_family_name_in_pds + ) ): return False logger.info( @@ -195,6 +199,8 @@ def validate_patient_name_using_full_name_history( ) for name in pds_patient_details.name: + if not name.given or not name.family: + continue historic_first_name_in_pds: str = name.given[0] historic_family_name_in_pds = name.family if validate_patient_name_strict( From cc2491d033d6acf7609e53489fc5d60f5e08bf1a Mon Sep 17 00:00:00 2001 From: NogaNHS Date: Wed, 29 Jan 2025 11:38:49 +0000 Subject: [PATCH 12/13] [PRMP-1367] function name change --- .../unit/utils/test_lloyd_george_validator.py | 24 +++++++++---------- lambdas/utils/lloyd_george_validator.py | 4 ++-- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/lambdas/tests/unit/utils/test_lloyd_george_validator.py b/lambdas/tests/unit/utils/test_lloyd_george_validator.py index 001b17d6d..77bbf8a42 100644 --- a/lambdas/tests/unit/utils/test_lloyd_george_validator.py +++ b/lambdas/tests/unit/utils/test_lloyd_george_validator.py @@ -37,7 +37,7 @@ from utils.lloyd_george_validator import ( LGInvalidFilesException, allowed_to_ingest_ods_code, - calculate_validation_score, + calculate_validation_score_for_lenient_check, check_for_duplicate_files, check_for_file_names_agrees_with_each_other, check_for_number_of_files_match_expected, @@ -314,7 +314,7 @@ def test_validate_name_with_correct_name_lenient(mocker, mock_pds_patient): family_name_match="Smith", ) actual_score, actual_is_name_validation_based_on_historic_name, result_message = ( - calculate_validation_score(lg_file_patient_name, mock_pds_patient) + calculate_validation_score_for_lenient_check(lg_file_patient_name, mock_pds_patient) ) assert expected_message == result_message @@ -379,7 +379,7 @@ def test_validate_name_with_additional_middle_name_in_file_mismatching_pds_lenie family_name_match="Smith", ) actual_score, actual_is_name_validation_based_on_historic_name, result_message = ( - calculate_validation_score(lg_file_patient_name, patient) + calculate_validation_score_for_lenient_check(lg_file_patient_name, patient) ) assert expected_message == result_message @@ -422,7 +422,7 @@ def test_validate_name_with_additional_middle_name_in_file_but_none_in_pds( family_name_match="Smith", ) actual_score, actual_is_name_validation_based_on_historic_name, result_message = ( - calculate_validation_score(lg_file_patient_name, mock_pds_patient) + calculate_validation_score_for_lenient_check(lg_file_patient_name, mock_pds_patient) ) assert expected_message == result_message @@ -519,7 +519,7 @@ def test_validate_name_with_historical_name_lenient(mocker, mock_pds_patient): ), ] actual_score, actual_is_validate_on_historic, result_message = ( - calculate_validation_score(lg_file_patient_name, mock_pds_patient) + calculate_validation_score_for_lenient_check(lg_file_patient_name, mock_pds_patient) ) assert result_message == expected_message @@ -553,7 +553,7 @@ def test_validate_name_without_given_name_lenient(mocker, mock_pds_patient): "utils.lloyd_george_validator.validate_patient_name_lenient" ) actual_score, actual_is_validate_on_historic, result_message = ( - calculate_validation_score(lg_file_patient_name, mock_pds_patient) + calculate_validation_score_for_lenient_check(lg_file_patient_name, mock_pds_patient) ) assert result_message == expected_message @@ -597,7 +597,7 @@ def test_validate_patient_name_with_two_words_family_name_lenient( ): actual_score, actual_is_validate_on_historic, result_message = ( - calculate_validation_score(patient_name_in_file_name, patient_details) + calculate_validation_score_for_lenient_check(patient_name_in_file_name, patient_details) ) if should_accept_name: assert actual_is_validate_on_historic is False @@ -643,7 +643,7 @@ def test_validate_patient_name_with_family_name_with_hyphen_lenient( should_accept_name: bool, ): actual_score, actual_is_validate_on_historic, result_message = ( - calculate_validation_score(patient_name_in_file_name, patient_details) + calculate_validation_score_for_lenient_check(patient_name_in_file_name, patient_details) ) if should_accept_name: expected_message = "matched on 1 family_name and 1 given name" @@ -688,7 +688,7 @@ def test_validate_patient_name_with_two_words_given_name_lenient( should_accept_name: bool, ): actual_score, actual_is_validate_on_historic, result_message = ( - calculate_validation_score(patient_name_in_file_name, patient_details) + calculate_validation_score_for_lenient_check(patient_name_in_file_name, patient_details) ) if should_accept_name: expected_message = "matched on 1 family_name and 1 given name" @@ -754,7 +754,7 @@ def test_validate_patient_name_with_two_words_family_name_and_given_name_lenient should_accept_name: bool, ): actual_score, actual_is_validate_on_historic, result_message = ( - calculate_validation_score(patient_name_in_file_name, patient_details) + calculate_validation_score_for_lenient_check(patient_name_in_file_name, patient_details) ) if should_accept_name: expected_message = "matched on 1 family_name and 1 given name" @@ -1274,7 +1274,7 @@ def test_validate_patient_name_lenient_return_true( ("Paul Anderson", ValidationScore.PARTIAL_MATCH, True), ], ) -def test_calculate_validation_score( +def test_calculate_validation_score_for_lenient_check( file_patient_name, expected_score, expected_historical_match ): name_1 = build_test_name(start="1990-01-01", end=None, given=["Jane"]) @@ -1286,7 +1286,7 @@ def test_calculate_validation_score( test_patient = build_test_patient_with_names([name_1, name_2, name_3, name_4]) - actual_result, historical, _ = calculate_validation_score( + actual_result, historical, _ = calculate_validation_score_for_lenient_check( file_patient_name, test_patient ) assert historical == expected_historical_match diff --git a/lambdas/utils/lloyd_george_validator.py b/lambdas/utils/lloyd_george_validator.py index 43b1e6ab4..d4c0c7619 100644 --- a/lambdas/utils/lloyd_george_validator.py +++ b/lambdas/utils/lloyd_george_validator.py @@ -219,7 +219,7 @@ def validate_filename_with_patient_details_lenient( file_patient_name = file_name_info["patient_name"] file_date_of_birth = file_name_info["date_of_birth"] name_validation_score, historical_match, result_message = ( - calculate_validation_score(file_patient_name, patient_details) + calculate_validation_score_for_lenient_check(file_patient_name, patient_details) ) if name_validation_score == ValidationScore.NO_MATCH: raise LGInvalidFilesException("Patient name does not match our records") @@ -250,7 +250,7 @@ def validate_filename_with_patient_details_lenient( raise LGInvalidFilesException(e) -def calculate_validation_score( +def calculate_validation_score_for_lenient_check( file_patient_name: str, patient_details: Patient ) -> (ValidationScore, bool, str): matched_on_given_name = set() From c9d07d1f3dc72be6115f4988223781346d39eaa1 Mon Sep 17 00:00:00 2001 From: NogaNHS Date: Wed, 29 Jan 2025 12:46:31 +0000 Subject: [PATCH 13/13] PRMP-1367 remove example given name --- ..._patient_9000000104_M85143_gp_given_name_with_whitespace.json | 1 - 1 file changed, 1 deletion(-) diff --git a/lambdas/services/mock_data/pds_patient_9000000104_M85143_gp_given_name_with_whitespace.json b/lambdas/services/mock_data/pds_patient_9000000104_M85143_gp_given_name_with_whitespace.json index 43d913533..870ed128e 100644 --- a/lambdas/services/mock_data/pds_patient_9000000104_M85143_gp_given_name_with_whitespace.json +++ b/lambdas/services/mock_data/pds_patient_9000000104_M85143_gp_given_name_with_whitespace.json @@ -53,7 +53,6 @@ "end": "2025-12-31" }, "family": "Stevens", - "given": [""], "prefix": ["Mr"], "suffix": ["MBE"] }