From 0904a5b5f350cc479c855fb66b28d49f899b6d66 Mon Sep 17 00:00:00 2001 From: steph-torres-nhs <173282814+steph-torres-nhs@users.noreply.github.com> Date: Tue, 6 Jan 2026 10:10:13 +0000 Subject: [PATCH 01/17] [PRMP-975] add util to extract relevant querystring params --- .../document_reference_search_handler.py | 10 +++++++++- .../test_document_reference_search_handler.py | 20 ++++++++++++++++++- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/lambdas/handlers/document_reference_search_handler.py b/lambdas/handlers/document_reference_search_handler.py index 7e1aafdef..e34d55d36 100755 --- a/lambdas/handlers/document_reference_search_handler.py +++ b/lambdas/handlers/document_reference_search_handler.py @@ -28,7 +28,7 @@ def lambda_handler(event, context): request_context.app_interaction = LoggingAppInteraction.VIEW_PATIENT.value logger.info("Starting document reference search process") - nhs_number = extract_nhs_number_from_event(event) + nhs_number, next_page_token = extract_querystring_params(event) request_context.patient_nhs_no = nhs_number document_reference_search_service = DocumentReferenceSearchService() @@ -54,3 +54,11 @@ def lambda_handler(event, context): return ApiGatewayResponse( 204, json.dumps([]), "GET" ).create_api_gateway_response() + + + +def extract_querystring_params(event): + nhs_number = extract_nhs_number_from_event(event) + next_page_token = event["queryStringParameters"].get("nextPageToken") + + return nhs_number, next_page_token diff --git a/lambdas/tests/unit/handlers/test_document_reference_search_handler.py b/lambdas/tests/unit/handlers/test_document_reference_search_handler.py index c9411818f..4dcd30df1 100755 --- a/lambdas/tests/unit/handlers/test_document_reference_search_handler.py +++ b/lambdas/tests/unit/handlers/test_document_reference_search_handler.py @@ -1,9 +1,10 @@ import json +from copy import deepcopy from enum import Enum import pytest from enums.feature_flags import FeatureFlags -from handlers.document_reference_search_handler import lambda_handler +from handlers.document_reference_search_handler import lambda_handler, extract_querystring_params from tests.unit.helpers.data.dynamo.dynamo_responses import EXPECTED_RESPONSE from utils.lambda_exceptions import DocumentRefSearchException from utils.lambda_response import ApiGatewayResponse @@ -193,3 +194,20 @@ def test_lambda_handler_with_feature_flag_disabled_no_doc_status_filter( check_upload_completed=True, additional_filters=None, ) + + +def test_extract_querystring_params_next_page_token_present(valid_id_event_without_auth_header): + event = deepcopy(valid_id_event_without_auth_header) + event["queryStringParameters"].update({"nextPageToken": "abc"}) + + expected = ("9000000009", "abc") + + actual = extract_querystring_params(event) + + assert expected == actual + + +def test_extract_querystring_params_no_next_page_token(valid_id_event_without_auth_header): + expected = ("9000000009", None) + actual = extract_querystring_params(valid_id_event_without_auth_header) + assert expected == actual From d6a88c5974622d15ab12c3e3fe3c294acfc91cbc Mon Sep 17 00:00:00 2001 From: steph-torres-nhs <173282814+steph-torres-nhs@users.noreply.github.com> Date: Tue, 6 Jan 2026 14:00:25 +0000 Subject: [PATCH 02/17] [PRMP-975] remove functionality of searching multiple tables --- .../document_reference_search_service.py | 60 +++++++-------- .../test_document_reference_search_service.py | 76 ++++++++----------- ..._fhir_document_reference_search_service.py | 4 +- 3 files changed, 59 insertions(+), 81 deletions(-) diff --git a/lambdas/services/document_reference_search_service.py b/lambdas/services/document_reference_search_service.py index 0784c7dab..a7ba8d343 100644 --- a/lambdas/services/document_reference_search_service.py +++ b/lambdas/services/document_reference_search_service.py @@ -4,7 +4,7 @@ from botocore.exceptions import ClientError from enums.dynamo_filter import AttributeOperator -from enums.infrastructure import MAP_MTLS_TO_DYNAMO +from enums.infrastructure import MAP_MTLS_TO_DYNAMO, DynamoTables from enums.lambda_error import LambdaError from enums.metadata_field_names import DocumentReferenceMetadataFields from enums.mtls import MtlsCommonNames @@ -67,54 +67,46 @@ def get_document_references( ) raise DocumentRefSearchException(500, LambdaError.DocRefClient) - def _get_table_names(self, common_name: MtlsCommonNames | None) -> list[str]: - table_list = [] - try: - table_list = json.loads(os.environ["DYNAMODB_TABLE_LIST"]) - except JSONDecodeError as e: - logger.error(f"Failed to decode table list: {str(e)}") - raise - + def _get_table_names(self, common_name: MtlsCommonNames | None) -> str: if not common_name or common_name not in MtlsCommonNames: - return table_list + return str(DynamoTables.LLOYD_GEORGE) - return [str(MAP_MTLS_TO_DYNAMO[common_name])] + return str(MAP_MTLS_TO_DYNAMO[common_name]) def _search_tables_for_documents( self, nhs_number: str, - table_names: list[str], + table_name: str, return_fhir: bool, filters=None, check_upload_completed=False, ): document_resources = [] - for table_name in table_names: - logger.info(f"Searching for results in {table_name}") - filter_expression = self._get_filter_expression( - filters, upload_completed=check_upload_completed - ) + logger.info(f"Searching for results in {table_name}") + filter_expression = self._get_filter_expression( + filters, upload_completed=check_upload_completed + ) - if "coredocumentmetadata" not in table_name.lower(): - documents = self.fetch_documents_from_table_with_nhs_number( - nhs_number, table_name, query_filter=filter_expression - ) - else: - documents = self.fetch_documents_from_table( - search_condition=nhs_number, - search_key="NhsNumber", - table_name=table_name, - query_filter=filter_expression, - ) + if "coredocumentmetadata" not in table_name.lower(): + documents = self.fetch_documents_from_table_with_nhs_number( + nhs_number, table_name, query_filter=filter_expression + ) + else: + documents = self.fetch_documents_from_table( + search_condition=nhs_number, + search_key="NhsNumber", + table_name=table_name, + query_filter=filter_expression, + ) - if check_upload_completed: - self._validate_upload_status(documents) + if check_upload_completed: + self._validate_upload_status(documents) - processed_documents = self._process_documents( - documents, return_fhir=return_fhir - ) - document_resources.extend(processed_documents) + processed_documents = self._process_documents( + documents, return_fhir=return_fhir + ) + document_resources.extend(processed_documents) logger.info(f"Found {len(document_resources)} document references") diff --git a/lambdas/tests/unit/services/test_document_reference_search_service.py b/lambdas/tests/unit/services/test_document_reference_search_service.py index 096d3f83c..7d1f9a857 100644 --- a/lambdas/tests/unit/services/test_document_reference_search_service.py +++ b/lambdas/tests/unit/services/test_document_reference_search_service.py @@ -1,5 +1,4 @@ import json -from json import JSONDecodeError from unittest.mock import MagicMock, call import pytest @@ -58,14 +57,6 @@ def mock_filter_builder(mocker): return mock_filter -def test_get_document_references_raise_json_error_when_no_table_list( - mock_document_service, monkeypatch -): - monkeypatch.setenv("DYNAMODB_TABLE_LIST", "") - with pytest.raises(JSONDecodeError): - mock_document_service._get_table_names(None) - - def test_search_tables_for_documents_raise_validation_error( mock_document_service, validation_error ): @@ -74,7 +65,7 @@ def test_search_tables_for_documents_raise_validation_error( ) with pytest.raises(ValidationError): mock_document_service._search_tables_for_documents( - "1234567890", ["table1", "table2"], return_fhir=True + "1234567890", "table1", return_fhir=True ) @@ -92,7 +83,7 @@ def test_get_document_references_raise_client_error(mock_document_service): ) with pytest.raises(ClientError): mock_document_service._search_tables_for_documents( - "1234567890", ["table1", "table2"], return_fhir=True + "1234567890", "table1", return_fhir=True ) @@ -103,7 +94,7 @@ def test_get_document_references_raise_dynamodb_error(mock_document_service): with pytest.raises(DynamoServiceException): mock_document_service._search_tables_for_documents( "1234567890", - ["table1", "table2"], + "table1", return_fhir=True, check_upload_completed=False, ) @@ -115,7 +106,7 @@ def test_get_document_references_dynamo_return_empty_response_with_fhir( mock_document_service.fetch_documents_from_table_with_nhs_number.return_value = [] actual = mock_document_service._search_tables_for_documents( - "1234567890", ["table1", "table2"], return_fhir=True + "1234567890", "table1", return_fhir=True ) assert actual["resourceType"] == "Bundle" assert actual["entry"] == [] @@ -126,7 +117,7 @@ def test_get_document_references_dynamo_return_empty_response(mock_document_serv mock_document_service.fetch_documents_from_table_with_nhs_number.return_value = [] actual = mock_document_service._search_tables_for_documents( - "1234567890", ["table1", "table2"], return_fhir=False + "1234567890", "table1", return_fhir=False ) assert actual is None @@ -154,24 +145,24 @@ def test_build_document_model_response(mock_document_service, monkeypatch): assert actual == expected_results -def test_get_document_references_dynamo_return_successful_response_multiple_tables( - mock_document_service, mocker -): - mock_fetch_documents = mocker.MagicMock(return_value=MOCK_DOCUMENT_REFERENCE) - mock_document_service.fetch_documents_from_table_with_nhs_number = ( - mock_fetch_documents - ) - mock_document_service._validate_upload_status = mocker.MagicMock() - mock_document_service._process_documents = mocker.MagicMock( - return_value=[EXPECTED_RESPONSE] - ) - expected_results = [EXPECTED_RESPONSE, EXPECTED_RESPONSE] - - actual = mock_document_service._search_tables_for_documents( - "1111111111", ["table1", "table2"], False - ) - - assert actual == expected_results +# def test_get_document_references_dynamo_return_successful_response_multiple_tables( +# mock_document_service, mocker +# ): +# mock_fetch_documents = mocker.MagicMock(return_value=MOCK_DOCUMENT_REFERENCE) +# mock_document_service.fetch_documents_from_table_with_nhs_number = ( +# mock_fetch_documents +# ) +# mock_document_service._validate_upload_status = mocker.MagicMock() +# mock_document_service._process_documents = mocker.MagicMock( +# return_value=[EXPECTED_RESPONSE] +# ) +# expected_results = [EXPECTED_RESPONSE, EXPECTED_RESPONSE] +# +# actual = mock_document_service._search_tables_for_documents( +# "1111111111", "table1", False +# ) +# +# assert actual == expected_results def test_get_document_references_raise_error_when_upload_is_in_process( @@ -184,7 +175,7 @@ def test_get_document_references_raise_error_when_upload_is_in_process( def test_get_document_references_success(mock_document_service, mocker): - mock_get_table_names = mocker.MagicMock(return_value=["table1", "table2"]) + mock_get_table_names = mocker.MagicMock(return_value="table1") mock_document_service._get_table_names = mock_get_table_names mock_search_document = mocker.MagicMock(return_value=[{"id": "123"}]) mock_document_service._search_tables_for_documents = mock_search_document @@ -196,7 +187,7 @@ def test_get_document_references_success(mock_document_service, mocker): assert result == [{"id": "123"}] mock_get_table_names.assert_called_once() mock_search_document.assert_called_once_with( - "1234567890", ["table1", "table2"], False, None, True + "1234567890", "table1", False, None, True ) @@ -224,25 +215,23 @@ def test_search_tables_for_documents_non_fhir(mock_document_service, mocker): mock_document_service._process_documents = mock_process_document_non_fhir result_non_fhir = mock_document_service._search_tables_for_documents( "1234567890", - ["table1", "table2"], + "table1", return_fhir=False, check_upload_completed=True, ) - assert result_non_fhir == [mock_document_id, mock_document_id] + assert result_non_fhir == [mock_document_id] mock_process_document_non_fhir.assert_has_calls( [ call(MOCK_DOCUMENT_REFERENCE, return_fhir=False), - call(MOCK_DOCUMENT_REFERENCE, return_fhir=False), ] ) - assert mock_fetch_document_method.call_count == 2 + assert mock_fetch_document_method.call_count == 1 mock_fetch_document_method.assert_has_calls( [ call("1234567890", "table1", query_filter=UploadCompleted), - call("1234567890", "table2", query_filter=UploadCompleted), ] ) @@ -259,28 +248,25 @@ def test_search_tables_for_documents_fhir(mock_document_service, mocker): mock_document_service._process_documents = mock_process_document_fhir result_fhir = mock_document_service._search_tables_for_documents( "1234567890", - ["table1", "table2"], + "table1", return_fhir=True, check_upload_completed=True, ) assert result_fhir["resourceType"] == "Bundle" assert result_fhir["type"] == "searchset" - assert result_fhir["total"] == 2 - assert len(result_fhir["entry"]) == 2 + assert result_fhir["total"] == 1 + assert len(result_fhir["entry"]) == 1 assert result_fhir["entry"][0]["resource"] == mock_fhir_doc - assert result_fhir["entry"][1]["resource"] == mock_fhir_doc mock_fetch_document_method.assert_has_calls( [ call("1234567890", "table1", query_filter=UploadCompleted), - call("1234567890", "table2", query_filter=UploadCompleted), ] ) mock_process_document_fhir.assert_has_calls( [ call(MOCK_DOCUMENT_REFERENCE, return_fhir=True), - call(MOCK_DOCUMENT_REFERENCE, return_fhir=True), ] ) diff --git a/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_search_service.py b/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_search_service.py index 8d78c40e5..b586730b5 100644 --- a/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_search_service.py +++ b/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_search_service.py @@ -67,9 +67,9 @@ def mock_filter_builder(mocker): }, }, }, - ["dev_COREDocumentMetadata"], + "dev_COREDocumentMetadata", ), - ({}, ["test_pdm_dynamoDB_table", "test_lg_dynamoDB_table"]), + ({}, "dev_LloydGeorgeReferenceMetadata"), ], ) def test_get_pdm_table(set_env, mock_document_service, common_name, expected): From 0aae7338893c10f5fb0300e9fa947ee40fd0850c Mon Sep 17 00:00:00 2001 From: steph-torres-nhs <173282814+steph-torres-nhs@users.noreply.github.com> Date: Mon, 12 Jan 2026 10:45:52 +0000 Subject: [PATCH 03/17] [PRMP-975] extract limit from querystring --- lambdas/handlers/document_reference_search_handler.py | 5 +++-- lambdas/services/document_reference_search_service.py | 6 +++--- .../unit/handlers/test_document_reference_search_handler.py | 4 ++-- .../unit/services/test_document_reference_search_service.py | 4 ++-- .../test_pdm_get_fhir_document_reference_search_service.py | 2 +- 5 files changed, 11 insertions(+), 10 deletions(-) diff --git a/lambdas/handlers/document_reference_search_handler.py b/lambdas/handlers/document_reference_search_handler.py index e34d55d36..9a60e878c 100755 --- a/lambdas/handlers/document_reference_search_handler.py +++ b/lambdas/handlers/document_reference_search_handler.py @@ -28,7 +28,7 @@ def lambda_handler(event, context): request_context.app_interaction = LoggingAppInteraction.VIEW_PATIENT.value logger.info("Starting document reference search process") - nhs_number, next_page_token = extract_querystring_params(event) + nhs_number, next_page_token, limit = extract_querystring_params(event) request_context.patient_nhs_no = nhs_number document_reference_search_service = DocumentReferenceSearchService() @@ -60,5 +60,6 @@ def lambda_handler(event, context): def extract_querystring_params(event): nhs_number = extract_nhs_number_from_event(event) next_page_token = event["queryStringParameters"].get("nextPageToken") + limit = event["queryStringParameters"].get("limit") - return nhs_number, next_page_token + return nhs_number, next_page_token, limit diff --git a/lambdas/services/document_reference_search_service.py b/lambdas/services/document_reference_search_service.py index a7ba8d343..551003d4c 100644 --- a/lambdas/services/document_reference_search_service.py +++ b/lambdas/services/document_reference_search_service.py @@ -46,10 +46,10 @@ def get_document_references( api_request_context=api_request_context ) try: - list_of_table_names = self._get_table_names(common_name) + table_name = self._get_table_name(common_name) results = self._search_tables_for_documents( nhs_number, - list_of_table_names, + table_name, return_fhir, additional_filters, check_upload_completed, @@ -67,7 +67,7 @@ def get_document_references( ) raise DocumentRefSearchException(500, LambdaError.DocRefClient) - def _get_table_names(self, common_name: MtlsCommonNames | None) -> str: + def _get_table_name(self, common_name: MtlsCommonNames | None) -> str: if not common_name or common_name not in MtlsCommonNames: return str(DynamoTables.LLOYD_GEORGE) diff --git a/lambdas/tests/unit/handlers/test_document_reference_search_handler.py b/lambdas/tests/unit/handlers/test_document_reference_search_handler.py index 4dcd30df1..878aa2f26 100755 --- a/lambdas/tests/unit/handlers/test_document_reference_search_handler.py +++ b/lambdas/tests/unit/handlers/test_document_reference_search_handler.py @@ -200,7 +200,7 @@ def test_extract_querystring_params_next_page_token_present(valid_id_event_witho event = deepcopy(valid_id_event_without_auth_header) event["queryStringParameters"].update({"nextPageToken": "abc"}) - expected = ("9000000009", "abc") + expected = ("9000000009", "abc", None) actual = extract_querystring_params(event) @@ -208,6 +208,6 @@ def test_extract_querystring_params_next_page_token_present(valid_id_event_witho def test_extract_querystring_params_no_next_page_token(valid_id_event_without_auth_header): - expected = ("9000000009", None) + expected = ("9000000009", None, None) actual = extract_querystring_params(valid_id_event_without_auth_header) assert expected == actual diff --git a/lambdas/tests/unit/services/test_document_reference_search_service.py b/lambdas/tests/unit/services/test_document_reference_search_service.py index 7d1f9a857..64a02e4c5 100644 --- a/lambdas/tests/unit/services/test_document_reference_search_service.py +++ b/lambdas/tests/unit/services/test_document_reference_search_service.py @@ -176,7 +176,7 @@ def test_get_document_references_raise_error_when_upload_is_in_process( def test_get_document_references_success(mock_document_service, mocker): mock_get_table_names = mocker.MagicMock(return_value="table1") - mock_document_service._get_table_names = mock_get_table_names + mock_document_service._get_table_name = mock_get_table_names mock_search_document = mocker.MagicMock(return_value=[{"id": "123"}]) mock_document_service._search_tables_for_documents = mock_search_document @@ -192,7 +192,7 @@ def test_get_document_references_success(mock_document_service, mocker): def test_get_document_references_exception(mock_document_service, mocker): - mock_document_service._get_table_names = mocker.MagicMock( + mock_document_service._get_table_name = mocker.MagicMock( side_effect=DynamoServiceException ) diff --git a/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_search_service.py b/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_search_service.py index 357d802a9..64d39a19e 100644 --- a/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_search_service.py +++ b/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_search_service.py @@ -86,7 +86,7 @@ def test_get_pdm_table( set_env, mock_document_service, common_name, expected, mock_mtls_common_names ): cn = validate_common_name_in_mtls(common_name) - tables = mock_document_service._get_table_names(cn) + tables = mock_document_service._get_table_name(cn) assert tables == expected From 65e3debaad20276088eb0897278df24b12ab777a Mon Sep 17 00:00:00 2001 From: steph-torres-nhs <173282814+steph-torres-nhs@users.noreply.github.com> Date: Mon, 12 Jan 2026 16:06:35 +0000 Subject: [PATCH 04/17] [PRMP-975] add pagination to doc ref search service --- .../document_reference_search_handler.py | 15 +++++++---- .../document_reference_search_service.py | 27 +++++++++++++++++++ 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/lambdas/handlers/document_reference_search_handler.py b/lambdas/handlers/document_reference_search_handler.py index 9a60e878c..5a1354b71 100755 --- a/lambdas/handlers/document_reference_search_handler.py +++ b/lambdas/handlers/document_reference_search_handler.py @@ -10,7 +10,6 @@ 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 ( - extract_nhs_number_from_event, validate_patient_id, ) from utils.lambda_response import ApiGatewayResponse @@ -41,9 +40,15 @@ def lambda_handler(event, context): doc_status_filter = ( {"doc_status": "final"} if doc_upload_iteration2_enabled else None ) - response = document_reference_search_service.get_document_references( - nhs_number, check_upload_completed=True, additional_filters=doc_status_filter - ) + + if limit or next_page_token: + document_reference_search_service.get_paginated_references_by_nhs_number( + nhs_number=nhs_number, limit=limit, next_page_token=next_page_token, filter=doc_status_filter + ) + else: + response = document_reference_search_service.get_document_references( + nhs_number, check_upload_completed=True, additional_filters=doc_status_filter + ) logger.info("User is able to view docs", {"Result": "Successful viewing docs"}) if response: @@ -58,7 +63,7 @@ def lambda_handler(event, context): def extract_querystring_params(event): - nhs_number = extract_nhs_number_from_event(event) + nhs_number = event["queryStringParameters"]["patientId"] next_page_token = event["queryStringParameters"].get("nextPageToken") limit = event["queryStringParameters"].get("limit") diff --git a/lambdas/services/document_reference_search_service.py b/lambdas/services/document_reference_search_service.py index 551003d4c..8d6f490f4 100644 --- a/lambdas/services/document_reference_search_service.py +++ b/lambdas/services/document_reference_search_service.py @@ -17,6 +17,7 @@ from utils.audit_logging_setup import LoggingService from utils.common_query_filters import NotDeleted, UploadCompleted from utils.dynamo_query_filter_builder import DynamoQueryFilterBuilder +from utils.dynamo_utils import build_mixed_condition_expression from utils.exceptions import DynamoServiceException from utils.lambda_exceptions import DocumentRefSearchException from utils.lambda_header_utils import validate_common_name_in_mtls @@ -233,3 +234,29 @@ def create_document_reference_fhir_response( .model_dump(exclude_none=True) ) return fhir_document_reference + + + def get_paginated_references_by_nhs_number( + self, + nhs_number: str, + limit: int | None = None, + next_page_token: str | None = None, + filter: dict | None = None, + ): + + filter_expression, condition_attribute_names, condition_attribute_values = None, None, None + + if filter: + filter_expression, condition_attribute_names, condition_attribute_values = ( + build_mixed_condition_expression([filter]) + ) + self.query_table_with_paginator( + index_name="NhsNumberIndex", + search_key="NhsNumber", + search_condition=nhs_number, + limit=limit, + start_key=next_page_token, + filter_expression=filter_expression, + expression_attribute_names=condition_attribute_names, + expression_attribute_values=condition_attribute_values, + ) \ No newline at end of file From 50a322b02b23eac578cb3e9df2c961cc3308e4ce Mon Sep 17 00:00:00 2001 From: steph-torres-nhs <173282814+steph-torres-nhs@users.noreply.github.com> Date: Mon, 12 Jan 2026 16:49:36 +0000 Subject: [PATCH 05/17] [PRMP-975] add query with pagination to doc ref search --- .../handlers/document_reference_search_handler.py | 2 +- .../services/document_reference_search_service.py | 13 +++++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/lambdas/handlers/document_reference_search_handler.py b/lambdas/handlers/document_reference_search_handler.py index 5a1354b71..fa56a8aac 100755 --- a/lambdas/handlers/document_reference_search_handler.py +++ b/lambdas/handlers/document_reference_search_handler.py @@ -42,7 +42,7 @@ def lambda_handler(event, context): ) if limit or next_page_token: - document_reference_search_service.get_paginated_references_by_nhs_number( + response = document_reference_search_service.get_paginated_references_by_nhs_number( nhs_number=nhs_number, limit=limit, next_page_token=next_page_token, filter=doc_status_filter ) else: diff --git a/lambdas/services/document_reference_search_service.py b/lambdas/services/document_reference_search_service.py index 8d6f490f4..cc091ad68 100644 --- a/lambdas/services/document_reference_search_service.py +++ b/lambdas/services/document_reference_search_service.py @@ -242,6 +242,7 @@ def get_paginated_references_by_nhs_number( limit: int | None = None, next_page_token: str | None = None, filter: dict | None = None, + api_request_context: dict = {} ): filter_expression, condition_attribute_names, condition_attribute_values = None, None, None @@ -250,7 +251,13 @@ def get_paginated_references_by_nhs_number( filter_expression, condition_attribute_names, condition_attribute_values = ( build_mixed_condition_expression([filter]) ) - self.query_table_with_paginator( + + common_name = validate_common_name_in_mtls( + api_request_context=api_request_context + ) + + references, next_page_token = self.query_table_with_paginator( + table_name=self._get_table_name(common_name), index_name="NhsNumberIndex", search_key="NhsNumber", search_condition=nhs_number, @@ -259,4 +266,6 @@ def get_paginated_references_by_nhs_number( filter_expression=filter_expression, expression_attribute_names=condition_attribute_names, expression_attribute_values=condition_attribute_values, - ) \ No newline at end of file + ) + + return {"references": [self._build_document_model(ref) for ref in references], "next_page_token": next_page_token} \ No newline at end of file From 44d0f47f5e705d0d41f961c5bfcef556c5b50e49 Mon Sep 17 00:00:00 2001 From: steph-torres-nhs <173282814+steph-torres-nhs@users.noreply.github.com> Date: Tue, 13 Jan 2026 09:52:15 +0000 Subject: [PATCH 06/17] [PRMP-975] amend how filter is handled --- lambdas/handlers/document_reference_search_handler.py | 1 + lambdas/services/document_reference_search_service.py | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/lambdas/handlers/document_reference_search_handler.py b/lambdas/handlers/document_reference_search_handler.py index fa56a8aac..89d6974e6 100755 --- a/lambdas/handlers/document_reference_search_handler.py +++ b/lambdas/handlers/document_reference_search_handler.py @@ -42,6 +42,7 @@ def lambda_handler(event, context): ) if limit or next_page_token: + logger.info("Searching for patient references with pagination.") response = document_reference_search_service.get_paginated_references_by_nhs_number( nhs_number=nhs_number, limit=limit, next_page_token=next_page_token, filter=doc_status_filter ) diff --git a/lambdas/services/document_reference_search_service.py b/lambdas/services/document_reference_search_service.py index cc091ad68..5ee0ab388 100644 --- a/lambdas/services/document_reference_search_service.py +++ b/lambdas/services/document_reference_search_service.py @@ -248,8 +248,10 @@ def get_paginated_references_by_nhs_number( filter_expression, condition_attribute_names, condition_attribute_values = None, None, None if filter: + logger.info(f"Building filter expression for {filter}") + conditions = [{"field": key, "operator": "=", "value": value} for key, value in filter.items()] filter_expression, condition_attribute_names, condition_attribute_values = ( - build_mixed_condition_expression([filter]) + build_mixed_condition_expression(conditions) ) common_name = validate_common_name_in_mtls( From ed30447dc2c7876307b4c4f7c7acd0eb3280f347 Mon Sep 17 00:00:00 2001 From: steph-torres-nhs <173282814+steph-torres-nhs@users.noreply.github.com> Date: Tue, 13 Jan 2026 10:38:31 +0000 Subject: [PATCH 07/17] [PRMP-975] update lg dynamo mock response --- lambdas/tests/unit/conftest.py | 1 + .../helpers/data/dynamo/dynamo_responses.py | 11 ++++-- .../tests/unit/helpers/data/test_documents.py | 5 ++- .../test_document_reference_search_service.py | 38 ++++++++++++++++--- ...est_get_fhir_document_reference_service.py | 1 + .../services/test_pdf_stitching_service.py | 6 +++ 6 files changed, 53 insertions(+), 9 deletions(-) diff --git a/lambdas/tests/unit/conftest.py b/lambdas/tests/unit/conftest.py index df5d7820e..20101d7d0 100644 --- a/lambdas/tests/unit/conftest.py +++ b/lambdas/tests/unit/conftest.py @@ -84,6 +84,7 @@ TEST_UUID = "1234-4567-8912-HSDF-TEST" TEST_FILE_KEY = "test_file_key" TEST_FILE_NAME = "test.pdf" +TEST_FILE_SIZE = 24000 TEST_VIRUS_SCANNER_RESULT = "not_scanned" TEST_DOCUMENT_LOCATION = f"s3://{MOCK_BUCKET}/{TEST_FILE_KEY}" TEST_CURRENT_GP_ODS = "Y12345" diff --git a/lambdas/tests/unit/helpers/data/dynamo/dynamo_responses.py b/lambdas/tests/unit/helpers/data/dynamo/dynamo_responses.py index b957c819f..3c2b50332 100755 --- a/lambdas/tests/unit/helpers/data/dynamo/dynamo_responses.py +++ b/lambdas/tests/unit/helpers/data/dynamo/dynamo_responses.py @@ -1,3 +1,5 @@ +from tests.unit.conftest import TEST_FILE_SIZE, TEST_NHS_NUMBER + MOCK_SEARCH_RESPONSE = { "Items": [ { @@ -7,7 +9,8 @@ "DocStatus": "final", "FileLocation": "s3://test-s3-bucket/9000000009/test-key-123", "FileName": "document.csv", - "NhsNumber": "9000000009", + "FileSize": TEST_FILE_SIZE, + "NhsNumber": TEST_NHS_NUMBER, "VirusScannerResult": "Clean", "CurrentGpOds": "Y12345", "Uploaded": "True", @@ -22,7 +25,8 @@ "DocStatus": "final", "FileLocation": "s3://test-s3-bucket/9000000009/test-key-223", "FileName": "results.pdf", - "NhsNumber": "9000000009", + "FileSize": TEST_FILE_SIZE, + "NhsNumber": TEST_NHS_NUMBER, "VirusScannerResult": "Clean", "CurrentGpOds": "Y12345", "Uploaded": "True", @@ -37,7 +41,8 @@ "DocStatus": "final", "FileLocation": "s3://test-s3-bucket/9000000009/test-key-323", "FileName": "output.csv", - "NhsNumber": "9000000009", + "FileSize": TEST_FILE_SIZE, + "NhsNumber": TEST_NHS_NUMBER, "VirusScannerResult": "Clean", "CurrentGpOds": "Y12345", "Uploaded": "True", diff --git a/lambdas/tests/unit/helpers/data/test_documents.py b/lambdas/tests/unit/helpers/data/test_documents.py index 7f573e042..b3ce442a6 100644 --- a/lambdas/tests/unit/helpers/data/test_documents.py +++ b/lambdas/tests/unit/helpers/data/test_documents.py @@ -8,7 +8,7 @@ MOCK_LG_BUCKET, MOCK_LG_STAGING_STORE_BUCKET_ENV_NAME, TEST_NHS_NUMBER, - TEST_UUID, + TEST_UUID, TEST_FILE_SIZE, ) from tests.unit.helpers.data.dynamo.dynamo_responses import MOCK_SEARCH_RESPONSE from enums.snomed_codes import SnomedCodes @@ -55,14 +55,17 @@ def create_test_lloyd_george_doc_store_refs( refs[0].file_name = filename_1 refs[0].s3_file_key = f"{TEST_NHS_NUMBER}/test-key-1" refs[0].file_location = f"s3://{MOCK_LG_BUCKET}/{TEST_NHS_NUMBER}/test-key-1" + refs[0].file_size = TEST_FILE_SIZE refs[0].s3_bucket_name = MOCK_LG_BUCKET refs[1].file_name = filename_2 refs[1].s3_file_key = f"{TEST_NHS_NUMBER}/test-key-2" refs[1].file_location = f"s3://{MOCK_LG_BUCKET}/{TEST_NHS_NUMBER}/test-key-2" + refs[1].file_size = TEST_FILE_SIZE refs[1].s3_bucket_name = MOCK_LG_BUCKET refs[2].file_name = filename_3 refs[2].s3_file_key = f"{TEST_NHS_NUMBER}/test-key-3" refs[2].file_location = f"s3://{MOCK_LG_BUCKET}/{TEST_NHS_NUMBER}/test-key-3" + refs[2].file_size = TEST_FILE_SIZE refs[2].s3_bucket_name = MOCK_LG_BUCKET if override: diff --git a/lambdas/tests/unit/services/test_document_reference_search_service.py b/lambdas/tests/unit/services/test_document_reference_search_service.py index 64a02e4c5..c890249de 100644 --- a/lambdas/tests/unit/services/test_document_reference_search_service.py +++ b/lambdas/tests/unit/services/test_document_reference_search_service.py @@ -13,7 +13,7 @@ from pydantic import ValidationError from services.document_reference_search_service import DocumentReferenceSearchService from tests.unit.helpers.data.dynamo.dynamo_responses import MOCK_SEARCH_RESPONSE -from tests.unit.conftest import APIM_API_URL +from tests.unit.conftest import APIM_API_URL, TEST_FILE_SIZE, TEST_NHS_NUMBER from utils.common_query_filters import NotDeleted, UploadCompleted from utils.exceptions import DynamoServiceException from utils.lambda_exceptions import DocumentRefSearchException @@ -22,25 +22,25 @@ DocumentReference.model_validate(MOCK_SEARCH_RESPONSE["Items"][0]) ] -MOCK_FILE_SIZE = 24000 - EXPECTED_RESPONSE = { "created": "2024-01-01T12:00:00.000Z", "fileName": "document.csv", "virusScannerResult": "Clean", "id": "3d8683b9-1665-40d2-8499-6e8302d507ff", - "fileSize": MOCK_FILE_SIZE, + "fileSize": TEST_FILE_SIZE, "version": "1", "contentType": "application/pdf", "documentSnomedCodeType": SnomedCodes.LLOYD_GEORGE.value.code, } +MOCK_NEXT_PAGE_TOKEN = "thisisaencodedtoken" + @pytest.fixture def mock_document_service(mocker, set_env): service = DocumentReferenceSearchService() mock_s3_service = mocker.patch.object(service, "s3_service") - mocker.patch.object(mock_s3_service, "get_file_size", return_value=MOCK_FILE_SIZE) + mocker.patch.object(mock_s3_service, "get_file_size", return_value=TEST_FILE_SIZE) mocker.patch.object(service, "dynamo_service") mocker.patch.object(service, "fetch_documents_from_table_with_nhs_number") mocker.patch.object(service, "is_upload_in_process", return_value=False) @@ -489,3 +489,31 @@ def test_build_filter_expression_defaults(mock_document_service): actual_filter = mock_document_service._build_filter_expression(filter_values) assert actual_filter == expected_filter + + +def test_get_paginated_reference_by_nhs_number_returns_references_and_token( + mock_document_service, + mocker +): + mock_paginate = mocker.patch.object(mock_document_service, "query_table_with_paginator") + mock_paginate.return_value = (MOCK_DOCUMENT_REFERENCE, MOCK_NEXT_PAGE_TOKEN) + expected = {"references": [EXPECTED_RESPONSE], "next_page_token": MOCK_NEXT_PAGE_TOKEN} + + actual = mock_document_service.get_paginated_references_by_nhs_number( + nhs_number=TEST_NHS_NUMBER, + ) + + mock_paginate.assert_called_with( + table_name="dev_LloydGeorgeReferenceMetadata", + index_name="NhsNumberIndex", + search_key="NhsNumber", + search_condition=TEST_NHS_NUMBER, + limit=None, + start_key=None, + filter_expression=None, + expression_attribute_names=None, + expression_attribute_values=None + ) + + assert actual == expected + diff --git a/lambdas/tests/unit/services/test_get_fhir_document_reference_service.py b/lambdas/tests/unit/services/test_get_fhir_document_reference_service.py index ece775b23..c0a0c67fe 100644 --- a/lambdas/tests/unit/services/test_get_fhir_document_reference_service.py +++ b/lambdas/tests/unit/services/test_get_fhir_document_reference_service.py @@ -125,6 +125,7 @@ def test_create_document_reference_fhir_response_with_presign_url_document_data( # Modify the document reference to test different values modified_doc = copy.deepcopy(test_doc) modified_doc.file_name = "different_file.pdf" + modified_doc.file_size = 8000000 modified_doc.created = "2023-05-15T10:30:00.000Z" modified_doc.document_scan_creation = "2023-05-15" diff --git a/lambdas/tests/unit/services/test_pdf_stitching_service.py b/lambdas/tests/unit/services/test_pdf_stitching_service.py index a4babeca8..72504f20d 100644 --- a/lambdas/tests/unit/services/test_pdf_stitching_service.py +++ b/lambdas/tests/unit/services/test_pdf_stitching_service.py @@ -326,6 +326,7 @@ def test_migrate_multipart_references(mock_service): "DocumentSnomedCodeType": "16521000000101", "FileLocation": f"{TEST_DOCUMENT_REFERENCES[0].file_location}", "FileName": f"{TEST_DOCUMENT_REFERENCES[0].file_name}", + "FileSize": TEST_DOCUMENT_REFERENCES[0].file_size, "ID": f"{TEST_DOCUMENT_REFERENCES[0].id}", "LastUpdated": 1704110400, "NhsNumber": f"{TEST_DOCUMENT_REFERENCES[0].nhs_number}", @@ -347,6 +348,7 @@ def test_migrate_multipart_references(mock_service): "DocumentSnomedCodeType": "16521000000101", "FileLocation": f"{TEST_DOCUMENT_REFERENCES[1].file_location}", "FileName": f"{TEST_DOCUMENT_REFERENCES[1].file_name}", + "FileSize": TEST_DOCUMENT_REFERENCES[1].file_size, "ID": f"{TEST_DOCUMENT_REFERENCES[1].id}", "LastUpdated": 1704110400, "NhsNumber": f"{TEST_DOCUMENT_REFERENCES[1].nhs_number}", @@ -368,6 +370,7 @@ def test_migrate_multipart_references(mock_service): "DocumentSnomedCodeType": "16521000000101", "FileLocation": f"{TEST_DOCUMENT_REFERENCES[2].file_location}", "FileName": f"{TEST_DOCUMENT_REFERENCES[2].file_name}", + "FileSize": TEST_DOCUMENT_REFERENCES[2].file_size, "ID": f"{TEST_DOCUMENT_REFERENCES[2].id}", "LastUpdated": 1704110400, "NhsNumber": f"{TEST_DOCUMENT_REFERENCES[2].nhs_number}", @@ -596,6 +599,7 @@ def test_rollback_reference_migration(mock_service): "DocumentSnomedCodeType": "16521000000101", "FileLocation": f"{TEST_DOCUMENT_REFERENCES[0].file_location}", "FileName": f"{TEST_DOCUMENT_REFERENCES[0].file_name}", + "FileSize": TEST_DOCUMENT_REFERENCES[0].file_size, "ID": f"{TEST_DOCUMENT_REFERENCES[0].id}", "LastUpdated": 1704110400, "NhsNumber": f"{TEST_DOCUMENT_REFERENCES[0].nhs_number}", @@ -618,6 +622,7 @@ def test_rollback_reference_migration(mock_service): "DocumentSnomedCodeType": "16521000000101", "FileLocation": f"{TEST_DOCUMENT_REFERENCES[1].file_location}", "FileName": f"{TEST_DOCUMENT_REFERENCES[1].file_name}", + "FileSize": TEST_DOCUMENT_REFERENCES[1].file_size, "ID": f"{TEST_DOCUMENT_REFERENCES[1].id}", "LastUpdated": 1704110400, "NhsNumber": f"{TEST_DOCUMENT_REFERENCES[1].nhs_number}", @@ -640,6 +645,7 @@ def test_rollback_reference_migration(mock_service): "DocumentSnomedCodeType": "16521000000101", "FileLocation": f"{TEST_DOCUMENT_REFERENCES[2].file_location}", "FileName": f"{TEST_DOCUMENT_REFERENCES[2].file_name}", + "FileSize": TEST_DOCUMENT_REFERENCES[2].file_size, "ID": f"{TEST_DOCUMENT_REFERENCES[2].id}", "LastUpdated": 1704110400, "NhsNumber": f"{TEST_DOCUMENT_REFERENCES[2].nhs_number}", From 7ff97f9e6600f8ba0cefdd589be0824beb59caa5 Mon Sep 17 00:00:00 2001 From: steph-torres-nhs <173282814+steph-torres-nhs@users.noreply.github.com> Date: Tue, 13 Jan 2026 10:38:45 +0000 Subject: [PATCH 08/17] [PRMP-975] update lg dynamo mock response --- .../unit/services/test_get_fhir_document_reference_service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lambdas/tests/unit/services/test_get_fhir_document_reference_service.py b/lambdas/tests/unit/services/test_get_fhir_document_reference_service.py index c0a0c67fe..6ff70c3d8 100644 --- a/lambdas/tests/unit/services/test_get_fhir_document_reference_service.py +++ b/lambdas/tests/unit/services/test_get_fhir_document_reference_service.py @@ -129,7 +129,7 @@ def test_create_document_reference_fhir_response_with_presign_url_document_data( modified_doc.created = "2023-05-15T10:30:00.000Z" modified_doc.document_scan_creation = "2023-05-15" - patched_service.s3_service.get_file_size.return_value = 8000000 + # patched_service.s3_service.get_file_size.return_value = 8000000 patched_service.get_presigned_url = mocker.MagicMock( return_value="https://new-test-url.com" ) From 45ce75d96e5ae022fa21b3e500765ad5cdf304cb Mon Sep 17 00:00:00 2001 From: steph-torres-nhs <173282814+steph-torres-nhs@users.noreply.github.com> Date: Tue, 13 Jan 2026 11:25:17 +0000 Subject: [PATCH 09/17] [PRMP-975] add filter build helper --- lambdas/enums/dynamo_filter.py | 2 ++ .../document_reference_search_service.py | 33 ++++++++++++++++--- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/lambdas/enums/dynamo_filter.py b/lambdas/enums/dynamo_filter.py index 5a3749f0e..aa0f6e77d 100644 --- a/lambdas/enums/dynamo_filter.py +++ b/lambdas/enums/dynamo_filter.py @@ -15,3 +15,5 @@ class AttributeOperator(Enum): class ConditionOperator(Enum): OR = "|" AND = "&" + EQUAL = "=" + NOT_EQUAL = "<>" diff --git a/lambdas/services/document_reference_search_service.py b/lambdas/services/document_reference_search_service.py index 5ee0ab388..dafce21c6 100644 --- a/lambdas/services/document_reference_search_service.py +++ b/lambdas/services/document_reference_search_service.py @@ -1,9 +1,8 @@ -import json import os from json import JSONDecodeError from botocore.exceptions import ClientError -from enums.dynamo_filter import AttributeOperator +from enums.dynamo_filter import AttributeOperator, ConditionOperator from enums.infrastructure import MAP_MTLS_TO_DYNAMO, DynamoTables from enums.lambda_error import LambdaError from enums.metadata_field_names import DocumentReferenceMetadataFields @@ -249,9 +248,8 @@ def get_paginated_references_by_nhs_number( if filter: logger.info(f"Building filter expression for {filter}") - conditions = [{"field": key, "operator": "=", "value": value} for key, value in filter.items()] filter_expression, condition_attribute_names, condition_attribute_values = ( - build_mixed_condition_expression(conditions) + self._build_pagination_filter(filter) ) common_name = validate_common_name_in_mtls( @@ -270,4 +268,29 @@ def get_paginated_references_by_nhs_number( expression_attribute_values=condition_attribute_values, ) - return {"references": [self._build_document_model(ref) for ref in references], "next_page_token": next_page_token} \ No newline at end of file + return {"references": [self._build_document_model(ref) for ref in references], "next_page_token": next_page_token} + + + def _build_pagination_filter(self, filter_values: dict[str, str]): + conditions = [{ + "field": DocumentReferenceMetadataFields.DELETED.value, + "operator": ConditionOperator.NOT_EQUAL.value, + "value": True + }] + for filter_key, filter_value in filter_values.items(): + if filter_key == "custodian": + conditions.append( + {"field": DocumentReferenceMetadataFields.CURRENT_GP_ODS.value, + "operator": ConditionOperator.EQUAL.value, + "value": filter_value,} + ) + elif filter_key == "file_type": + # placeholder for future filtering + pass + elif filter_key == "doc_status": + conditions.append( + {"field": DocumentReferenceMetadataFields.DOC_STATUS.value, + "operator": ConditionOperator.EQUAL.value, + "value": filter_value, } + ) + return build_mixed_condition_expression(conditions) \ No newline at end of file From 21135cda06a0401702447804a229be49dbc2d80d Mon Sep 17 00:00:00 2001 From: steph-torres-nhs <173282814+steph-torres-nhs@users.noreply.github.com> Date: Tue, 13 Jan 2026 11:29:14 +0000 Subject: [PATCH 10/17] [PRMP-975] camelize response --- lambdas/handlers/document_reference_search_handler.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lambdas/handlers/document_reference_search_handler.py b/lambdas/handlers/document_reference_search_handler.py index 89d6974e6..d8961f5b3 100755 --- a/lambdas/handlers/document_reference_search_handler.py +++ b/lambdas/handlers/document_reference_search_handler.py @@ -14,6 +14,7 @@ ) from utils.lambda_response import ApiGatewayResponse from utils.request_context import request_context +from utils.utilities import camelize_dict logger = LoggingService(__name__) @@ -43,9 +44,9 @@ def lambda_handler(event, context): if limit or next_page_token: logger.info("Searching for patient references with pagination.") - response = document_reference_search_service.get_paginated_references_by_nhs_number( + response = camelize_dict(document_reference_search_service.get_paginated_references_by_nhs_number( nhs_number=nhs_number, limit=limit, next_page_token=next_page_token, filter=doc_status_filter - ) + )) else: response = document_reference_search_service.get_document_references( nhs_number, check_upload_completed=True, additional_filters=doc_status_filter From 47520bb469eda6223a9b24993262b7cc73d8c019 Mon Sep 17 00:00:00 2001 From: steph-torres-nhs <173282814+steph-torres-nhs@users.noreply.github.com> Date: Tue, 13 Jan 2026 12:37:13 +0000 Subject: [PATCH 11/17] [PRMP-975] add test --- .../document_reference_search_service.py | 45 ++++++------ .../test_document_reference_search_handler.py | 28 ++++++++ .../test_document_reference_search_service.py | 72 ++++++++++++++++--- 3 files changed, 113 insertions(+), 32 deletions(-) diff --git a/lambdas/services/document_reference_search_service.py b/lambdas/services/document_reference_search_service.py index dafce21c6..a1acd9cd9 100644 --- a/lambdas/services/document_reference_search_service.py +++ b/lambdas/services/document_reference_search_service.py @@ -244,13 +244,9 @@ def get_paginated_references_by_nhs_number( api_request_context: dict = {} ): - filter_expression, condition_attribute_names, condition_attribute_values = None, None, None - - if filter: - logger.info(f"Building filter expression for {filter}") - filter_expression, condition_attribute_names, condition_attribute_values = ( - self._build_pagination_filter(filter) - ) + filter_expression, condition_attribute_names, condition_attribute_values = ( + self._build_pagination_filter(filter) + ) common_name = validate_common_name_in_mtls( api_request_context=api_request_context @@ -271,26 +267,27 @@ def get_paginated_references_by_nhs_number( return {"references": [self._build_document_model(ref) for ref in references], "next_page_token": next_page_token} - def _build_pagination_filter(self, filter_values: dict[str, str]): + def _build_pagination_filter(self, filter_values: dict[str, str] | None) -> tuple[str, dict, dict]: conditions = [{ "field": DocumentReferenceMetadataFields.DELETED.value, "operator": ConditionOperator.NOT_EQUAL.value, "value": True }] - for filter_key, filter_value in filter_values.items(): - if filter_key == "custodian": - conditions.append( - {"field": DocumentReferenceMetadataFields.CURRENT_GP_ODS.value, - "operator": ConditionOperator.EQUAL.value, - "value": filter_value,} - ) - elif filter_key == "file_type": - # placeholder for future filtering - pass - elif filter_key == "doc_status": - conditions.append( - {"field": DocumentReferenceMetadataFields.DOC_STATUS.value, - "operator": ConditionOperator.EQUAL.value, - "value": filter_value, } - ) + if filter_values: + for filter_key, filter_value in filter_values.items(): + if filter_key == "custodian": + conditions.append( + {"field": DocumentReferenceMetadataFields.CURRENT_GP_ODS.value, + "operator": ConditionOperator.EQUAL.value, + "value": filter_value,} + ) + elif filter_key == "file_type": + # placeholder for future filtering + pass + elif filter_key == "doc_status": + conditions.append( + {"field": DocumentReferenceMetadataFields.DOC_STATUS.value, + "operator": ConditionOperator.EQUAL.value, + "value": filter_value, } + ) return build_mixed_condition_expression(conditions) \ No newline at end of file diff --git a/lambdas/tests/unit/handlers/test_document_reference_search_handler.py b/lambdas/tests/unit/handlers/test_document_reference_search_handler.py index 878aa2f26..4c19152f9 100755 --- a/lambdas/tests/unit/handlers/test_document_reference_search_handler.py +++ b/lambdas/tests/unit/handlers/test_document_reference_search_handler.py @@ -211,3 +211,31 @@ def test_extract_querystring_params_no_next_page_token(valid_id_event_without_au expected = ("9000000009", None, None) actual = extract_querystring_params(valid_id_event_without_auth_header) assert expected == actual + + +def test_extract_querystring_params_limit_passed(valid_id_event_without_auth_header): + event = deepcopy(valid_id_event_without_auth_header) + event["queryStringParameters"].update({"limit": "10"}) + + expected = ("9000000009", None, "10") + actual = extract_querystring_params(event) + + assert expected == actual + +def test_handler_uses_pagination_expected_params_passed( + valid_id_event_without_auth_header, + mocked_service, + context, +): + + limit_event = deepcopy(valid_id_event_without_auth_header) + limit_event["queryStringParameters"].update({"limit": "10"}) + + token_event = deepcopy(valid_id_event_without_auth_header) + token_event["queryStringParameters"].update({"nextPageToken": "abc"}) + + events = [limit_event, token_event] + + for event in events: + lambda_handler(event, context) + mocked_service.get_paginated_references_by_nhs_number.assert_called() \ No newline at end of file diff --git a/lambdas/tests/unit/services/test_document_reference_search_service.py b/lambdas/tests/unit/services/test_document_reference_search_service.py index c890249de..9ce6cc6f3 100644 --- a/lambdas/tests/unit/services/test_document_reference_search_service.py +++ b/lambdas/tests/unit/services/test_document_reference_search_service.py @@ -44,6 +44,7 @@ def mock_document_service(mocker, set_env): mocker.patch.object(service, "dynamo_service") mocker.patch.object(service, "fetch_documents_from_table_with_nhs_number") mocker.patch.object(service, "is_upload_in_process", return_value=False) + mocker.patch.object(service, "query_table_with_paginator") return service @@ -491,29 +492,84 @@ def test_build_filter_expression_defaults(mock_document_service): assert actual_filter == expected_filter -def test_get_paginated_reference_by_nhs_number_returns_references_and_token( +def test_get_paginated_references_by_nhs_number_returns_references_and_token( mock_document_service, - mocker ): - mock_paginate = mocker.patch.object(mock_document_service, "query_table_with_paginator") - mock_paginate.return_value = (MOCK_DOCUMENT_REFERENCE, MOCK_NEXT_PAGE_TOKEN) + mock_document_service.query_table_with_paginator.return_value = ( + MOCK_DOCUMENT_REFERENCE, MOCK_NEXT_PAGE_TOKEN + ) expected = {"references": [EXPECTED_RESPONSE], "next_page_token": MOCK_NEXT_PAGE_TOKEN} actual = mock_document_service.get_paginated_references_by_nhs_number( nhs_number=TEST_NHS_NUMBER, ) - mock_paginate.assert_called_with( + mock_document_service.query_table_with_paginator.assert_called_with( table_name="dev_LloydGeorgeReferenceMetadata", index_name="NhsNumberIndex", search_key="NhsNumber", search_condition=TEST_NHS_NUMBER, limit=None, start_key=None, - filter_expression=None, - expression_attribute_names=None, - expression_attribute_values=None + filter_expression="#Deleted_attr <> :Deleted_condition_val", + expression_attribute_names={"#Deleted_attr": "Deleted"}, + expression_attribute_values={":Deleted_condition_val": True} ) assert actual == expected + +def test_get_paginated_references_by_nhs_number_handles_filters( + mock_document_service +): + mock_document_service.query_table_with_paginator.return_value = ( + MOCK_DOCUMENT_REFERENCE, MOCK_NEXT_PAGE_TOKEN + ) + + mock_document_service.get_paginated_references_by_nhs_number( + nhs_number=TEST_NHS_NUMBER, filter={"doc_status": "final"} + ) + + mock_document_service.query_table_with_paginator.assert_called_with( + table_name="dev_LloydGeorgeReferenceMetadata", + index_name="NhsNumberIndex", + search_key="NhsNumber", + search_condition=TEST_NHS_NUMBER, + limit=None, + start_key=None, + filter_expression="#Deleted_attr <> :Deleted_condition_val AND #DocStatus_attr = :DocStatus_condition_val", + expression_attribute_names={"#Deleted_attr": "Deleted", "#DocStatus_attr": "DocStatus"}, + expression_attribute_values={":Deleted_condition_val": True, ":DocStatus_condition_val": "final"}, + ) + + +def test_build_pagination_filter_no_addition_filter_passed_returns_not_deleted_filter( + mock_document_service +): + expected_filter_expression = "#Deleted_attr <> :Deleted_condition_val" + expected_condition_attribute_names = {"#Deleted_attr": "Deleted"} + expected_condition_attribute_values = {":Deleted_condition_val": True} + + actual_filter_expression, actual_condition_attribute_names, actual_condition_attribute_values = ( + mock_document_service._build_pagination_filter(None) + ) + + assert expected_filter_expression == actual_filter_expression + assert expected_condition_attribute_names == actual_condition_attribute_names + assert expected_condition_attribute_values == actual_condition_attribute_values + + +def test_build_pagination_filter_handles_additional_filters( + mock_document_service +): + expected_filter_expression = "#Deleted_attr <> :Deleted_condition_val AND #DocStatus_attr = :DocStatus_condition_val" + expected_condition_attribute_names = {"#Deleted_attr": "Deleted", "#DocStatus_attr": "DocStatus"} + expected_condition_attribute_values = {":Deleted_condition_val": True, ":DocStatus_condition_val": "final"} + + actual_filter_expression, actual_condition_attribute_names, actual_condition_attribute_values = ( + mock_document_service._build_pagination_filter({"doc_status": "final"}) + ) + + assert expected_filter_expression == actual_filter_expression + assert expected_condition_attribute_names == actual_condition_attribute_names + assert expected_condition_attribute_values == actual_condition_attribute_values \ No newline at end of file From 0968a903c96b6ff2cfcd105701ecbd55dad0cfa5 Mon Sep 17 00:00:00 2001 From: steph-torres-nhs <173282814+steph-torres-nhs@users.noreply.github.com> Date: Wed, 14 Jan 2026 11:51:56 +0000 Subject: [PATCH 12/17] [PRMP-975] address PR comments --- lambdas/handlers/document_reference_search_handler.py | 7 ++++--- lambdas/services/document_reference_search_service.py | 6 +++--- .../test_document_reference_search_handler.py | 11 ++++++----- .../test_document_reference_search_service.py | 6 +++--- .../test_get_fhir_document_reference_service.py | 1 - ..._pdm_get_fhir_document_reference_search_service.py | 4 ++-- 6 files changed, 18 insertions(+), 17 deletions(-) diff --git a/lambdas/handlers/document_reference_search_handler.py b/lambdas/handlers/document_reference_search_handler.py index d8961f5b3..c612d1487 100755 --- a/lambdas/handlers/document_reference_search_handler.py +++ b/lambdas/handlers/document_reference_search_handler.py @@ -21,7 +21,7 @@ @set_request_context_for_logging @validate_patient_id -@ensure_environment_variables(names=["DYNAMODB_TABLE_LIST"]) +@ensure_environment_variables(names=["LLOYD_GEORGE_DYNAMODB_NAME"]) @override_error_check @handle_lambda_exceptions def lambda_handler(event, context): @@ -44,9 +44,10 @@ def lambda_handler(event, context): if limit or next_page_token: logger.info("Searching for patient references with pagination.") - response = camelize_dict(document_reference_search_service.get_paginated_references_by_nhs_number( + response_dict =document_reference_search_service.get_paginated_references_by_nhs_number( nhs_number=nhs_number, limit=limit, next_page_token=next_page_token, filter=doc_status_filter - )) + ) + response = camelize_dict(response_dict) else: response = document_reference_search_service.get_document_references( nhs_number, check_upload_completed=True, additional_filters=doc_status_filter diff --git a/lambdas/services/document_reference_search_service.py b/lambdas/services/document_reference_search_service.py index a1acd9cd9..dafcbdcc2 100644 --- a/lambdas/services/document_reference_search_service.py +++ b/lambdas/services/document_reference_search_service.py @@ -3,7 +3,7 @@ from botocore.exceptions import ClientError from enums.dynamo_filter import AttributeOperator, ConditionOperator -from enums.infrastructure import MAP_MTLS_TO_DYNAMO, DynamoTables +from enums.infrastructure import MAP_MTLS_TO_DYNAMO from enums.lambda_error import LambdaError from enums.metadata_field_names import DocumentReferenceMetadataFields from enums.mtls import MtlsCommonNames @@ -69,7 +69,7 @@ def get_document_references( def _get_table_name(self, common_name: MtlsCommonNames | None) -> str: if not common_name or common_name not in MtlsCommonNames: - return str(DynamoTables.LLOYD_GEORGE) + return os.environ["LLOYD_GEORGE_DYNAMODB_NAME"] return str(MAP_MTLS_TO_DYNAMO[common_name]) @@ -277,7 +277,7 @@ def _build_pagination_filter(self, filter_values: dict[str, str] | None) -> tupl for filter_key, filter_value in filter_values.items(): if filter_key == "custodian": conditions.append( - {"field": DocumentReferenceMetadataFields.CURRENT_GP_ODS.value, + {"field": DocumentReferenceMetadataFields.CUSTODIAN.value, "operator": ConditionOperator.EQUAL.value, "value": filter_value,} ) diff --git a/lambdas/tests/unit/handlers/test_document_reference_search_handler.py b/lambdas/tests/unit/handlers/test_document_reference_search_handler.py index 4c19152f9..7adfafcad 100755 --- a/lambdas/tests/unit/handlers/test_document_reference_search_handler.py +++ b/lambdas/tests/unit/handlers/test_document_reference_search_handler.py @@ -5,6 +5,7 @@ import pytest from enums.feature_flags import FeatureFlags from handlers.document_reference_search_handler import lambda_handler, extract_querystring_params +from tests.unit.handlers.test_patch_document_review_handler import TEST_PATIENT_ID from tests.unit.helpers.data.dynamo.dynamo_responses import EXPECTED_RESPONSE from utils.lambda_exceptions import DocumentRefSearchException from utils.lambda_response import ApiGatewayResponse @@ -107,12 +108,12 @@ def test_lambda_handler_when_id_not_supplied_returns_400( assert expected == actual -def test_lambda_handler_when_dynamo_tables_env_variable_not_supplied_then_return_400_response( +def test_lambda_handler_when_dynamo_tables_env_variable_not_supplied_then_return_500_response( valid_id_event_without_auth_header, context ): expected_body = json.dumps( { - "message": "An error occurred due to missing environment variable: 'DYNAMODB_TABLE_LIST'", + "message": "An error occurred due to missing environment variable: 'LLOYD_GEORGE_DYNAMODB_NAME'", "err_code": "ENV_5001", "interaction_id": "88888888-4444-4444-4444-121212121212", } @@ -200,7 +201,7 @@ def test_extract_querystring_params_next_page_token_present(valid_id_event_witho event = deepcopy(valid_id_event_without_auth_header) event["queryStringParameters"].update({"nextPageToken": "abc"}) - expected = ("9000000009", "abc", None) + expected = (TEST_PATIENT_ID, "abc", None) actual = extract_querystring_params(event) @@ -208,7 +209,7 @@ def test_extract_querystring_params_next_page_token_present(valid_id_event_witho def test_extract_querystring_params_no_next_page_token(valid_id_event_without_auth_header): - expected = ("9000000009", None, None) + expected = (TEST_PATIENT_ID, None, None) actual = extract_querystring_params(valid_id_event_without_auth_header) assert expected == actual @@ -217,7 +218,7 @@ def test_extract_querystring_params_limit_passed(valid_id_event_without_auth_hea event = deepcopy(valid_id_event_without_auth_header) event["queryStringParameters"].update({"limit": "10"}) - expected = ("9000000009", None, "10") + expected = (TEST_PATIENT_ID, None, "10") actual = extract_querystring_params(event) assert expected == actual diff --git a/lambdas/tests/unit/services/test_document_reference_search_service.py b/lambdas/tests/unit/services/test_document_reference_search_service.py index 9ce6cc6f3..327f90720 100644 --- a/lambdas/tests/unit/services/test_document_reference_search_service.py +++ b/lambdas/tests/unit/services/test_document_reference_search_service.py @@ -13,7 +13,7 @@ from pydantic import ValidationError from services.document_reference_search_service import DocumentReferenceSearchService from tests.unit.helpers.data.dynamo.dynamo_responses import MOCK_SEARCH_RESPONSE -from tests.unit.conftest import APIM_API_URL, TEST_FILE_SIZE, TEST_NHS_NUMBER +from tests.unit.conftest import APIM_API_URL, TEST_FILE_SIZE, TEST_NHS_NUMBER, MOCK_LG_TABLE_NAME from utils.common_query_filters import NotDeleted, UploadCompleted from utils.exceptions import DynamoServiceException from utils.lambda_exceptions import DocumentRefSearchException @@ -505,7 +505,7 @@ def test_get_paginated_references_by_nhs_number_returns_references_and_token( ) mock_document_service.query_table_with_paginator.assert_called_with( - table_name="dev_LloydGeorgeReferenceMetadata", + table_name=MOCK_LG_TABLE_NAME, index_name="NhsNumberIndex", search_key="NhsNumber", search_condition=TEST_NHS_NUMBER, @@ -531,7 +531,7 @@ def test_get_paginated_references_by_nhs_number_handles_filters( ) mock_document_service.query_table_with_paginator.assert_called_with( - table_name="dev_LloydGeorgeReferenceMetadata", + table_name=MOCK_LG_TABLE_NAME, index_name="NhsNumberIndex", search_key="NhsNumber", search_condition=TEST_NHS_NUMBER, diff --git a/lambdas/tests/unit/services/test_get_fhir_document_reference_service.py b/lambdas/tests/unit/services/test_get_fhir_document_reference_service.py index 6ff70c3d8..0f9670371 100644 --- a/lambdas/tests/unit/services/test_get_fhir_document_reference_service.py +++ b/lambdas/tests/unit/services/test_get_fhir_document_reference_service.py @@ -129,7 +129,6 @@ def test_create_document_reference_fhir_response_with_presign_url_document_data( modified_doc.created = "2023-05-15T10:30:00.000Z" modified_doc.document_scan_creation = "2023-05-15" - # patched_service.s3_service.get_file_size.return_value = 8000000 patched_service.get_presigned_url = mocker.MagicMock( return_value="https://new-test-url.com" ) diff --git a/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_search_service.py b/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_search_service.py index 64d39a19e..95066408b 100644 --- a/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_search_service.py +++ b/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_search_service.py @@ -4,7 +4,7 @@ from freezegun import freeze_time from models.document_reference import DocumentReference from services.document_reference_search_service import DocumentReferenceSearchService -from tests.unit.conftest import APIM_API_URL +from tests.unit.conftest import APIM_API_URL, MOCK_LG_TABLE_NAME from tests.unit.helpers.data.dynamo.dynamo_responses import MOCK_SEARCH_RESPONSE from utils.lambda_header_utils import validate_common_name_in_mtls @@ -79,7 +79,7 @@ def mock_mtls_common_names(monkeypatch): }, "dev_COREDocumentMetadata", ), - ({}, "dev_LloydGeorgeReferenceMetadata"), + ({}, MOCK_LG_TABLE_NAME), ], ) def test_get_pdm_table( From 402ac05cbea1c818e9723d95aab0011be01578f1 Mon Sep 17 00:00:00 2001 From: steph-torres-nhs <173282814+steph-torres-nhs@users.noreply.github.com> Date: Wed, 14 Jan 2026 11:56:47 +0000 Subject: [PATCH 13/17] [PRMP-975] format --- .../document_reference_search_handler.py | 18 +++-- .../document_reference_search_service.py | 45 +++++++----- .../test_document_reference_search_handler.py | 16 +++-- .../tests/unit/helpers/data/test_documents.py | 12 ++-- .../test_document_reference_search_service.py | 72 ++++++++++++------- ...est_get_fhir_document_reference_service.py | 2 +- 6 files changed, 104 insertions(+), 61 deletions(-) diff --git a/lambdas/handlers/document_reference_search_handler.py b/lambdas/handlers/document_reference_search_handler.py index c612d1487..f4b6be07f 100755 --- a/lambdas/handlers/document_reference_search_handler.py +++ b/lambdas/handlers/document_reference_search_handler.py @@ -9,9 +9,7 @@ from utils.decorators.handle_lambda_exceptions import handle_lambda_exceptions 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.decorators.validate_patient_id import validate_patient_id from utils.lambda_response import ApiGatewayResponse from utils.request_context import request_context from utils.utilities import camelize_dict @@ -44,13 +42,20 @@ def lambda_handler(event, context): if limit or next_page_token: logger.info("Searching for patient references with pagination.") - response_dict =document_reference_search_service.get_paginated_references_by_nhs_number( - nhs_number=nhs_number, limit=limit, next_page_token=next_page_token, filter=doc_status_filter + response_dict = ( + document_reference_search_service.get_paginated_references_by_nhs_number( + nhs_number=nhs_number, + limit=limit, + next_page_token=next_page_token, + filter=doc_status_filter, + ) ) response = camelize_dict(response_dict) else: response = document_reference_search_service.get_document_references( - nhs_number, check_upload_completed=True, additional_filters=doc_status_filter + nhs_number, + check_upload_completed=True, + additional_filters=doc_status_filter, ) logger.info("User is able to view docs", {"Result": "Successful viewing docs"}) @@ -64,7 +69,6 @@ def lambda_handler(event, context): ).create_api_gateway_response() - def extract_querystring_params(event): nhs_number = event["queryStringParameters"]["patientId"] next_page_token = event["queryStringParameters"].get("nextPageToken") diff --git a/lambdas/services/document_reference_search_service.py b/lambdas/services/document_reference_search_service.py index dafcbdcc2..eadfe60d0 100644 --- a/lambdas/services/document_reference_search_service.py +++ b/lambdas/services/document_reference_search_service.py @@ -234,14 +234,13 @@ def create_document_reference_fhir_response( ) return fhir_document_reference - def get_paginated_references_by_nhs_number( self, nhs_number: str, limit: int | None = None, next_page_token: str | None = None, filter: dict | None = None, - api_request_context: dict = {} + api_request_context: dict = {}, ): filter_expression, condition_attribute_names, condition_attribute_values = ( @@ -264,30 +263,40 @@ def get_paginated_references_by_nhs_number( expression_attribute_values=condition_attribute_values, ) - return {"references": [self._build_document_model(ref) for ref in references], "next_page_token": next_page_token} - - - def _build_pagination_filter(self, filter_values: dict[str, str] | None) -> tuple[str, dict, dict]: - conditions = [{ - "field": DocumentReferenceMetadataFields.DELETED.value, - "operator": ConditionOperator.NOT_EQUAL.value, - "value": True - }] + return { + "references": [self._build_document_model(ref) for ref in references], + "next_page_token": next_page_token, + } + + def _build_pagination_filter( + self, filter_values: dict[str, str] | None + ) -> tuple[str, dict, dict]: + conditions = [ + { + "field": DocumentReferenceMetadataFields.DELETED.value, + "operator": ConditionOperator.NOT_EQUAL.value, + "value": True, + } + ] if filter_values: for filter_key, filter_value in filter_values.items(): if filter_key == "custodian": conditions.append( - {"field": DocumentReferenceMetadataFields.CUSTODIAN.value, - "operator": ConditionOperator.EQUAL.value, - "value": filter_value,} + { + "field": DocumentReferenceMetadataFields.CUSTODIAN.value, + "operator": ConditionOperator.EQUAL.value, + "value": filter_value, + } ) elif filter_key == "file_type": # placeholder for future filtering pass elif filter_key == "doc_status": conditions.append( - {"field": DocumentReferenceMetadataFields.DOC_STATUS.value, - "operator": ConditionOperator.EQUAL.value, - "value": filter_value, } + { + "field": DocumentReferenceMetadataFields.DOC_STATUS.value, + "operator": ConditionOperator.EQUAL.value, + "value": filter_value, + } ) - return build_mixed_condition_expression(conditions) \ No newline at end of file + return build_mixed_condition_expression(conditions) diff --git a/lambdas/tests/unit/handlers/test_document_reference_search_handler.py b/lambdas/tests/unit/handlers/test_document_reference_search_handler.py index 7adfafcad..1367f1d27 100755 --- a/lambdas/tests/unit/handlers/test_document_reference_search_handler.py +++ b/lambdas/tests/unit/handlers/test_document_reference_search_handler.py @@ -4,7 +4,10 @@ import pytest from enums.feature_flags import FeatureFlags -from handlers.document_reference_search_handler import lambda_handler, extract_querystring_params +from handlers.document_reference_search_handler import ( + extract_querystring_params, + lambda_handler, +) from tests.unit.handlers.test_patch_document_review_handler import TEST_PATIENT_ID from tests.unit.helpers.data.dynamo.dynamo_responses import EXPECTED_RESPONSE from utils.lambda_exceptions import DocumentRefSearchException @@ -197,7 +200,9 @@ def test_lambda_handler_with_feature_flag_disabled_no_doc_status_filter( ) -def test_extract_querystring_params_next_page_token_present(valid_id_event_without_auth_header): +def test_extract_querystring_params_next_page_token_present( + valid_id_event_without_auth_header, +): event = deepcopy(valid_id_event_without_auth_header) event["queryStringParameters"].update({"nextPageToken": "abc"}) @@ -208,7 +213,9 @@ def test_extract_querystring_params_next_page_token_present(valid_id_event_witho assert expected == actual -def test_extract_querystring_params_no_next_page_token(valid_id_event_without_auth_header): +def test_extract_querystring_params_no_next_page_token( + valid_id_event_without_auth_header, +): expected = (TEST_PATIENT_ID, None, None) actual = extract_querystring_params(valid_id_event_without_auth_header) assert expected == actual @@ -223,6 +230,7 @@ def test_extract_querystring_params_limit_passed(valid_id_event_without_auth_hea assert expected == actual + def test_handler_uses_pagination_expected_params_passed( valid_id_event_without_auth_header, mocked_service, @@ -239,4 +247,4 @@ def test_handler_uses_pagination_expected_params_passed( for event in events: lambda_handler(event, context) - mocked_service.get_paginated_references_by_nhs_number.assert_called() \ No newline at end of file + mocked_service.get_paginated_references_by_nhs_number.assert_called() diff --git a/lambdas/tests/unit/helpers/data/test_documents.py b/lambdas/tests/unit/helpers/data/test_documents.py index b3ce442a6..4f54b7b72 100644 --- a/lambdas/tests/unit/helpers/data/test_documents.py +++ b/lambdas/tests/unit/helpers/data/test_documents.py @@ -1,5 +1,7 @@ +import json from typing import Dict, List, Optional +from enums.snomed_codes import SnomedCodes from enums.supported_document_types import SupportedDocumentTypes from freezegun import freeze_time from models.document_reference import DocumentReference @@ -7,12 +9,11 @@ MOCK_ARF_BUCKET, MOCK_LG_BUCKET, MOCK_LG_STAGING_STORE_BUCKET_ENV_NAME, + TEST_FILE_SIZE, TEST_NHS_NUMBER, - TEST_UUID, TEST_FILE_SIZE, + TEST_UUID, ) from tests.unit.helpers.data.dynamo.dynamo_responses import MOCK_SEARCH_RESPONSE -from enums.snomed_codes import SnomedCodes -import json def create_test_doc_store_refs(): @@ -126,6 +127,7 @@ def create_test_doc_refs_as_dict( for doc_ref in test_doc_refs ] + def create_valid_fhir_doc_json(nhs_number: str = "9000000009"): return json.dumps( { @@ -171,8 +173,6 @@ def create_valid_fhir_doc_json(nhs_number: str = "9000000009"): } } ], - "meta": { - "versionId": "1" - } + "meta": {"versionId": "1"}, } ) diff --git a/lambdas/tests/unit/services/test_document_reference_search_service.py b/lambdas/tests/unit/services/test_document_reference_search_service.py index 327f90720..af2ddeeeb 100644 --- a/lambdas/tests/unit/services/test_document_reference_search_service.py +++ b/lambdas/tests/unit/services/test_document_reference_search_service.py @@ -12,8 +12,13 @@ from models.document_reference import DocumentReference from pydantic import ValidationError from services.document_reference_search_service import DocumentReferenceSearchService +from tests.unit.conftest import ( + APIM_API_URL, + MOCK_LG_TABLE_NAME, + TEST_FILE_SIZE, + TEST_NHS_NUMBER, +) from tests.unit.helpers.data.dynamo.dynamo_responses import MOCK_SEARCH_RESPONSE -from tests.unit.conftest import APIM_API_URL, TEST_FILE_SIZE, TEST_NHS_NUMBER, MOCK_LG_TABLE_NAME from utils.common_query_filters import NotDeleted, UploadCompleted from utils.exceptions import DynamoServiceException from utils.lambda_exceptions import DocumentRefSearchException @@ -496,13 +501,17 @@ def test_get_paginated_references_by_nhs_number_returns_references_and_token( mock_document_service, ): mock_document_service.query_table_with_paginator.return_value = ( - MOCK_DOCUMENT_REFERENCE, MOCK_NEXT_PAGE_TOKEN + MOCK_DOCUMENT_REFERENCE, + MOCK_NEXT_PAGE_TOKEN, ) - expected = {"references": [EXPECTED_RESPONSE], "next_page_token": MOCK_NEXT_PAGE_TOKEN} + expected = { + "references": [EXPECTED_RESPONSE], + "next_page_token": MOCK_NEXT_PAGE_TOKEN, + } actual = mock_document_service.get_paginated_references_by_nhs_number( nhs_number=TEST_NHS_NUMBER, - ) + ) mock_document_service.query_table_with_paginator.assert_called_with( table_name=MOCK_LG_TABLE_NAME, @@ -513,22 +522,21 @@ def test_get_paginated_references_by_nhs_number_returns_references_and_token( start_key=None, filter_expression="#Deleted_attr <> :Deleted_condition_val", expression_attribute_names={"#Deleted_attr": "Deleted"}, - expression_attribute_values={":Deleted_condition_val": True} + expression_attribute_values={":Deleted_condition_val": True}, ) assert actual == expected -def test_get_paginated_references_by_nhs_number_handles_filters( - mock_document_service -): +def test_get_paginated_references_by_nhs_number_handles_filters(mock_document_service): mock_document_service.query_table_with_paginator.return_value = ( - MOCK_DOCUMENT_REFERENCE, MOCK_NEXT_PAGE_TOKEN + MOCK_DOCUMENT_REFERENCE, + MOCK_NEXT_PAGE_TOKEN, ) mock_document_service.get_paginated_references_by_nhs_number( nhs_number=TEST_NHS_NUMBER, filter={"doc_status": "final"} - ) + ) mock_document_service.query_table_with_paginator.assert_called_with( table_name=MOCK_LG_TABLE_NAME, @@ -538,38 +546,52 @@ def test_get_paginated_references_by_nhs_number_handles_filters( limit=None, start_key=None, filter_expression="#Deleted_attr <> :Deleted_condition_val AND #DocStatus_attr = :DocStatus_condition_val", - expression_attribute_names={"#Deleted_attr": "Deleted", "#DocStatus_attr": "DocStatus"}, - expression_attribute_values={":Deleted_condition_val": True, ":DocStatus_condition_val": "final"}, + expression_attribute_names={ + "#Deleted_attr": "Deleted", + "#DocStatus_attr": "DocStatus", + }, + expression_attribute_values={ + ":Deleted_condition_val": True, + ":DocStatus_condition_val": "final", + }, ) def test_build_pagination_filter_no_addition_filter_passed_returns_not_deleted_filter( - mock_document_service + mock_document_service, ): expected_filter_expression = "#Deleted_attr <> :Deleted_condition_val" expected_condition_attribute_names = {"#Deleted_attr": "Deleted"} expected_condition_attribute_values = {":Deleted_condition_val": True} - actual_filter_expression, actual_condition_attribute_names, actual_condition_attribute_values = ( - mock_document_service._build_pagination_filter(None) - ) + ( + actual_filter_expression, + actual_condition_attribute_names, + actual_condition_attribute_values, + ) = mock_document_service._build_pagination_filter(None) assert expected_filter_expression == actual_filter_expression assert expected_condition_attribute_names == actual_condition_attribute_names assert expected_condition_attribute_values == actual_condition_attribute_values -def test_build_pagination_filter_handles_additional_filters( - mock_document_service -): +def test_build_pagination_filter_handles_additional_filters(mock_document_service): expected_filter_expression = "#Deleted_attr <> :Deleted_condition_val AND #DocStatus_attr = :DocStatus_condition_val" - expected_condition_attribute_names = {"#Deleted_attr": "Deleted", "#DocStatus_attr": "DocStatus"} - expected_condition_attribute_values = {":Deleted_condition_val": True, ":DocStatus_condition_val": "final"} + expected_condition_attribute_names = { + "#Deleted_attr": "Deleted", + "#DocStatus_attr": "DocStatus", + } + expected_condition_attribute_values = { + ":Deleted_condition_val": True, + ":DocStatus_condition_val": "final", + } - actual_filter_expression, actual_condition_attribute_names, actual_condition_attribute_values = ( - mock_document_service._build_pagination_filter({"doc_status": "final"}) - ) + ( + actual_filter_expression, + actual_condition_attribute_names, + actual_condition_attribute_values, + ) = mock_document_service._build_pagination_filter({"doc_status": "final"}) assert expected_filter_expression == actual_filter_expression assert expected_condition_attribute_names == actual_condition_attribute_names - assert expected_condition_attribute_values == actual_condition_attribute_values \ No newline at end of file + assert expected_condition_attribute_values == actual_condition_attribute_values diff --git a/lambdas/tests/unit/services/test_get_fhir_document_reference_service.py b/lambdas/tests/unit/services/test_get_fhir_document_reference_service.py index 0f9670371..5f7c0c7b6 100644 --- a/lambdas/tests/unit/services/test_get_fhir_document_reference_service.py +++ b/lambdas/tests/unit/services/test_get_fhir_document_reference_service.py @@ -6,7 +6,7 @@ import pytest from enums.infrastructure import DynamoTables from enums.lambda_error import LambdaError -from enums.snomed_codes import SnomedCodes, SnomedCode +from enums.snomed_codes import SnomedCode, SnomedCodes from services.get_fhir_document_reference_service import GetFhirDocumentReferenceService from tests.unit.conftest import MOCK_LG_TABLE_NAME from tests.unit.helpers.data.test_documents import create_test_doc_store_refs From f86d414f490cbd650a44998e0614f3bcc4a280a9 Mon Sep 17 00:00:00 2001 From: steph-torres-nhs <173282814+steph-torres-nhs@users.noreply.github.com> Date: Wed, 14 Jan 2026 14:29:25 +0000 Subject: [PATCH 14/17] [PRMP-975] adjust filter expression --- .../document_reference_search_service.py | 35 ++++++++++++++++--- .../document_upload_review_service.py | 8 ++--- .../test_document_reference_search_service.py | 30 ++++++++++------ 3 files changed, 53 insertions(+), 20 deletions(-) diff --git a/lambdas/services/document_reference_search_service.py b/lambdas/services/document_reference_search_service.py index eadfe60d0..2fd110d62 100644 --- a/lambdas/services/document_reference_search_service.py +++ b/lambdas/services/document_reference_search_service.py @@ -275,13 +275,23 @@ def _build_pagination_filter( { "field": DocumentReferenceMetadataFields.DELETED.value, "operator": ConditionOperator.NOT_EQUAL.value, - "value": True, - } + "value": "", + }, + { + "field": DocumentReferenceMetadataFields.DELETED.value, + "operator": "attribute_not_exists", + }, ] + + filter, condition_attribute_names, condition_attribute_values = ( + build_mixed_condition_expression(conditions=conditions, join_operator="OR") + ) + if filter_values: + additional_conditions = [] for filter_key, filter_value in filter_values.items(): if filter_key == "custodian": - conditions.append( + additional_conditions.append( { "field": DocumentReferenceMetadataFields.CUSTODIAN.value, "operator": ConditionOperator.EQUAL.value, @@ -292,11 +302,26 @@ def _build_pagination_filter( # placeholder for future filtering pass elif filter_key == "doc_status": - conditions.append( + additional_conditions.append( { "field": DocumentReferenceMetadataFields.DOC_STATUS.value, "operator": ConditionOperator.EQUAL.value, "value": filter_value, } ) - return build_mixed_condition_expression(conditions) + + ( + additional_filter, + additional_condition_attribute_names, + additional_condition_attribute_values, + ) = build_mixed_condition_expression(conditions=additional_conditions) + condition_attribute_names.update(additional_condition_attribute_names) + condition_attribute_values.update(additional_condition_attribute_values) + + return ( + f"({filter}) AND " + additional_filter, + condition_attribute_names, + condition_attribute_values, + ) + + return filter, condition_attribute_names, condition_attribute_values diff --git a/lambdas/services/document_upload_review_service.py b/lambdas/services/document_upload_review_service.py index cbbd5478e..54802b120 100644 --- a/lambdas/services/document_upload_review_service.py +++ b/lambdas/services/document_upload_review_service.py @@ -4,7 +4,7 @@ from boto3.dynamodb.conditions import Attr, ConditionBase from botocore.exceptions import ClientError from enums.document_review_status import DocumentReviewStatus -from enums.dynamo_filter import AttributeOperator +from enums.dynamo_filter import AttributeOperator, ConditionOperator from enums.lambda_error import ErrorMessage from enums.metadata_field_names import DocumentReferenceMetadataFields from models.document_review import DocumentUploadReviewReference @@ -94,7 +94,7 @@ def build_paginator_query_filter( conditions = [ { "field": "ReviewStatus", - "operator": "=", + "operator": ConditionOperator.EQUAL.value, "value": DocumentReviewStatus.PENDING_REVIEW.value, } ] @@ -102,7 +102,7 @@ def build_paginator_query_filter( conditions.append( { "field": "NhsNumber", - "operator": "=", + "operator": ConditionOperator.EQUAL.value, "value": nhs_number, } ) @@ -111,7 +111,7 @@ def build_paginator_query_filter( conditions.append( { "field": "Author", - "operator": "=", + "operator": ConditionOperator.EQUAL.value, "value": uploader, } ) diff --git a/lambdas/tests/unit/services/test_document_reference_search_service.py b/lambdas/tests/unit/services/test_document_reference_search_service.py index af2ddeeeb..8ba636293 100644 --- a/lambdas/tests/unit/services/test_document_reference_search_service.py +++ b/lambdas/tests/unit/services/test_document_reference_search_service.py @@ -520,9 +520,9 @@ def test_get_paginated_references_by_nhs_number_returns_references_and_token( search_condition=TEST_NHS_NUMBER, limit=None, start_key=None, - filter_expression="#Deleted_attr <> :Deleted_condition_val", + filter_expression="#Deleted_attr <> :Deleted_condition_val OR attribute_not_exists(#Deleted_attr)", expression_attribute_names={"#Deleted_attr": "Deleted"}, - expression_attribute_values={":Deleted_condition_val": True}, + expression_attribute_values={":Deleted_condition_val": ""}, ) assert actual == expected @@ -545,13 +545,16 @@ def test_get_paginated_references_by_nhs_number_handles_filters(mock_document_se search_condition=TEST_NHS_NUMBER, limit=None, start_key=None, - filter_expression="#Deleted_attr <> :Deleted_condition_val AND #DocStatus_attr = :DocStatus_condition_val", + filter_expression=( + "(#Deleted_attr <> :Deleted_condition_val OR attribute_not_exists(#Deleted_attr)) " + "AND #DocStatus_attr = :DocStatus_condition_val" + ), expression_attribute_names={ "#Deleted_attr": "Deleted", "#DocStatus_attr": "DocStatus", }, expression_attribute_values={ - ":Deleted_condition_val": True, + ":Deleted_condition_val": "", ":DocStatus_condition_val": "final", }, ) @@ -560,9 +563,11 @@ def test_get_paginated_references_by_nhs_number_handles_filters(mock_document_se def test_build_pagination_filter_no_addition_filter_passed_returns_not_deleted_filter( mock_document_service, ): - expected_filter_expression = "#Deleted_attr <> :Deleted_condition_val" + expected_filter_expression = ( + "#Deleted_attr <> :Deleted_condition_val OR attribute_not_exists(#Deleted_attr)" + ) expected_condition_attribute_names = {"#Deleted_attr": "Deleted"} - expected_condition_attribute_values = {":Deleted_condition_val": True} + expected_condition_attribute_values = {":Deleted_condition_val": ""} ( actual_filter_expression, @@ -576,13 +581,16 @@ def test_build_pagination_filter_no_addition_filter_passed_returns_not_deleted_f def test_build_pagination_filter_handles_additional_filters(mock_document_service): - expected_filter_expression = "#Deleted_attr <> :Deleted_condition_val AND #DocStatus_attr = :DocStatus_condition_val" + expected_filter_expression = ( + "(#Deleted_attr <> :Deleted_condition_val OR attribute_not_exists(#Deleted_attr)) " + "AND #DocStatus_attr = :DocStatus_condition_val" + ) expected_condition_attribute_names = { "#Deleted_attr": "Deleted", "#DocStatus_attr": "DocStatus", } expected_condition_attribute_values = { - ":Deleted_condition_val": True, + ":Deleted_condition_val": "", ":DocStatus_condition_val": "final", } @@ -592,6 +600,6 @@ def test_build_pagination_filter_handles_additional_filters(mock_document_servic actual_condition_attribute_values, ) = mock_document_service._build_pagination_filter({"doc_status": "final"}) - assert expected_filter_expression == actual_filter_expression - assert expected_condition_attribute_names == actual_condition_attribute_names - assert expected_condition_attribute_values == actual_condition_attribute_values + assert actual_filter_expression == expected_filter_expression + assert actual_condition_attribute_names == expected_condition_attribute_names + assert actual_condition_attribute_values == expected_condition_attribute_values From 887bebb69d11f342135ebbca1d4e5ff9cab1250a Mon Sep 17 00:00:00 2001 From: steph-torres-nhs <173282814+steph-torres-nhs@users.noreply.github.com> Date: Thu, 15 Jan 2026 16:00:33 +0000 Subject: [PATCH 15/17] [PRMP-975] change getDocSearchResults response object --- .../removeRecordStage/RemoveRecordStage.test.tsx | 14 ++++++++------ .../requests/getDocumentSearchResults.test.ts | 2 +- .../helpers/requests/getDocumentSearchResults.ts | 7 ++++--- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/app/src/components/blocks/_delete/removeRecordStage/RemoveRecordStage.test.tsx b/app/src/components/blocks/_delete/removeRecordStage/RemoveRecordStage.test.tsx index 710b338bb..46756b3c9 100644 --- a/app/src/components/blocks/_delete/removeRecordStage/RemoveRecordStage.test.tsx +++ b/app/src/components/blocks/_delete/removeRecordStage/RemoveRecordStage.test.tsx @@ -38,10 +38,12 @@ const mockUseConfig = useConfig as Mock; const testFileName1 = 'John_1'; const testFileName2 = 'John_2'; -const searchResults = [ - buildSearchResult({ fileName: testFileName1 }), - buildSearchResult({ fileName: testFileName2 }), -]; +const searchResults = { + references: [ + buildSearchResult({ fileName: testFileName1 }), + buildSearchResult({ fileName: testFileName2 }), + ], +}; let history = createMemoryHistory({ initialEntries: ['/'], @@ -124,8 +126,8 @@ describe('RemoveRecordStage', () => { ).toBeInTheDocument(); }); - expect(screen.getByText(searchResults[0].fileName)).toBeInTheDocument(); - expect(screen.getByText(searchResults[1].fileName)).toBeInTheDocument(); + expect(screen.getByText(searchResults.references[0].fileName)).toBeInTheDocument(); + expect(screen.getByText(searchResults.references[1].fileName)).toBeInTheDocument(); }); }); diff --git a/app/src/helpers/requests/getDocumentSearchResults.test.ts b/app/src/helpers/requests/getDocumentSearchResults.test.ts index 16f97eaf4..b6a51f87f 100644 --- a/app/src/helpers/requests/getDocumentSearchResults.test.ts +++ b/app/src/helpers/requests/getDocumentSearchResults.test.ts @@ -21,7 +21,7 @@ describe('[GET] getDocumentSearchResults', () => { test('Document search results handles a 2XX response', async () => { const searchResult = buildSearchResult(); - const mockResults = [searchResult]; + const mockResults = { references: [searchResult] }; mockedAxios.get.mockImplementation(() => Promise.resolve({ status: 200, data: mockResults }), ); diff --git a/app/src/helpers/requests/getDocumentSearchResults.ts b/app/src/helpers/requests/getDocumentSearchResults.ts index 8c2bc587e..b58ed9bdc 100644 --- a/app/src/helpers/requests/getDocumentSearchResults.ts +++ b/app/src/helpers/requests/getDocumentSearchResults.ts @@ -14,7 +14,7 @@ export type DocumentSearchResultsArgs = { }; export type GetDocumentSearchResultsResponse = { - data: Array; + references: Array; }; const getDocumentSearchResults = async ({ @@ -26,16 +26,17 @@ const getDocumentSearchResults = async ({ const gatewayUrl = baseUrl + endpoints.DOCUMENT_SEARCH; try { - const response: GetDocumentSearchResultsResponse = await axios.get(gatewayUrl, { + const { data } = await axios.get(gatewayUrl, { headers: { ...baseHeaders, }, params: { patientId: nhsNumber?.replaceAll(/\s/g, ''), // replace whitespace docType: docType, + limit: 9999, }, }); - return response?.data; + return data.references; } catch (e) { if (isLocal) { return [ From e3f19ed31e4eaa57341e1ccc13f4447d4f5c3ac2 Mon Sep 17 00:00:00 2001 From: steph-torres-nhs <173282814+steph-torres-nhs@users.noreply.github.com> Date: Fri, 16 Jan 2026 10:21:26 +0000 Subject: [PATCH 16/17] [PRMP-975] check 204 response, upload complete filter --- .../document_reference_search_handler.py | 26 ++---- .../test_document_reference_search_handler.py | 92 ++++++++++++------- 2 files changed, 70 insertions(+), 48 deletions(-) diff --git a/lambdas/handlers/document_reference_search_handler.py b/lambdas/handlers/document_reference_search_handler.py index f4b6be07f..a7fb7b00c 100755 --- a/lambdas/handlers/document_reference_search_handler.py +++ b/lambdas/handlers/document_reference_search_handler.py @@ -40,23 +40,17 @@ def lambda_handler(event, context): {"doc_status": "final"} if doc_upload_iteration2_enabled else None ) - if limit or next_page_token: - logger.info("Searching for patient references with pagination.") - response_dict = ( - document_reference_search_service.get_paginated_references_by_nhs_number( - nhs_number=nhs_number, - limit=limit, - next_page_token=next_page_token, - filter=doc_status_filter, - ) - ) - response = camelize_dict(response_dict) - else: - response = document_reference_search_service.get_document_references( - nhs_number, - check_upload_completed=True, - additional_filters=doc_status_filter, + logger.info("Searching for patient references with pagination.") + + response_dict = ( + document_reference_search_service.get_paginated_references_by_nhs_number( + nhs_number=nhs_number, + limit=limit, + next_page_token=next_page_token, + filter=doc_status_filter, ) + ) + response = camelize_dict(response_dict) logger.info("User is able to view docs", {"Result": "Successful viewing docs"}) if response: diff --git a/lambdas/tests/unit/handlers/test_document_reference_search_handler.py b/lambdas/tests/unit/handlers/test_document_reference_search_handler.py index 1367f1d27..98f3ec3de 100755 --- a/lambdas/tests/unit/handlers/test_document_reference_search_handler.py +++ b/lambdas/tests/unit/handlers/test_document_reference_search_handler.py @@ -8,7 +8,7 @@ extract_querystring_params, lambda_handler, ) -from tests.unit.handlers.test_patch_document_review_handler import TEST_PATIENT_ID +from tests.unit.conftest import TEST_NHS_NUMBER from tests.unit.helpers.data.dynamo.dynamo_responses import EXPECTED_RESPONSE from utils.lambda_exceptions import DocumentRefSearchException from utils.lambda_response import ApiGatewayResponse @@ -37,10 +37,16 @@ def mocked_service(set_env, mocker): def test_lambda_handler_returns_200( mocked_service, valid_id_event_without_auth_header, context ): - mocked_service.get_document_references.return_value = EXPECTED_RESPONSE * 2 + mocked_service.get_paginated_references_by_nhs_number.return_value = { + "references": EXPECTED_RESPONSE * 2, + "next_page_token": None + } expected = ApiGatewayResponse( - 200, json.dumps(EXPECTED_RESPONSE * 2), "GET" + 200, json.dumps({ + "references": EXPECTED_RESPONSE * 2, + "nextPageToken": None + }), "GET" ).create_api_gateway_response() actual = lambda_handler(valid_id_event_without_auth_header, context) @@ -48,24 +54,31 @@ def test_lambda_handler_returns_200( assert expected == actual -def test_lambda_handler_returns_204( - mocked_service, valid_id_event_without_auth_header, context -): - mocked_service.get_document_references.return_value = [] - - expected = ApiGatewayResponse( - 204, json.dumps([]), "GET" - ).create_api_gateway_response() - - actual = lambda_handler(valid_id_event_without_auth_header, context) - - assert expected == actual +# TODO do we still need this, are we handling a 204? +# def test_lambda_handler_returns_204( +# mocked_service, valid_id_event_without_auth_header, context +# ): +# mocked_service.get_paginated_references_by_nhs_number.return_value = { +# "references": [], +# "next_page_token": None +# } +# +# expected = ApiGatewayResponse( +# 204, json.dumps({ +# "references": [], +# "nextPageToken": None +# }), "GET" +# ).create_api_gateway_response() +# +# actual = lambda_handler(valid_id_event_without_auth_header, context) +# +# assert expected == actual def test_lambda_handler_raises_exception_returns_500( mocked_service, valid_id_event_without_auth_header, context ): - mocked_service.get_document_references.side_effect = DocumentRefSearchException( + mocked_service.get_paginated_references_by_nhs_number.side_effect = DocumentRefSearchException( 500, MockError.Error ) expected = ApiGatewayResponse( @@ -138,7 +151,10 @@ def test_lambda_handler_with_feature_flag_enabled_applies_doc_status_filter( "handlers.document_reference_search_handler.DocumentReferenceSearchService" ) mocked_service = mocked_service_class.return_value - mocked_service.get_document_references.return_value = EXPECTED_RESPONSE + mocked_service.get_paginated_references_by_nhs_number.return_value = { + "references": EXPECTED_RESPONSE, + "next_page_token": None, + } mocked_feature_flag_service = mocker.patch( "handlers.document_reference_search_handler.FeatureFlagService" @@ -149,7 +165,10 @@ def test_lambda_handler_with_feature_flag_enabled_applies_doc_status_filter( } expected = ApiGatewayResponse( - 200, json.dumps(EXPECTED_RESPONSE), "GET" + 200, json.dumps({ + "references": EXPECTED_RESPONSE, + "nextPageToken": None, + }), "GET" ).create_api_gateway_response() actual = lambda_handler(valid_id_event_without_auth_header, context) @@ -158,12 +177,14 @@ def test_lambda_handler_with_feature_flag_enabled_applies_doc_status_filter( mocked_feature_flag_instance.get_feature_flags_by_flag.assert_called_once_with( FeatureFlags.UPLOAD_DOCUMENT_ITERATION_2_ENABLED ) - mocked_service.get_document_references.assert_called_once_with( - "9000000009", - check_upload_completed=True, - additional_filters={"doc_status": "final"}, + mocked_service.get_paginated_references_by_nhs_number.assert_called_once_with( + nhs_number = TEST_NHS_NUMBER, + limit = None, + next_page_token = None, + # check_upload_completed=True, + filter={"doc_status": "final"}, ) - +# TODO do we still need to check upload complete? def test_lambda_handler_with_feature_flag_disabled_no_doc_status_filter( set_env, mocker, valid_id_event_without_auth_header, context @@ -173,7 +194,10 @@ def test_lambda_handler_with_feature_flag_disabled_no_doc_status_filter( "handlers.document_reference_search_handler.DocumentReferenceSearchService" ) mocked_service = mocked_service_class.return_value - mocked_service.get_document_references.return_value = EXPECTED_RESPONSE + mocked_service.get_paginated_references_by_nhs_number.return_value = { + "references": EXPECTED_RESPONSE, + "next_page_token": None, + } mocked_feature_flag_service = mocker.patch( "handlers.document_reference_search_handler.FeatureFlagService" @@ -184,7 +208,10 @@ def test_lambda_handler_with_feature_flag_disabled_no_doc_status_filter( } expected = ApiGatewayResponse( - 200, json.dumps(EXPECTED_RESPONSE), "GET" + 200, json.dumps({ + "references": EXPECTED_RESPONSE, + "nextPageToken": None, + }), "GET" ).create_api_gateway_response() actual = lambda_handler(valid_id_event_without_auth_header, context) @@ -193,10 +220,11 @@ def test_lambda_handler_with_feature_flag_disabled_no_doc_status_filter( mocked_feature_flag_instance.get_feature_flags_by_flag.assert_called_once_with( FeatureFlags.UPLOAD_DOCUMENT_ITERATION_2_ENABLED ) - mocked_service.get_document_references.assert_called_once_with( - "9000000009", - check_upload_completed=True, - additional_filters=None, + mocked_service.get_paginated_references_by_nhs_number.assert_called_once_with( + nhs_number=TEST_NHS_NUMBER, + limit = None, + next_page_token = None, + filter=None, ) @@ -206,7 +234,7 @@ def test_extract_querystring_params_next_page_token_present( event = deepcopy(valid_id_event_without_auth_header) event["queryStringParameters"].update({"nextPageToken": "abc"}) - expected = (TEST_PATIENT_ID, "abc", None) + expected = (TEST_NHS_NUMBER, "abc", None) actual = extract_querystring_params(event) @@ -216,7 +244,7 @@ def test_extract_querystring_params_next_page_token_present( def test_extract_querystring_params_no_next_page_token( valid_id_event_without_auth_header, ): - expected = (TEST_PATIENT_ID, None, None) + expected = (TEST_NHS_NUMBER, None, None) actual = extract_querystring_params(valid_id_event_without_auth_header) assert expected == actual @@ -225,7 +253,7 @@ def test_extract_querystring_params_limit_passed(valid_id_event_without_auth_hea event = deepcopy(valid_id_event_without_auth_header) event["queryStringParameters"].update({"limit": "10"}) - expected = (TEST_PATIENT_ID, None, "10") + expected = (TEST_NHS_NUMBER, None, "10") actual = extract_querystring_params(event) assert expected == actual From 176fb90e3b6f47a0d7acabfac5a4000104f1f506 Mon Sep 17 00:00:00 2001 From: steph-torres-nhs <173282814+steph-torres-nhs@users.noreply.github.com> Date: Mon, 19 Jan 2026 10:47:07 +0000 Subject: [PATCH 17/17] [PRMP-975] amend stubbed response e2e tests --- .../download_lloyd_george_workflow.cy.js | 15 ++++++++++++--- .../download_patient_files_workflow.cy.js | 5 ++++- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/app/cypress/e2e/0-ndr-core-tests/gp_user_workflows/download_lloyd_george_workflow.cy.js b/app/cypress/e2e/0-ndr-core-tests/gp_user_workflows/download_lloyd_george_workflow.cy.js index 11e441100..f36f81411 100644 --- a/app/cypress/e2e/0-ndr-core-tests/gp_user_workflows/download_lloyd_george_workflow.cy.js +++ b/app/cypress/e2e/0-ndr-core-tests/gp_user_workflows/download_lloyd_george_workflow.cy.js @@ -122,7 +122,10 @@ describe('GP Workflow: View Lloyd George record', () => { cy.intercept('GET', '/SearchDocumentReferences*', { statusCode: 200, - body: testFiles, + body: { + references: testFiles, + nextPageToken: 'abc', + }, }).as('searchDocumentReferences'); cy.get('#verify-submit').click(); @@ -142,7 +145,10 @@ describe('GP Workflow: View Lloyd George record', () => { cy.intercept('GET', '/SearchDocumentReferences*', { statusCode: 200, - body: testFiles, + body: { + references: testFiles, + nextPageToken: 'abc', + }, }).as('searchDocumentReferences'); setUpDownloadManifestIntercepts(); @@ -259,7 +265,10 @@ describe('GP Workflow: View Lloyd George record', () => { cy.intercept('GET', '/SearchDocumentReferences*', { statusCode: 200, - body: singleTestFile, + body: { + references: singleTestFile, + nextPageToken: 'abc' + }, }).as('searchDocumentReferences'); setUpDownloadManifestIntercepts(); diff --git a/app/cypress/e2e/0-ndr-core-tests/pcse_user_workflows/download_patient_files_workflow.cy.js b/app/cypress/e2e/0-ndr-core-tests/pcse_user_workflows/download_patient_files_workflow.cy.js index bc7337aa0..eeca32ea2 100644 --- a/app/cypress/e2e/0-ndr-core-tests/pcse_user_workflows/download_patient_files_workflow.cy.js +++ b/app/cypress/e2e/0-ndr-core-tests/pcse_user_workflows/download_patient_files_workflow.cy.js @@ -40,7 +40,10 @@ describe('PCSE Workflow: Access and download found files', () => { cy.intercept('GET', '/SearchDocumentReferences*', { statusCode: 200, - body: searchDocumentReferencesResponse, + body: { + references: searchDocumentReferencesResponse, + nextPageToken: 'abc', + }, }).as('documentSearch'); cy.get('#verify-submit').click();