[PRMP-813] Enable pilot practices to search patients outside of their ods (#902)

Co-authored-by: adamwhitingnhs <[email protected]>

Author: kamenbachvarov-nhs

lambdas/handlers/search_patient_details_handler.py

@@ -1,5 +1,7 @@
+from enums.feature_flags import FeatureFlags
 from enums.lambda_error import LambdaError
 from enums.logging_app_interaction import LoggingAppInteraction
+from services.feature_flags_service import FeatureFlagService
 from services.search_patient_details_service import SearchPatientDetailsService
 from utils.audit_logging_setup import LoggingService
 from utils.decorators.ensure_env_var import ensure_environment_variables
@@ -38,13 +40,27 @@ def lambda_handler(event, context):
         )
         raise SearchPatientException(400, LambdaError.SearchPatientMissing)
 
+    feature_flag_service = FeatureFlagService()
+    feature_flag = feature_flag_service.get_feature_flags_by_flag(
+        FeatureFlags.UPLOAD_DOCUMENT_ITERATION_3_ENABLED
+    )
+    document_upload_iteration3_enabled = feature_flag[
+        FeatureFlags.UPLOAD_DOCUMENT_ITERATION_3_ENABLED
+    ]
+    can_access_not_my_record = (
+        feature_flag_service.check_if_ods_code_is_in_pilot()
+        and document_upload_iteration3_enabled
+    )
+
     search_service = SearchPatientDetailsService(
         user_role=user_role, user_ods_code=user_ods_code
     )
 
     # Get patient details from service
     patient_details = search_service.handle_search_patient_request(
-        nhs_number,
+        nhs_number=nhs_number,
+        update_session=True,
+        can_access_not_my_record=can_access_not_my_record,
     )
     formatted_response = patient_details.model_dump_json(
         by_alias=True,

lambdas/models/pds_models.py

@@ -86,6 +86,7 @@ class PatientDetails(BaseModel):
     active: Optional[bool] = None
     deceased: bool = False
     death_notification_status: Optional[DeathNotificationStatus] = None
+    can_manage_record: Optional[bool] = None
 
 
 class Patient(BaseModel):

lambdas/scripts/dynamodb_migration_prmp_198.py

@@ -1,12 +1,13 @@
 import os
-import boto3
 from typing import Callable, Iterable
-from boto3.dynamodb.conditions import Key, Attr
 
 from scripts.MigrationBase import MigrationBase
 from services.base.dynamo_service import DynamoDBService
 from utils.audit_logging_setup import LoggingService
-from utils.exceptions import MigrationUnrecoverableException, MigrationRetryableException
+from utils.exceptions import (
+    MigrationRetryableException,
+    MigrationUnrecoverableException,
+)
 
 
 class AuthorMigration(MigrationBase):
@@ -77,7 +78,8 @@ def get_update_author_items(self, entry: dict) -> dict | None:
                 f"No completed bulk upload found for NHS number: {nhs_number}"
             )
             raise MigrationUnrecoverableException(
-                message=f"No completed bulk upload found for NHS number: {nhs_number}", item_id=entry.get("ID")
+                message=f"No completed bulk upload found for NHS number: {nhs_number}",
+                item_id=entry.get("ID"),
             )
 
         new_author = bulk_upload_row.get("UploaderOdsCode")
@@ -86,7 +88,8 @@ def get_update_author_items(self, entry: dict) -> dict | None:
                 f"No uploader ODS code found for NHS number: {nhs_number}"
             )
             raise MigrationUnrecoverableException(
-                message=f"No uploader ODS code found for NHS number: {nhs_number}", item_id=entry.get("ID")
+                message=f"No uploader ODS code found for NHS number: {nhs_number}",
+                item_id=entry.get("ID"),
             )
 
         return {"Author": new_author}
@@ -105,7 +108,7 @@ def build_bulk_upload_lookup(self, entries: Iterable[dict]) -> dict[str, dict]:
             nhs = entry.get("NhsNumber")
             if nhs:
                 unique_nhs_numbers.add(nhs)
-        
+
         self.logger.info(f"Found {len(unique_nhs_numbers)} unique NHS numbers to query")
 
         if not unique_nhs_numbers:
@@ -121,9 +124,9 @@ def build_bulk_upload_lookup(self, entries: Iterable[dict]) -> dict[str, dict]:
                     table_name=self.bulk_upload_report_table,
                     search_key="NhsNumber",
                     search_condition=nhs_number,
-                    index_name="NhsNumberIndex",  
+                    index_name="NhsNumberIndex",
                 )
