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 11e4411005..f36f814113 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 bc7337aa09..eeca32ea22 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(); diff --git a/app/src/components/blocks/_delete/removeRecordStage/RemoveRecordStage.test.tsx b/app/src/components/blocks/_delete/removeRecordStage/RemoveRecordStage.test.tsx index 710b338bb8..46756b3c96 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 16f97eaf4c..b6a51f87f1 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 6afb16317f..a40ea58428 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 == DOCUMENT_TYPE.ALL ? undefined : docType, + limit: 9999, }, }); - return response?.data; + return data.references; } catch (e) { if (isLocal) { return [ diff --git a/lambdas/enums/dynamo_filter.py b/lambdas/enums/dynamo_filter.py index 5a3749f0ee..aa0f6e77dd 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/handlers/document_reference_search_handler.py b/lambdas/handlers/document_reference_search_handler.py index 74c5d37bff..e7020292e2 100755 --- a/lambdas/handlers/document_reference_search_handler.py +++ b/lambdas/handlers/document_reference_search_handler.py @@ -11,31 +11,33 @@ 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.document_type_utils import extract_document_type_to_enum from utils.lambda_exceptions import DocumentRefSearchException from utils.lambda_response import ApiGatewayResponse from utils.request_context import request_context +from utils.utilities import camelize_dict logger = LoggingService(__name__) @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): 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, limit = extract_querystring_params(event) + doc_type = event.get("queryStringParameters", {}).get("docType", None) try: - document_snomed_code = extract_document_type_to_enum(doc_type) if doc_type else None + document_snomed_code = ( + extract_document_type_to_enum(doc_type) if doc_type else None + ) except ValueError: raise DocumentRefSearchException(400, LambdaError.DocTypeInvalid) @@ -48,25 +50,33 @@ def lambda_handler(event, context): doc_upload_iteration2_enabled = upload_lambda_enabled_flag_object[ FeatureFlags.UPLOAD_DOCUMENT_ITERATION_2_ENABLED ] - additional_filters = {} if doc_upload_iteration2_enabled: additional_filters["doc_status"] = "final" if document_snomed_code: additional_filters["document_snomed_code"] = document_snomed_code[0].value - response = document_reference_search_service.get_document_references( - nhs_number, - check_upload_completed=True, - additional_filters=additional_filters + 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=additional_filters, + ) ) + response = camelize_dict(response_dict) logger.info("User is able to view docs", {"Result": "Successful viewing docs"}) - if response: - return ApiGatewayResponse( - 200, json.dumps(response), "GET" - ).create_api_gateway_response() - else: - return ApiGatewayResponse( - 204, json.dumps([]), "GET" - ).create_api_gateway_response() + return ApiGatewayResponse( + 200, json.dumps(response), "GET" + ).create_api_gateway_response() + + +def extract_querystring_params(event): + nhs_number = event["queryStringParameters"]["patientId"] + next_page_token = event["queryStringParameters"].get("nextPageToken") + limit = event["queryStringParameters"].get("limit") + + 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 a4046bb45e..791113427f 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 from enums.lambda_error import LambdaError from enums.metadata_field_names import DocumentReferenceMetadataFields @@ -17,6 +16,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 @@ -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,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_name(self, common_name: MtlsCommonNames | None) -> str: if not common_name or common_name not in MtlsCommonNames: - return table_list + return os.environ["LLOYD_GEORGE_DYNAMODB_NAME"] - 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") @@ -249,3 +241,105 @@ 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, + api_request_context: dict = {}, + return_fhir: bool = False, + ): + + 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 + ) + + 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, + limit=limit, + start_key=next_page_token, + filter_expression=filter_expression, + expression_attribute_names=condition_attribute_names, + expression_attribute_values=condition_attribute_values, + ) + + self._validate_upload_status(references) + + document_references = self._process_documents(references, return_fhir=return_fhir) + + return { + "references": document_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": "", + }, + { + "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": + additional_conditions.append( + { + "field": DocumentReferenceMetadataFields.CUSTODIAN.value, + "operator": ConditionOperator.EQUAL.value, + "value": filter_value, + } + ) + elif filter_key == "document_snomed_code": + additional_conditions.append( + { + "field": DocumentReferenceMetadataFields.DOCUMENT_SNOMED_CODE_TYPE.value, + "operator": ConditionOperator.EQUAL.value, + "value": filter_value, + } + ) + elif filter_key == "doc_status": + additional_conditions.append( + { + "field": DocumentReferenceMetadataFields.DOC_STATUS.value, + "operator": ConditionOperator.EQUAL.value, + "value": filter_value, + } + ) + + ( + 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 cbbd5478ec..54802b1207 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/conftest.py b/lambdas/tests/unit/conftest.py index b3afba0f82..f649fed096 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/handlers/test_document_reference_search_handler.py b/lambdas/tests/unit/handlers/test_document_reference_search_handler.py index 4b6523dad2..609cb05235 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,15 @@ 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 enums.snomed_codes import SnomedCodes +from handlers.document_reference_search_handler import ( + extract_querystring_params, + lambda_handler, +) +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 @@ -22,34 +28,30 @@ def mocked_service(set_env, mocker): mocked_class = mocker.patch( "handlers.document_reference_search_handler.DocumentReferenceSearchService" ) - mocker.patch( - "handlers.document_reference_search_handler.FeatureFlagService.get_feature_flags_by_flag" - ) mocked_service = mocked_class.return_value yield mocked_service -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 - - expected = ApiGatewayResponse( - 200, json.dumps(EXPECTED_RESPONSE * 2), "GET" - ).create_api_gateway_response() - - actual = lambda_handler(valid_id_event_without_auth_header, context) - - assert expected == actual +@pytest.fixture +def mocked_feature_flags(mocker): + feature_flag_service = mocker.patch( + "handlers.document_reference_search_handler.FeatureFlagService" + ) + yield feature_flag_service.return_value -def test_lambda_handler_returns_204( - mocked_service, valid_id_event_without_auth_header, context +def test_lambda_handler_returns_200( + mocked_service, valid_id_event_without_auth_header, context, mocked_feature_flags ): - mocked_service.get_document_references.return_value = [] + mocked_service.get_paginated_references_by_nhs_number.return_value = { + "references": EXPECTED_RESPONSE * 2, + "next_page_token": None, + } expected = ApiGatewayResponse( - 204, json.dumps([]), "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) @@ -57,11 +59,12 @@ def test_lambda_handler_returns_204( assert expected == actual + def test_lambda_handler_raises_exception_returns_500( - mocked_service, valid_id_event_without_auth_header, context + mocked_service, valid_id_event_without_auth_header, context, mocked_feature_flags ): - mocked_service.get_document_references.side_effect = DocumentRefSearchException( - 500, MockError.Error + mocked_service.get_paginated_references_by_nhs_number.side_effect = ( + DocumentRefSearchException(500, MockError.Error) ) expected = ApiGatewayResponse( 500, @@ -73,7 +76,7 @@ def test_lambda_handler_raises_exception_returns_500( def test_lambda_handler_when_id_not_valid_returns_400( - set_env, invalid_id_event, context + set_env, invalid_id_event, context, mocked_feature_flags ): expected_body = json.dumps( { @@ -90,7 +93,7 @@ def test_lambda_handler_when_id_not_valid_returns_400( def test_lambda_handler_when_id_not_supplied_returns_400( - set_env, missing_id_event, context + set_env, missing_id_event, context, mocked_feature_flags ): expected_body = json.dumps( { @@ -106,12 +109,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( - valid_id_event_without_auth_header, context +def test_lambda_handler_when_dynamo_tables_env_variable_not_supplied_then_return_500_response( + valid_id_event_without_auth_header, context, mocked_feature_flags ): 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", } @@ -126,121 +129,188 @@ def test_lambda_handler_when_dynamo_tables_env_variable_not_supplied_then_return def test_lambda_handler_with_feature_flag_enabled_applies_doc_status_filter( - set_env, mocker, valid_id_event_without_auth_header, context + set_env, + valid_id_event_without_auth_header, + context, + mocked_service, + mocked_feature_flags, ): - mocked_service_class = mocker.patch( - "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" - ) - mocked_feature_flag_instance = mocked_feature_flag_service.return_value - mocked_feature_flag_instance.get_feature_flags_by_flag.return_value = { + mocked_feature_flags.get_feature_flags_by_flag.return_value = { FeatureFlags.UPLOAD_DOCUMENT_ITERATION_2_ENABLED: True } 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) assert expected == actual - mocked_feature_flag_instance.get_feature_flags_by_flag.assert_called_once_with( + mocked_feature_flags.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, + filter={"doc_status": "final"}, ) - def test_lambda_handler_with_feature_flag_disabled_no_doc_status_filter( - set_env, mocker, valid_id_event_without_auth_header, context + set_env, + valid_id_event_without_auth_header, + context, + mocked_service, + mocked_feature_flags, ): - mocked_service_class = mocker.patch( - "handlers.document_reference_search_handler.DocumentReferenceSearchService" - ) - mocked_service = mocked_service_class.return_value - mocked_service.get_document_references.return_value = EXPECTED_RESPONSE - mocked_feature_flag_service = mocker.patch( - "handlers.document_reference_search_handler.FeatureFlagService" - ) - mocked_feature_flag_instance = mocked_feature_flag_service.return_value - mocked_feature_flag_instance.get_feature_flags_by_flag.return_value = { + mocked_service.get_paginated_references_by_nhs_number.return_value = { + "references": EXPECTED_RESPONSE, + "next_page_token": None, + } + + mocked_feature_flags.get_feature_flags_by_flag.return_value = { FeatureFlags.UPLOAD_DOCUMENT_ITERATION_2_ENABLED: False } 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) assert expected == actual - mocked_feature_flag_instance.get_feature_flags_by_flag.assert_called_once_with( + mocked_feature_flags.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={}, + mocked_service.get_paginated_references_by_nhs_number.assert_called_once_with( + nhs_number=TEST_NHS_NUMBER, + limit=None, + next_page_token=None, + filter={}, ) + def test_lambda_handler_with_doc_type_applies_doc_type_filter( - set_env, mocker, valid_id_event_without_auth_header, context + set_env, + valid_id_event_without_auth_header, + context, + mocked_service, + mocked_feature_flags, ): - mocked_service_class = mocker.patch( - "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" - ) - mocked_feature_flag_instance = mocked_feature_flag_service.return_value - mocked_feature_flag_instance.get_feature_flags_by_flag.return_value = { + mocked_feature_flags.get_feature_flags_by_flag.return_value = { FeatureFlags.UPLOAD_DOCUMENT_ITERATION_2_ENABLED: False } expected = ApiGatewayResponse( - 200, json.dumps(EXPECTED_RESPONSE), "GET" + 200, json.dumps({"references": EXPECTED_RESPONSE, "nextPageToken": None}), "GET" ).create_api_gateway_response() - doc_type = "16521000000101" + doc_type = SnomedCodes.LLOYD_GEORGE.value.code valid_id_event_without_auth_header["queryStringParameters"]["docType"] = doc_type actual = lambda_handler(valid_id_event_without_auth_header, context) assert expected == actual - mocked_feature_flag_instance.get_feature_flags_by_flag.assert_called_once_with( + mocked_feature_flags.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={"document_snomed_code": doc_type}, + 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={"document_snomed_code": doc_type}, ) +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 = (TEST_NHS_NUMBER, "abc", None) + + actual = extract_querystring_params(event) + + assert expected == actual + + +def test_extract_querystring_params_no_next_page_token( + valid_id_event_without_auth_header, +): + expected = (TEST_NHS_NUMBER, 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 = (TEST_NHS_NUMBER, 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, + mocked_feature_flags, +): + + 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() + + def test_lambda_handler_with_invalid_doc_type_returns_400( - set_env, mocker, valid_id_event_without_auth_header, context + set_env, + valid_id_event_without_auth_header, + context, + mocked_service, + mocked_feature_flags, ): - mocker.patch( - "handlers.document_reference_search_handler.DocumentReferenceSearchService" - ) - mocker.patch( - "handlers.document_reference_search_handler.FeatureFlagService" - ) invalid_doc_type = "invalid_doc_type" - valid_id_event_without_auth_header["queryStringParameters"]["docType"] = invalid_doc_type + valid_id_event_without_auth_header["queryStringParameters"][ + "docType" + ] = invalid_doc_type expected_body = json.dumps( { @@ -255,4 +325,4 @@ def test_lambda_handler_with_invalid_doc_type_returns_400( actual = lambda_handler(valid_id_event_without_auth_header, context) - assert expected == actual \ No newline at end of file + assert expected == actual diff --git a/lambdas/tests/unit/helpers/data/dynamo/dynamo_responses.py b/lambdas/tests/unit/helpers/data/dynamo/dynamo_responses.py index b957c819f4..3c2b503323 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 6035c1e474..f38e4118fc 100644 --- a/lambdas/tests/unit/helpers/data/test_documents.py +++ b/lambdas/tests/unit/helpers/data/test_documents.py @@ -9,6 +9,7 @@ MOCK_ARF_BUCKET, MOCK_LG_BUCKET, MOCK_LG_STAGING_STORE_BUCKET_ENV_NAME, + TEST_FILE_SIZE, TEST_NHS_NUMBER, TEST_UUID, ) @@ -55,16 +56,19 @@ 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[0].document_snomed_code_type = SnomedCodes.LLOYD_GEORGE.value.code 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[1].document_snomed_code_type = SnomedCodes.LLOYD_GEORGE.value.code 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 refs[2].document_snomed_code_type = SnomedCodes.LLOYD_GEORGE.value.code 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 65396a4490..bc17233338 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 @@ -13,7 +12,12 @@ 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 +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 utils.common_query_filters import NotDeleted, UploadCompleted from utils.exceptions import DynamoServiceException @@ -23,28 +27,29 @@ 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) + mocker.patch.object(service, "query_table_with_paginator") return service @@ -58,14 +63,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 +71,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 +89,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 +100,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 +112,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 +123,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 +151,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,8 +181,8 @@ 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_document_service._get_table_names = mock_get_table_names + mock_get_table_names = mocker.MagicMock(return_value="table1") + 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 @@ -196,12 +193,12 @@ 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 ) 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 ) @@ -224,25 +221,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 +254,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), ] ) @@ -504,6 +496,114 @@ def test_build_filter_expression_defaults(mock_document_service): assert actual_filter == expected_filter + +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, + ) + 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, + index_name="NhsNumberIndex", + search_key="NhsNumber", + search_condition=TEST_NHS_NUMBER, + limit=None, + start_key=None, + 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": ""}, + ) + + 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=MOCK_LG_TABLE_NAME, + index_name="NhsNumberIndex", + search_key="NhsNumber", + search_condition=TEST_NHS_NUMBER, + limit=None, + start_key=None, + 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": "", + ":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 OR attribute_not_exists(#Deleted_attr)" + ) + expected_condition_attribute_names = {"#Deleted_attr": "Deleted"} + expected_condition_attribute_values = {":Deleted_condition_val": ""} + + ( + 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 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": "", + ":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 actual_filter_expression == expected_filter_expression + assert actual_condition_attribute_names == expected_condition_attribute_names + assert actual_condition_attribute_values == expected_condition_attribute_values + def test_build_filter_expression_document_snomed_code(mock_document_service): filter_values = {"document_snomed_code": "16521000000101"} expected_filter = Attr("DocumentSnomedCodeType").eq("16521000000101") & ( 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 47ed6dcd96..5f7c0c7b61 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,10 +125,10 @@ 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" - 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_pdf_stitching_service.py b/lambdas/tests/unit/services/test_pdf_stitching_service.py index 4be6959917..f0de141c5b 100644 --- a/lambdas/tests/unit/services/test_pdf_stitching_service.py +++ b/lambdas/tests/unit/services/test_pdf_stitching_service.py @@ -366,6 +366,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}", @@ -387,6 +388,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}", @@ -408,6 +410,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}", @@ -593,8 +596,96 @@ def test_rollback_reference_migration(mock_service): mock_service.rollback_reference_migration() - assert mock_service.dynamo_service.create_item.call_count == 3 - assert mock_service.dynamo_service.delete_item.call_count == 3 + mock_service.dynamo_service.create_item.assert_has_calls( + [ + call( + table_name=MOCK_LG_TABLE_NAME, + item={ + "ContentType": "application/pdf", + "Created": TEST_DOCUMENT_REFERENCES[0].created, + "CurrentGpOds": TEST_DOCUMENT_REFERENCES[0].current_gp_ods, + "DocStatus": "final", + "DocumentScanCreation": "2024-01-01", + "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}", + "S3FileKey": f"{TEST_DOCUMENT_REFERENCES[0].s3_file_key}", + "Status": "current", + "Version": "1", + "Uploaded": True, + "Uploading": False, + "VirusScannerResult": "Clean", + }, + ), + call( + table_name=MOCK_LG_TABLE_NAME, + item={ + "ContentType": "application/pdf", + "Created": TEST_DOCUMENT_REFERENCES[1].created, + "CurrentGpOds": TEST_DOCUMENT_REFERENCES[1].current_gp_ods, + "DocStatus": "final", + "DocumentScanCreation": "2024-01-01", + "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}", + "S3FileKey": f"{TEST_DOCUMENT_REFERENCES[1].s3_file_key}", + "Status": "current", + "Version": "1", + "Uploaded": True, + "Uploading": False, + "VirusScannerResult": "Clean", + }, + ), + call( + table_name=MOCK_LG_TABLE_NAME, + item={ + "ContentType": "application/pdf", + "Created": TEST_DOCUMENT_REFERENCES[2].created, + "CurrentGpOds": TEST_DOCUMENT_REFERENCES[2].current_gp_ods, + "DocStatus": "final", + "DocumentScanCreation": "2024-01-01", + "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}", + "S3FileKey": f"{TEST_DOCUMENT_REFERENCES[2].s3_file_key}", + "Status": "current", + "Version": "1", + "Uploaded": True, + "Uploading": False, + "VirusScannerResult": "Clean", + }, + ), + ] + ) + + mock_service.dynamo_service.delete_item.assert_has_calls( + [ + call( + table_name=MOCK_UNSTITCHED_LG_TABLE_NAME, + key={"ID": TEST_DOCUMENT_REFERENCES[0].id}, + ), + call( + table_name=MOCK_UNSTITCHED_LG_TABLE_NAME, + key={"ID": TEST_DOCUMENT_REFERENCES[1].id}, + ), + call( + table_name=MOCK_UNSTITCHED_LG_TABLE_NAME, + key={"ID": TEST_DOCUMENT_REFERENCES[2].id}, + ), + ] + ) def test_rollback_reference_migration_handles_exception(mock_service): 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 3ec59484fa..95066408b3 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 @@ -77,16 +77,16 @@ def mock_mtls_common_names(monkeypatch): }, }, }, - ["dev_COREDocumentMetadata"], + "dev_COREDocumentMetadata", ), - ({}, ["test_pdm_dynamoDB_table", "test_lg_dynamoDB_table"]), + ({}, MOCK_LG_TABLE_NAME), ], ) 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