[PRMP-1376] Add validation score, and changes to validation to return…

… score

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

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.")

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 = (

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)}")

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",

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():

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()