diff --git a/app/src/components/blocks/deleteDocumentsStage/DeleteDocumentsStage.test.tsx b/app/src/components/blocks/deleteDocumentsStage/DeleteDocumentsStage.test.tsx index b56dcb1f3..313f3258c 100644 --- a/app/src/components/blocks/deleteDocumentsStage/DeleteDocumentsStage.test.tsx +++ b/app/src/components/blocks/deleteDocumentsStage/DeleteDocumentsStage.test.tsx @@ -13,7 +13,9 @@ import { LG_RECORD_STAGE } from '../../../pages/lloydGeorgeRecordPage/LloydGeorg import { DOCUMENT_TYPE } from '../../../types/pages/UploadDocumentsPage/types'; import axios from 'axios/index'; import { USER_ROLE } from '../../../types/generic/roles'; -import { BrowserRouter } from 'react-router-dom'; +import * as ReactRouter from 'react-router'; +import { createMemoryHistory } from 'history'; +import { routes } from '../../../types/generic/routes'; jest.mock('axios'); @@ -42,7 +44,7 @@ describe('DeleteAllDocumentsStage', () => { await waitFor(async () => { expect( - screen.getByText('Are you sure you want to permanently delete files for:'), + screen.getByText('Are you sure you want to permanently delete files for:') ).toBeInTheDocument(); }); @@ -95,7 +97,7 @@ describe('DeleteAllDocumentsStage', () => { await waitFor(async () => { expect( - screen.getByText('Are you sure you want to permanently delete files for:'), + screen.getByText('Are you sure you want to permanently delete files for:') ).toBeInTheDocument(); }); @@ -137,11 +139,70 @@ describe('DeleteAllDocumentsStage', () => { expect(screen.getByText('Deletion complete')).toBeInTheDocument(); }); }); + + it('renders a service error when service is down', async () => { + const errorResponse = { + response: { + status: 500, + message: 'Client Error.', + }, + }; + mockedAxios.delete.mockImplementation(() => Promise.reject(errorResponse)); + + renderComponent(USER_ROLE.PCSE, DOCUMENT_TYPE.ALL); + + expect(screen.getByRole('radio', { name: 'Yes' })).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Continue' })).toBeInTheDocument(); + + act(() => { + userEvent.click(screen.getByRole('radio', { name: 'Yes' })); + userEvent.click(screen.getByRole('button', { name: 'Continue' })); + }); + + await waitFor(() => { + expect( + screen.getByText('Sorry, the service is currently unavailable.') + ).toBeInTheDocument(); + }); + }); + }); + + describe('Navigation', () => { + it('navigates to home page when API call returns 403', async () => { + const history = createMemoryHistory({ + initialEntries: ['/example'], + initialIndex: 1, + }); + + const errorResponse = { + response: { + status: 403, + message: 'Forbidden', + }, + }; + mockedAxios.delete.mockImplementation(() => Promise.reject(errorResponse)); + + renderComponent(USER_ROLE.PCSE, DOCUMENT_TYPE.ALL, history); + + expect(history.location.pathname).toBe('/example'); + + expect(screen.getByRole('radio', { name: 'Yes' })).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Continue' })).toBeInTheDocument(); + + act(() => { + userEvent.click(screen.getByRole('radio', { name: 'Yes' })); + userEvent.click(screen.getByRole('button', { name: 'Continue' })); + }); + + await waitFor(() => { + expect(history.location.pathname).toBe(routes.HOME); + }); + }); }); }); const TestApp = ( - props: Omit, + props: Omit ) => { return ( { +const homeRoute = '/example'; +const renderComponent = ( + userType: USER_ROLE, + docType: DOCUMENT_TYPE, + history = createMemoryHistory({ + initialEntries: [homeRoute], + initialIndex: 1, + }) +) => { const auth: Session = { auth: buildUserAuth(), isLoggedIn: true, @@ -167,10 +236,10 @@ const renderComponent = (userType: USER_ROLE, docType: DOCUMENT_TYPE) => { }; render( - + - , + ); }; diff --git a/app/src/components/blocks/deleteDocumentsStage/DeleteDocumentsStage.tsx b/app/src/components/blocks/deleteDocumentsStage/DeleteDocumentsStage.tsx index 63fbb19fa..0e74e09f0 100644 --- a/app/src/components/blocks/deleteDocumentsStage/DeleteDocumentsStage.tsx +++ b/app/src/components/blocks/deleteDocumentsStage/DeleteDocumentsStage.tsx @@ -15,6 +15,9 @@ import ServiceError from '../../layout/serviceErrorBox/ServiceErrorBox'; import { SUBMISSION_STATE } from '../../../types/pages/documentSearchResultsPage/types'; import { USER_ROLE } from '../../../types/generic/roles'; import { formatNhsNumber } from '../../../helpers/utils/formatNhsNumber'; +import { AxiosError } from 'axios'; +import { routes } from '../../../types/generic/routes'; +import { useNavigate } from 'react-router-dom'; export type Props = { docType: DOCUMENT_TYPE; @@ -45,6 +48,7 @@ function DeleteDocumentsStage({ const [deletionStage, setDeletionStage] = useState(SUBMISSION_STATE.INITIAL); const baseUrl = useBaseAPIUrl(); const baseHeaders = useBaseAPIHeaders(); + const navigate = useNavigate(); const nhsNumber: string = patientDetails?.nhsNumber || ''; const formattedNhsNumber = formatNhsNumber(nhsNumber); @@ -64,39 +68,29 @@ function DeleteDocumentsStage({ ); - const deleteDocumentsFor = (type: DOCUMENT_TYPE) => - deleteAllDocuments({ - docType: type, - nhsNumber: nhsNumber, - baseUrl, - baseHeaders, - }); - const handleYesOption = async () => { setDeletionStage(SUBMISSION_STATE.PENDING); - let documentPromises: Array> = []; - if (docType === DOCUMENT_TYPE.LLOYD_GEORGE) { - documentPromises = [deleteDocumentsFor(DOCUMENT_TYPE.LLOYD_GEORGE)]; - } else if (docType === DOCUMENT_TYPE.ALL) { - documentPromises = [ - deleteDocumentsFor(DOCUMENT_TYPE.LLOYD_GEORGE), - deleteDocumentsFor(DOCUMENT_TYPE.ARF), - ]; - } try { - Promise.all(documentPromises).then((responses) => { - const hasSucceeded = !responses.some((res) => res.status === 403); + const response: DeleteResponse = await deleteAllDocuments({ + docType: docType, + nhsNumber: nhsNumber, + baseUrl, + baseHeaders, + }); - if (hasSucceeded) { - setDeletionStage(SUBMISSION_STATE.SUCCEEDED); + if (response.status === 200) { + setDeletionStage(SUBMISSION_STATE.SUCCEEDED); - if (setDownloadStage) { - setDownloadStage(DOWNLOAD_STAGE.FAILED); - } + if (setDownloadStage) { + setDownloadStage(DOWNLOAD_STAGE.FAILED); } - }); + } } catch (e) { + const error = e as AxiosError; + if (error.response?.status === 403) { + navigate(routes.HOME); + } setDeletionStage(SUBMISSION_STATE.FAILED); } }; diff --git a/app/src/components/blocks/lloydGeorgeRecordDetails/LloydGeorgeRecordDetails.test.tsx b/app/src/components/blocks/lloydGeorgeRecordDetails/LloydGeorgeRecordDetails.test.tsx index 402dadc24..5fd29f1ff 100644 --- a/app/src/components/blocks/lloydGeorgeRecordDetails/LloydGeorgeRecordDetails.test.tsx +++ b/app/src/components/blocks/lloydGeorgeRecordDetails/LloydGeorgeRecordDetails.test.tsx @@ -15,7 +15,7 @@ describe('LloydGeorgeRecordDetails', () => { const actionLinkStrings = [ { label: 'See all files', expectedStage: LG_RECORD_STAGE.SEE_ALL }, { label: 'Download all files', expectedStage: LG_RECORD_STAGE.DOWNLOAD_ALL }, - { label: 'Delete a selection of files', expectedStage: LG_RECORD_STAGE.DELETE_ANY }, + { label: 'Delete all files', expectedStage: LG_RECORD_STAGE.DELETE_ALL }, { label: 'Delete file', expectedStage: LG_RECORD_STAGE.DELETE_ONE }, ]; diff --git a/app/src/components/blocks/lloydGeorgeRecordDetails/LloydGeorgeRecordDetails.tsx b/app/src/components/blocks/lloydGeorgeRecordDetails/LloydGeorgeRecordDetails.tsx index a0ab470a5..fdd13dbaa 100644 --- a/app/src/components/blocks/lloydGeorgeRecordDetails/LloydGeorgeRecordDetails.tsx +++ b/app/src/components/blocks/lloydGeorgeRecordDetails/LloydGeorgeRecordDetails.tsx @@ -46,9 +46,9 @@ function LloydGeorgeRecordDetails({ handler: () => setStage(LG_RECORD_STAGE.DOWNLOAD_ALL), }, { - label: 'Delete a selection of files', - key: 'delete-any-files-link', - handler: () => setStage(LG_RECORD_STAGE.DELETE_ANY), + label: 'Delete all files', + key: 'delete-all-files-link', + handler: () => setStage(LG_RECORD_STAGE.DELETE_ALL), }, { label: 'Delete file', diff --git a/app/src/pages/lloydGeorgeRecordPage/LloydGeorgeRecordPage.tsx b/app/src/pages/lloydGeorgeRecordPage/LloydGeorgeRecordPage.tsx index 94f5b5d54..dfff02364 100644 --- a/app/src/pages/lloydGeorgeRecordPage/LloydGeorgeRecordPage.tsx +++ b/app/src/pages/lloydGeorgeRecordPage/LloydGeorgeRecordPage.tsx @@ -16,7 +16,7 @@ export enum LG_RECORD_STAGE { RECORD = 0, DOWNLOAD_ALL = 1, SEE_ALL = 2, - DELETE_ANY = 3, + DELETE_ALL = 3, DELETE_ONE = 4, } function LloydGeorgeRecordPage() { @@ -97,7 +97,7 @@ function LloydGeorgeRecordPage() { /> ) ); - case LG_RECORD_STAGE.DELETE_ANY: + case LG_RECORD_STAGE.DELETE_ALL: return ( patientDetails && ( List[str]: + return [str(doc_type.value) for doc_type in SupportedDocumentTypes.list()] @staticmethod - def get_from_field_name(enum_value): - for supported_document_type in SupportedDocumentTypes.list(): - if supported_document_type.name == enum_value: - return supported_document_type + def get_from_field_name(value: str): + if value in SupportedDocumentTypes.list_names(): + return value return None diff --git a/lambdas/handlers/bulk_upload_report_handler.py b/lambdas/handlers/bulk_upload_report_handler.py index f9a5a7f1e..22a233560 100644 --- a/lambdas/handlers/bulk_upload_report_handler.py +++ b/lambdas/handlers/bulk_upload_report_handler.py @@ -5,7 +5,6 @@ from boto3.dynamodb.conditions import Attr from botocore.exceptions import ClientError - from services.dynamo_service import DynamoDBService from services.s3_service import S3Service from utils.decorators.ensure_env_var import ensure_environment_variables @@ -62,7 +61,7 @@ def get_dynamodb_report_items( bulk_upload_table_name, filter_expression=filter_time ) - if not "Items" in db_response: + if "Items" not in db_response: return None items = db_response["Items"] while "LastEvaluatedKey" in db_response: diff --git a/lambdas/handlers/create_document_reference_handler.py b/lambdas/handlers/create_document_reference_handler.py index d895e3f82..62c123378 100644 --- a/lambdas/handlers/create_document_reference_handler.py +++ b/lambdas/handlers/create_document_reference_handler.py @@ -97,7 +97,7 @@ def lambda_handler(event, context): document_reference: NHSDocumentReference - if document_type == SupportedDocumentTypes.LG: + if document_type == SupportedDocumentTypes.LG.value: document_reference = NHSDocumentReference( nhs_number=nhs_number, s3_bucket_name=lg_s3_bucket_name, @@ -106,7 +106,7 @@ def lambda_handler(event, context): file_name=document.fileName, ) lg_documents.append(document_reference) - elif document_type == SupportedDocumentTypes.ARF: + elif document_type == SupportedDocumentTypes.ARF.value: document_reference = NHSDocumentReference( nhs_number=nhs_number, s3_bucket_name=arf_s3_bucket_name, diff --git a/lambdas/handlers/delete_document_reference_handler.py b/lambdas/handlers/delete_document_reference_handler.py index 1e7d70f55..520ab0a1d 100644 --- a/lambdas/handlers/delete_document_reference_handler.py +++ b/lambdas/handlers/delete_document_reference_handler.py @@ -2,12 +2,12 @@ import os from botocore.exceptions import ClientError -from enums.metadata_field_names import DocumentReferenceMetadataFields from enums.s3_lifecycle_tags import S3LifecycleTags from enums.supported_document_types import SupportedDocumentTypes from services.document_service import DocumentService from utils.decorators.ensure_env_var import ensure_environment_variables -from utils.decorators.validate_document_type import validate_document_type +from utils.decorators.validate_document_type import (extract_document_type, + validate_document_type) from utils.decorators.validate_patient_id import validate_patient_id from utils.lambda_response import ApiGatewayResponse @@ -25,38 +25,70 @@ ) def lambda_handler(event, context): nhs_number = event["queryStringParameters"]["patientId"] - doc_type = event["queryStringParameters"]["docType"] + doc_type = extract_document_type(event["queryStringParameters"]["docType"]) document_service = DocumentService() try: - table = "" - if SupportedDocumentTypes.ARF.name == doc_type: - logger.info("Retrieving ARF documents for deletion") - table = os.environ["DOCUMENT_STORE_DYNAMODB_NAME"] + if doc_type == SupportedDocumentTypes.ALL.value: + arf_delete_response = handle_delete( + document_service, nhs_number, str(SupportedDocumentTypes.ARF.value) + ) + lg_delete_response = handle_delete( + document_service, nhs_number, str(SupportedDocumentTypes.LG.value) + ) - if SupportedDocumentTypes.LG.name == doc_type: - logger.info("Retrieving Lloyd George documents for deletion") - table = os.environ["LLOYD_GEORGE_DYNAMODB_NAME"] + if ( + arf_delete_response["statusCode"] == 404 + and lg_delete_response["statusCode"] == 404 + ): + return ApiGatewayResponse( + 404, "No documents available", "DELETE" + ).create_api_gateway_response() - results = document_service.fetch_documents_from_table_with_filter( - nhs_number, table, {DocumentReferenceMetadataFields.DELETED.value: ""} - ) - - if not results: return ApiGatewayResponse( - 404, "No documents available", "DELETE" + 200, "Success", "DELETE" ).create_api_gateway_response() - document_service.delete_documents( - table_name=table, - document_references=results, - type_of_delete=str(S3LifecycleTags.SOFT_DELETE.value), - ) + if doc_type == SupportedDocumentTypes.ARF.value: + return handle_delete( + document_service, nhs_number, str(SupportedDocumentTypes.ARF.value) + ) + + if doc_type == SupportedDocumentTypes.LG.value: + return handle_delete( + document_service, nhs_number, str(SupportedDocumentTypes.LG.value) + ) + except ClientError as e: logger.info(str(e)) return ApiGatewayResponse( 500, "Failed to delete documents", "DELETE" ).create_api_gateway_response() + +def handle_delete( + document_service: DocumentService, nhs_number: str, doc_type: str +) -> dict: + table_name = "" + if doc_type == SupportedDocumentTypes.ARF.value: + table_name = os.environ["DOCUMENT_STORE_DYNAMODB_NAME"] + if doc_type == SupportedDocumentTypes.LG.value: + table_name = os.environ["LLOYD_GEORGE_DYNAMODB_NAME"] + + results = document_service.fetch_available_document_references_by_type( + nhs_number, doc_type + ) + + if not results: + return ApiGatewayResponse( + 404, "No documents available", "DELETE" + ).create_api_gateway_response() + + document_service.delete_documents( + table_name=table_name, + document_references=results, + type_of_delete=str(S3LifecycleTags.SOFT_DELETE.value), + ) + return ApiGatewayResponse(200, "Success", "DELETE").create_api_gateway_response() diff --git a/lambdas/handlers/document_manifest_by_nhs_number_handler.py b/lambdas/handlers/document_manifest_by_nhs_number_handler.py index d2bcb93e1..694750f5e 100644 --- a/lambdas/handlers/document_manifest_by_nhs_number_handler.py +++ b/lambdas/handlers/document_manifest_by_nhs_number_handler.py @@ -6,7 +6,8 @@ from services.document_manifest_service import DocumentManifestService from services.document_service import DocumentService from utils.decorators.ensure_env_var import ensure_environment_variables -from utils.decorators.validate_document_type import validate_document_type +from utils.decorators.validate_document_type import (extract_document_type, + validate_document_type) from utils.decorators.validate_patient_id import validate_patient_id from utils.exceptions import DynamoDbException, ManifestDownloadException from utils.lambda_response import ApiGatewayResponse @@ -30,7 +31,7 @@ def lambda_handler(event, context): try: nhs_number = event["queryStringParameters"]["patientId"] - doc_type = event["queryStringParameters"]["docType"] + doc_type = extract_document_type(event["queryStringParameters"]["docType"]) zip_output_bucket = os.environ["ZIPPED_STORE_BUCKET_NAME"] zip_trace_table_name = os.environ["ZIPPED_STORE_DYNAMODB_NAME"] diff --git a/lambdas/services/document_service.py b/lambdas/services/document_service.py index 4cd480473..391cbfc30 100644 --- a/lambdas/services/document_service.py +++ b/lambdas/services/document_service.py @@ -19,29 +19,36 @@ def __init__(self): self.s3_service = S3Service() def fetch_available_document_references_by_type( - self, nhs_number: str, doc_types: str + self, nhs_number: str, doc_type: str ) -> list[DocumentReference]: - arf_documents = [] - lg_documents = [] - + results: list[DocumentReference] = [] delete_filter = {DocumentReferenceMetadataFields.DELETED.value: ""} - if SupportedDocumentTypes.ARF.name in doc_types: - logger.info("Retrieving ARF documents") - arf_documents = self.fetch_documents_from_table_with_filter( + if doc_type == SupportedDocumentTypes.ALL.value: + results = self.fetch_documents_from_table_with_filter( nhs_number, os.environ["DOCUMENT_STORE_DYNAMODB_NAME"], attr_filter=delete_filter, - ) - if SupportedDocumentTypes.LG.name in doc_types: - logger.info("Retrieving Lloyd George documents") - lg_documents = self.fetch_documents_from_table_with_filter( + ) + self.fetch_documents_from_table_with_filter( nhs_number, os.environ["LLOYD_GEORGE_DYNAMODB_NAME"], attr_filter=delete_filter, ) - return arf_documents + lg_documents + if doc_type == SupportedDocumentTypes.ARF.value: + results = self.fetch_documents_from_table_with_filter( + nhs_number, + os.environ["DOCUMENT_STORE_DYNAMODB_NAME"], + attr_filter=delete_filter, + ) + + if doc_type == SupportedDocumentTypes.LG.value: + results = self.fetch_documents_from_table_with_filter( + nhs_number, + os.environ["LLOYD_GEORGE_DYNAMODB_NAME"], + attr_filter=delete_filter, + ) + return results def fetch_documents_from_table( self, nhs_number: str, table: str diff --git a/lambdas/tests/unit/conftest.py b/lambdas/tests/unit/conftest.py index 008c06e6e..e8b446325 100644 --- a/lambdas/tests/unit/conftest.py +++ b/lambdas/tests/unit/conftest.py @@ -33,7 +33,7 @@ MOCK_LG_METADATA_SQS_QUEUE = "test_bulk_upload_metadata_queue" MOCK_LG_INVALID_SQS_QUEUE = "INVALID_SQS_QUEUE_URL" -TEST_NHS_NUMBER = "1111111111" +TEST_NHS_NUMBER = "9000000009" TEST_OBJECT_KEY = "1234-4567-8912-HSDF-TEST" TEST_FILE_KEY = "test_file_key" TEST_FILE_NAME = "test.pdf" diff --git a/lambdas/tests/unit/handlers/test_bulk_upload_report_handler.py b/lambdas/tests/unit/handlers/test_bulk_upload_report_handler.py index 76120b0dc..858bdff53 100644 --- a/lambdas/tests/unit/handlers/test_bulk_upload_report_handler.py +++ b/lambdas/tests/unit/handlers/test_bulk_upload_report_handler.py @@ -5,25 +5,17 @@ from boto3.dynamodb.conditions import Attr from freezegun import freeze_time - -from handlers.bulk_upload_report_handler import ( - get_times_for_scan, - write_items_to_csv, - get_dynamodb_report_items, - report_handler, - write_empty_report -) +from handlers.bulk_upload_report_handler import (get_dynamodb_report_items, + get_times_for_scan, + report_handler, + write_empty_report, + write_items_to_csv) +from tests.unit.conftest import (MOCK_BULK_REPORT_TABLE_NAME, + MOCK_LG_STAGING_STORE_BUCKET) from tests.unit.helpers.data.dynamo_scan_response import ( - MOCK_RESPONSE, - EXPECTED_RESPONSE, - MOCK_RESPONSE_WITH_LAST_KEY, - MOCK_EMPTY_RESPONSE, - UNEXPECTED_RESPONSE, -) -from tests.unit.conftest import ( - MOCK_BULK_REPORT_TABLE_NAME, - MOCK_LG_STAGING_STORE_BUCKET, -) + EXPECTED_RESPONSE, MOCK_EMPTY_RESPONSE, MOCK_RESPONSE, + MOCK_RESPONSE_WITH_LAST_KEY, UNEXPECTED_RESPONSE) + @freeze_time("2012-01-14 7:20:01") def test_get_time_for_scan_after_7am(): @@ -70,6 +62,7 @@ def test_write_items_to_csv(): assert row == item os.remove("test_file") + def test_write_empty_file_to_txt(): write_empty_report("test_file") diff --git a/lambdas/tests/unit/handlers/test_create_document_manifest_by_nhs_number_handler.py b/lambdas/tests/unit/handlers/test_create_document_manifest_by_nhs_number_handler.py index 828bd78d6..e81bfcc2d 100644 --- a/lambdas/tests/unit/handlers/test_create_document_manifest_by_nhs_number_handler.py +++ b/lambdas/tests/unit/handlers/test_create_document_manifest_by_nhs_number_handler.py @@ -48,11 +48,13 @@ def test_lambda_handler_returns_400_when_doc_type_invalid_response( assert expected == actual -def manifest_service_side_effect(nhs_number, doc_types): - if SupportedDocumentTypes.ARF.name in doc_types: +def manifest_service_side_effect(nhs_number, doc_type): + if doc_type == SupportedDocumentTypes.ARF.value: return create_test_doc_store_refs() - if SupportedDocumentTypes.LG.name in doc_types: + if doc_type == SupportedDocumentTypes.LG.value: return create_test_lloyd_george_doc_store_refs() + if doc_type == SupportedDocumentTypes.ALL.value: + return create_test_doc_store_refs() + create_test_lloyd_george_doc_store_refs() return [] @@ -61,10 +63,10 @@ def test_lambda_handler_valid_parameters_arf_doc_type_request_returns_200( ): expected_url = "test-url" - mock_dynamo = mocker.patch( + mock_document_query = mocker.patch( "services.document_service.DocumentService.fetch_available_document_references_by_type" ) - mock_dynamo.side_effect = manifest_service_side_effect + mock_document_query.side_effect = manifest_service_side_effect mock_doc_manifest_url = mocker.patch( "services.document_manifest_service.DocumentManifestService.create_document_manifest_presigned_url" @@ -76,7 +78,7 @@ def test_lambda_handler_valid_parameters_arf_doc_type_request_returns_200( ).create_api_gateway_response() actual = lambda_handler(valid_id_and_arf_doctype_event, context) - mock_dynamo.assert_called_once_with("9000000009", "ARF") + mock_document_query.assert_called_once_with("9000000009", "ARF") assert expected == actual @@ -85,10 +87,10 @@ def test_lambda_handler_valid_parameters_lg_doc_type_request_returns_200( ): expected_url = "test-url" - mock_dynamo = mocker.patch( + mock_document_query = mocker.patch( "services.document_service.DocumentService.fetch_available_document_references_by_type" ) - mock_dynamo.side_effect = manifest_service_side_effect + mock_document_query.side_effect = manifest_service_side_effect mock_doc_manifest_url = mocker.patch( "services.document_manifest_service.DocumentManifestService.create_document_manifest_presigned_url" @@ -100,7 +102,7 @@ def test_lambda_handler_valid_parameters_lg_doc_type_request_returns_200( ).create_api_gateway_response() actual = lambda_handler(valid_id_and_lg_doctype_event, context) - mock_dynamo.assert_called_once_with("9000000009", "LG") + mock_document_query.assert_called_once_with("9000000009", "LG") assert expected == actual @@ -109,10 +111,10 @@ def test_lambda_handler_valid_parameters_both_doc_type_request_returns_200( ): expected_url = "test-url" - mock_dynamo = mocker.patch( + mock_document_query = mocker.patch( "services.document_service.DocumentService.fetch_available_document_references_by_type" ) - mock_dynamo.side_effect = manifest_service_side_effect + mock_document_query.side_effect = manifest_service_side_effect mock_doc_manifest_url = mocker.patch( "services.document_manifest_service.DocumentManifestService.create_document_manifest_presigned_url" @@ -124,9 +126,9 @@ def test_lambda_handler_valid_parameters_both_doc_type_request_returns_200( ).create_api_gateway_response() actual = lambda_handler(valid_id_and_both_doctype_event, context) - mock_dynamo.assert_has_calls( + mock_document_query.assert_has_calls( [ - call("9000000009", "LG,ARF"), + call("9000000009", "ALL"), ] ) assert expected == actual diff --git a/lambdas/tests/unit/handlers/test_create_document_reference_handler.py b/lambdas/tests/unit/handlers/test_create_document_reference_handler.py index d08e7d180..c203ae935 100644 --- a/lambdas/tests/unit/handlers/test_create_document_reference_handler.py +++ b/lambdas/tests/unit/handlers/test_create_document_reference_handler.py @@ -42,7 +42,7 @@ def test_create_document_reference_valid_both_lg_and_arf_type_returns_200( "enums.supported_document_types.SupportedDocumentTypes.get_from_field_name" ) mock_supported_document_get_from_field_name.return_value = ( - SupportedDocumentTypes.ARF + SupportedDocumentTypes.ARF.value ) mocker.patch("services.dynamo_service.DynamoDBService.create_item") @@ -67,7 +67,7 @@ def test_create_document_reference_valid_arf_type_returns_200( "enums.supported_document_types.SupportedDocumentTypes.get_from_field_name" ) mock_supported_document_get_from_field_name.return_value = ( - SupportedDocumentTypes.ARF + SupportedDocumentTypes.ARF.value ) mocker.patch("services.dynamo_service.DynamoDBService.create_item") @@ -91,7 +91,9 @@ def test_create_document_reference_valid_lg_type_returns_200( mock_supported_document_get_from_field_name = mocker.patch( "enums.supported_document_types.SupportedDocumentTypes.get_from_field_name" ) - mock_supported_document_get_from_field_name.return_value = SupportedDocumentTypes.LG + mock_supported_document_get_from_field_name.return_value = ( + SupportedDocumentTypes.LG.value + ) mocker.patch("services.dynamo_service.DynamoDBService.create_item") mock_presigned = mocker.patch( "services.s3_service.S3Service.create_document_presigned_url_handler" @@ -115,7 +117,7 @@ def test_create_document_reference_valid_arf_type_uses_arf_s3_bucket( "enums.supported_document_types.SupportedDocumentTypes.get_from_field_name" ) mock_supported_document_get_from_field_name.return_value = ( - SupportedDocumentTypes.ARF + SupportedDocumentTypes.ARF.value ) mock_presigned = mocker.patch( "services.s3_service.S3Service.create_document_presigned_url_handler" @@ -136,7 +138,7 @@ def test_create_document_reference_valid_arf_type_adds_nhs_number_as_s3_folder( "enums.supported_document_types.SupportedDocumentTypes.get_from_field_name" ) mock_supported_document_get_from_field_name.return_value = ( - SupportedDocumentTypes.ARF + SupportedDocumentTypes.ARF.value ) mock_presigned = mocker.patch( "services.s3_service.S3Service.create_document_presigned_url_handler" @@ -163,7 +165,7 @@ def test_create_document_reference_valid_arf_type_uses_arf_dynamo_table( "enums.supported_document_types.SupportedDocumentTypes.get_from_field_name" ) mock_supported_document_get_from_field_name.return_value = ( - SupportedDocumentTypes.ARF + SupportedDocumentTypes.ARF.value ) mock_presigned = mocker.patch( "services.s3_service.S3Service.create_document_presigned_url_handler" @@ -182,7 +184,9 @@ def test_create_document_reference_valid_lg_type_uses_lg_s3_bucket( mock_supported_document_get_from_field_name = mocker.patch( "enums.supported_document_types.SupportedDocumentTypes.get_from_field_name" ) - mock_supported_document_get_from_field_name.return_value = SupportedDocumentTypes.LG + mock_supported_document_get_from_field_name.return_value = ( + SupportedDocumentTypes.LG.value + ) mock_presigned = mocker.patch( "services.s3_service.S3Service.create_document_presigned_url_handler" ) @@ -203,7 +207,9 @@ def test_create_document_reference_valid_lg_type_uses_lg_dynamo_table( mock_supported_document_get_from_field_name = mocker.patch( "enums.supported_document_types.SupportedDocumentTypes.get_from_field_name" ) - mock_supported_document_get_from_field_name.return_value = SupportedDocumentTypes.LG + mock_supported_document_get_from_field_name.return_value = ( + SupportedDocumentTypes.LG.value + ) mock_presigned = mocker.patch( "services.s3_service.S3Service.create_document_presigned_url_handler" ) @@ -226,7 +232,7 @@ def test_create_document_reference_arf_type_dynamo_ClientError_returns_500( "enums.supported_document_types.SupportedDocumentTypes.get_from_field_name" ) mock_supported_document_get_from_field_name.return_value = ( - SupportedDocumentTypes.ARF + SupportedDocumentTypes.ARF.value ) mock_presigned = mocker.patch( "services.s3_service.S3Service.create_document_presigned_url_handler" @@ -250,7 +256,7 @@ def test_create_document_reference_arf_type_s3_ClientError_returns_500( "enums.supported_document_types.SupportedDocumentTypes.get_from_field_name" ) mock_supported_document_get_from_field_name.return_value = ( - SupportedDocumentTypes.ARF + SupportedDocumentTypes.ARF.value ) mock_presigned = mocker.patch( "services.s3_service.S3Service.create_document_presigned_url_handler" @@ -278,7 +284,9 @@ def test_invalid_file_type_for_lg_return_400(set_env, context, mocker, event_bod mock_supported_document_get_from_field_name = mocker.patch( "enums.supported_document_types.SupportedDocumentTypes.get_from_field_name" ) - mock_supported_document_get_from_field_name.return_value = SupportedDocumentTypes.LG + mock_supported_document_get_from_field_name.return_value = ( + SupportedDocumentTypes.LG.value + ) mocker.patch("services.dynamo_service.DynamoDBService.create_item") mock_presigned = mocker.patch( "services.s3_service.S3Service.create_document_presigned_url_handler" @@ -324,7 +332,9 @@ def test_lambda_handler_missing_environment_variables_type_lg_returns_400( mock_supported_document_get_from_field_name = mocker.patch( "enums.supported_document_types.SupportedDocumentTypes.get_from_field_name" ) - mock_supported_document_get_from_field_name.return_value = SupportedDocumentTypes.LG + mock_supported_document_get_from_field_name.return_value = ( + SupportedDocumentTypes.LG.value + ) monkeypatch.delenv(environmentVariable) expected = ApiGatewayResponse( 500, @@ -343,7 +353,7 @@ def test_lambda_handler_missing_environment_variables_type_arf_returns_400( "enums.supported_document_types.SupportedDocumentTypes.get_from_field_name" ) mock_supported_document_get_from_field_name.return_value = ( - SupportedDocumentTypes.ARF + SupportedDocumentTypes.ARF.value ) monkeypatch.delenv(environmentVariable) expected = ApiGatewayResponse( diff --git a/lambdas/tests/unit/handlers/test_delete_document_reference_handler.py b/lambdas/tests/unit/handlers/test_delete_document_reference_handler.py index 115e4f380..c737c6f08 100644 --- a/lambdas/tests/unit/handlers/test_delete_document_reference_handler.py +++ b/lambdas/tests/unit/handlers/test_delete_document_reference_handler.py @@ -1,3 +1,4 @@ +import pytest from handlers.delete_document_reference_handler import lambda_handler from tests.unit.helpers.data.test_documents import ( create_test_doc_store_refs, create_test_lloyd_george_doc_store_refs) @@ -7,7 +8,69 @@ TEST_LG_DOC_STORE_REFERENCES = create_test_lloyd_george_doc_store_refs() -def test_lambda_handler_valid_arf_docs_successful_delete_returns_204( +@pytest.mark.parametrize( + "event_body", + [ + { + "httpMethod": "GET", + "queryStringParameters": {"patientId": "9000000009", "docType": "LG,ARF"}, + }, + { + "httpMethod": "GET", + "queryStringParameters": {"patientId": "9000000009", "docType": "ARF,LG"}, + }, + { + "httpMethod": "GET", + "queryStringParameters": {"patientId": "9000000009", "docType": "LG , ARF"}, + }, + { + "httpMethod": "GET", + "queryStringParameters": {"patientId": "9000000009", "docType": "ARF, LG"}, + }, + ], +) +def test_lambda_handler_valid_both_doc_types_successful_delete_returns_200( + mocker, set_env, event_body, context +): + mock_document_query = mocker.patch( + "services.document_service.DocumentService.fetch_available_document_references_by_type" + ) + mock_document_query.side_effect = [ + TEST_DOC_STORE_REFERENCES, + TEST_LG_DOC_STORE_REFERENCES, + ] + + mocker.patch("services.document_service.DocumentService.delete_documents") + + expected = ApiGatewayResponse( + 200, "Success", "DELETE" + ).create_api_gateway_response() + + actual = lambda_handler(event_body, context) + + assert expected == actual + + +def test_lambda_handler_valid_both_doc_types_empty_arf_doc_refs_successful_delete_returns_200( + mocker, set_env, valid_id_and_both_doctype_event, context +): + mock_document_query = mocker.patch( + "services.document_service.DocumentService.fetch_available_document_references_by_type" + ) + mock_document_query.side_effect = [[], TEST_LG_DOC_STORE_REFERENCES] + + mocker.patch("services.document_service.DocumentService.delete_documents") + + expected = ApiGatewayResponse( + 200, "Success", "DELETE" + ).create_api_gateway_response() + + actual = lambda_handler(valid_id_and_both_doctype_event, context) + + assert expected == actual + + +def test_lambda_handler_valid_arf_docs_successful_delete_returns_200( mocker, set_env, valid_id_and_arf_doctype_event, context ): mock_document_query = mocker.patch( @@ -26,7 +89,7 @@ def test_lambda_handler_valid_arf_docs_successful_delete_returns_204( assert expected == actual -def test_lambda_handler_valid_lg_docs_successful_delete_returns_204( +def test_lambda_handler_valid_lg_docs_successful_delete_returns_200( mocker, set_env, valid_id_and_lg_doctype_event, context ): mock_document_query = mocker.patch( @@ -45,6 +108,25 @@ def test_lambda_handler_valid_lg_docs_successful_delete_returns_204( assert expected == actual +def test_lambda_handler_valid_both_doc_type_no_documents_found_returns_404( + mocker, set_env, valid_id_and_both_doctype_event, context +): + mock_document_query = mocker.patch( + "services.document_service.DocumentService.fetch_available_document_references_by_type" + ) + mock_document_query.side_effect = [[], []] + + mocker.patch("services.document_service.DocumentService.delete_documents") + + expected = ApiGatewayResponse( + 404, "No documents available", "DELETE" + ).create_api_gateway_response() + + actual = lambda_handler(valid_id_and_both_doctype_event, context) + + assert expected == actual + + def test_lambda_handler_no_documents_found_returns_404( mocker, set_env, valid_id_and_arf_doctype_event, context ): diff --git a/lambdas/tests/unit/services/test_document_service.py b/lambdas/tests/unit/services/test_document_service.py index bb0c9447c..d564af8d9 100644 --- a/lambdas/tests/unit/services/test_document_service.py +++ b/lambdas/tests/unit/services/test_document_service.py @@ -1,13 +1,11 @@ -import os from datetime import datetime, timedelta -from unittest.mock import MagicMock, patch -import boto3 import pytest from enums.s3_lifecycle_tags import S3LifecycleDays, S3LifecycleTags from freezegun import freeze_time from models.document_reference import DocumentReference -from tests.unit.conftest import MOCK_TABLE_NAME +from tests.unit.conftest import (MOCK_ARF_TABLE_NAME, MOCK_LG_TABLE_NAME, + MOCK_TABLE_NAME, TEST_NHS_NUMBER) from tests.unit.helpers.data.dynamo_responses import (MOCK_EMPTY_RESPONSE, MOCK_SEARCH_RESPONSE) @@ -30,74 +28,115 @@ def nhs_number(): return "9000000009" -def test_returns_list_of_documents_when_results_are_returned(nhs_number): - expected_table = "expected_table_name" - os.environ["LLOYD_GEORGE_DYNAMODB_NAME"] = expected_table - os.environ["DOCUMENT_STORE_DYNAMODB_NAME"] = "no-table" +def test_returns_list_of_lg_document_references_when_results_are_returned( + set_env, mocker +): + mocker.patch("boto3.resource") + service = DocumentService() - with patch.object(boto3, "resource", return_value=MagicMock()) as mock_dynamo: - mock_table = MagicMock() - mock_dynamo.return_value.Table.return_value = mock_table - mock_table.query.return_value = MOCK_SEARCH_RESPONSE - result = DocumentService().fetch_available_document_references_by_type( - nhs_number, "LG" - ) + mock_dynamo_table = mocker.patch.object(service.dynamodb, "Table") + mock_dynamo_table.return_value.query.return_value = MOCK_SEARCH_RESPONSE - mock_dynamo.return_value.Table.assert_called_with(expected_table) + results = service.fetch_available_document_references_by_type(TEST_NHS_NUMBER, "LG") - assert len(result) == 3 - assert type(result[0]) == DocumentReference + mock_dynamo_table.assert_called_with(MOCK_LG_TABLE_NAME) + assert len(results) == 3 + for result in results: + assert isinstance(result, DocumentReference) -def test_only_retrieves_documents_from_lloyd_george_table(nhs_number): - expected_table = "expected_table_name" - os.environ["LLOYD_GEORGE_DYNAMODB_NAME"] = expected_table - os.environ["DOCUMENT_STORE_DYNAMODB_NAME"] = "no-table" - with patch.object(boto3, "resource", return_value=MagicMock()) as mock_dynamo: - mock_table = MagicMock() - mock_dynamo.return_value.Table.return_value = mock_table - mock_table.query.return_value = MOCK_EMPTY_RESPONSE - result = DocumentService().fetch_available_document_references_by_type( - nhs_number, "LG" - ) +def test_returns_list_of_arf_document_references_when_results_are_returned( + set_env, mocker +): + mocker.patch("boto3.resource") + service = DocumentService() - mock_dynamo.return_value.Table.assert_called_with(expected_table) + mock_dynamo_table = mocker.patch.object(service.dynamodb, "Table") + mock_dynamo_table.return_value.query.return_value = MOCK_SEARCH_RESPONSE - assert len(result) == 0 + results = service.fetch_available_document_references_by_type( + TEST_NHS_NUMBER, "ARF" + ) + + mock_dynamo_table.assert_called_with(MOCK_ARF_TABLE_NAME) + + assert len(results) == 3 + for result in results: + assert isinstance(result, DocumentReference) + + +def test_returns_list_of_both_type_document_references_when_results_are_returned( + set_env, mocker +): + mocker.patch("boto3.resource") + service = DocumentService() + mock_dynamo_table = mocker.patch.object(service.dynamodb, "Table") + mock_dynamo_table.return_value.query.side_effect = [ + MOCK_SEARCH_RESPONSE, + MOCK_SEARCH_RESPONSE, + ] -def test_only_retrieves_documents_from_electronic_health_record_table(nhs_number): - expected_table = "expected_table_name" - os.environ["LLOYD_GEORGE_DYNAMODB_NAME"] = "no-table" - os.environ["DOCUMENT_STORE_DYNAMODB_NAME"] = expected_table + results = service.fetch_available_document_references_by_type( + TEST_NHS_NUMBER, "ALL" + ) + + assert mock_dynamo_table.call_count == 2 + + assert len(results) == 6 + for result in results: + assert isinstance(result, DocumentReference) - with patch.object(boto3, "resource", return_value=MagicMock()) as mock_dynamo: - mock_table = MagicMock() - mock_dynamo.return_value.Table.return_value = mock_table - mock_table.query.return_value = MOCK_EMPTY_RESPONSE - result = DocumentService().fetch_available_document_references_by_type( - nhs_number, "ARF" - ) - mock_dynamo.return_value.Table.assert_called_with(expected_table) +def test_returns_list_of_both_type_document_references_when_only_one_result_is_returned( + set_env, mocker +): + mocker.patch("boto3.resource") + service = DocumentService() - assert len(result) == 0 + mock_dynamo_table = mocker.patch.object(service.dynamodb, "Table") + mock_dynamo_table.return_value.query.side_effect = [ + MOCK_SEARCH_RESPONSE, + MOCK_EMPTY_RESPONSE, + ] + + results = service.fetch_available_document_references_by_type( + TEST_NHS_NUMBER, "ALL" + ) + assert mock_dynamo_table.call_count == 2 -def test_nothing_returned_when_invalid_doctype_supplied(nhs_number): - expected_table = "expected_table_name" - os.environ["LLOYD_GEORGE_DYNAMODB_NAME"] = "no-table" - os.environ["DOCUMENT_STORE_DYNAMODB_NAME"] = expected_table + assert len(results) == 3 + for result in results: + assert isinstance(result, DocumentReference) - with patch.object(boto3, "resource", return_value=MagicMock()) as mock_dynamo: - mock_table = MagicMock() - mock_dynamo.return_value.Table.return_value = mock_table - result = DocumentService().fetch_available_document_references_by_type( - nhs_number, "" - ) - assert len(result) == 0 +def test_returns_empty_list_of_lg_document_references_when_no_results_are_returned( + set_env, mocker +): + mocker.patch("boto3.resource") + service = DocumentService() + + mock_dynamo_table = mocker.patch.object(service.dynamodb, "Table") + mock_dynamo_table.return_value.query.return_value = MOCK_EMPTY_RESPONSE + + result = service.fetch_available_document_references_by_type(TEST_NHS_NUMBER, "LG") + + mock_dynamo_table.assert_called_with(MOCK_LG_TABLE_NAME) + + assert len(result) == 0 + + +def test_returns_empty_list_when_invalid_doctype_supplied(set_env, mocker): + mocker.patch("boto3.resource") + service = DocumentService() + + result = service.fetch_available_document_references_by_type( + TEST_NHS_NUMBER, "INVALID" + ) + + assert len(result) == 0 @freeze_time("2023-10-1 13:00:00") diff --git a/lambdas/tests/unit/utils/decorators/test_validate_document_type.py b/lambdas/tests/unit/utils/decorators/test_validate_document_type.py index 06b8556f4..43e796118 100755 --- a/lambdas/tests/unit/utils/decorators/test_validate_document_type.py +++ b/lambdas/tests/unit/utils/decorators/test_validate_document_type.py @@ -1,7 +1,9 @@ +import pytest +from enums.supported_document_types import SupportedDocumentTypes from utils.lambda_response import ApiGatewayResponse -from lambdas.utils.decorators.validate_document_type import \ - validate_document_type +from lambdas.utils.decorators.validate_document_type import ( + extract_document_type, validate_document_type) @validate_document_type @@ -93,3 +95,50 @@ def test_returns_400_response_when_doctype_field_not_in_event( actual = lambda_handler(valid_id_and_none_doctype_event, context) assert actual == expected + + +@pytest.mark.parametrize( + "value", + [ + "LG, ARF", + "ARF,LG", + " ARF, LG", + "LG , ARF", + ], +) +def test_extract_document_type_both(value): + expected = SupportedDocumentTypes.ALL.value + + actual = extract_document_type(value) + + assert expected == actual + + +@pytest.mark.parametrize( + "value", + [ + "LG ", + " LG", + ], +) +def test_extract_document_type_lg(value): + expected = SupportedDocumentTypes.LG.value + + actual = extract_document_type(value) + + assert expected == actual + + +@pytest.mark.parametrize( + "value", + [ + "ARF ", + " ARF", + ], +) +def test_extract_document_type_arf(value): + expected = SupportedDocumentTypes.ARF.value + + actual = extract_document_type(value) + + assert expected == actual diff --git a/lambdas/utils/decorators/validate_document_type.py b/lambdas/utils/decorators/validate_document_type.py index bdb012081..8c6f94fb0 100755 --- a/lambdas/utils/decorators/validate_document_type.py +++ b/lambdas/utils/decorators/validate_document_type.py @@ -39,9 +39,33 @@ def interceptor(event, context): return interceptor -def doc_type_is_valid(doc_types: str) -> bool: - doc_types_requested = doc_types.split(",") +def doc_type_is_valid(value: str) -> bool: + doc_types_requested = value.replace(" ", "").split(",") for doc_type_requested in doc_types_requested: if SupportedDocumentTypes.get_from_field_name(doc_type_requested) is None: return False return True + + +def extract_document_type(value: str) -> str: + doc_type = value.replace(" ", "") + + if doc_type == SupportedDocumentTypes.LG.value: + return str(SupportedDocumentTypes.LG.value) + if doc_type == SupportedDocumentTypes.ARF.value: + return str(SupportedDocumentTypes.ARF.value) + + doc_types_requested = doc_type.split(",") + + doc_types = [] + for doc_type in doc_types_requested: + if SupportedDocumentTypes.get_from_field_name(doc_type): + doc_types.append(doc_type) + + doc_type_intersection = set(doc_types) | set(SupportedDocumentTypes.list_names()) + + if ( + SupportedDocumentTypes.LG.value in doc_type_intersection + and SupportedDocumentTypes.ARF.value in doc_type_intersection + ): + return str(SupportedDocumentTypes.ALL.value) diff --git a/lambdas/utils/lambda_response.py b/lambdas/utils/lambda_response.py index 7b7b818e5..b1f28fc3c 100644 --- a/lambdas/utils/lambda_response.py +++ b/lambdas/utils/lambda_response.py @@ -1,10 +1,10 @@ class ApiGatewayResponse: - def __init__(self, status_code, body, methods) -> None: + def __init__(self, status_code: int, body: str, methods: str) -> None: self.status_code = status_code self.body = body self.methods = methods - def create_api_gateway_response(self, headers=None) -> object: + def create_api_gateway_response(self, headers=None) -> dict: if headers is None: headers = {} return {