From c651640eb0ad25de6d1a9afe69cfaf4fc6336e0e Mon Sep 17 00:00:00 2001 From: steph-torres-nhs <173282814+steph-torres-nhs@users.noreply.github.com> Date: Thu, 11 Dec 2025 14:58:10 +0000 Subject: [PATCH 1/3] [PRMP-1078] add util --- lambdas/tests/unit/utils/test_ods_utils.py | 26 +++++++++++++++++++++- lambdas/utils/ods_utils.py | 16 +++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/lambdas/tests/unit/utils/test_ods_utils.py b/lambdas/tests/unit/utils/test_ods_utils.py index ecb48c59af..d29f301dab 100644 --- a/lambdas/tests/unit/utils/test_ods_utils.py +++ b/lambdas/tests/unit/utils/test_ods_utils.py @@ -1,11 +1,23 @@ import pytest from enums.patient_ods_inactive_status import PatientOdsInactiveStatus +from tests.unit.conftest import TEST_CURRENT_GP_ODS +from utils.exceptions import OdsErrorException from utils.ods_utils import ( + extract_ods_code_from_request_context, extract_ods_role_code_with_r_prefix_from_role_codes_string, - is_ods_code_active, + is_ods_code_active ) +@pytest.fixture() +def mocked_request_context_with_ods(mocker): + mocked_context = mocker.MagicMock() + mocked_context.authorization = { + "selected_organisation": {"org_ods_code": TEST_CURRENT_GP_ODS}, + } + yield mocker.patch("utils.ods_utils.request_context", mocked_context) + + @pytest.mark.parametrize( "ods_code,expected", [ @@ -35,3 +47,15 @@ def test_process_role_code_returns_correct_role(role_code, expected): extract_ods_role_code_with_r_prefix_from_role_codes_string(role_code) == expected ) + + +def test_get_ods_code_from_request(mocked_request_context_with_ods): + + assert extract_ods_code_from_request_context() == TEST_CURRENT_GP_ODS + + +def test_get_ods_code_from_request_throws_exception_no_auth(mocker): + mocker.patch("utils.ods_utils.request_context", {}) + + with pytest.raises(OdsErrorException): + extract_ods_code_from_request_context() diff --git a/lambdas/utils/ods_utils.py b/lambdas/utils/ods_utils.py index 57e1c1a188..f335ae06ac 100644 --- a/lambdas/utils/ods_utils.py +++ b/lambdas/utils/ods_utils.py @@ -1,4 +1,6 @@ from enums.patient_ods_inactive_status import PatientOdsInactiveStatus +from utils.exceptions import OdsErrorException +from utils.request_context import request_context """ On PDS, GP ODS codes must be 6 characters long, see the 'epraccur' document here for info: @@ -21,3 +23,17 @@ def extract_ods_role_code_with_r_prefix_from_role_codes_string(role_codes) -> st for role_code in role_codes.split(":"): if role_code.startswith("R"): return role_code + + +def extract_ods_code_from_request_context() -> str: + try: + ods_code = request_context.authorization.get("selected_organisation", {}).get( + "org_ods_code" + ) + if not ods_code: + raise OdsErrorException() + + return ods_code + + except AttributeError: + raise OdsErrorException() \ No newline at end of file From 07a4c5e9045b8e40e511d241f6294e3586038aaf Mon Sep 17 00:00:00 2001 From: steph-torres-nhs <173282814+steph-torres-nhs@users.noreply.github.com> Date: Fri, 12 Dec 2025 13:55:25 +0000 Subject: [PATCH 2/3] [PRMP-1078] use ods util in patch review lambda --- .../handlers/patch_document_review_handler.py | 10 ++++---- .../test_patch_document_review_handler.py | 25 +++++++++---------- 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/lambdas/handlers/patch_document_review_handler.py b/lambdas/handlers/patch_document_review_handler.py index 2d0ca35bc7..56e27e6357 100644 --- a/lambdas/handlers/patch_document_review_handler.py +++ b/lambdas/handlers/patch_document_review_handler.py @@ -11,9 +11,11 @@ from utils.decorators.override_error_check import override_error_check from utils.decorators.set_audit_arg import set_request_context_for_logging from utils.decorators.validate_patient_id import validate_patient_id +from utils.exceptions import OdsErrorException from utils.lambda_exceptions import UpdateDocumentReviewException from utils.lambda_handler_utils import validate_review_path_parameters from utils.lambda_response import ApiGatewayResponse +from utils.ods_utils import extract_ods_code_from_request_context from utils.request_context import request_context logger = LoggingService(__name__) @@ -46,12 +48,10 @@ def lambda_handler(event, context): document_id, document_version = validate_review_path_parameters(event) - reviewer_ods_code = request_context.authorization.get( - "selected_organisation", {} - ).get("org_ods_code") + try: + reviewer_ods_code = extract_ods_code_from_request_context() - if not reviewer_ods_code: - logger.error("Missing ODS code in authorization token") + except OdsErrorException: raise UpdateDocumentReviewException( 401, LambdaError.DocumentReferenceUnauthorised ) diff --git a/lambdas/tests/unit/handlers/test_patch_document_review_handler.py b/lambdas/tests/unit/handlers/test_patch_document_review_handler.py index 721b65088c..cf8279bc63 100644 --- a/lambdas/tests/unit/handlers/test_patch_document_review_handler.py +++ b/lambdas/tests/unit/handlers/test_patch_document_review_handler.py @@ -4,6 +4,8 @@ from enums.document_review_status import DocumentReviewStatus from enums.lambda_error import LambdaError from handlers.patch_document_review_handler import lambda_handler +from tests.unit.conftest import TEST_CURRENT_GP_ODS +from utils.exceptions import OdsErrorException from utils.lambda_exceptions import UpdateDocumentReviewException from utils.lambda_response import ApiGatewayResponse @@ -130,25 +132,18 @@ def mocked_service(set_env, mocker): @pytest.fixture def mock_authorization(mocker): - mocked_context = mocker.MagicMock() - mocked_context.authorization = { - "selected_organisation": {"org_ods_code": TEST_REVIEWER_ODS_CODE}, - } - yield mocker.patch( - "handlers.patch_document_review_handler.request_context", mocked_context + return mocker.patch( + "handlers.patch_document_review_handler.extract_ods_code_from_request_context" ) @pytest.fixture def mock_missing_authorization(mocker): - mocked_context = mocker.MagicMock() - mocked_context.authorization = { - "selected_organisation": {"org_ods_code": None}, - } - yield mocker.patch( - "handlers.patch_document_review_handler.request_context", mocked_context + mock_auth = mocker.patch( + "handlers.patch_document_review_handler.extract_ods_code_from_request_context" ) - + mock_auth.side_effect = OdsErrorException() + yield mock_auth def test_lambda_handler_returns_200_when_document_review_approved( mocked_service, @@ -159,6 +154,7 @@ def test_lambda_handler_returns_200_when_document_review_approved( mock_upload_document_iteration_3_enabled, ): mocked_service.update_document_review.return_value = None + mock_authorization.return_value = TEST_CURRENT_GP_ODS expected = ApiGatewayResponse(200, "", "PATCH").create_api_gateway_response() @@ -186,6 +182,7 @@ def test_lambda_handler_returns_200_when_document_review_rejected( mock_upload_document_iteration_3_enabled, ): mocked_service.update_document_review.return_value = None + mock_authorization.return_value = TEST_CURRENT_GP_ODS expected = ApiGatewayResponse(200, "", "PATCH").create_api_gateway_response() @@ -209,6 +206,8 @@ def test_lambda_handler_returns_400_when_patient_id_missing( mock_authorization, mock_upload_document_iteration_3_enabled, ): + mock_authorization.return_value = TEST_CURRENT_GP_ODS + actual = lambda_handler(missing_patient_id_event, context) expected = ApiGatewayResponse( From b2c273d8f33ef5aae742dc015212d66399d59843 Mon Sep 17 00:00:00 2001 From: steph-torres-nhs <173282814+steph-torres-nhs@users.noreply.github.com> Date: Thu, 18 Dec 2025 11:43:35 +0000 Subject: [PATCH 3/3] [PRMP-1078] use util in search review handler --- .../search_document_review_handler.py | 20 ++--------- .../test_search_document_review_handler.py | 36 +++++-------------- 2 files changed, 11 insertions(+), 45 deletions(-) diff --git a/lambdas/handlers/search_document_review_handler.py b/lambdas/handlers/search_document_review_handler.py index 63f1525e57..c2ec8e027c 100644 --- a/lambdas/handlers/search_document_review_handler.py +++ b/lambdas/handlers/search_document_review_handler.py @@ -15,7 +15,7 @@ from utils.exceptions import OdsErrorException from utils.lambda_exceptions import DocumentReviewException from utils.lambda_response import ApiGatewayResponse -from utils.request_context import request_context +from utils.ods_utils import extract_ods_code_from_request_context logger = LoggingService(__name__) @@ -56,7 +56,7 @@ def lambda_handler(event, context): logger.info("Feature flag not enabled, event will not be processed") raise DocumentReviewException(403, LambdaError.FeatureFlagDisabled) - ods_code = get_ods_code_from_request_context() + ods_code = extract_ods_code_from_request_context() params = parse_querystring_parameters(event) @@ -86,22 +86,6 @@ def lambda_handler(event, context): ).create_api_gateway_response() -def get_ods_code_from_request_context(): - logger.info("Getting ODS code from request context") - try: - ods_code = request_context.authorization.get("selected_organisation", {}).get( - "org_ods_code" - ) - if not ods_code: - raise OdsErrorException() - - return ods_code - - except AttributeError as e: - logger.error(e) - raise DocumentReviewException(401, LambdaError.DocumentReviewMissingODS) - - def parse_querystring_parameters(event): logger.info("Parsing query string parameters.") params = event.get("queryStringParameters", {}) diff --git a/lambdas/tests/unit/handlers/test_search_document_review_handler.py b/lambdas/tests/unit/handlers/test_search_document_review_handler.py index a1db7793fb..93f81f9d4b 100644 --- a/lambdas/tests/unit/handlers/test_search_document_review_handler.py +++ b/lambdas/tests/unit/handlers/test_search_document_review_handler.py @@ -4,7 +4,6 @@ import pytest from enums.lambda_error import LambdaError from handlers.search_document_review_handler import ( - get_ods_code_from_request_context, lambda_handler, parse_querystring_parameters, ) @@ -13,6 +12,7 @@ from tests.unit.helpers.data.search_document_review.dynamo_response import ( MOCK_DOCUMENT_REVIEW_SEARCH_RESPONSE, ) +from utils.exceptions import OdsErrorException from utils.lambda_exceptions import DocumentReviewException from utils.lambda_response import ApiGatewayResponse @@ -92,38 +92,20 @@ def event_with_all_params(): @pytest.fixture() def mocked_request_context_with_ods(mocker): - mocked_context = mocker.MagicMock() - mocked_context.authorization = { - "selected_organisation": {"org_ods_code": TEST_CURRENT_GP_ODS}, - } - yield mocker.patch( - "handlers.search_document_review_handler.request_context", mocked_context + mock_extract = mocker.patch( + "handlers.search_document_review_handler.extract_ods_code_from_request_context" ) + mock_extract.return_value = TEST_CURRENT_GP_ODS + yield mock_extract @pytest.fixture() def mocked_request_context_without_ods(mocker): - mocked_context = mocker.MagicMock() - mocked_context.authorization = { - "selected_organisation": {"org_ods_code": ""}, - } - yield mocker.patch( - "handlers.search_document_review_handler.request_context", mocked_context + mocked_extract = mocker.patch( + "handlers.search_document_review_handler.extract_ods_code_from_request_context", ) - - -def test_get_ods_code_from_request(mocked_request_context_with_ods): - - assert get_ods_code_from_request_context() == TEST_CURRENT_GP_ODS - - -def test_get_ods_code_from_request_throws_exception_no_auth(mocker): - mocker.patch("handlers.search_document_review_handler.request_context", {}) - - with pytest.raises(DocumentReviewException) as e: - get_ods_code_from_request_context() - - assert e.value.status_code == 401 + mocked_extract.side_effect = OdsErrorException() + yield mocked_extract def test_handler_returns_401_response_no_ods_code_in_request_context(