-                
+
                 # Find the most recent completed upload for this NHS number
                 for row in items:
                     status = row.get("UploadStatus")

lambdas/services/authoriser_service.py

@@ -72,6 +72,7 @@ def deny_access_policy(self, path, user_role, nhs_number: str = None):
         patient_access_is_allowed = (
             nhs_number in self.allowed_nhs_numbers if nhs_number else False
         )
+
         access_to_deceased_patient = (
             nhs_number in self.deceased_nhs_numbers if nhs_number else False
         )

lambdas/services/document_reference_search_service.py

@@ -9,7 +9,6 @@
 from enums.metadata_field_names import DocumentReferenceMetadataFields
 from enums.mtls import MtlsCommonNames
 from enums.snomed_codes import SnomedCodes
-from enums.supported_document_types import SupportedDocumentTypes
 from models.document_reference import DocumentReference
 from models.fhir.R4.bundle import Bundle, BundleEntry
 from models.fhir.R4.fhir_document_reference import Attachment, DocumentReferenceInfo

lambdas/services/search_patient_details_service.py

@@ -1,9 +1,7 @@
-from enums.feature_flags import FeatureFlags
 from enums.lambda_error import LambdaError
 from enums.repository_role import RepositoryRole
 from pydantic import ValidationError
 from pydantic_core import PydanticSerializationError
