From c9304b0dd59adddf48b6e4b9772113d1d2d5a459 Mon Sep 17 00:00:00 2001 From: steph-torres-nhs Date: Thu, 16 Jan 2025 09:17:34 +0000 Subject: [PATCH 1/4] [PRMP-1322] Update content of Cancel link on LG Download stage (#509) Change content from "Cancel" to "Cancel and return to patient record" --- .../lloydGeorgeDownloadStage/LloydGeorgeDownloadStage.test.tsx | 3 +++ .../lloydGeorgeDownloadStage/LloydGeorgeDownloadStage.tsx | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/app/src/components/blocks/_lloydGeorge/lloydGeorgeDownloadStage/LloydGeorgeDownloadStage.test.tsx b/app/src/components/blocks/_lloydGeorge/lloydGeorgeDownloadStage/LloydGeorgeDownloadStage.test.tsx index 92712f6d7..382a1e399 100644 --- a/app/src/components/blocks/_lloydGeorge/lloydGeorgeDownloadStage/LloydGeorgeDownloadStage.test.tsx +++ b/app/src/components/blocks/_lloydGeorge/lloydGeorgeDownloadStage/LloydGeorgeDownloadStage.test.tsx @@ -79,6 +79,9 @@ describe('LloydGeorgeDownloadStage', () => { const expectedTestId = 'download-file-header-' + mockPdf.numberOfFiles + '-files'; expect(screen.getByTestId(expectedTestId)).toBeInTheDocument(); + expect(screen.getByTestId('cancel-download-link')).toHaveTextContent( + 'Cancel and return to patient record', + ); }); it('renders a progress bar', () => { diff --git a/app/src/components/blocks/_lloydGeorge/lloydGeorgeDownloadStage/LloydGeorgeDownloadStage.tsx b/app/src/components/blocks/_lloydGeorge/lloydGeorgeDownloadStage/LloydGeorgeDownloadStage.tsx index 5e8da201e..910acb237 100644 --- a/app/src/components/blocks/_lloydGeorge/lloydGeorgeDownloadStage/LloydGeorgeDownloadStage.tsx +++ b/app/src/components/blocks/_lloydGeorge/lloydGeorgeDownloadStage/LloydGeorgeDownloadStage.tsx @@ -193,7 +193,7 @@ function LloydGeorgeDownloadStage({ navigate(routes.LLOYD_GEORGE); }} > - Cancel + Cancel and return to patient record From d5d44069abc12ba81d4302fd425a1fc612dffd09 Mon Sep 17 00:00:00 2001 From: abid-nhs Date: Thu, 16 Jan 2025 09:37:46 +0000 Subject: [PATCH 2/4] PRMP-1292: Download LG Record Screen | Add file size column (#493) * adds fileSize to document reference * PRMP-1292 - comment out as its breaking tests * PRMP-1292 - ensure files table includes a file size information such as file size measure units * PRMP-1292 - lower case size and fix test * [PRMP-1292] Updated Document Search to include file size in response as well as update tests * PRMP-1292 fix unhandled error with invalid argument name * PRMP-1292 fix broken cypress tests * increase test coverage: missing formatFileSize test --------- Co-authored-by: Jack Sutton Co-authored-by: Ollie Beumkes Co-authored-by: mark start --- .../download_lloyd_george_workflow.cy.js | 4 ++ .../LloydGeorgeSelectSearchResults.test.tsx | 15 ++++++- .../LloydGeorgeSelectSearchResults.tsx | 8 ++++ app/src/helpers/test/testBuilders.ts | 1 + app/src/helpers/utils/formatFileSize.test.tsx | 41 +++++++++++++++++++ app/src/types/generic/searchResult.ts | 1 + .../document_reference_search_service.py | 19 +++++++-- .../test_document_reference_search_service.py | 8 +++- 8 files changed, 90 insertions(+), 7 deletions(-) create mode 100644 app/src/helpers/utils/formatFileSize.test.tsx diff --git a/app/cypress/e2e/0-ndr-core-tests/gp_user_workflows/download_lloyd_george_workflow.cy.js b/app/cypress/e2e/0-ndr-core-tests/gp_user_workflows/download_lloyd_george_workflow.cy.js index 8aa60367b..051463d05 100644 --- a/app/cypress/e2e/0-ndr-core-tests/gp_user_workflows/download_lloyd_george_workflow.cy.js +++ b/app/cypress/e2e/0-ndr-core-tests/gp_user_workflows/download_lloyd_george_workflow.cy.js @@ -19,18 +19,21 @@ const testFiles = [ created: '2024-05-07T14:52:00.827602Z', virusScannerResult: 'Clean', id: 'test-id', + fileSize: 200, }, { fileName: '2of2_testy_test.pdf', created: '2024-05-07T14:52:00.827602Z', virusScannerResult: 'Clean', id: 'test-id-2', + fileSize: 200, }, { fileName: '1of1_lone_test_file.pdf', created: '2024-01-01T14:52:00.827602Z', virusScannerResult: 'Clean', id: 'test-id-3', + fileSize: 200, }, ]; @@ -40,6 +43,7 @@ const singleTestFile = [ created: '2024-01-01T14:52:00.827602Z', virusScannerResult: 'Clean', id: 'test-id-3', + fileSize: 200, }, ]; diff --git a/app/src/components/blocks/_lloydGeorge/lloydGeorgeSelectSearchResults/LloydGeorgeSelectSearchResults.test.tsx b/app/src/components/blocks/_lloydGeorge/lloydGeorgeSelectSearchResults/LloydGeorgeSelectSearchResults.test.tsx index 877a70902..048ae2d04 100644 --- a/app/src/components/blocks/_lloydGeorge/lloydGeorgeSelectSearchResults/LloydGeorgeSelectSearchResults.test.tsx +++ b/app/src/components/blocks/_lloydGeorge/lloydGeorgeSelectSearchResults/LloydGeorgeSelectSearchResults.test.tsx @@ -5,7 +5,6 @@ import usePatient from '../../../../helpers/hooks/usePatient'; import { LinkProps } from 'react-router-dom'; import LloydGeorgeSelectSearchResults, { Props } from './LloydGeorgeSelectSearchResults'; import userEvent from '@testing-library/user-event'; -import { routes } from '../../../../types/generic/routes'; import { SEARCH_AND_DOWNLOAD_STATE } from '../../../../types/pages/documentSearchResultsPage/types'; import { runAxeTest } from '../../../../helpers/test/axeTestHelper'; @@ -57,6 +56,20 @@ describe('LloydGeorgeSelectSearchResults', () => { expect(screen.getByTestId('toggle-selection-btn')).toBeInTheDocument(); }); + it('renders the correct table headers', () => { + renderComponent({ selectedDocuments: mockSelectedDocuments }); + + const headers = screen.getAllByRole('columnheader'); + const expectedHeaders = ['Selected', 'Filename', 'Upload date', 'File size']; + + expectedHeaders.forEach((headerText, index) => { + expect(headers[index]).toHaveTextContent(headerText); + }); + + const filesTable = screen.getByTestId('available-files-table-title'); + expect(filesTable).toHaveTextContent(/bytes|KB|MB|GB/); + }); + it('shows error box when download selected files button is clicked but no files selected', async () => { renderComponent({ selectedDocuments: [] }); diff --git a/app/src/components/blocks/_lloydGeorge/lloydGeorgeSelectSearchResults/LloydGeorgeSelectSearchResults.tsx b/app/src/components/blocks/_lloydGeorge/lloydGeorgeSelectSearchResults/LloydGeorgeSelectSearchResults.tsx index 8a7da7ecc..8a009b5c8 100644 --- a/app/src/components/blocks/_lloydGeorge/lloydGeorgeSelectSearchResults/LloydGeorgeSelectSearchResults.tsx +++ b/app/src/components/blocks/_lloydGeorge/lloydGeorgeSelectSearchResults/LloydGeorgeSelectSearchResults.tsx @@ -8,6 +8,7 @@ import { SEARCH_AND_DOWNLOAD_STATE } from '../../../../types/pages/documentSearc import ErrorBox from '../../../layout/errorBox/ErrorBox'; import PatientSummary from '../../../generic/patientSummary/PatientSummary'; import BackButton from '../../../generic/backButton/BackButton'; +import formatFileSize from '../../../../helpers/utils/formatFileSize'; export type Props = { searchResults: Array; @@ -108,6 +109,7 @@ const AvailableFilesTable = ({ )} Filename Upload date + File size @@ -147,6 +149,12 @@ const AvailableFilesTable = ({ > {getFormattedDatetime(new Date(result.created))} + + {formatFileSize(result.fileSize)} + ))} diff --git a/app/src/helpers/test/testBuilders.ts b/app/src/helpers/test/testBuilders.ts index b73100c67..53523f93c 100644 --- a/app/src/helpers/test/testBuilders.ts +++ b/app/src/helpers/test/testBuilders.ts @@ -123,6 +123,7 @@ const buildSearchResult = (searchResultOverride?: Partial) => { created: moment().format(), virusScannerResult: 'Clean', ID: '1234qwer-241ewewr', + fileSize: 224, ...searchResultOverride, }; return result; diff --git a/app/src/helpers/utils/formatFileSize.test.tsx b/app/src/helpers/utils/formatFileSize.test.tsx new file mode 100644 index 000000000..748d1e0a0 --- /dev/null +++ b/app/src/helpers/utils/formatFileSize.test.tsx @@ -0,0 +1,41 @@ +import formatFileSize from "./formatFileSize"; + +describe('formatFileSize', () => { + + it('returns rounded file size formats for valid inputs', () => { + + expect(formatFileSize(0)).toBe('0 bytes'); + expect(formatFileSize(-0)).toBe('0 bytes'); + expect(formatFileSize(1)).toBe('1 bytes'); + expect(formatFileSize(1.5)).toBe('2 bytes'); + + expect(formatFileSize(1023)).toBe('1023 bytes'); + expect(formatFileSize(1024)).toBe('1 KB'); + expect(formatFileSize(1025)).toBe('1 KB'); + + expect(formatFileSize(1535)).toBe('1 KB'); + expect(formatFileSize(1536)).toBe('2 KB'); + expect(formatFileSize(2048)).toBe('2 KB'); + + expect(formatFileSize(Math.pow(2, 20) - 1)).toBe('1024 KB'); + expect(formatFileSize(Math.pow(2, 20))).toBe('1 MB'); + expect(formatFileSize(Math.pow(2, 20) + 1)).toBe('1 MB'); + + expect(formatFileSize(Math.pow(2, 30) - 1)).toBe('1024 MB'); + expect(formatFileSize(Math.pow(2, 30))).toBe('1 GB'); + expect(formatFileSize(Math.pow(2, 30) + 1)).toBe('1 GB'); + + }); + + it('throws "Invalid file size" exception for invalid inputs', () => { + + expect(() => formatFileSize(Number.MIN_SAFE_INTEGER)).toThrow('Invalid file size'); + expect(() => formatFileSize(-1)).toThrow('Invalid file size'); + expect(() => formatFileSize(NaN)).toThrow('Invalid file size'); + expect(() => formatFileSize(undefined as unknown as number)).toThrow('Invalid file size'); + expect(() => formatFileSize(Math.pow(2, 40))).toThrow('Invalid file size'); // 1TB + expect(() => formatFileSize(Number.MAX_SAFE_INTEGER)).toThrow('Invalid file size'); + + }); + +}); \ No newline at end of file diff --git a/app/src/types/generic/searchResult.ts b/app/src/types/generic/searchResult.ts index 769d93871..9225ba18e 100644 --- a/app/src/types/generic/searchResult.ts +++ b/app/src/types/generic/searchResult.ts @@ -3,4 +3,5 @@ export type SearchResult = { created: string; virusScannerResult: string; ID: string; + fileSize: number; }; diff --git a/lambdas/services/document_reference_search_service.py b/lambdas/services/document_reference_search_service.py index 8f1f4c2bd..d3d2fb3d9 100644 --- a/lambdas/services/document_reference_search_service.py +++ b/lambdas/services/document_reference_search_service.py @@ -49,10 +49,21 @@ def get_document_references(self, nhs_number: str): 423, LambdaError.UploadInProgressError ) results.extend( - document.model_dump( - include={"file_name", "created", "virus_scanner_result", "id"}, - by_alias=True, - ) + { + **document.model_dump( + include={ + "file_name", + "created", + "virus_scanner_result", + "id", + }, + by_alias=True, + ), + "fileSize": self.s3_service.get_file_size( + s3_bucket_name=document.get_file_bucket(), + object_key=document.get_file_key(), + ), + } for document in documents ) return results diff --git a/lambdas/tests/unit/services/test_document_reference_search_service.py b/lambdas/tests/unit/services/test_document_reference_search_service.py index 667fd3c9e..10ff8fdae 100644 --- a/lambdas/tests/unit/services/test_document_reference_search_service.py +++ b/lambdas/tests/unit/services/test_document_reference_search_service.py @@ -12,18 +12,22 @@ DocumentReference.model_validate(MOCK_SEARCH_RESPONSE["Items"][0]) ] +MOCK_FILE_SIZE = 24000 + EXPECTED_RESPONSE = { "created": "2024-01-01T12:00:00.000Z", "fileName": "document.csv", "virusScannerResult": "Clean", "ID": "3d8683b9-1665-40d2-8499-6e8302d507ff", + "fileSize": MOCK_FILE_SIZE, } @pytest.fixture def patched_service(mocker, set_env): service = DocumentReferenceSearchService() - mocker.patch.object(service, "s3_service") + mock_s3_service = mocker.patch.object(service, "s3_service") + mocker.patch.object(mock_s3_service, "get_file_size", return_value=MOCK_FILE_SIZE) mocker.patch.object(service, "dynamo_service") mocker.patch.object(service, "fetch_documents_from_table_with_filter") mocker.patch.object(service, "is_upload_in_process", return_value=False) @@ -83,10 +87,10 @@ def test_get_document_references_dynamo_return_successful_response_single_table( patched_service, monkeypatch ): monkeypatch.setenv("DYNAMODB_TABLE_LIST", json.dumps(["test_table"])) - patched_service.fetch_documents_from_table_with_filter.return_value = ( MOCK_DOCUMENT_REFERENCE ) + expected_results = [EXPECTED_RESPONSE] actual = patched_service.get_document_references("1111111111") From f6b1733e2d12e8bc83879a3164321a316f366043 Mon Sep 17 00:00:00 2001 From: abid-nhs Date: Fri, 17 Jan 2025 14:03:38 +0000 Subject: [PATCH 3/4] PRMP-1288 - change specified h1 tags to h2 (#507) --- app/src/pages/privacyPage/PrivacyPage.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/pages/privacyPage/PrivacyPage.tsx b/app/src/pages/privacyPage/PrivacyPage.tsx index ce3213d3c..a53e2b61c 100644 --- a/app/src/pages/privacyPage/PrivacyPage.tsx +++ b/app/src/pages/privacyPage/PrivacyPage.tsx @@ -91,7 +91,7 @@ function PrivacyPage() {
-

Our permission to process and store patient data

+

Our permission to process and store patient data

This service has legal permission to process and store patient data through the National Data Processing Deed. @@ -137,7 +137,7 @@ function PrivacyPage() {

-

Contact us

+

Contact us

If you have any questions about the National Data Processing Deed, or our privacy policy, you can contact the team on{' '} From 1caa1ad611ed8b5e3f1e585d705893f1491c6a75 Mon Sep 17 00:00:00 2001 From: steph-torres-nhs Date: Fri, 17 Jan 2025 14:30:44 +0000 Subject: [PATCH 4/4] [PRMP-1257] Add log messages to adhere to audit requirements (#479) Add log messages to adhere to audit requirements --------- Co-authored-by: robg-nhs --- .../handlers/manage_nrl_pointer_handler.py | 10 ++++++ lambdas/models/nrl_fhir_document_reference.py | 18 ++++++++-- lambdas/models/nrl_sqs_message.py | 16 +++++---- lambdas/services/bulk_upload_service.py | 1 + lambdas/services/nrl_api_service.py | 33 ++++++++++++++++++- .../unit/services/test_nrl_api_service.py | 1 + 6 files changed, 70 insertions(+), 9 deletions(-) diff --git a/lambdas/handlers/manage_nrl_pointer_handler.py b/lambdas/handlers/manage_nrl_pointer_handler.py index cc0c24e27..4b8525113 100644 --- a/lambdas/handlers/manage_nrl_pointer_handler.py +++ b/lambdas/handlers/manage_nrl_pointer_handler.py @@ -1,4 +1,5 @@ import json +from datetime import datetime from enums.nrl_sqs_upload import NrlActionTypes from models.nrl_fhir_document_reference import FhirDocumentReference @@ -58,7 +59,16 @@ def lambda_handler(event, context): .json() ) + logger.info( + f"Create pointer request: Body: {json.loads(document)}, " + f"RequestURL: {nrl_api_service.endpoint}, " + "HTTP Verb: POST, " + f"NHS Number: {nrl_message.nhs_number}, " + f"ODS Code: {nrl_api_service.end_user_ods_code}, " + f"Datetime: {int(datetime.now().timestamp())} " + ) nrl_api_service.create_new_pointer(json.loads(document)) + case NrlActionTypes.DELETE: nrl_api_service.delete_pointer( nrl_message.nhs_number, nrl_message.snomed_code_doc_type diff --git a/lambdas/models/nrl_fhir_document_reference.py b/lambdas/models/nrl_fhir_document_reference.py index 196d03ecf..291744ac5 100644 --- a/lambdas/models/nrl_fhir_document_reference.py +++ b/lambdas/models/nrl_fhir_document_reference.py @@ -66,13 +66,27 @@ def build_fhir_dict(self): "content": [ { "attachment": self.attachment.model_dump( - by_alias=True, exclude_none=True, exclude_defaults=True + by_alias=True, exclude_none=True ), "format": { "system": "https://fhir.nhs.uk/England/CodeSystem/England-NRLFormatCode", "code": "urn:nhs-ic:unstructured", "display": "Unstructured document", }, + "extension": [ + { + "url": "https://fhir.nhs.uk/England/StructureDefinition/Extension-England-ContentStability", + "valueCodeableConcept": { + "coding": [ + { + "system": "https://fhir.nhs.uk/England/CodeSystem/England-NRLContentStability", + "code": "static", + "display": "Static", + } + ] + }, + } + ], } ], "context": { @@ -84,7 +98,7 @@ def build_fhir_dict(self): "display": self.snomed_code_practice_setting.display_name, } ] - } + }, }, } return DocumentReference(**structure_json) diff --git a/lambdas/models/nrl_sqs_message.py b/lambdas/models/nrl_sqs_message.py index aa3b9dbdd..e6cbbd315 100644 --- a/lambdas/models/nrl_sqs_message.py +++ b/lambdas/models/nrl_sqs_message.py @@ -6,13 +6,17 @@ class NrlAttachment(BaseModel): + model_config = ConfigDict( + alias_generator=AliasGenerator(serialization_alias=to_camel), + use_enum_values=True, + ) content_type: str = "application/pdf" language: str = "en-UK" - url: str = "" - size: int = 0 - hash: str = "" - title: str = "" - creation: str = "" + url: Optional[str] = None + size: Optional[int] = None + hash: Optional[str] = None + title: Optional[str] = None + creation: Optional[str] = None class NrlSqsMessage(BaseModel): @@ -27,6 +31,6 @@ class NrlSqsMessage(BaseModel): snomed_code_practice_setting: SnomedCode = ( SnomedCodes.GENERAL_MEDICAL_PRACTICE.value ) - description: str = "" + description: Optional[str] = None attachment: Optional[NrlAttachment] = None action: str diff --git a/lambdas/services/bulk_upload_service.py b/lambdas/services/bulk_upload_service.py index 72cd35499..b4f30a29d 100644 --- a/lambdas/services/bulk_upload_service.py +++ b/lambdas/services/bulk_upload_service.py @@ -281,6 +281,7 @@ def handle_sqs_message(self, message: dict): ) doc_details = NrlAttachment( url=document_api_endpoint, + content_type="application/pdf", ) nrl_sqs_message = NrlSqsMessage( nhs_number=staging_metadata.nhs_number, diff --git a/lambdas/services/nrl_api_service.py b/lambdas/services/nrl_api_service.py index 52f3ed5bb..3fe8b3599 100644 --- a/lambdas/services/nrl_api_service.py +++ b/lambdas/services/nrl_api_service.py @@ -1,5 +1,6 @@ import os import uuid +from datetime import datetime import requests from enums.snomed_codes import SnomedCode @@ -10,6 +11,7 @@ from utils.exceptions import NrlApiException logger = LoggingService(__name__) +NRL_USER_ID = "National-Document-Repository" class NrlApiService: @@ -45,6 +47,11 @@ def create_new_pointer(self, body, retry_on_expired: bool = True): response = self.session.post( url=self.endpoint, headers=self.headers, json=body ) + logger.info( + f"Create pointer response: Status code: {response.status_code}, " + f"Body: {response.json()}, " + f"Headers: {response.headers}" + ) response.raise_for_status() logger.info("Successfully created new pointer") except (ConnectionError, Timeout, HTTPError) as e: @@ -70,6 +77,18 @@ def get_pointer( response = self.session.get( url=self.endpoint, params=params, headers=self.headers ) + logger.info( + f"Get pointer request: URL: {response.url}, " + "HTTP Verb: GET, " + f"ODS Code: {self.end_user_ods_code}, " + f"Datetime: {int(datetime.now().timestamp())}, " + f"UserID: {self.end_user_ods_code} - {NRL_USER_ID}" + ) + logger.info( + f"Get pointer response: Status code: {response.status_code}, " + f"Body: {response.json()}, " + f"Headers: {response.headers}" + ) response.raise_for_status() return response.json() except HTTPError as e: @@ -90,7 +109,19 @@ def delete_pointer(self, nhs_number, record_type: SnomedCode = None): url_endpoint = self.endpoint + f"/{pointer_id}" try: response = self.session.delete(url=url_endpoint, headers=self.headers) - logger.info(response.json()) + 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()) diff --git a/lambdas/tests/unit/services/test_nrl_api_service.py b/lambdas/tests/unit/services/test_nrl_api_service.py index 0c0241bd4..4cf18d465 100644 --- a/lambdas/tests/unit/services/test_nrl_api_service.py +++ b/lambdas/tests/unit/services/test_nrl_api_service.py @@ -31,6 +31,7 @@ def test_create_new_pointer(nrl_service): def test_create_new_pointer_raise_error(nrl_service): mock_body = {"test": "tests"} response = Response() + response._content = b"{}" response.status_code = 400 nrl_service.session.post.return_value = response pytest.raises(NrlApiException, nrl_service.create_new_pointer, mock_body)