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(); 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 [ 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/handlers/document_reference_search_handler.py b/lambdas/handlers/document_reference_search_handler.py index 7e1aafdef..a7fb7b00c 100755 --- a/lambdas/handlers/document_reference_search_handler.py +++ b/lambdas/handlers/document_reference_search_handler.py @@ -9,26 +9,24 @@ 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 ( - extract_nhs_number_from_event, - 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 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) request_context.patient_nhs_no = nhs_number document_reference_search_service = DocumentReferenceSearchService() @@ -41,9 +39,18 @@ 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 + + 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: @@ -54,3 +61,11 @@ def lambda_handler(event, context): return ApiGatewayResponse( 204, json.dumps([]), "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 0784c7dab..2fd110d62 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") @@ -241,3 +233,95 @@ 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 = {}, + ): + + 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, + ) + + 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": "", + }, + { + "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 == "file_type": + # placeholder for future filtering + pass + 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 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/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/handlers/test_document_reference_search_handler.py b/lambdas/tests/unit/handlers/test_document_reference_search_handler.py index c9411818f..98f3ec3de 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,14 @@ 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 ( + 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 @@ -32,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) @@ -43,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( @@ -106,12 +124,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", } @@ -133,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" @@ -144,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) @@ -153,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 @@ -168,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" @@ -179,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) @@ -188,8 +220,59 @@ 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, ) + + +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, +): + + 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() 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..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, ) 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(): @@ -55,14 +56,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: @@ -123,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( { @@ -168,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 096d3f83c..8ba636293 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,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 from utils.common_query_filters import NotDeleted, UploadCompleted from utils.exceptions import DynamoServiceException from utils.lambda_exceptions import DocumentRefSearchException @@ -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), ] ) @@ -503,3 +495,111 @@ 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_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 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..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 @@ -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 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}", 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 3ec59484f..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 @@ -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