[PRMP-839] Fixed suspended patient is active logic & added unit test (#…

…416) * [PRMP-839] Fixed suspended patient is active logic & added unit test * [PRMP-839] Added ODS code validation for inactive statuses * PRMP-839 fix tests and format add method to PDS model * PRMP-839 add return type and remove SUSP string from tests * PRMP-839 remove invalid code --------- Co-authored-by: Andy Flint <[email protected]> Co-authored-by: NogaNHS <[email protected]>
nhsconnect · Aug 29, 2024 · 5fa3eaf · 5fa3eaf
1 parent adb9dde
commit 5fa3eaf
Show file tree

Hide file tree

Showing 8 changed files with 124 additions and 24 deletions.
diff --git a/lambdas/enums/patient_ods_inactive_status.py b/lambdas/enums/patient_ods_inactive_status.py
@@ -0,0 +1,10 @@
+from enum import StrEnum
+
+
+class PatientOdsInactiveStatus(StrEnum):
+    SUSPENDED = "SUSP"
+    DECEASED = "DECE"
+
+    @staticmethod
+    def list() -> list[str]:
+        return list(PatientOdsInactiveStatus.__members__.values())
diff --git a/lambdas/models/pds_models.py b/lambdas/models/pds_models.py
@@ -2,9 +2,11 @@
 from typing import Optional, Tuple
 
 from enums.death_notification_status import DeathNotificationStatus
+from enums.patient_ods_inactive_status import PatientOdsInactiveStatus
 from pydantic import BaseModel, ConfigDict
 from pydantic.alias_generators import to_camel
 from utils.audit_logging_setup import LoggingService
+from utils.ods_utils import is_ods_code_active
 
 logger = LoggingService(__name__)
 conf = ConfigDict(alias_generator=to_camel)
@@ -146,6 +148,13 @@ def get_current_home_address(self) -> Optional[Address]:
                 if entry.use.lower() == "home":
                     return entry
 
+    def get_ods_code_or_inactive_status_for_gp(self) -> str:
+        return (
+            self.get_active_ods_code_for_gp()
+            or self.get_status_for_inactive_patient()
+            or ""
+        )
+
     def get_active_ods_code_for_gp(self) -> str:
         for entry in self.general_practitioner:
             period = entry.identifier.period
@@ -155,14 +164,14 @@ def get_active_ods_code_for_gp(self) -> str:
             if not gp_end_date or gp_end_date >= date.today():
                 return entry.identifier.value
 
+    def get_status_for_inactive_patient(self) -> str:
         death_notification_status = self.get_death_notification_status()
         if not is_deceased(death_notification_status) and self.is_unrestricted():
-            return "SUSP"
-        return ""
+            return PatientOdsInactiveStatus.SUSPENDED
 
     def get_is_active_status(self) -> bool:
-        gp_ods = self.get_active_ods_code_for_gp()
-        return bool(gp_ods)
+        gp_ods = self.get_ods_code_or_inactive_status_for_gp()
+        return is_ods_code_active(gp_ods)
 
     def get_death_notification_status(self) -> Optional[DeathNotificationStatus]:
         if not self.deceased_date_time:
@@ -211,7 +220,7 @@ def get_patient_details(self, nhs_number) -> PatientDetails:
             nhsNumber=self.id,
             superseded=bool(nhs_number == id),
             restricted=not self.is_unrestricted(),
-            generalPracticeOds=self.get_active_ods_code_for_gp(),
+            generalPracticeOds=self.get_ods_code_or_inactive_status_for_gp(),
             active=self.get_is_active_status(),
             deceased=is_deceased(death_notification_status),
             deathNotificationStatus=death_notification_status,
@@ -228,7 +237,9 @@ def get_minimum_patient_details(self, nhs_number) -> PatientDetails:
             familyName=family_name,
             birthDate=self.birth_date,
             generalPracticeOds=(
-                self.get_active_ods_code_for_gp() if self.is_unrestricted() else ""
+                self.get_ods_code_or_inactive_status_for_gp()
+                if self.is_unrestricted()
+                else ""
             ),
             nhsNumber=self.id,
             superseded=bool(nhs_number == id),

diff --git a/lambdas/services/search_patient_details_service.py b/lambdas/services/search_patient_details_service.py
@@ -11,6 +11,7 @@
     UserNotAuthorisedException,
 )
 from utils.lambda_exceptions import SearchPatientException
+from utils.ods_utils import is_ods_code_active
 from utils.utilities import get_pds_service
 
 logger = LoggingService(__name__)
@@ -65,7 +66,7 @@ def handle_search_patient_request(self, nhs_number):
             raise SearchPatientException(400, LambdaError.SearchPatientNoParse)
 
     def check_if_user_authorise(self, gp_ods_for_patient):
-        patient_is_active = bool(gp_ods_for_patient)
+        patient_is_active = is_ods_code_active(gp_ods_for_patient)
         match self.user_role:
             case RepositoryRole.GP_ADMIN.value:
                 # Not raising error here if gp_ods is null / empty