-from services.feature_flags_service import FeatureFlagService
 from services.manage_user_session_access import ManageUserSessionAccess
 from utils.audit_logging_setup import LoggingService
 from utils.exceptions import (
@@ -24,9 +22,13 @@ def __init__(self, user_role, user_ods_code):
         self.user_role = user_role
         self.user_ods_code = user_ods_code
         self.manage_user_session_service = ManageUserSessionAccess()
-        self.feature_flag_service = FeatureFlagService()
 
-    def handle_search_patient_request(self, nhs_number, update_session=True):
+    def handle_search_patient_request(
+        self,
+        nhs_number,
+        update_session=True,
+        can_access_not_my_record=False,
+    ):
         """
         Handle search patient request and return patient details if authorised.
 
@@ -43,15 +45,20 @@ def handle_search_patient_request(self, nhs_number, update_session=True):
         try:
             patient_details = self._fetch_patient_details(nhs_number)
 
+            can_manage_record = patient_details.deceased
+
             if not patient_details.deceased:
-                self._check_authorization(patient_details.general_practice_ods)
+                can_manage_record = self._check_authorization(
+                    patient_details.general_practice_ods, can_access_not_my_record
+                )
 
             logger.info("Searched for patient details", {"Result": "Patient found"})
 
-            if update_session:
+            if update_session and can_manage_record:
                 self._update_session(nhs_number, patient_details.deceased)
 
-            # Return the patient details object directly
+            patient_details.can_manage_record = can_manage_record
+
             return patient_details
 
         except PatientNotFoundException as e:
@@ -86,7 +93,9 @@ def _fetch_patient_details(self, nhs_number):
         pds_service = get_pds_service()
         return pds_service.fetch_patient_details(nhs_number)
 
-    def _check_authorization(self, gp_ods_for_patient):
+    def _check_authorization(
+        self, gp_ods_for_patient, can_access_not_my_record
+    ) -> bool:
         """
         Check if the current user is authorised to view the patient details.
 
@@ -97,33 +106,18 @@ def _check_authorization(self, gp_ods_for_patient):
             UserNotAuthorisedException: If the user is not authorised
         """
         patient_is_active = is_ods_code_active(gp_ods_for_patient)
-        is_arf_journey_on = self._is_arf_upload_enabled()
+        user_is_data_controller = gp_ods_for_patient == self.user_ods_code
 
         match self.user_role:
-            case RepositoryRole.GP_ADMIN.value:
-                if patient_is_active and gp_ods_for_patient != self.user_ods_code:
-                    raise UserNotAuthorisedException
-                elif not patient_is_active and not is_arf_journey_on:
-                    raise UserNotAuthorisedException
-
-            case RepositoryRole.GP_CLINICAL.value:
-                if not patient_is_active or gp_ods_for_patient != self.user_ods_code:
-                    raise UserNotAuthorisedException
+            case RepositoryRole.GP_ADMIN.value | RepositoryRole.GP_CLINICAL.value:
+                if user_is_data_controller or can_access_not_my_record:
+                    return user_is_data_controller
 
             case RepositoryRole.PCSE.value:
-                if patient_is_active:
-                    raise UserNotAuthorisedException
+                if not patient_is_active:
+                    return True
 
-            case _:
-                raise UserNotAuthorisedException
-
-    def _is_arf_upload_enabled(self):
-        """Check if ARF upload workflow is enabled via feature flags"""
-        upload_flag_name = FeatureFlags.UPLOAD_ARF_WORKFLOW_ENABLED.value
-        upload_lambda_enabled_flag_object = (
-            self.feature_flag_service.get_feature_flags_by_flag(upload_flag_name)
-        )
-        return upload_lambda_enabled_flag_object[upload_flag_name]
+        raise UserNotAuthorisedException
 
     def _update_session(self, nhs_number, is_deceased):
         """Update the user session with permitted search information"""

lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_failure.py

@@ -11,7 +11,6 @@
     create_mtls_session,
     fetch_with_retry_mtls,
 )
-from lambdas.tests.e2e.conftest import APIM_ENDPOINT, PDM_SNOMED
 from lambdas.tests.e2e.helpers.data_helper import PdmDataHelper
 
 pdm_data_helper = PdmDataHelper()
@@ -60,7 +59,9 @@ def test_create_document_virus(test_data):
     }
 
     # Attach EICAR data
-    eicar_string = r"X5O!P%@AP[4\PZX54(P^)7CC)7}$EICAR-STANDARD-ANTIVIRUS-TEST-FILE!$H+H*"
+    eicar_string = (
+        r"X5O!P%@AP[4\PZX54(P^)7CC)7}$EICAR-STANDARD-ANTIVIRUS-TEST-FILE!$H+H*"
+    )
     record["data"] = base64.b64encode(eicar_string.encode()).decode()
     payload = pdm_data_helper.create_upload_payload(record)
 
@@ -74,8 +75,8 @@ def test_create_document_virus(test_data):
     def condition(response_json):
         logging.info(response_json)
         return response_json.get("docStatus") in (
-        "cancelled",
-        "final",
+            "cancelled",
+            "final",
         )
 
     raw_retrieve_response = retrieve_document_with_retry(

lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_success.py

@@ -2,9 +2,6 @@
 import logging
 import os
 
-import pytest
-import requests
-
 from lambdas.tests.e2e.api.fhir.conftest import (
     MTLS_ENDPOINT,
     create_mtls_session,

lambdas/tests/unit/handlers/test_get_document_reference_handler.py

@@ -43,7 +43,6 @@ def mock_interaction_id():
 @pytest.fixture
 def mocked_bad_env_vars():
     env_vars = {
-        # "LLOYD_GEORGE_DYNAMODB_NAME": "mock_dynamodb_name",
         "PRESIGNED_ASSUME_ROLE": "mock_presigned_role",
         "APPCONFIG_APPLICATION": "mock_value",
         "APPCONFIG_ENVIRONMENT": "mock_value",

lambdas/tests/unit/handlers/test_search_patient_details_handler.py

@@ -6,6 +6,7 @@
 import pytest
 from handlers.search_patient_details_handler import lambda_handler
 from models.pds_models import PatientDetails
+from services.feature_flags_service import FeatureFlagService
 from utils.lambda_exceptions import SearchPatientException
 from utils.lambda_response import ApiGatewayResponse
 
@@ -40,12 +41,23 @@ def mocked_context(mocker):
     )
 
 
+@pytest.fixture
+def mock_check_if_ods_code_is_in_pilot(mocker):
+    mock_function = mocker.patch.object(
+        FeatureFlagService, "check_if_ods_code_is_in_pilot"
+    )
+    ods_in_pilot = mock_function.return_value = True
+    yield ods_in_pilot
+
+
 def test_lambda_handler_valid_id_returns_200(
     set_env,
     valid_id_event_with_auth_header,
     context,
     mocker,
     mocked_context,
+    mock_upload_document_iteration_3_enabled,
+    mock_check_if_ods_code_is_in_pilot,
 ):
     patient_details_object = PatientDetails(
         givenName=["Jane"],
@@ -100,7 +112,13 @@ def test_lambda_handler_invalid_id_returns_400(invalid_id_event, context):
 
 
 def test_lambda_handler_valid_id_not_in_pds_returns_404(
-    set_env, valid_id_event_with_auth_header, context, mocker, mocked_context
+    set_env,
+    valid_id_event_with_auth_header,
+    context,
+    mocker,
+    mocked_context,
+    mock_upload_document_iteration_3_enabled,
+    mock_check_if_ods_code_is_in_pilot,
 ):
     mocker.patch(
         "handlers.search_patient_details_handler.SearchPatientDetailsService.handle_search_patient_request",