diff --git a/app/src/components/blocks/_delete/deleteSubmitStage/DeleteSubmitStage.test.tsx b/app/src/components/blocks/_delete/deleteSubmitStage/DeleteSubmitStage.test.tsx index aa1c28778b..058d6174c9 100644 --- a/app/src/components/blocks/_delete/deleteSubmitStage/DeleteSubmitStage.test.tsx +++ b/app/src/components/blocks/_delete/deleteSubmitStage/DeleteSubmitStage.test.tsx @@ -147,7 +147,7 @@ describe('DeleteSubmitStage', () => { }); }); - it('renders DeletionConfirmationStage when the Yes is selected and Continue clicked, when user role is GP_ADMIN', async () => { + it('renders lloyd george DeletionCompleteStage when the Yes is selected and Continue clicked, when user role is GP_ADMIN and feature flag is disabled', async () => { mockedUseRole.mockReturnValue(REPOSITORY_ROLE.GP_ADMIN); mockedAxios.delete.mockReturnValue(Promise.resolve({ status: 200, data: 'Success' })); @@ -162,13 +162,18 @@ describe('DeleteSubmitStage', () => { await waitFor(() => { expect(mockedUseNavigate).toHaveBeenCalledWith( - routeChildren.DOCUMENT_DELETE_COMPLETE, + routeChildren.LLOYD_GEORGE_DELETE_COMPLETE, ); }); }); - it('calls resetDocState when the Yes is selected and Continue clicked', async () => { + it('renders DeletionCompleteStage when the Yes is selected and Continue clicked, when user role is GP_ADMIN and feature flag is enabled', async () => { mockedUseRole.mockReturnValue(REPOSITORY_ROLE.GP_ADMIN); + mockuseConfig.mockReturnValue({ + featureFlags: { + uploadDocumentIteration3Enabled: true, + }, + }); mockedAxios.delete.mockReturnValue(Promise.resolve({ status: 200, data: 'Success' })); @@ -181,12 +186,14 @@ describe('DeleteSubmitStage', () => { await userEvent.click(screen.getByRole('button', { name: 'Continue' })); await waitFor(() => { - expect(mockResetDocState).toHaveBeenCalled(); + expect(mockedUseNavigate).toHaveBeenCalledWith( + routeChildren.DOCUMENT_DELETE_COMPLETE, + ); }); }); - it('renders DeletionConfirmationStage when the Yes is selected and Continue clicked, when user role is PCSE', async () => { - mockedUseRole.mockReturnValue(REPOSITORY_ROLE.PCSE); + it('calls resetDocState when the Yes is selected and Continue clicked', async () => { + mockedUseRole.mockReturnValue(REPOSITORY_ROLE.GP_ADMIN); mockedAxios.delete.mockReturnValue(Promise.resolve({ status: 200, data: 'Success' })); @@ -199,9 +206,7 @@ describe('DeleteSubmitStage', () => { await userEvent.click(screen.getByRole('button', { name: 'Continue' })); await waitFor(() => { - expect(mockedUseNavigate).toHaveBeenCalledWith( - routeChildren.DOCUMENT_DELETE_COMPLETE, - ); + expect(mockResetDocState).toHaveBeenCalled(); }); }); diff --git a/app/src/components/blocks/_delete/deleteSubmitStage/DeleteSubmitStage.tsx b/app/src/components/blocks/_delete/deleteSubmitStage/DeleteSubmitStage.tsx index 75b0b5b7d7..dc6cb66f36 100644 --- a/app/src/components/blocks/_delete/deleteSubmitStage/DeleteSubmitStage.tsx +++ b/app/src/components/blocks/_delete/deleteSubmitStage/DeleteSubmitStage.tsx @@ -8,7 +8,10 @@ import useBaseAPIHeaders from '../../../../helpers/hooks/useBaseAPIHeaders'; import { DOWNLOAD_STAGE } from '../../../../types/generic/downloadStage'; import SpinnerButton from '../../../generic/spinnerButton/SpinnerButton'; import ServiceError from '../../../layout/serviceErrorBox/ServiceErrorBox'; -import { SUBMISSION_STATE } from '../../../../types/pages/documentSearchResultsPage/types'; +import { + DocumentReference, + SUBMISSION_STATE, +} from '../../../../types/pages/documentSearchResultsPage/types'; import { AxiosError } from 'axios'; import { routeChildren, routes } from '../../../../types/generic/routes'; import { Outlet, Route, Routes, useNavigate } from 'react-router-dom'; @@ -23,12 +26,18 @@ import useTitle from '../../../../helpers/hooks/useTitle'; import ErrorBox from '../../../layout/errorBox/ErrorBox'; import BackButton from '../../../generic/backButton/BackButton'; import PatientSummary, { PatientInfo } from '../../../generic/patientSummary/PatientSummary'; -import { DOCUMENT_TYPE, getDocumentTypeLabel } from '../../../../helpers/utils/documentType'; +import { + DOCUMENT_TYPE, + DOCUMENT_TYPE_CONFIG, + getConfigForDocType, + getDocumentTypeLabel, +} from '../../../../helpers/utils/documentType'; import { getLastURLPath } from '../../../../helpers/utils/urlManipulations'; import DeleteResultStage from '../deleteResultStage/DeleteResultStage'; export type Props = { - docType: DOCUMENT_TYPE; + document?: DocumentReference; + docType?: DOCUMENT_TYPE; setDownloadStage?: Dispatch>; resetDocState: () => void; }; @@ -39,11 +48,13 @@ enum DELETE_DOCUMENTS_OPTION { } type IndexViewProps = { - docType: DOCUMENT_TYPE; + document?: DocumentReference; + docType?: DOCUMENT_TYPE; resetDocState: () => void; }; -const DeleteSubmitStageIndexView = ({ +export const DeleteSubmitStageIndexView = ({ + document, docType, resetDocState, }: IndexViewProps): React.JSX.Element => { @@ -62,15 +73,26 @@ const DeleteSubmitStageIndexView = ({ 'Select whether you want to permanently delete these patient files'; const userIsGP = role === REPOSITORY_ROLE.GP_ADMIN || role === REPOSITORY_ROLE.GP_CLINICAL; + let documentConfig: DOCUMENT_TYPE_CONFIG | null = null; + if (docType !== DOCUMENT_TYPE.ALL) { + documentConfig = getConfigForDocType(document?.documentSnomedCodeType ?? docType!) + docType = documentConfig.snomedCode as DOCUMENT_TYPE; + } + const handleYesOption = async (): Promise => { const onSuccess = (): void => { resetDocState(); setDeletionStage(SUBMISSION_STATE.SUCCEEDED); - navigate(routeChildren.DOCUMENT_DELETE_COMPLETE); + navigate( + config.featureFlags.uploadDocumentIteration3Enabled + ? routeChildren.DOCUMENT_DELETE_COMPLETE + : routeChildren.LLOYD_GEORGE_DELETE_COMPLETE, + ); }; try { setDeletionStage(SUBMISSION_STATE.PENDING); const response: DeleteResponse = await deleteAllDocuments({ + documentId: !documentConfig?.singleDocumentOnly ? document?.id : undefined, docType: docType, nhsNumber: nhsNumber, baseUrl, @@ -116,8 +138,15 @@ const DeleteSubmitStageIndexView = ({ } }; - const pageTitle = `You are removing the ${getDocumentTypeLabel(docType) ?? 'records'} of`; - useTitle({ pageTitle }); + const pageTitle = (): string => { + if (docType) { + return `You are removing the ${getDocumentTypeLabel(docType) ?? 'records'} of`; + } + + return `You are removing a record from patient`; + }; + + useTitle({ pageTitle: pageTitle() }); return ( <> @@ -141,13 +170,23 @@ const DeleteSubmitStageIndexView = ({ )}
- {pageTitle}: + {pageTitle()}: + {document && ( + <> +

+ Record type:{' '} + {documentConfig?.displayName} +

+

Filename: {document.fileName}

+ + )} + {!userIsGP && ( Before removing @@ -223,6 +262,7 @@ const DeleteSubmitStageIndexView = ({ }; const DeleteSubmitStage = ({ + document, docType, setDownloadStage, resetDocState, @@ -234,21 +274,12 @@ const DeleteSubmitStage = ({ index element={ } /> - - } - /> } + element={ + + } > { ); @@ -321,15 +321,13 @@ describe('DocumentView', () => { }); describe('Document actions', () => { - it('calls removeDocuments when remove action is triggered', () => { + it('calls removeDocument when remove action is triggered', () => { renderComponent(); // Assuming the first record link is remove action const removeRecordLink = screen.getByTestId(lloydGeorgeRecordLinks[0].key); fireEvent.click(removeRecordLink); - expect(mockRemoveDocuments).toHaveBeenCalledWith( - mockDocumentReference.documentSnomedCodeType, - ); + expect(mockRemoveDocument).toHaveBeenCalled(); }); }); diff --git a/app/src/components/blocks/_patientDocuments/documentView/DocumentView.tsx b/app/src/components/blocks/_patientDocuments/documentView/DocumentView.tsx index 9f63905e08..7dd61c6f76 100644 --- a/app/src/components/blocks/_patientDocuments/documentView/DocumentView.tsx +++ b/app/src/components/blocks/_patientDocuments/documentView/DocumentView.tsx @@ -23,12 +23,12 @@ import useRole from '../../../../helpers/hooks/useRole'; type Props = { documentReference: DocumentReference | null; - removeDocuments: (docType: DOCUMENT_TYPE) => void; + removeDocument: () => void; }; const DocumentView = ({ documentReference, - removeDocuments, + removeDocument, }: Readonly): React.JSX.Element => { const [session, setUserSession] = useSessionContext(); const role = useRole(); @@ -95,7 +95,7 @@ const DocumentView = ({ const removeClicked = (): void => { disableFullscreen(); - removeDocuments(documentReference.documentSnomedCodeType); + removeDocument(); }; const getCardLinks = (): Array => { diff --git a/app/src/helpers/requests/deleteAllDocuments.ts b/app/src/helpers/requests/deleteAllDocuments.ts index 17259634ba..f5c217bc7c 100644 --- a/app/src/helpers/requests/deleteAllDocuments.ts +++ b/app/src/helpers/requests/deleteAllDocuments.ts @@ -3,7 +3,8 @@ import { AuthHeaders } from '../../types/blocks/authHeaders'; import { DOCUMENT_TYPE } from '../utils/documentType'; type Args = { - docType: DOCUMENT_TYPE; + documentId?: string; + docType?: DOCUMENT_TYPE; nhsNumber: string; baseUrl: string; baseHeaders: AuthHeaders; @@ -14,6 +15,7 @@ export type DeleteResponse = { status: number; }; const deleteAllDocuments = async ({ + documentId, docType, nhsNumber, baseUrl, @@ -29,6 +31,7 @@ const deleteAllDocuments = async ({ params: { patientId: nhsNumber, docType, + documentId, }, }); return response; diff --git a/app/src/pages/documentSearchResultsPage/DocumentSearchResultsPage.tsx b/app/src/pages/documentSearchResultsPage/DocumentSearchResultsPage.tsx index d6ec003a21..163d15a361 100644 --- a/app/src/pages/documentSearchResultsPage/DocumentSearchResultsPage.tsx +++ b/app/src/pages/documentSearchResultsPage/DocumentSearchResultsPage.tsx @@ -48,7 +48,6 @@ const DocumentSearchResultsPage = (): React.JSX.Element => { const config = useConfig(); const mounted = useRef(false); const activeSearchResult = useRef(null); - const [removeDocType, setRemoveDocType] = useState(undefined); useEffect(() => { const onPageLoad = async (): Promise => { @@ -159,8 +158,7 @@ const DocumentSearchResultsPage = (): React.JSX.Element => { return URL.createObjectURL(data); }; - const removeDocuments = (docType: DOCUMENT_TYPE): void => { - setRemoveDocType(docType); + const removeDocument = (): void => { navigate(routeChildren.DOCUMENT_DELETE); }; @@ -186,7 +184,7 @@ const DocumentSearchResultsPage = (): React.JSX.Element => { element={ } /> @@ -194,7 +192,8 @@ const DocumentSearchResultsPage = (): React.JSX.Element => { path={getLastURLPath(routeChildren.DOCUMENT_DELETE) + '/*'} element={ { mounted.current = false; }} diff --git a/lambdas/enums/lambda_error.py b/lambdas/enums/lambda_error.py index 192fd4d303..09b4a52cbd 100644 --- a/lambdas/enums/lambda_error.py +++ b/lambdas/enums/lambda_error.py @@ -315,6 +315,10 @@ def create_error_body(self, params: Optional[dict] = None, **kwargs) -> str: "err_code": "DDS_4002", "message": "Failed to delete document object", } + DocDelInvalidRequest = { + "err_code": "DDS_4003", + "message": "Invalid Request" + } DocDelClient = { "err_code": "DDS_5001", "message": "Failed to delete documents", diff --git a/lambdas/handlers/delete_document_reference_handler.py b/lambdas/handlers/delete_document_reference_handler.py index 558b71e449..b2f11ed358 100644 --- a/lambdas/handlers/delete_document_reference_handler.py +++ b/lambdas/handlers/delete_document_reference_handler.py @@ -6,7 +6,6 @@ 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_document_type import validate_document_type from utils.decorators.validate_patient_id import ( extract_nhs_number_from_event, validate_patient_id, @@ -15,12 +14,13 @@ from utils.lambda_response import ApiGatewayResponse from utils.request_context import request_context +from lambdas.enums.supported_document_types import SupportedDocumentTypes + logger = LoggingService(__name__) @set_request_context_for_logging @validate_patient_id -@validate_document_type @ensure_environment_variables( names=[ "DOCUMENT_STORE_DYNAMODB_NAME", @@ -36,15 +36,49 @@ def lambda_handler(event, context): logger.info("Delete Document Reference handler has been triggered") nhs_number = extract_nhs_number_from_event(event) - document_types = extract_document_type_to_enum( - event["queryStringParameters"]["docType"] - ) + document_types_query = event.get("queryStringParameters", {}).get("docType", None) + + document_types = [] + try: + document_types = ( + extract_document_type_to_enum(document_types_query) + if document_types_query + else [] + ) + except ValueError: + logger.error( + LambdaError.DocTypeInvalid.to_str(), {"Result": "Invalid document type"} + ) + return ApiGatewayResponse( + 400, LambdaError.DocTypeInvalid.create_error_body(), "DELETE" + ).create_api_gateway_response() + + document_id = event.get("queryStringParameters", {}).get("documentId", None) + + if (len(document_types) > 0 and document_id) or ( + len(document_types) == 0 and document_id is None + ): + logger.error( + LambdaError.DocDelInvalidRequest.to_str(), {"Result": "Invalid request"} + ) + + return ApiGatewayResponse( + 400, LambdaError.DocDelInvalidRequest.create_error_body(), "DELETE" + ).create_api_gateway_response() request_context.patient_nhs_no = nhs_number + return process_delete(nhs_number, document_types, document_id) + + +def process_delete( + nhs_number: str, document_types: list[SupportedDocumentTypes], document_id: str +): deletion_service = DocumentDeletionService() - files_deleted = deletion_service.handle_reference_delete(nhs_number, document_types) + files_deleted = deletion_service.handle_reference_delete( + nhs_number, document_types, document_id + ) if files_deleted: logger.info( "Documents were deleted successfully", {"Result": "Successful deletion"} diff --git a/lambdas/handlers/manage_nrl_pointer_handler.py b/lambdas/handlers/manage_nrl_pointer_handler.py index efd66d3b6d..84bb21a561 100644 --- a/lambdas/handlers/manage_nrl_pointer_handler.py +++ b/lambdas/handlers/manage_nrl_pointer_handler.py @@ -64,7 +64,9 @@ def lambda_handler(event, context): case NrlActionTypes.DELETE: nrl_api_service.delete_pointer( - nrl_message.nhs_number, nrl_message.snomed_code_doc_type + nrl_message.nhs_number, + nrl_message.snomed_code_doc_type, + nrl_message.attachment.url or None, ) except Exception as error: logger.info(f"Failed to process current message due to error: {error}") diff --git a/lambdas/services/document_deletion_service.py b/lambdas/services/document_deletion_service.py index 0a89328c5f..d07f200247 100644 --- a/lambdas/services/document_deletion_service.py +++ b/lambdas/services/document_deletion_service.py @@ -1,6 +1,6 @@ import os import uuid -from typing import Literal +from typing import Literal, Optional from urllib.parse import urlparse from botocore.exceptions import ClientError @@ -8,16 +8,18 @@ from enums.lambda_error import LambdaError from enums.metadata_field_names import DocumentReferenceMetadataFields from enums.nrl_sqs_upload import NrlActionTypes -from enums.snomed_codes import SnomedCodes +from enums.snomed_codes import SnomedCode, SnomedCodes from enums.supported_document_types import SupportedDocumentTypes from inflection import underscore from models.document_reference import DocumentReference +from models.fhir.R4.fhir_document_reference import Attachment from models.sqs.nrl_sqs_message import NrlSqsMessage from services.base.sqs_service import SQSService from services.document_service import DocumentService from services.lloyd_george_stitch_job_service import LloydGeorgeStitchJobService from utils.audit_logging_setup import LoggingService -from utils.common_query_filters import NotDeleted +from utils.common_query_filters import NotDeleted, get_document_type_filter +from utils.dynamo_query_filter_builder import DynamoQueryFilterBuilder from utils.exceptions import DocumentServiceException, DynamoServiceException from utils.lambda_exceptions import DocumentDeletionServiceException @@ -31,17 +33,56 @@ def __init__(self): self.sqs_service = SQSService() def handle_reference_delete( - self, nhs_number: str, doc_types: list[SupportedDocumentTypes] + self, + nhs_number: str, + doc_types: list[SupportedDocumentTypes], + document_id: str | None = None, ) -> list[DocumentReference]: + if document_id: + self.delete_document_by_id(nhs_number, document_id) + return [document_id] + else: + return self.delete_documents_by_types(nhs_number, doc_types) + + def delete_document_by_id(self, nhs_number: str, document_id: str): + doc_ref_list: DocumentReference = ( + self.document_service.fetch_documents_from_table( + search_condition=document_id, + search_key="ID", + model_class=DocumentReference, + ) + ) + + if len(doc_ref_list) == 0: + raise DocumentDeletionServiceException(404, LambdaError.DocDelNull) + + document_ref: DocumentReference = doc_ref_list[0] + + self.document_service.delete_document_references( + table_name=self.document_service.table_name, + document_references=[document_ref], + document_ttl_days=DocumentRetentionDays.SOFT_DELETE, + ) + + self.send_sqs_message_to_remove_pointer( + nhs_number, + snomed=SnomedCodes.find_by_code(document_ref.document_snomed_code_type), + doc_ref=document_ref + ) + + def delete_documents_by_types( + self, nhs_number: str, doc_types: list[SupportedDocumentTypes] + ): files_deleted = [] for doc_type in doc_types: + snomed = SnomedCodes.find_by_code(doc_type) files_deleted += self.delete_specific_doc_type(nhs_number, doc_type) + self.send_sqs_message_to_remove_pointer(nhs_number, snomed) - self.delete_documents_references_in_stitch_table(nhs_number) if SupportedDocumentTypes.LG in doc_types: + self.delete_documents_references_in_stitch_table(nhs_number) self.delete_unstitched_document_reference(nhs_number) - self.send_sqs_message_to_remove_pointer(nhs_number) return files_deleted @@ -78,8 +119,14 @@ def get_documents_references_in_storage( nhs_number: str, doc_type: Literal[SupportedDocumentTypes.ARF, SupportedDocumentTypes.LG], ) -> list[DocumentReference]: - results = self.document_service.fetch_available_document_references_by_type( - nhs_number, doc_type, NotDeleted + query_filter = get_document_type_filter( + DynamoQueryFilterBuilder(), doc_type.value + ) + + results = self.document_service.fetch_documents_from_table_with_nhs_number( + nhs_number=nhs_number, + query_filter=query_filter, + model_class=DocumentReference, ) return results @@ -125,13 +172,23 @@ def delete_specific_doc_type( ) raise DocumentDeletionServiceException(500, LambdaError.DocDelClient) - def send_sqs_message_to_remove_pointer(self, nhs_number: str): + def send_sqs_message_to_remove_pointer( + self, + nhs_number: str, + snomed: SnomedCode, + doc_ref: Optional[DocumentReference] = None + ): delete_nrl_message = NrlSqsMessage( nhs_number=nhs_number, action=NrlActionTypes.DELETE, - snomed_code_doc_type=SnomedCodes.LLOYD_GEORGE.value, + snomed_code_doc_type=snomed, snomed_code_category=SnomedCodes.CARE_PLAN.value, ) + + if doc_ref: + attachment = Attachment(url=doc_ref.file_location) + delete_nrl_message.attachment = attachment + sqs_group_id = f"NRL_delete_{uuid.uuid4()}" nrl_queue_url = os.environ["NRL_SQS_QUEUE_URL"] self.sqs_service.send_message_fifo( diff --git a/lambdas/services/nrl_api_service.py b/lambdas/services/nrl_api_service.py index 5cbbeae072..41983a66c3 100644 --- a/lambdas/services/nrl_api_service.py +++ b/lambdas/services/nrl_api_service.py @@ -21,7 +21,7 @@ def __init__(self, ssm_service, auth_service): retry_strategy = Retry( total=3, status_forcelist=[429, 500, 502, 503, 504], - allowed_methods=["GET", "POST", "DELETE", "OPTIONS"], + allowed_methods=["GET", "PUT", "POST", "DELETE", "OPTIONS"], backoff_factor=1, ) adapter = HTTPAdapter(max_retries=retry_strategy) @@ -75,6 +75,39 @@ def create_new_pointer( else: raise NrlApiException("Error while creating new NRL Pointer") + def update_pointer(self, pointer: dict, nhs_number: str): + pointer_id = pointer.get("resource", {}).get("id") + url_endpoint = self.endpoint + f"/{pointer_id}" + self.set_x_request_id() + + try: + response = self.session.put( + url=url_endpoint, headers=self.headers, json=pointer + ) + logger.info( + f"Update pointer request: URL: {url_endpoint}, " + f"HTTP Verb: PUT, " + f"ODS Code: {self.end_user_ods_code}, " + f"NHS Number: {nhs_number}, " + f"Datetime: {int(datetime.now().timestamp())}, " + f"UserId: {self.end_user_ods_code} - {NRL_USER_ID}." + ) + logger.info( + f"Update pointer response: Body: {response.json()}, " + f"Status Code: {response.status_code}, " + f"Headers: {response.headers}" + ) + response.raise_for_status() + except HTTPError as e: + logger.error(e.response.json()) + if e.response.status_code == 401: + self.headers["Authorization"] = ( + f"Bearer {self.auth_service.get_active_access_token()}" + ) + self.session.put(url=url_endpoint, headers=self.headers, json=pointer) + else: + logger.error(f"Unable to update NRL Pointer: {pointer}") + def get_pointer( self, nhs_number: str, @@ -115,38 +148,91 @@ def get_pointer( else: raise NrlApiException("Error while getting NRL Pointer") - def delete_pointer(self, nhs_number: str, record_type: SnomedCode = None): - search_results = self.get_pointer(nhs_number, record_type).get("entry", []) + def get_pointer_by_url( + self, + nhs_number: str, + record_type: SnomedCode, + url: str, + retry_on_expired: bool = True, + ): + search_results = self.get_pointer( + nhs_number, record_type, retry_on_expired + ).get("entry", []) + + entry_with_url = None + for entry in search_results: - self.set_x_request_id() - pointer_id = entry.get("resource", {}).get("id") - url_endpoint = self.endpoint + f"/{pointer_id}" - try: - response = self.session.delete(url=url_endpoint, headers=self.headers) - logger.info( - f"Delete pointer request: URL: {url_endpoint}, " - f"HTTP Verb: DELETE, " - f"ODS Code: {self.end_user_ods_code}, " - f"NHS Number: {nhs_number}, " - f"Datetime: {int(datetime.now().timestamp())}, " - f"UserID: {self.end_user_ods_code} - {NRL_USER_ID}." - ) - logger.info( - f"Delete pointer response: Body: {response.json()}, " - f"Status Code: {response.status_code}, " - f"Headers: {response.headers}" + if entry_with_url: + break + content_list = entry.get("content", []) + for content in content_list: + content_url = content.get("attachment", {}).get("url", None) + + if content_url and content_url == url: + entry_with_url = entry + break + + return entry_with_url + + def delete_pointer( + self, nhs_number: str, record_type: SnomedCode = None, url: str = None + ): + if url: + self.delete_pointer_by_url(nhs_number, record_type, url) + else: + self.delete_pointer_by_record_type(nhs_number, record_type) + + def delete_pointer_by_url(self, nhs_number: str, record_type: SnomedCode, url: str): + pointer = self.get_pointer_by_url(nhs_number, record_type, url) + + if pointer: + content_list: list = pointer["content"] + + if len(content_list) > 1: + for content in content_list: + if content["attachment"]["url"] == url: + content_list.remove(content) + + self.update_pointer(pointer, nhs_number) + else: + self.delete_pointer_by_id(pointer, nhs_number) + + def delete_pointer_by_id(self, entry: dict, nhs_number: str): + self.set_x_request_id() + pointer_id = entry.get("resource", {}).get("id") + url_endpoint = self.endpoint + f"/{pointer_id}" + try: + response = self.session.delete(url=url_endpoint, headers=self.headers) + logger.info( + f"Delete pointer request: URL: {url_endpoint}, " + f"HTTP Verb: DELETE, " + f"ODS Code: {self.end_user_ods_code}, " + f"NHS Number: {nhs_number}, " + f"Datetime: {int(datetime.now().timestamp())}, " + f"UserID: {self.end_user_ods_code} - {NRL_USER_ID}." + ) + logger.info( + f"Delete pointer response: Body: {response.json()}, " + f"Status Code: {response.status_code}, " + f"Headers: {response.headers}" + ) + response.raise_for_status() + except HTTPError as e: + logger.error(e.response.json()) + if e.response.status_code == 401: + self.headers["Authorization"] = ( + f"Bearer {self.auth_service.get_active_access_token()}" ) - response.raise_for_status() - except HTTPError as e: - logger.error(e.response.json()) - if e.response.status_code == 401: - self.headers["Authorization"] = ( - f"Bearer {self.auth_service.get_active_access_token()}" - ) - self.session.delete(url=self.endpoint, headers=self.headers) - else: - logger.error(f"Unable to delete NRL Pointer: {entry}") - continue + self.session.delete(url=url_endpoint, headers=self.headers) + else: + logger.error(f"Unable to delete NRL Pointer: {entry}") + + def delete_pointer_by_record_type( + self, nhs_number: str, record_type: SnomedCode = None + ): + search_results = self.get_pointer(nhs_number, record_type).get("entry", []) + for entry in search_results: + self.delete_pointer_by_id(entry, nhs_number) def set_x_request_id(self): self.headers["X-Request-ID"] = str(uuid.uuid4()) 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 520060a615..14c05f8d6e 100644 --- a/lambdas/tests/unit/handlers/test_delete_document_reference_handler.py +++ b/lambdas/tests/unit/handlers/test_delete_document_reference_handler.py @@ -2,6 +2,7 @@ import pytest from botocore.exceptions import ClientError +from enums.lambda_error import LambdaError from handlers.delete_document_reference_handler import lambda_handler from services.document_deletion_service import DocumentDeletionService from tests.unit.conftest import MockError @@ -26,19 +27,31 @@ def mock_handle_delete(mocker): [ { "httpMethod": "GET", - "queryStringParameters": {"patientId": "9000000009", "docType": "16521000000101,ARF"}, + "queryStringParameters": { + "patientId": "9000000009", + "docType": "16521000000101,ARF", + }, }, { "httpMethod": "GET", - "queryStringParameters": {"patientId": "9000000009", "docType": "ARF,16521000000101"}, + "queryStringParameters": { + "patientId": "9000000009", + "docType": "ARF,16521000000101", + }, }, { "httpMethod": "GET", - "queryStringParameters": {"patientId": "9000000009", "docType": "16521000000101 , ARF"}, + "queryStringParameters": { + "patientId": "9000000009", + "docType": "16521000000101 , ARF", + }, }, { "httpMethod": "GET", - "queryStringParameters": {"patientId": "9000000009", "docType": "ARF, 16521000000101"}, + "queryStringParameters": { + "patientId": "9000000009", + "docType": "ARF, 16521000000101", + }, }, ], ) @@ -158,37 +171,38 @@ def test_lambda_handler_id_not_valid_returns_400(set_env, invalid_id_event, cont assert expected == actual -def test_lambda_handler_when_id_not_supplied_returns_400( - set_env, missing_id_event, context +def test_lambda_handler_returns_400_when_doc_type_and_doc_id_not_supplied( + set_env, valid_id_event_with_auth_header, context ): expected_body = json.dumps( { - "message": "An error occurred due to missing key", - "err_code": "PN_4002", + "message": "Invalid Request", + "err_code": "DDS_4003", "interaction_id": "88888888-4444-4444-4444-121212121212", } ) expected = ApiGatewayResponse( - 400, expected_body, "GET" + 400, expected_body, "DELETE" ).create_api_gateway_response() - actual = lambda_handler(missing_id_event, context) + actual = lambda_handler(valid_id_event_with_auth_header, context) assert expected == actual -def test_lambda_handler_returns_400_when_doc_type_not_supplied( - set_env, valid_id_event_without_auth_header, context +def test_lambda_handler_returns_400_when_doc_type_is_invalid( + set_env, valid_id_and_invalid_doctype_event, context ): + expected_body = json.dumps( { - "message": "An error occurred due to missing key", - "err_code": "VDT_4003", + "message": "Invalid document type requested", + "err_code": "VDT_4002", "interaction_id": "88888888-4444-4444-4444-121212121212", } ) expected = ApiGatewayResponse( - 400, expected_body, "GET" + 400, expected_body, "DELETE" ).create_api_gateway_response() - actual = lambda_handler(valid_id_event_without_auth_header, context) + actual = lambda_handler(valid_id_and_invalid_doctype_event, context) assert expected == actual @@ -253,3 +267,19 @@ def test_lambda_handler_handle_lambda_exception( ).create_api_gateway_response() actual = lambda_handler(valid_id_and_lg_doctype_delete_event, context) assert expected == actual + + +def test_lambda_handler_both_params_set_errors( + set_env, valid_id_and_lg_doctype_delete_event, context +): + valid_id_and_lg_doctype_delete_event["queryStringParameters"][ + "documentId" + ] = "mock_document_id" + + expected_result = ApiGatewayResponse( + 400, LambdaError.DocDelInvalidRequest.create_error_body(), "DELETE" + ).create_api_gateway_response() + + response = lambda_handler(valid_id_and_lg_doctype_delete_event, context) + + assert expected_result == response diff --git a/lambdas/tests/unit/services/test_document_deletion_service.py b/lambdas/tests/unit/services/test_document_deletion_service.py index d0ffe4e1d9..d086acaac8 100644 --- a/lambdas/tests/unit/services/test_document_deletion_service.py +++ b/lambdas/tests/unit/services/test_document_deletion_service.py @@ -5,6 +5,7 @@ from enums.lambda_error import LambdaError from enums.snomed_codes import SnomedCodes from enums.supported_document_types import SupportedDocumentTypes +from models.document_reference import DocumentReference from services.document_deletion_service import DocumentDeletionService from tests.unit.conftest import ( MOCK_ARF_TABLE_NAME, @@ -120,18 +121,15 @@ def test_handle_delete_for_all_doc_type( mock_delete_documents_references_in_stitch_table, mock_delete_unstitched_document_reference, ): - expected = TEST_DOC_STORE_REFERENCES + TEST_LG_DOC_STORE_REFERENCES + expected = TEST_LG_DOC_STORE_REFERENCES actual = mock_deletion_service.handle_reference_delete( - TEST_NHS_NUMBER, [SupportedDocumentTypes.ARF, SupportedDocumentTypes.LG] + TEST_NHS_NUMBER, [SupportedDocumentTypes.LG] ) assert expected == actual - assert mock_delete_specific_doc_type.call_count == 2 - mock_delete_specific_doc_type.assert_any_call( - TEST_NHS_NUMBER, SupportedDocumentTypes.ARF - ) + assert mock_delete_specific_doc_type.call_count == 1 mock_delete_specific_doc_type.assert_any_call( TEST_NHS_NUMBER, SupportedDocumentTypes.LG ) @@ -139,6 +137,50 @@ def test_handle_delete_for_all_doc_type( mock_delete_unstitched_document_reference.assert_called() +def test_handle_delete_for_doc_id( + mock_delete_specific_doc_type, + mock_deletion_service, + mock_delete_documents_references_in_stitch_table, + mock_delete_unstitched_document_reference, + mocker, +): + expected_document_id = "mock_document_id" + + mocked_document_reference = DocumentReference( + id=expected_document_id, + file_location="https://example.com/mocked_file_location", + file_name="mock_file_name", + nhs_number=TEST_NHS_NUMBER, + ) + + mocked_document_service = mock_deletion_service.document_service + mocked_document_service.fetch_documents_from_table.return_value = [ + mocked_document_reference + ] + + actual = mock_deletion_service.handle_reference_delete( + TEST_NHS_NUMBER, [], expected_document_id + ) + + assert [expected_document_id] == actual + + mock_delete_unstitched_document_reference.assert_not_called() + mock_delete_specific_doc_type.assert_not_called() + mock_delete_documents_references_in_stitch_table.assert_not_called() + + mocked_document_service.fetch_documents_from_table.assert_called_with( + search_condition=expected_document_id, + search_key="ID", + model_class=DocumentReference, + ) + + mocked_document_service.delete_document_references.assert_called_with( + table_name=mocked_document_service.table_name, + document_references=[mocked_document_reference], + document_ttl_days=DocumentRetentionDays.SOFT_DELETE, + ) + + def test_handle_delete_all_doc_type_when_only_lg_records_available( mock_delete_specific_doc_type, mock_deletion_service, @@ -149,15 +191,12 @@ def test_handle_delete_all_doc_type_when_only_lg_records_available( expected = TEST_LG_DOC_STORE_REFERENCES actual = mock_deletion_service.handle_reference_delete( - nhs_number, [SupportedDocumentTypes.LG, SupportedDocumentTypes.ARF] + nhs_number, [SupportedDocumentTypes.LG] ) assert expected == actual - assert mock_delete_specific_doc_type.call_count == 2 - mock_delete_specific_doc_type.assert_any_call( - nhs_number, SupportedDocumentTypes.ARF - ) + assert mock_delete_specific_doc_type.call_count == 1 mock_delete_specific_doc_type.assert_any_call(nhs_number, SupportedDocumentTypes.LG) mock_delete_unstitched_document_reference.assert_called() @@ -166,7 +205,6 @@ def test_handle_delete_all_doc_type_when_only_lg_records_available( @pytest.mark.parametrize( ["doc_type", "expected"], [ - (SupportedDocumentTypes.ARF, TEST_DOC_STORE_REFERENCES), (SupportedDocumentTypes.LG, TEST_LG_DOC_STORE_REFERENCES), ], ) @@ -193,7 +231,7 @@ def test_handle_delete_when_no_record_for_patient_return_empty_list( expected = [] actual = mock_deletion_service.handle_reference_delete( TEST_NHS_NUMBER_WITH_NO_RECORD, - [SupportedDocumentTypes.LG, SupportedDocumentTypes.ARF], + [SupportedDocumentTypes.LG], ) assert actual == expected @@ -202,7 +240,6 @@ def test_handle_delete_when_no_record_for_patient_return_empty_list( @pytest.mark.parametrize( ["doc_type", "table_name", "doc_ref"], [ - (SupportedDocumentTypes.ARF, MOCK_ARF_TABLE_NAME, TEST_DOC_STORE_REFERENCES), (SupportedDocumentTypes.LG, MOCK_LG_TABLE_NAME, TEST_LG_DOC_STORE_REFERENCES), ], ) @@ -230,7 +267,7 @@ def test_delete_specific_doc_type( @pytest.mark.parametrize( "doc_type", - [SupportedDocumentTypes.ARF, SupportedDocumentTypes.LG], + [SupportedDocumentTypes.LG], ) def test_delete_specific_doc_type_when_no_record_for_given_patient( doc_type, @@ -277,6 +314,7 @@ def test_delete_documents_references_in_stitch_table(mock_deletion_service): def test_send_sqs_message_to_remove_pointer(mocker, mock_deletion_service): mocker.patch("uuid.uuid4", return_value="test_uuid") + snomed=SnomedCodes.LLOYD_GEORGE.value expected_message_body = ( '{{"nhs_number":"{}",' '"snomed_code_doc_type":{},' @@ -284,11 +322,11 @@ def test_send_sqs_message_to_remove_pointer(mocker, mock_deletion_service): '"action":"delete"}}' ).format( TEST_NHS_NUMBER, - SnomedCodes.LLOYD_GEORGE.value.model_dump_json(), + snomed.model_dump_json(), SnomedCodes.CARE_PLAN.value.model_dump_json(), ) - mock_deletion_service.send_sqs_message_to_remove_pointer(TEST_NHS_NUMBER) + mock_deletion_service.send_sqs_message_to_remove_pointer(TEST_NHS_NUMBER, snomed) assert mock_deletion_service.sqs_service.send_message_fifo.call_count == 1 @@ -308,15 +346,6 @@ def test_delete_unstitched_document_reference_called_for_LG( mock_delete_unstitched_document_reference.assert_called() -def test_delete_unstitched_document_reference_not_called_for_ARF( - mock_deletion_service, mocker, mock_delete_unstitched_document_reference -): - mock_deletion_service.handle_reference_delete( - TEST_NHS_NUMBER, [SupportedDocumentTypes.ARF] - ) - mock_delete_unstitched_document_reference.assert_not_called() - - def test_delete_unstitched_document_reference_updates_correct_dynamo_table( mock_deletion_service, mock_fetch_documents_with_nhs_number ): @@ -399,13 +428,54 @@ def test_handle_object_delete_invalid_filepath_raises_exception( def test_handle_object_delete_DocumentService_exception_raises_exception( - mock_deletion_service, mock_delete_document_object, caplog + mock_deletion_service, mock_delete_document_object, caplog, mocker ): test_reference = MOCK_OLD_IMAGE_MODEL + test_reference.file_location = f"s3://{MOCK_BUCKET}/{TEST_FILE_KEY}" mock_delete_document_object.side_effect = DocumentServiceException() - with pytest.raises(DocumentDeletionServiceException): + with pytest.raises(DocumentDeletionServiceException) as excinfo: mock_deletion_service.handle_object_delete(test_reference) - mock_delete_document_object.assert_called_once_with( - bucket=MOCK_BUCKET, key=TEST_FILE_KEY + + mock_delete_document_object.assert_called_once_with( + bucket=MOCK_BUCKET, key=TEST_FILE_KEY + ) + + assert excinfo.value.error == LambdaError.DocDelObjectFailure + assert excinfo.value.status_code == 400 + + +def test_handle_reference_delete_single_document_not_found_raises_exception( + mock_deletion_service, mocker +): + mocker.patch.object( + mock_deletion_service.document_service, + "fetch_document_from_table", + return_value=[], + ) + + with pytest.raises(DocumentDeletionServiceException) as excinfo: + mock_deletion_service.handle_reference_delete( + "mock_nhs_number", [], "mock_document_id" + ) + + assert excinfo.value.error == LambdaError.DocDelNull + assert excinfo.value.status_code == 404 + + +def test_delete_specific_doc_type_client_error_raises_exception( + mock_deletion_service, mocker +): + mocker.patch.object( + mock_deletion_service.document_service, + "delete_document_references", + side_effect=MOCK_CLIENT_ERROR, + ) + + with pytest.raises(DocumentDeletionServiceException) as excinfo: + mock_deletion_service.delete_specific_doc_type( + "mock_nhs_number", SupportedDocumentTypes.LG ) + + assert excinfo.value.error == LambdaError.DocDelClient + assert excinfo.value.status_code == 500 diff --git a/lambdas/tests/unit/services/test_nrl_api_service.py b/lambdas/tests/unit/services/test_nrl_api_service.py index c02bd3dac4..7207e0435d 100644 --- a/lambdas/tests/unit/services/test_nrl_api_service.py +++ b/lambdas/tests/unit/services/test_nrl_api_service.py @@ -1,6 +1,9 @@ +from unittest.mock import call + import pytest from enums.snomed_codes import SnomedCodes from requests import Response +from requests.exceptions import HTTPError from services.nrl_api_service import NrlApiService from tests.unit.conftest import FAKE_URL, TEST_NHS_NUMBER from tests.unit.helpers.mock_services import FakeSSMService, FakOAuthService @@ -32,6 +35,78 @@ def mock_get_pointer(nrl_service, mocker): yield nrl_service.get_pointer +@pytest.fixture +def mock_search_url(): + yield "https://example.com/search_url" + + +@pytest.fixture +def mock_pointer_id(): + yield "222" + + +@pytest.fixture +def mock_nrl_pointer_entry(mock_search_url, mock_pointer_id): + mock_nrl_entry = { + "resource": { + "resourceType": "DocumentReference", + "id": mock_pointer_id, + }, + "content": [{"attachment": {"url": mock_search_url}}], + } + + yield mock_nrl_entry + + +@pytest.fixture +def mock_multi_doc_nrl_pointer_entry(mock_search_url, mock_pointer_id): + mock_nrl_entry = { + "resource": { + "resourceType": "DocumentReference", + "id": mock_pointer_id, + }, + "content": [ + {"attachment": {"url": "https://example.com/other_doc_url"}}, + {"attachment": {"url": "https://example.com/other_doc_url"}}, + {"attachment": {"url": mock_search_url}}, + ], + } + + yield mock_nrl_entry + + +@pytest.fixture +def mock_nrl_multiple_entry_response(mock_nrl_pointer_entry): + mock_nrl_response = { + "resourceType": "Bundle", + "type": "searchset", + "total": 3, + "entry": [ + { + "resource": { + "resourceType": "DocumentReference", + "id": "222", + }, + "content": [ + {"attachment": {"url": "https://example.com/other_mock_url"}} + ], + }, + mock_nrl_pointer_entry, + { + "resource": { + "resourceType": "DocumentReference", + "id": "222", + }, + "content": [ + {"attachment": {"url": "https://example.com/other_mock_url"}} + ], + }, + ], + } + + yield mock_nrl_response + + def test_create_new_pointer(nrl_service, mock_session_post, mock_get_pointer): mock_get_pointer.return_value = {} @@ -150,6 +225,23 @@ def test_get_pointer_with_record_type(mocker, nrl_service): ) +def test_get_pointer_without_record_type(mocker, nrl_service): + mocker.patch("uuid.uuid4", return_value="test_uuid") + mock_params = { + "subject:identifier": f"https://fhir.nhs.uk/Id/nhs-number|{TEST_NHS_NUMBER}", + } + mock_headers = { + "Authorization": f"Bearer {ACCESS_TOKEN}", + "NHSD-End-User-Organisation-ODS": "test_value_test_nrl_user_ods_ssm_key", + "Accept": "application/json", + "X-Request-ID": "test_uuid", + } + nrl_service.get_pointer(TEST_NHS_NUMBER) + nrl_service.session.get.assert_called_with( + params=mock_params, url=FAKE_URL, headers=mock_headers + ) + + def test_get_pointer_with_record_type_no_retry(mocker, nrl_service): mock_type = SnomedCodes.LLOYD_GEORGE.value mocker.patch("uuid.uuid4", return_value="test_uuid") @@ -313,3 +405,199 @@ def test_delete_pointer_not_raise_error(mock_get_pointer, nrl_service): except Exception: assert False nrl_service.session.delete.assert_called_once() + + +def test_update_pointer_success(nrl_service, mocker): + mock_pointer_id = "222" + mock_pointer = { + "resource": {"resourceType": "DocumentReference", "id": mock_pointer_id} + } + + mock_response = Response() + mock_response.status_code = 200 + mock_response._content = b"{}" + + nrl_service.session.put.return_value = mock_response + + nrl_service.update_pointer(mock_pointer, TEST_NHS_NUMBER) + + nrl_service.session.put.assert_called_once_with( + url=nrl_service.endpoint + f"/{mock_pointer_id}", + headers=nrl_service.headers, + json=mock_pointer, + ) + + +def test_update_pointer_nrl_token_expired_retries(nrl_service): + mock_pointer_id = "222" + mock_pointer = { + "resource": {"resourceType": "DocumentReference", "id": mock_pointer_id} + } + + mock_token_expired_response = Response() + mock_token_expired_response.status_code = 401 + mock_token_expired_response._content = b"{}" + mock_http_error = HTTPError(response=mock_token_expired_response) + + mock_successful_response = Response() + mock_successful_response.status_code = 200 + mock_successful_response._content = b"{}" + + nrl_service.session.put.side_effect = [mock_http_error, mock_successful_response] + + nrl_service.update_pointer(mock_pointer, TEST_NHS_NUMBER) + + assert nrl_service.session.put.call_count == 2 + + nrl_service.session.put.assert_has_calls( + [ + call( + url=nrl_service.endpoint + f"/{mock_pointer_id}", + headers=nrl_service.headers, + json=mock_pointer, + ), + call( + url=nrl_service.endpoint + f"/{mock_pointer_id}", + headers=nrl_service.headers, + json=mock_pointer, + ), + ] + ) + + +def test_update_pointer_not_raise_error(nrl_service): + response = Response() + response.status_code = 400 + response._content = b"{}" + mock_pointer_id = "222" + mock_pointer = { + "resource": {"resourceType": "DocumentReference", "id": mock_pointer_id} + } + nrl_service.session.put.return_value = response + + nrl_service.update_pointer(mock_pointer, TEST_NHS_NUMBER) + + nrl_service.session.put.assert_called_once() + + +def test_get_pointer_by_url_returns_correct_entry( + nrl_service, + mock_get_pointer, + mock_search_url, + mock_nrl_multiple_entry_response, + mock_nrl_pointer_entry, +): + mock_get_pointer.return_value = mock_nrl_multiple_entry_response + mock_type = SnomedCodes.LLOYD_GEORGE.value + + result = nrl_service.get_pointer_by_url(TEST_NHS_NUMBER, mock_type, mock_search_url) + + assert result == mock_nrl_pointer_entry + + +def test_delete_pointer_with_url_single_document_deletes_pointer( + nrl_service, mock_nrl_pointer_entry, mocker, mock_search_url +): + mocker.patch.object( + nrl_service, "get_pointer_by_url", return_value=mock_nrl_pointer_entry + ) + + mock_type = SnomedCodes.LLOYD_GEORGE.value + + mocker.patch.object(nrl_service, "update_pointer") + mocker.patch.object(nrl_service, "delete_pointer_by_id") + mocker.patch.object(nrl_service, "delete_pointer_by_record_type") + + nrl_service.delete_pointer(TEST_NHS_NUMBER, mock_type, mock_search_url) + + nrl_service.delete_pointer_by_record_type.assert_not_called() + nrl_service.update_pointer.assert_not_called() + + nrl_service.delete_pointer_by_id.assert_called_once_with( + mock_nrl_pointer_entry, TEST_NHS_NUMBER + ) + + +def test_delete_pointer_with_url_multiple_documents_deletes_pointer( + nrl_service, mock_multi_doc_nrl_pointer_entry, mocker, mock_search_url +): + mocker.patch.object( + nrl_service, "get_pointer_by_url", return_value=mock_multi_doc_nrl_pointer_entry + ) + + mock_type = SnomedCodes.LLOYD_GEORGE.value + + mocker.patch.object(nrl_service, "update_pointer") + mocker.patch.object(nrl_service, "delete_pointer_by_id") + mocker.patch.object(nrl_service, "delete_pointer_by_record_type") + + nrl_service.delete_pointer(TEST_NHS_NUMBER, mock_type, mock_search_url) + + nrl_service.delete_pointer_by_record_type.assert_not_called() + nrl_service.delete_pointer_by_id.assert_not_called() + + nrl_service.update_pointer.assert_called_once_with( + mock_multi_doc_nrl_pointer_entry, TEST_NHS_NUMBER + ) + + +def test_delete_pointer_with_url_no_pointers_found_no_error( + nrl_service, mocker, mock_search_url +): + mocker.patch.object(nrl_service, "get_pointer_by_url", return_value=None) + + mock_type = SnomedCodes.LLOYD_GEORGE.value + + mocker.patch.object(nrl_service, "update_pointer") + mocker.patch.object(nrl_service, "delete_pointer_by_id") + mocker.patch.object(nrl_service, "delete_pointer_by_record_type") + + nrl_service.delete_pointer(TEST_NHS_NUMBER, mock_type, mock_search_url) + + nrl_service.delete_pointer_by_record_type.assert_not_called() + nrl_service.delete_pointer_by_id.assert_not_called() + nrl_service.update_pointer.assert_not_called() + + +def test_delete_pointer_by_id_refresh_expired_token( + nrl_service, mock_nrl_pointer_entry, mock_pointer_id +): + mock_token_expired_response = Response() + mock_token_expired_response.status_code = 401 + mock_token_expired_response._content = b"{}" + mock_http_error = HTTPError(response=mock_token_expired_response) + + mock_successful_response = Response() + mock_successful_response.status_code = 200 + mock_successful_response._content = b"{}" + + nrl_service.session.delete.side_effect = [mock_http_error, mock_successful_response] + + nrl_service.delete_pointer_by_id(mock_nrl_pointer_entry, TEST_NHS_NUMBER) + + assert nrl_service.session.delete.call_count == 2 + + nrl_service.session.delete.assert_has_calls( + [ + call( + url=nrl_service.endpoint + f"/{mock_pointer_id}", + headers=nrl_service.headers, + ), + call( + url=nrl_service.endpoint + f"/{mock_pointer_id}", + headers=nrl_service.headers, + ), + ] + ) + + +def test_get_pointer_by_url_no_results_returns_none( + nrl_service, mock_get_pointer, mock_search_url +): + mock_get_pointer.return_result = {"entry": []} + + mock_type = SnomedCodes.LLOYD_GEORGE.value + + result = nrl_service.get_pointer_by_url(TEST_NHS_NUMBER, mock_type, mock_search_url) + + assert result is None diff --git a/lambdas/tests/unit/utils/test_document_type_utils.py b/lambdas/tests/unit/utils/test_document_type_utils.py index 864ca19825..a2de8ef5ea 100644 --- a/lambdas/tests/unit/utils/test_document_type_utils.py +++ b/lambdas/tests/unit/utils/test_document_type_utils.py @@ -67,3 +67,10 @@ def test_extract_document_type_as_enum(value, expected): actual = extract_document_type_to_enum(value) assert expected == actual + +@pytest.mark.parametrize( + "value", ["MANGO"], +) +def test_extract_invalid_document_type_throws_exception(value): + with pytest.raises(ValueError): + extract_document_type_to_enum(value) \ No newline at end of file diff --git a/lambdas/utils/common_query_filters.py b/lambdas/utils/common_query_filters.py index f9335826e4..4f9a0e2708 100644 --- a/lambdas/utils/common_query_filters.py +++ b/lambdas/utils/common_query_filters.py @@ -10,6 +10,12 @@ def get_not_deleted_filter(filter_builder: DynamoQueryFilterBuilder): delete_does_not_exist_filter_expression = filter_builder.build() return delete_filter_expression | delete_does_not_exist_filter_expression +def get_document_type_filter(filter_builder: DynamoQueryFilterBuilder, document_type: str): + filter_builder.add_condition("DocumentSnomedCodeType", AttributeOperator.EQUAL, document_type) + document_type_filter = filter_builder.build() + filter_not_deleted = get_not_deleted_filter(filter_builder) + return document_type_filter & filter_not_deleted + def get_upload_complete_filter(filter_builder: DynamoQueryFilterBuilder): filter_builder.add_condition("Uploaded", AttributeOperator.EQUAL, True) diff --git a/lambdas/utils/decorators/validate_document_type.py b/lambdas/utils/decorators/validate_document_type.py index 842477a371..fdd6b439f3 100755 --- a/lambdas/utils/decorators/validate_document_type.py +++ b/lambdas/utils/decorators/validate_document_type.py @@ -2,7 +2,7 @@ from enums.lambda_error import LambdaError from utils.audit_logging_setup import LoggingService -from utils.document_type_utils import doc_type_is_valid +from utils.document_type_utils import extract_document_type_to_enum from utils.lambda_response import ApiGatewayResponse logger = LoggingService(__name__) @@ -28,12 +28,15 @@ def interceptor(event, context): LambdaError.DocTypeNull.create_error_body(), event["httpMethod"], ).create_api_gateway_response() - if not doc_type_is_valid(doc_type): - return ApiGatewayResponse( - 400, - LambdaError.DocTypeInvalid.create_error_body(), - event["httpMethod"], - ).create_api_gateway_response() + + extract_document_type_to_enum(doc_type) + + except ValueError: + return ApiGatewayResponse( + 400, + LambdaError.DocTypeInvalid.create_error_body(), + event["httpMethod"], + ).create_api_gateway_response() except KeyError as e: logger.info({str(e)}, {"Result": "An error occurred due to missing key"}) return ApiGatewayResponse( diff --git a/lambdas/utils/document_type_utils.py b/lambdas/utils/document_type_utils.py index 98c949417a..fda6d87a24 100644 --- a/lambdas/utils/document_type_utils.py +++ b/lambdas/utils/document_type_utils.py @@ -1,18 +1,9 @@ from enums.supported_document_types import SupportedDocumentTypes - -def doc_type_is_valid(value: str) -> bool: - if not len(extract_document_type_to_enum(value)): - return False - return True - - def extract_document_type_to_enum(value: str) -> list[SupportedDocumentTypes]: received_document_types = value.replace(" ", "").split(",") converted_document_types = [] for document_type in received_document_types: - try: - converted_document_types.append(SupportedDocumentTypes(document_type)) - except ValueError: - continue + converted_document_types.append(SupportedDocumentTypes(document_type)) + return converted_document_types