diff --git a/lambdas/tests/unit/helpers/data/pds/pds_patient_response.py b/lambdas/tests/unit/helpers/data/pds/pds_patient_response.py
@@ -592,3 +592,6 @@
     "display": "Informal - death notice received via an update from a local NHS "
     "Organisation such as GENERAL PRACTITIONER or NHS Trust",
 }
+
+PDS_PATIENT_SUSPENDED = copy.deepcopy(PDS_PATIENT)
+PDS_PATIENT_SUSPENDED["generalPractitioner"][0]["identifier"].pop("period")
diff --git a/lambdas/tests/unit/models/test_pds_models.py b/lambdas/tests/unit/models/test_pds_models.py
@@ -1,6 +1,7 @@
 import copy
 
 from enums.death_notification_status import DeathNotificationStatus
+from enums.patient_ods_inactive_status import PatientOdsInactiveStatus
 from freezegun import freeze_time
 from models.pds_models import PatientDetails
 from tests.unit.conftest import EXPECTED_PARSED_PATIENT_BASE_CASE
@@ -13,6 +14,7 @@
     PDS_PATIENT_NO_PERIOD_IN_GENERAL_PRACTITIONER_IDENTIFIER,
     PDS_PATIENT_NO_PERIOD_IN_NAME_MODEL,
     PDS_PATIENT_RESTRICTED,
+    PDS_PATIENT_SUSPENDED,
     PDS_PATIENT_WITH_GP_END_DATE,
     PDS_PATIENT_WITHOUT_ACTIVE_GP,
     PDS_PATIENT_WITHOUT_ADDRESS,
@@ -61,6 +63,26 @@ def test_get_restricted_patient_details():
     assert expected_patient_details == result
 
 
+def test_get_suspended_patient_details():
+    patient = create_patient(PDS_PATIENT_SUSPENDED)
+
+    expected_patient_details = PatientDetails(
+        givenName=["Jane"],
+        familyName="Smith",
+        birthDate="2010-10-22",
+        postalCode="LS1 6AE",
+        nhsNumber="9000000009",
+        superseded=False,
+        restricted=False,
+        generalPracticeOds=PatientOdsInactiveStatus.SUSPENDED,
+        active=False,
+    )
+
+    result = patient.get_patient_details(patient.id)
+
+    assert expected_patient_details == result
+
+
 def test_get_minimum_patient_details():
     patient = create_patient(PDS_PATIENT)
 
@@ -100,15 +122,15 @@ def test_gp_ods_susp_when_gp_end_date_indicates_inactive():
 
     response = patient.get_minimum_patient_details(patient.id)
 
-    assert response.general_practice_ods == "SUSP"
+    assert response.general_practice_ods == PatientOdsInactiveStatus.SUSPENDED
 
 
 def test_not_raise_error_when_no_gp_in_response():
     patient = create_patient(PDS_PATIENT_WITHOUT_ACTIVE_GP)
 
     response = patient.get_minimum_patient_details(patient.id)
 
-    assert response.general_practice_ods == "SUSP"
+    assert response.general_practice_ods == PatientOdsInactiveStatus.SUSPENDED
 
 
 @freeze_time("2021-12-31")

diff --git a/lambdas/tests/unit/services/test_search_patient_details_service.py b/lambdas/tests/unit/services/test_search_patient_details_service.py
@@ -9,6 +9,10 @@
 )
 from utils.lambda_exceptions import SearchPatientException
 
+USER_VALID_ODS_CODE = "ABC123"
+PATIENT_VALID_ODS_CODE = "XYZ789"
+EMPTY_ODS_CODE = ""
+
 
 @pytest.fixture(scope="function")
 def mock_service(request, mocker):
@@ -33,7 +37,11 @@ def mock_check_if_user_authorise(mocker, mock_service):
 
 @pytest.mark.parametrize(
     "mock_service",
-    (("GP_ADMIN", "test_ods_code"), ("GP_CLINICAL", "test_ods_code"), ("PCSE", "")),
+    (
+        ("GP_ADMIN", USER_VALID_ODS_CODE),
+        ("GP_CLINICAL", USER_VALID_ODS_CODE),
+        ("PCSE", ""),
+    ),
     indirect=True,
 )
 def test_check_if_user_authorise_valid(mock_service):
@@ -47,24 +55,26 @@ def test_check_if_user_authorise_valid(mock_service):
 @pytest.mark.parametrize(
     "mock_service",
     (
-        ("GP_ADMIN", "test_ods_code"),
-        ("GP_ADMIN", ""),
-        ("GP_CLINICAL", "test_ods_code"),
-        ("GP_CLINICAL", ""),
-        ("PCSE", ""),
-        ("PCSE", "new_gp_ods_code"),
-        ("Not_valid", "new_gp_ods_code"),
-        ("Not_valid", ""),
+        ("GP_ADMIN", USER_VALID_ODS_CODE),
+        ("GP_ADMIN", EMPTY_ODS_CODE),
+        ("GP_CLINICAL", USER_VALID_ODS_CODE),
+        ("GP_CLINICAL", EMPTY_ODS_CODE),
+        ("PCSE", EMPTY_ODS_CODE),
+        ("PCSE", PATIENT_VALID_ODS_CODE),
+        ("Not_valid", PATIENT_VALID_ODS_CODE),
+        ("Not_valid", EMPTY_ODS_CODE),
     ),
     indirect=True,
 )
 def test_check_if_user_authorise_raise_error(mock_service):
     with pytest.raises(UserNotAuthorisedException):
