From f9e225ab1a5a2c7823404999f355dc30fa6b5e79 Mon Sep 17 00:00:00 2001 From: adamwhitingnhs Date: Tue, 9 Dec 2025 12:03:21 +0000 Subject: [PATCH 01/13] [PRMP-907] Changes for delete single document --- .../DeleteSubmitStage.test.tsx | 23 +++++---- .../deleteSubmitStage/DeleteSubmitStage.tsx | 51 ++++++++++++------- .../removeRecordStage/RemoveRecordStage.tsx | 4 +- .../documentView/DocumentView.test.tsx | 10 ++-- .../documentView/DocumentView.tsx | 6 +-- .../helpers/requests/deleteAllDocuments.ts | 5 +- .../DocumentSearchResultsPage.tsx | 8 ++- 7 files changed, 62 insertions(+), 45 deletions(-) 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..f780bb52c7 100644 --- a/app/src/components/blocks/_delete/deleteSubmitStage/DeleteSubmitStage.tsx +++ b/app/src/components/blocks/_delete/deleteSubmitStage/DeleteSubmitStage.tsx @@ -8,7 +8,7 @@ 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 +23,13 @@ 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, 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 +40,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 => { @@ -66,11 +69,15 @@ const DeleteSubmitStageIndexView = ({ 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: document?.id, docType: docType, nhsNumber: nhsNumber, baseUrl, @@ -116,8 +123,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 +155,20 @@ const DeleteSubmitStageIndexView = ({ )}
- {pageTitle}: + {pageTitle()}: + {document && ( + <> +

Record type: {getConfigForDocType(document.documentSnomedCodeType)?.displayName}

+

Filename: {document.fileName}

+ + )} + {!userIsGP && ( Before removing @@ -223,6 +244,7 @@ const DeleteSubmitStageIndexView = ({ }; const DeleteSubmitStage = ({ + document, docType, setDownloadStage, resetDocState, @@ -234,21 +256,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..9bc5c7bd11 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,7 @@ const DocumentSearchResultsPage = (): React.JSX.Element => { path={getLastURLPath(routeChildren.DOCUMENT_DELETE) + '/*'} element={ { mounted.current = false; }} From c92e48bd24fd9b9a1dce05490cbeaa617630c523 Mon Sep 17 00:00:00 2001 From: Fox Maltas Date: Thu, 11 Dec 2025 11:22:55 +0000 Subject: [PATCH 02/13] [PRMP-907] Implemented delete record by ID --- lambdas/enums/lambda_error.py | 4 + .../delete_document_reference_handler.py | 16 +- .../handlers/manage_nrl_pointer_handler.py | 2 +- lambdas/services/document_deletion_service.py | 53 +++- lambdas/services/nrl_api_service.py | 137 +++++++-- .../test_delete_document_reference_handler.py | 16 + .../test_document_deletion_service.py | 83 +++++- .../unit/services/test_nrl_api_service.py | 277 ++++++++++++++++++ lambdas/utils/common_query_filters.py | 6 + 9 files changed, 549 insertions(+), 45 deletions(-) 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..0abca048a2 100644 --- a/lambdas/handlers/delete_document_reference_handler.py +++ b/lambdas/handlers/delete_document_reference_handler.py @@ -40,11 +40,25 @@ def lambda_handler(event, context): event["queryStringParameters"]["docType"] ) + document_id = event.get("queryStringParameters", {}).get("documentId", None) + + if len(document_types) > 0 and document_id: + 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 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..c8ecf9e41b 100644 --- a/lambdas/handlers/manage_nrl_pointer_handler.py +++ b/lambdas/handlers/manage_nrl_pointer_handler.py @@ -64,7 +64,7 @@ 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..e161806cc9 100644 --- a/lambdas/services/document_deletion_service.py +++ b/lambdas/services/document_deletion_service.py @@ -17,9 +17,11 @@ 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.exceptions import DocumentServiceException, DynamoServiceException from utils.lambda_exceptions import DocumentDeletionServiceException +from utils.dynamo_query_filter_builder import DynamoQueryFilterBuilder +from models.fhir.R4.fhir_document_reference import Attachment logger = LoggingService(__name__) @@ -31,15 +33,40 @@ 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]: files_deleted = [] for doc_type in doc_types: files_deleted += self.delete_specific_doc_type(nhs_number, doc_type) - self.delete_documents_references_in_stitch_table(nhs_number) + + if document_id: + doc_ref_list: DocumentReference = self.document_service.fetch_documents_from_table( + search_condition="ID", + search_key=document_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, + ) + + files_deleted.append(document_id) + + self.handle_object_delete(document_ref) + + self.send_sqs_message_to_remove_pointer(nhs_number, document_ref.file_location) + 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) @@ -78,12 +105,16 @@ 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 - def delete_documents_references_in_stitch_table(self, nhs_number: str): + def delete_documents_references_in_stitch_table(self, nhs_number: str): documents_in_stitch_table = ( self.stitch_service.query_stitch_trace_with_nhs_number(nhs_number) or [] ) @@ -125,13 +156,21 @@ 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, document_url: str = None): delete_nrl_message = NrlSqsMessage( nhs_number=nhs_number, action=NrlActionTypes.DELETE, snomed_code_doc_type=SnomedCodes.LLOYD_GEORGE.value, snomed_code_category=SnomedCodes.CARE_PLAN.value, ) + + if document_url: + attachment = Attachment( + url=document_url + ) + + 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..0f6e7e5dd3 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) @@ -74,6 +74,38 @@ def create_new_pointer( self.create_new_pointer(nhs_number, body, retry_on_expired=False) 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, @@ -114,39 +146,82 @@ def get_pointer( self.get_pointer(nhs_number, record_type, retry_on_expired=False) else: raise NrlApiException("Error while getting NRL Pointer") + + 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 - def delete_pointer(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.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..b591de949a 100644 --- a/lambdas/tests/unit/handlers/test_delete_document_reference_handler.py +++ b/lambdas/tests/unit/handlers/test_delete_document_reference_handler.py @@ -11,6 +11,7 @@ ) from utils.lambda_exceptions import DocumentDeletionServiceException from utils.lambda_response import ApiGatewayResponse +from enums.lambda_error import LambdaError TEST_DOC_STORE_REFERENCES = create_test_doc_store_refs() TEST_LG_DOC_STORE_REFERENCES = create_test_lloyd_george_doc_store_refs() @@ -253,3 +254,18 @@ 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 \ No newline at end of file diff --git a/lambdas/tests/unit/services/test_document_deletion_service.py b/lambdas/tests/unit/services/test_document_deletion_service.py index d0ffe4e1d9..cdf0b06c6a 100644 --- a/lambdas/tests/unit/services/test_document_deletion_service.py +++ b/lambdas/tests/unit/services/test_document_deletion_service.py @@ -26,6 +26,8 @@ from lambdas.tests.unit.helpers.data.test_stitch_trace import get_list_test_stitch_trace +from models.document_reference import DocumentReference + TEST_DOC_STORE_REFERENCES = create_test_doc_store_refs() TEST_LG_DOC_STORE_REFERENCES = create_test_lloyd_george_doc_store_refs() TEST_STITCH_TRACE_REFERENCES = get_list_test_stitch_trace() @@ -139,6 +141,48 @@ 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="ID", + search_key=expected_document_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, @@ -399,13 +443,42 @@ 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 \ No newline at end of file diff --git a/lambdas/tests/unit/services/test_nrl_api_service.py b/lambdas/tests/unit/services/test_nrl_api_service.py index c02bd3dac4..6e39a87581 100644 --- a/lambdas/tests/unit/services/test_nrl_api_service.py +++ b/lambdas/tests/unit/services/test_nrl_api_service.py @@ -1,10 +1,12 @@ import pytest +from unittest.mock import call from enums.snomed_codes import SnomedCodes from requests import Response 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 from utils.exceptions import NrlApiException +from requests.exceptions import HTTPError ACCESS_TOKEN = "Sr5PGv19wTEHJdDr2wx2f7IGd0cw" @@ -32,6 +34,104 @@ 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 = {} @@ -313,3 +413,180 @@ 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, + )] + ) 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) From 3b222d113ad1d8853f1a82e13010fdba68e8ad97 Mon Sep 17 00:00:00 2001 From: Fox Maltas Date: Thu, 11 Dec 2025 13:17:37 +0000 Subject: [PATCH 03/13] [PRMP-907] Added missing tests --- .../unit/services/test_nrl_api_service.py | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/lambdas/tests/unit/services/test_nrl_api_service.py b/lambdas/tests/unit/services/test_nrl_api_service.py index 6e39a87581..44d137c9a5 100644 --- a/lambdas/tests/unit/services/test_nrl_api_service.py +++ b/lambdas/tests/unit/services/test_nrl_api_service.py @@ -250,6 +250,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") @@ -590,3 +607,16 @@ def test_delete_pointer_by_id_refresh_expired_token(nrl_service, mock_nrl_pointe 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 == None + From 4e2c59e53ffc5b557d42aadbaa1f768ce6409e49 Mon Sep 17 00:00:00 2001 From: Fox Maltas Date: Thu, 11 Dec 2025 14:10:55 +0000 Subject: [PATCH 04/13] [PRMP-907] Fixed test comparison to None --- lambdas/tests/unit/services/test_nrl_api_service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lambdas/tests/unit/services/test_nrl_api_service.py b/lambdas/tests/unit/services/test_nrl_api_service.py index 44d137c9a5..ba55d3c2c3 100644 --- a/lambdas/tests/unit/services/test_nrl_api_service.py +++ b/lambdas/tests/unit/services/test_nrl_api_service.py @@ -618,5 +618,5 @@ def test_get_pointer_by_url_no_results_returns_none(nrl_service, mock_get_pointe result = nrl_service.get_pointer_by_url(TEST_NHS_NUMBER, mock_type, mock_search_url) - assert result == None + assert result is None From e187f7e6783d523d77190777bb649aacf2e29be1 Mon Sep 17 00:00:00 2001 From: Fox Maltas Date: Fri, 12 Dec 2025 14:30:57 +0000 Subject: [PATCH 05/13] [PRMP-907] Fixed bug when deleting all records as PCSE --- .../documentSearchResultsPage/DocumentSearchResultsPage.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/src/pages/documentSearchResultsPage/DocumentSearchResultsPage.tsx b/app/src/pages/documentSearchResultsPage/DocumentSearchResultsPage.tsx index 9bc5c7bd11..163d15a361 100644 --- a/app/src/pages/documentSearchResultsPage/DocumentSearchResultsPage.tsx +++ b/app/src/pages/documentSearchResultsPage/DocumentSearchResultsPage.tsx @@ -192,7 +192,8 @@ const DocumentSearchResultsPage = (): React.JSX.Element => { path={getLastURLPath(routeChildren.DOCUMENT_DELETE) + '/*'} element={ { mounted.current = false; }} From da284924dd456bc5f8239f2f66205c75c9503c74 Mon Sep 17 00:00:00 2001 From: Fox Maltas Date: Fri, 12 Dec 2025 15:22:28 +0000 Subject: [PATCH 06/13] [PRMP-907] Fixed formatting --- .../delete_document_reference_handler.py | 11 +- .../handlers/manage_nrl_pointer_handler.py | 4 +- lambdas/services/document_deletion_service.py | 56 +++--- lambdas/services/nrl_api_service.py | 31 ++- .../test_delete_document_reference_handler.py | 35 ++-- .../test_document_deletion_service.py | 39 ++-- .../unit/services/test_nrl_api_service.py | 177 ++++++++---------- 7 files changed, 190 insertions(+), 163 deletions(-) diff --git a/lambdas/handlers/delete_document_reference_handler.py b/lambdas/handlers/delete_document_reference_handler.py index 0abca048a2..980bbaca0a 100644 --- a/lambdas/handlers/delete_document_reference_handler.py +++ b/lambdas/handlers/delete_document_reference_handler.py @@ -44,21 +44,20 @@ def lambda_handler(event, context): if len(document_types) > 0 and document_id: logger.error( - LambdaError.DocDelInvalidRequest.to_str(), - {"Result": "Invalid request"} + LambdaError.DocDelInvalidRequest.to_str(), {"Result": "Invalid request"} ) return ApiGatewayResponse( - 400, - LambdaError.DocDelInvalidRequest.create_error_body(), - "DELETE" + 400, LambdaError.DocDelInvalidRequest.create_error_body(), "DELETE" ).create_api_gateway_response() request_context.patient_nhs_no = nhs_number deletion_service = DocumentDeletionService() - files_deleted = deletion_service.handle_reference_delete(nhs_number, document_types, document_id) + 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 c8ecf9e41b..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.attachment.url or None + 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 e161806cc9..d49b87b2fb 100644 --- a/lambdas/services/document_deletion_service.py +++ b/lambdas/services/document_deletion_service.py @@ -12,16 +12,16 @@ 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, get_document_type_filter +from utils.dynamo_query_filter_builder import DynamoQueryFilterBuilder from utils.exceptions import DocumentServiceException, DynamoServiceException from utils.lambda_exceptions import DocumentDeletionServiceException -from utils.dynamo_query_filter_builder import DynamoQueryFilterBuilder -from models.fhir.R4.fhir_document_reference import Attachment logger = LoggingService(__name__) @@ -33,38 +33,44 @@ def __init__(self): self.sqs_service = SQSService() def handle_reference_delete( - self, nhs_number: str, doc_types: list[SupportedDocumentTypes], document_id: str | None = None + self, + nhs_number: str, + doc_types: list[SupportedDocumentTypes], + document_id: str | None = None, ) -> list[DocumentReference]: files_deleted = [] for doc_type in doc_types: files_deleted += self.delete_specific_doc_type(nhs_number, doc_type) - if document_id: - doc_ref_list: DocumentReference = self.document_service.fetch_documents_from_table( - search_condition="ID", - search_key=document_id, - model_class=DocumentReference + doc_ref_list: DocumentReference = ( + self.document_service.fetch_documents_from_table( + search_condition="ID", + search_key=document_id, + model_class=DocumentReference, + ) ) - if (len(doc_ref_list) == 0): + 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, - ) - + table_name=self.document_service.table_name, + document_references=[document_ref], + document_ttl_days=DocumentRetentionDays.SOFT_DELETE, + ) + files_deleted.append(document_id) - + self.handle_object_delete(document_ref) - self.send_sqs_message_to_remove_pointer(nhs_number, document_ref.file_location) - + self.send_sqs_message_to_remove_pointer( + nhs_number, document_ref.file_location + ) + if SupportedDocumentTypes.LG in doc_types: self.delete_documents_references_in_stitch_table(nhs_number) self.delete_unstitched_document_reference(nhs_number) @@ -105,7 +111,9 @@ def get_documents_references_in_storage( nhs_number: str, doc_type: Literal[SupportedDocumentTypes.ARF, SupportedDocumentTypes.LG], ) -> list[DocumentReference]: - query_filter = get_document_type_filter(DynamoQueryFilterBuilder(), doc_type.value) + 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, @@ -114,7 +122,7 @@ def get_documents_references_in_storage( ) return results - def delete_documents_references_in_stitch_table(self, nhs_number: str): + def delete_documents_references_in_stitch_table(self, nhs_number: str): documents_in_stitch_table = ( self.stitch_service.query_stitch_trace_with_nhs_number(nhs_number) or [] ) @@ -156,7 +164,9 @@ def delete_specific_doc_type( ) raise DocumentDeletionServiceException(500, LambdaError.DocDelClient) - def send_sqs_message_to_remove_pointer(self, nhs_number: str, document_url: str = None): + def send_sqs_message_to_remove_pointer( + self, nhs_number: str, document_url: str = None + ): delete_nrl_message = NrlSqsMessage( nhs_number=nhs_number, action=NrlActionTypes.DELETE, @@ -165,9 +175,7 @@ def send_sqs_message_to_remove_pointer(self, nhs_number: str, document_url: str ) if document_url: - attachment = Attachment( - url=document_url - ) + attachment = Attachment(url=document_url) delete_nrl_message.attachment = attachment diff --git a/lambdas/services/nrl_api_service.py b/lambdas/services/nrl_api_service.py index 0f6e7e5dd3..41983a66c3 100644 --- a/lambdas/services/nrl_api_service.py +++ b/lambdas/services/nrl_api_service.py @@ -74,14 +74,16 @@ def create_new_pointer( self.create_new_pointer(nhs_number, body, retry_on_expired=False) 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) + 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, " @@ -105,7 +107,6 @@ def update_pointer(self, pointer: dict, nhs_number: str): 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, @@ -146,9 +147,17 @@ def get_pointer( self.get_pointer(nhs_number, record_type, retry_on_expired=False) else: raise NrlApiException("Error while getting NRL Pointer") - - 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", []) + + 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 @@ -164,15 +173,15 @@ def get_pointer_by_url(self, nhs_number: str, record_type: SnomedCode, url: str, break return entry_with_url - - def delete_pointer(self, nhs_number: str, record_type: SnomedCode = None, url: str = None): + 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) @@ -218,7 +227,9 @@ def delete_pointer_by_id(self, entry: dict, nhs_number: str): else: logger.error(f"Unable to delete NRL Pointer: {entry}") - def delete_pointer_by_record_type(self, nhs_number: str, record_type: SnomedCode = None): + 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) 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 b591de949a..d71aeb0d04 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 @@ -11,7 +12,6 @@ ) from utils.lambda_exceptions import DocumentDeletionServiceException from utils.lambda_response import ApiGatewayResponse -from enums.lambda_error import LambdaError TEST_DOC_STORE_REFERENCES = create_test_doc_store_refs() TEST_LG_DOC_STORE_REFERENCES = create_test_lloyd_george_doc_store_refs() @@ -27,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", + }, }, ], ) @@ -255,17 +267,18 @@ def test_lambda_handler_handle_lambda_exception( 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 + set_env, valid_id_and_lg_doctype_delete_event, context ): - valid_id_and_lg_doctype_delete_event["queryStringParameters"]["documentId"] = "mock_document_id" + valid_id_and_lg_doctype_delete_event["queryStringParameters"][ + "documentId" + ] = "mock_document_id" expected_result = ApiGatewayResponse( - 400, - LambdaError.DocDelInvalidRequest.create_error_body(), - "DELETE" + 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 \ No newline at end of file + 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 cdf0b06c6a..cf9e86fb05 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, @@ -26,8 +27,6 @@ from lambdas.tests.unit.helpers.data.test_stitch_trace import get_list_test_stitch_trace -from models.document_reference import DocumentReference - TEST_DOC_STORE_REFERENCES = create_test_doc_store_refs() TEST_LG_DOC_STORE_REFERENCES = create_test_lloyd_george_doc_store_refs() TEST_STITCH_TRACE_REFERENCES = get_list_test_stitch_trace() @@ -146,7 +145,7 @@ def test_handle_delete_for_doc_id( mock_deletion_service, mock_delete_documents_references_in_stitch_table, mock_delete_unstitched_document_reference, - mocker + mocker, ): expected_document_id = "mock_document_id" @@ -158,7 +157,9 @@ def test_handle_delete_for_doc_id( ) mocked_document_service = mock_deletion_service.document_service - mocked_document_service.fetch_documents_from_table.return_value=[mocked_document_reference] + 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 @@ -173,7 +174,7 @@ def test_handle_delete_for_doc_id( mocked_document_service.fetch_documents_from_table.assert_called_with( search_condition="ID", search_key=expected_document_id, - model_class=DocumentReference + model_class=DocumentReference, ) mocked_document_service.delete_document_references.assert_called_with( @@ -451,7 +452,7 @@ def test_handle_object_delete_DocumentService_exception_raises_exception( 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 ) @@ -461,24 +462,36 @@ def test_handle_object_delete_DocumentService_exception_raises_exception( def test_handle_reference_delete_single_document_not_found_raises_exception( - mock_deletion_service, mocker + mock_deletion_service, mocker ): - mocker.patch.object(mock_deletion_service.document_service, "fetch_document_from_table", return_value=[]) + 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") + 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 + mock_deletion_service, mocker ): - mocker.patch.object(mock_deletion_service.document_service, "delete_document_references", side_effect=MOCK_CLIENT_ERROR) + 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) + mock_deletion_service.delete_specific_doc_type( + "mock_nhs_number", SupportedDocumentTypes.LG + ) assert excinfo.value.error == LambdaError.DocDelClient - assert excinfo.value.status_code == 500 \ No newline at end of file + 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 ba55d3c2c3..7207e0435d 100644 --- a/lambdas/tests/unit/services/test_nrl_api_service.py +++ b/lambdas/tests/unit/services/test_nrl_api_service.py @@ -1,12 +1,13 @@ -import pytest 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 from utils.exceptions import NrlApiException -from requests.exceptions import HTTPError ACCESS_TOKEN = "Sr5PGv19wTEHJdDr2wx2f7IGd0cw" @@ -51,13 +52,7 @@ def mock_nrl_pointer_entry(mock_search_url, mock_pointer_id): "resourceType": "DocumentReference", "id": mock_pointer_id, }, - "content": [ - { - "attachment": { - "url": mock_search_url - } - } - ] + "content": [{"attachment": {"url": mock_search_url}}], } yield mock_nrl_entry @@ -71,22 +66,10 @@ def mock_multi_doc_nrl_pointer_entry(mock_search_url, mock_pointer_id): "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 - } - } - ] + {"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 @@ -105,12 +88,8 @@ def mock_nrl_multiple_entry_response(mock_nrl_pointer_entry): "id": "222", }, "content": [ - { - "attachment": { - "url": "https://example.com/other_mock_url" - } - } - ] + {"attachment": {"url": "https://example.com/other_mock_url"}} + ], }, mock_nrl_pointer_entry, { @@ -119,12 +98,8 @@ def mock_nrl_multiple_entry_response(mock_nrl_pointer_entry): "id": "222", }, "content": [ - { - "attachment": { - "url": "https://example.com/other_mock_url" - } - } - ] + {"attachment": {"url": "https://example.com/other_mock_url"}} + ], }, ], } @@ -435,10 +410,7 @@ def test_delete_pointer_not_raise_error(mock_get_pointer, nrl_service): def test_update_pointer_success(nrl_service, mocker): mock_pointer_id = "222" mock_pointer = { - "resource": { - "resourceType": "DocumentReference", - "id": mock_pointer_id - } + "resource": {"resourceType": "DocumentReference", "id": mock_pointer_id} } mock_response = Response() @@ -451,51 +423,45 @@ def test_update_pointer_success(nrl_service, mocker): nrl_service.session.put.assert_called_once_with( url=nrl_service.endpoint + f"/{mock_pointer_id}", - headers = nrl_service.headers, - json=mock_pointer + 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 - } + "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_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.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 - )] + [ + 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, + ), + ] ) @@ -505,19 +471,22 @@ def test_update_pointer_not_raise_error(nrl_service): response._content = b"{}" mock_pointer_id = "222" mock_pointer = { - "resource": { - "resourceType": "DocumentReference", - "id": mock_pointer_id - } + "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): +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 @@ -526,8 +495,12 @@ def test_get_pointer_by_url_returns_correct_entry(nrl_service, mock_get_pointer, 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) +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 @@ -540,11 +513,17 @@ def test_delete_pointer_with_url_single_document_deletes_pointer(nrl_service, mo 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) + 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) +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 @@ -557,10 +536,14 @@ def test_delete_pointer_with_url_multiple_documents_deletes_pointer(nrl_service, 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) + 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): +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 @@ -576,47 +559,45 @@ def test_delete_pointer_with_url_no_pointers_found_no_error(nrl_service, mocker, 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): +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_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.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, - )] + [ + 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": [] - } +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 - From 979cbd550c8ef03d4ed5d1ffb0a0179c991cf2a0 Mon Sep 17 00:00:00 2001 From: adamwhitingnhs Date: Mon, 15 Dec 2025 14:54:37 +0000 Subject: [PATCH 07/13] [PRMP-907] fix extraction of doc type --- lambdas/handlers/delete_document_reference_handler.py | 8 ++++---- lambdas/services/document_deletion_service.py | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lambdas/handlers/delete_document_reference_handler.py b/lambdas/handlers/delete_document_reference_handler.py index 980bbaca0a..8efa2d87ad 100644 --- a/lambdas/handlers/delete_document_reference_handler.py +++ b/lambdas/handlers/delete_document_reference_handler.py @@ -36,13 +36,13 @@ 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 = extract_document_type_to_enum(document_types_query) if document_types_query else [] document_id = event.get("queryStringParameters", {}).get("documentId", None) - if len(document_types) > 0 and document_id: + 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"} ) diff --git a/lambdas/services/document_deletion_service.py b/lambdas/services/document_deletion_service.py index d49b87b2fb..dec310c6c5 100644 --- a/lambdas/services/document_deletion_service.py +++ b/lambdas/services/document_deletion_service.py @@ -46,8 +46,8 @@ def handle_reference_delete( if document_id: doc_ref_list: DocumentReference = ( self.document_service.fetch_documents_from_table( - search_condition="ID", - search_key=document_id, + search_condition=document_id, + search_key="ID", model_class=DocumentReference, ) ) From ff271ef2b17e4538818726f4f5d683d416f47453 Mon Sep 17 00:00:00 2001 From: adamwhitingnhs Date: Mon, 15 Dec 2025 14:58:06 +0000 Subject: [PATCH 08/13] [PRMP-907] fix tests --- lambdas/tests/unit/services/test_document_deletion_service.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lambdas/tests/unit/services/test_document_deletion_service.py b/lambdas/tests/unit/services/test_document_deletion_service.py index cf9e86fb05..1b492db661 100644 --- a/lambdas/tests/unit/services/test_document_deletion_service.py +++ b/lambdas/tests/unit/services/test_document_deletion_service.py @@ -172,8 +172,8 @@ def test_handle_delete_for_doc_id( mock_delete_documents_references_in_stitch_table.assert_not_called() mocked_document_service.fetch_documents_from_table.assert_called_with( - search_condition="ID", - search_key=expected_document_id, + search_condition=expected_document_id, + search_key="ID", model_class=DocumentReference, ) From 2918f1532859f63db0cfae11b4873977d35e1119 Mon Sep 17 00:00:00 2001 From: adamwhitingnhs Date: Mon, 15 Dec 2025 15:17:24 +0000 Subject: [PATCH 09/13] [PRMP-907] remove document type decorator --- .../delete_document_reference_handler.py | 12 ++++++++++-- .../test_delete_document_reference_handler.py | 18 ++++++++++-------- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/lambdas/handlers/delete_document_reference_handler.py b/lambdas/handlers/delete_document_reference_handler.py index 8efa2d87ad..263153dfcc 100644 --- a/lambdas/handlers/delete_document_reference_handler.py +++ b/lambdas/handlers/delete_document_reference_handler.py @@ -20,7 +20,6 @@ @set_request_context_for_logging @validate_patient_id -@validate_document_type @ensure_environment_variables( names=[ "DOCUMENT_STORE_DYNAMODB_NAME", @@ -38,7 +37,16 @@ def lambda_handler(event, context): nhs_number = extract_nhs_number_from_event(event) document_types_query = event.get("queryStringParameters", {}).get("docType", None) - document_types = extract_document_type_to_enum(document_types_query) if document_types_query else [] + 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) 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 d71aeb0d04..78546a7c10 100644 --- a/lambdas/tests/unit/handlers/test_delete_document_reference_handler.py +++ b/lambdas/tests/unit/handlers/test_delete_document_reference_handler.py @@ -3,6 +3,7 @@ import pytest from botocore.exceptions import ClientError from enums.lambda_error import LambdaError +from enums.supported_document_types import SupportedDocumentTypes from handlers.delete_document_reference_handler import lambda_handler from services.document_deletion_service import DocumentDeletionService from tests.unit.conftest import MockError @@ -171,37 +172,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", + "err_code": "VDT_4003", "interaction_id": "88888888-4444-4444-4444-121212121212", } ) expected = ApiGatewayResponse( 400, expected_body, "GET" ).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", + "err_code": "VDT_4002", "interaction_id": "88888888-4444-4444-4444-121212121212", } ) expected = ApiGatewayResponse( 400, expected_body, "GET" ).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 From 15a4af7b854c311646c99f7172c61898b518e2cc Mon Sep 17 00:00:00 2001 From: adamwhitingnhs Date: Mon, 15 Dec 2025 15:43:44 +0000 Subject: [PATCH 10/13] [PRMP-907] refactor tests --- .../test_delete_document_reference_handler.py | 10 +++++----- .../unit/utils/test_document_type_utils.py | 7 +++++++ .../utils/decorators/validate_document_type.py | 17 ++++++++++------- lambdas/utils/document_type_utils.py | 13 ++----------- 4 files changed, 24 insertions(+), 23 deletions(-) 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 78546a7c10..08ae6c06ec 100644 --- a/lambdas/tests/unit/handlers/test_delete_document_reference_handler.py +++ b/lambdas/tests/unit/handlers/test_delete_document_reference_handler.py @@ -177,13 +177,13 @@ def test_lambda_handler_returns_400_when_doc_type_and_doc_id_not_supplied( ): expected_body = json.dumps( { - "message": "An error occurred due to missing key", - "err_code": "VDT_4003", + "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(valid_id_event_with_auth_header, context) assert expected == actual @@ -195,13 +195,13 @@ def test_lambda_handler_returns_400_when_doc_type_is_invalid( expected_body = json.dumps( { - "message": "An error occurred due to missing key", + "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_and_invalid_doctype_event, context) assert expected == actual 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/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 From ccc1bccb75381a95ef2c9176cb7f04a6326d7d60 Mon Sep 17 00:00:00 2001 From: adamwhitingnhs Date: Mon, 15 Dec 2025 15:58:54 +0000 Subject: [PATCH 11/13] [PRMP-907] run prettier --- .../deleteSubmitStage/DeleteSubmitStage.tsx | 23 ++++++++++++++----- .../removeRecordStage/RemoveRecordStage.tsx | 7 +++++- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/app/src/components/blocks/_delete/deleteSubmitStage/DeleteSubmitStage.tsx b/app/src/components/blocks/_delete/deleteSubmitStage/DeleteSubmitStage.tsx index f780bb52c7..e658369f37 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 { DocumentReference, 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,7 +26,11 @@ 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, getConfigForDocType, getDocumentTypeLabel } from '../../../../helpers/utils/documentType'; +import { + DOCUMENT_TYPE, + getConfigForDocType, + getDocumentTypeLabel, +} from '../../../../helpers/utils/documentType'; import { getLastURLPath } from '../../../../helpers/utils/urlManipulations'; import DeleteResultStage from '../deleteResultStage/DeleteResultStage'; @@ -69,9 +76,10 @@ export const DeleteSubmitStageIndexView = ({ const onSuccess = (): void => { resetDocState(); setDeletionStage(SUBMISSION_STATE.SUCCEEDED); - navigate(config.featureFlags.uploadDocumentIteration3Enabled - ? routeChildren.DOCUMENT_DELETE_COMPLETE - : routeChildren.LLOYD_GEORGE_DELETE_COMPLETE + navigate( + config.featureFlags.uploadDocumentIteration3Enabled + ? routeChildren.DOCUMENT_DELETE_COMPLETE + : routeChildren.LLOYD_GEORGE_DELETE_COMPLETE, ); }; try { @@ -164,7 +172,10 @@ export const DeleteSubmitStageIndexView = ({ {document && ( <> -

Record type: {getConfigForDocType(document.documentSnomedCodeType)?.displayName}

+

+ Record type:{' '} + {getConfigForDocType(document.documentSnomedCodeType)?.displayName} +

Filename: {document.fileName}

)} diff --git a/app/src/components/blocks/_delete/removeRecordStage/RemoveRecordStage.tsx b/app/src/components/blocks/_delete/removeRecordStage/RemoveRecordStage.tsx index b256f56d7e..654abd5c38 100644 --- a/app/src/components/blocks/_delete/removeRecordStage/RemoveRecordStage.tsx +++ b/app/src/components/blocks/_delete/removeRecordStage/RemoveRecordStage.tsx @@ -204,7 +204,12 @@ const RemoveRecordStage = ({ } + element={ + + } > Date: Tue, 16 Dec 2025 10:05:47 +0000 Subject: [PATCH 12/13] [PRMP-907] Refactored functions and fixed formatting --- .../delete_document_reference_handler.py | 19 +++++-- lambdas/services/document_deletion_service.py | 53 ++++++++++--------- .../test_delete_document_reference_handler.py | 3 +- 3 files changed, 46 insertions(+), 29 deletions(-) diff --git a/lambdas/handlers/delete_document_reference_handler.py b/lambdas/handlers/delete_document_reference_handler.py index 263153dfcc..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,6 +14,8 @@ from utils.lambda_response import ApiGatewayResponse from utils.request_context import request_context +from lambdas.enums.supported_document_types import SupportedDocumentTypes + logger = LoggingService(__name__) @@ -39,7 +40,11 @@ def lambda_handler(event, context): document_types = [] try: - document_types = extract_document_type_to_enum(document_types_query) if document_types_query else [] + 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"} @@ -50,7 +55,9 @@ def lambda_handler(event, context): 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): + 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"} ) @@ -61,6 +68,12 @@ def lambda_handler(event, context): 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( diff --git a/lambdas/services/document_deletion_service.py b/lambdas/services/document_deletion_service.py index dec310c6c5..cceaf451c0 100644 --- a/lambdas/services/document_deletion_service.py +++ b/lambdas/services/document_deletion_service.py @@ -38,38 +38,43 @@ def handle_reference_delete( doc_types: list[SupportedDocumentTypes], document_id: str | None = None, ) -> list[DocumentReference]: - files_deleted = [] - - for doc_type in doc_types: - files_deleted += self.delete_specific_doc_type(nhs_number, doc_type) - if document_id: - doc_ref_list: DocumentReference = ( - self.document_service.fetch_documents_from_table( - search_condition=document_id, - search_key="ID", - model_class=DocumentReference, - ) + 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) + if len(doc_ref_list) == 0: + raise DocumentDeletionServiceException(404, LambdaError.DocDelNull) - document_ref: DocumentReference = doc_ref_list[0] + 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.document_service.delete_document_references( + table_name=self.document_service.table_name, + document_references=[document_ref], + document_ttl_days=DocumentRetentionDays.SOFT_DELETE, + ) - files_deleted.append(document_id) + self.handle_object_delete(document_ref) - self.handle_object_delete(document_ref) + self.send_sqs_message_to_remove_pointer(nhs_number, document_ref.file_location) - self.send_sqs_message_to_remove_pointer( - nhs_number, document_ref.file_location - ) + def delete_documents_by_types( + self, nhs_number: str, doc_types: list[SupportedDocumentTypes] + ): + files_deleted = [] + + for doc_type in doc_types: + files_deleted += self.delete_specific_doc_type(nhs_number, doc_type) if SupportedDocumentTypes.LG in doc_types: self.delete_documents_references_in_stitch_table(nhs_number) 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 08ae6c06ec..14c05f8d6e 100644 --- a/lambdas/tests/unit/handlers/test_delete_document_reference_handler.py +++ b/lambdas/tests/unit/handlers/test_delete_document_reference_handler.py @@ -3,7 +3,6 @@ import pytest from botocore.exceptions import ClientError from enums.lambda_error import LambdaError -from enums.supported_document_types import SupportedDocumentTypes from handlers.delete_document_reference_handler import lambda_handler from services.document_deletion_service import DocumentDeletionService from tests.unit.conftest import MockError @@ -192,7 +191,7 @@ def test_lambda_handler_returns_400_when_doc_type_and_doc_id_not_supplied( 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": "Invalid document type requested", From 7c8f5f57ff377a208eb069a8e944b67a06a04420 Mon Sep 17 00:00:00 2001 From: adamwhitingnhs Date: Tue, 16 Dec 2025 12:19:24 +0000 Subject: [PATCH 13/13] [PRMP-907] Adjust delete service to remove pointer for non LG --- .../deleteSubmitStage/DeleteSubmitStage.tsx | 11 ++++-- lambdas/services/document_deletion_service.py | 27 ++++++++------ .../test_document_deletion_service.py | 36 ++++++------------- 3 files changed, 35 insertions(+), 39 deletions(-) diff --git a/app/src/components/blocks/_delete/deleteSubmitStage/DeleteSubmitStage.tsx b/app/src/components/blocks/_delete/deleteSubmitStage/DeleteSubmitStage.tsx index e658369f37..dc6cb66f36 100644 --- a/app/src/components/blocks/_delete/deleteSubmitStage/DeleteSubmitStage.tsx +++ b/app/src/components/blocks/_delete/deleteSubmitStage/DeleteSubmitStage.tsx @@ -28,6 +28,7 @@ import BackButton from '../../../generic/backButton/BackButton'; import PatientSummary, { PatientInfo } from '../../../generic/patientSummary/PatientSummary'; import { DOCUMENT_TYPE, + DOCUMENT_TYPE_CONFIG, getConfigForDocType, getDocumentTypeLabel, } from '../../../../helpers/utils/documentType'; @@ -72,6 +73,12 @@ export 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(); @@ -85,7 +92,7 @@ export const DeleteSubmitStageIndexView = ({ try { setDeletionStage(SUBMISSION_STATE.PENDING); const response: DeleteResponse = await deleteAllDocuments({ - documentId: document?.id, + documentId: !documentConfig?.singleDocumentOnly ? document?.id : undefined, docType: docType, nhsNumber: nhsNumber, baseUrl, @@ -174,7 +181,7 @@ export const DeleteSubmitStageIndexView = ({ <>

Record type:{' '} - {getConfigForDocType(document.documentSnomedCodeType)?.displayName} + {documentConfig?.displayName}

Filename: {document.fileName}

diff --git a/lambdas/services/document_deletion_service.py b/lambdas/services/document_deletion_service.py index cceaf451c0..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,7 +8,7 @@ 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 @@ -64,9 +64,11 @@ def delete_document_by_id(self, nhs_number: str, document_id: str): document_ttl_days=DocumentRetentionDays.SOFT_DELETE, ) - self.handle_object_delete(document_ref) - - self.send_sqs_message_to_remove_pointer(nhs_number, document_ref.file_location) + 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] @@ -74,12 +76,13 @@ def delete_documents_by_types( 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) 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 @@ -170,18 +173,20 @@ def delete_specific_doc_type( raise DocumentDeletionServiceException(500, LambdaError.DocDelClient) def send_sqs_message_to_remove_pointer( - self, nhs_number: str, document_url: str = None + 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 document_url: - attachment = Attachment(url=document_url) - + if doc_ref: + attachment = Attachment(url=doc_ref.file_location) delete_nrl_message.attachment = attachment sqs_group_id = f"NRL_delete_{uuid.uuid4()}" diff --git a/lambdas/tests/unit/services/test_document_deletion_service.py b/lambdas/tests/unit/services/test_document_deletion_service.py index 1b492db661..d086acaac8 100644 --- a/lambdas/tests/unit/services/test_document_deletion_service.py +++ b/lambdas/tests/unit/services/test_document_deletion_service.py @@ -121,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 ) @@ -194,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() @@ -211,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), ], ) @@ -238,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 @@ -247,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), ], ) @@ -275,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, @@ -322,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":{},' @@ -329,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 @@ -353,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 ):