-        mock_service.check_if_user_authorise("new_gp_ods_code")
+        mock_service.check_if_user_authorise(PATIENT_VALID_ODS_CODE)
 
 
 @pytest.mark.parametrize(
-    "mock_service", (("GP_ADMIN", "test_ods_code"), ("GP_ADMIN", "")), indirect=True
+    "mock_service",
+    (("GP_ADMIN", USER_VALID_ODS_CODE), ("GP_ADMIN", EMPTY_ODS_CODE)),
+    indirect=True,
 )
 def test_handle_search_patient_request_valid(mock_service, mocker):
     pds_service_response = PatientDetails(
@@ -98,7 +108,9 @@ def test_handle_search_patient_request_valid(mock_service, mocker):
 
 
 @pytest.mark.parametrize(
-    "mock_service", (("GP_ADMIN", "test_ods_code"), ("GP_ADMIN", "")), indirect=True
+    "mock_service",
+    (("GP_ADMIN", USER_VALID_ODS_CODE), ("GP_ADMIN", EMPTY_ODS_CODE)),
+    indirect=True,
 )
 def test_handle_search_patient_request_raise_error_when_patient_not_found(
     mock_service, mock_pds_service_fetch
@@ -111,7 +123,9 @@ def test_handle_search_patient_request_raise_error_when_patient_not_found(
 
 
 @pytest.mark.parametrize(
-    "mock_service", (("GP_ADMIN", "test_ods_code"), ("GP_ADMIN", "")), indirect=True
+    "mock_service",
+    (("GP_ADMIN", USER_VALID_ODS_CODE), ("GP_ADMIN", EMPTY_ODS_CODE)),
+    indirect=True,
 )
 def test_handle_search_patient_request_raise_error_when_user_is_not_auth(
     mock_service, mock_pds_service_fetch, mock_check_if_user_authorise
@@ -125,7 +139,9 @@ def test_handle_search_patient_request_raise_error_when_user_is_not_auth(
 
 
 @pytest.mark.parametrize(
-    "mock_service", (("GP_ADMIN", "test_ods_code"), ("GP_ADMIN", "")), indirect=True
+    "mock_service",
+    (("GP_ADMIN", USER_VALID_ODS_CODE), ("GP_ADMIN", EMPTY_ODS_CODE)),
+    indirect=True,
 )
 def test_handle_search_patient_request_raise_error_when_invalid_patient(
     mock_service, mock_pds_service_fetch, mock_check_if_user_authorise
@@ -139,7 +155,9 @@ def test_handle_search_patient_request_raise_error_when_invalid_patient(
 
 
 @pytest.mark.parametrize(
-    "mock_service", (("GP_ADMIN", "test_ods_code"), ("GP_ADMIN", "")), indirect=True
+    "mock_service",
+    (("GP_ADMIN", USER_VALID_ODS_CODE), ("GP_ADMIN", EMPTY_ODS_CODE)),
+    indirect=True,
 )
 def test_handle_search_patient_request_raise_error_when_pds_error(
     mock_service, mock_pds_service_fetch, mock_check_if_user_authorise

diff --git a/lambdas/tests/unit/utils/test_ods_utils.py b/lambdas/tests/unit/utils/test_ods_utils.py
@@ -0,0 +1,19 @@
+import pytest
+from enums.patient_ods_inactive_status import PatientOdsInactiveStatus
+from utils.ods_utils import is_ods_code_active
+
+
+@pytest.mark.parametrize(
+    "ods_code,expected",
+    [
+        ["H81109", True],
+        [PatientOdsInactiveStatus.SUSPENDED, False],
+        [PatientOdsInactiveStatus.DECEASED, False],
+        ["", False],
+        [None, False],
+    ],
+)
+def test_is_ods_code_active(ods_code, expected):
+    actual = is_ods_code_active(ods_code)
+
+    assert actual == expected
diff --git a/lambdas/utils/ods_utils.py b/lambdas/utils/ods_utils.py
@@ -0,0 +1,16 @@
+from enums.patient_ods_inactive_status import PatientOdsInactiveStatus
+
+"""
+On PDS, GP ODS codes must be 6 characters long, see the 'epraccur' document here for info:
+https://digital.nhs.uk/services/organisation-data-service/export-data-files/csv-downloads/gp-and-gp-practice-related-data
+
+Sometimes, a patient will not have a generalPractitioner on PDS. Internally, we can also add codes to mark inactive
+patients for reporting purposes. The only values that should be considered 'active' are valid ODS codes.
+"""
+
+
+def is_ods_code_active(gp_ods) -> bool:
+    if gp_ods in PatientOdsInactiveStatus.list():
+        return False
+
+    return len(gp_ods or "") == 6