From b696dc1c0d4f813d77e536995c53df294a096989 Mon Sep 17 00:00:00 2001 From: Lillie Dae Date: Thu, 12 Feb 2026 09:05:30 +0000 Subject: [PATCH 1/2] [PRMP-1357] Error handling for invalid files during add more files --- .../ReviewDetailsDocumentSelectStage.test.tsx | 122 +++++++++++++++++- .../ReviewDetailsDocumentSelectStage.tsx | 10 ++ .../DocumentSelectStage.test.tsx | 27 ++++ .../DocumentSelectStage.tsx | 6 + .../pages/adminRoutesPage/AdminRoutesPage.tsx | 5 + app/src/types/generic/routes.ts | 1 + 6 files changed, 170 insertions(+), 1 deletion(-) diff --git a/app/src/components/blocks/_admin/reviewDetailsDocumentSelectStage/ReviewDetailsDocumentSelectStage.test.tsx b/app/src/components/blocks/_admin/reviewDetailsDocumentSelectStage/ReviewDetailsDocumentSelectStage.test.tsx index dd5c163e6..e095873f4 100644 --- a/app/src/components/blocks/_admin/reviewDetailsDocumentSelectStage/ReviewDetailsDocumentSelectStage.test.tsx +++ b/app/src/components/blocks/_admin/reviewDetailsDocumentSelectStage/ReviewDetailsDocumentSelectStage.test.tsx @@ -1,4 +1,6 @@ -import { render, screen, waitFor } from '@testing-library/react'; +// need to use happy-dom for this test file as jsdom doesn't support DOMMatrix and scrollIntoView +// @vitest-environment happy-dom +import { render, screen, waitFor, fireEvent } from '@testing-library/react'; import { describe, expect, it, vi, beforeEach } from 'vitest'; import userEvent from '@testing-library/user-event'; import ReviewDetailsDocumentSelectStage from './ReviewDetailsDocumentSelectStage'; @@ -10,6 +12,8 @@ import { UploadDocument, } from '../../../../types/pages/UploadDocumentsPage/types'; import { routeChildren } from '../../../../types/generic/routes'; +import { getDocument } from 'pdfjs-dist'; +import { PDF_PARSING_ERROR_TYPE } from '../../../../helpers/utils/fileUploadErrorMessages'; const mockNavigate = vi.fn(); @@ -338,4 +342,120 @@ describe('ReviewDetailsDocumentSelectStage', () => { }); }); }); + + describe('Error handling', () => { + const errorCases = [ + ['password protected file', PDF_PARSING_ERROR_TYPE.PASSWORD_MISSING], + ['invalid PDF structure', PDF_PARSING_ERROR_TYPE.INVALID_PDF_STRUCTURE], + ['empty PDF', PDF_PARSING_ERROR_TYPE.EMPTY_PDF], + ]; + + it.each(errorCases)( + 'navigates to admin file errors page when user selects a %s', + async (_description, errorType) => { + const testDocuments: UploadDocument[] = [ + { + id: 'test-id', + file: new File(['test'], 'test.pdf', { type: 'application/pdf' }), + state: DOCUMENT_UPLOAD_STATE.SELECTED, + progress: 0, + docType: testReviewSnoMed, + attempts: 0, + numPages: 1, + validated: false, + }, + ]; + + const { rerender } = render( + , + ); + + rerender( + , + ); + + await waitFor(() => { + expect(screen.queryByTestId('mock-spinner')).not.toBeInTheDocument(); + }); + + // Set up mock to throw error AFTER component is ready + vi.mocked(getDocument).mockImplementationOnce(() => { + throw new Error(errorType as string); + }); + + const errorFile = new File(['test'], 'error-file.pdf', { type: 'application/pdf' }); + const dropzone = screen.getByTestId('dropzone'); + fireEvent.drop(dropzone, { + dataTransfer: { files: [errorFile] }, + }); + + await waitFor(() => { + expect(mockNavigate).toHaveBeenCalledWith( + routeChildren.ADMIN_REVIEW_FILE_ERRORS.replaceAll( + ':reviewId', + 'test-review-id.1', + ), + ); + }); + }, + ); + + it('navigates to admin file errors page when user selects a non-PDF file', async () => { + const testDocuments: UploadDocument[] = [ + { + id: 'test-id', + file: new File(['test'], 'test.pdf', { type: 'application/pdf' }), + state: DOCUMENT_UPLOAD_STATE.SELECTED, + progress: 0, + docType: testReviewSnoMed, + attempts: 0, + numPages: 1, + validated: false, + }, + ]; + + const { rerender } = render( + , + ); + + rerender( + , + ); + + await waitFor(() => { + expect(screen.queryByTestId('mock-spinner')).not.toBeInTheDocument(); + }); + + const nonPdfFile = new File(['test'], 'nonPdfFile.txt', { type: 'text/plain' }); + const dropzone = screen.getByTestId('dropzone'); + fireEvent.drop(dropzone, { + dataTransfer: { files: [nonPdfFile] }, + }); + + await waitFor(() => { + expect(mockNavigate).toHaveBeenCalledWith( + routeChildren.ADMIN_REVIEW_FILE_ERRORS.replaceAll( + ':reviewId', + 'test-review-id.1', + ), + ); + }); + }); + }); }); diff --git a/app/src/components/blocks/_admin/reviewDetailsDocumentSelectStage/ReviewDetailsDocumentSelectStage.tsx b/app/src/components/blocks/_admin/reviewDetailsDocumentSelectStage/ReviewDetailsDocumentSelectStage.tsx index 045a38e4b..3dfa3bd04 100644 --- a/app/src/components/blocks/_admin/reviewDetailsDocumentSelectStage/ReviewDetailsDocumentSelectStage.tsx +++ b/app/src/components/blocks/_admin/reviewDetailsDocumentSelectStage/ReviewDetailsDocumentSelectStage.tsx @@ -55,6 +55,15 @@ const ReviewDetailsDocumentSelectStage = ({ ), ); }; + + const onError = (): void => { + navigate( + routeChildren.ADMIN_REVIEW_FILE_ERRORS.replaceAll( + ':reviewId', + `${reviewData?.id}.${reviewData?.version}`, + ), + ); + }; if (!reviewData?.snomedCode) { return ; } @@ -67,6 +76,7 @@ const ReviewDetailsDocumentSelectStage = ({ filesErrorRef={filesErrorRef} documentConfig={getConfigForDocType(reviewData.snomedCode)} onSuccessOverride={onSuccess} + onErrorOverride={onError} backLinkOverride={(): void => { navigate(-1); }} diff --git a/app/src/components/blocks/_documentUpload/documentSelectStage/DocumentSelectStage.test.tsx b/app/src/components/blocks/_documentUpload/documentSelectStage/DocumentSelectStage.test.tsx index 5b76b16f8..0e261866a 100644 --- a/app/src/components/blocks/_documentUpload/documentSelectStage/DocumentSelectStage.test.tsx +++ b/app/src/components/blocks/_documentUpload/documentSelectStage/DocumentSelectStage.test.tsx @@ -343,6 +343,30 @@ describe('DocumentSelectStage', () => { ); }); }); + + it('should call onErrorOverride when provided instead of navigating to default error route', async () => { + const mockOnErrorOverride = vi.fn(); + const lgDocumentOne = buildLgFile(1); + + vi.mocked(getDocument).mockImplementationOnce(() => { + throw new Error(PDF_PARSING_ERROR_TYPE.PASSWORD_MISSING); + }); + + renderApp(history, { onErrorOverride: mockOnErrorOverride }); + + const dropzone = screen.getByTestId('dropzone'); + fireEvent.drop(dropzone, { + dataTransfer: { files: [lgDocumentOne] }, + }); + + await waitFor(() => { + expect(mockOnErrorOverride).toHaveBeenCalled(); + }); + + expect(mockedUseNavigate).not.toHaveBeenCalledWith( + routeChildren.DOCUMENT_UPLOAD_FILE_ERRORS, + ); + }); }); describe('Update Journey', () => { @@ -741,6 +765,7 @@ describe('DocumentSelectStage', () => { documentConfig?: DOCUMENT_TYPE_CONFIG; isReview?: boolean; initialDocuments?: Array; + onErrorOverride?: () => void; }; const TestApp = ({ @@ -751,6 +776,7 @@ describe('DocumentSelectStage', () => { documentConfig, isReview, initialDocuments, + onErrorOverride, }: TestAppProps): JSX.Element => { const [documents, setDocuments] = useState>( initialDocuments ?? [], @@ -769,6 +795,7 @@ describe('DocumentSelectStage', () => { showSkiplink={showSkipLink} removeAllFilesLinkOverride={removeAllFilesLinkOverride} isReview={isReview} + onErrorOverride={onErrorOverride} /> ); }; diff --git a/app/src/components/blocks/_documentUpload/documentSelectStage/DocumentSelectStage.tsx b/app/src/components/blocks/_documentUpload/documentSelectStage/DocumentSelectStage.tsx index b79df0c56..bca233350 100644 --- a/app/src/components/blocks/_documentUpload/documentSelectStage/DocumentSelectStage.tsx +++ b/app/src/components/blocks/_documentUpload/documentSelectStage/DocumentSelectStage.tsx @@ -42,6 +42,7 @@ export type Props = { filesErrorRef: RefObject; documentConfig: DOCUMENT_TYPE_CONFIG; onSuccessOverride?: () => void; + onErrorOverride?: () => void; backLinkOverride?: () => void; removeAllFilesLinkOverride?: string; goToNextDocType?: () => void; @@ -59,6 +60,7 @@ const DocumentSelectStage = ({ filesErrorRef, documentConfig, onSuccessOverride, + onErrorOverride, backLinkOverride, removeAllFilesLinkOverride, goToNextDocType, @@ -197,6 +199,10 @@ const DocumentSelectStage = ({ if (failedDocs.length > 0) { filesErrorRef.current = true; setDocuments(failedDocs); + if (onErrorOverride) { + onErrorOverride(); + return; + } navigate(routeChildren.DOCUMENT_UPLOAD_FILE_ERRORS); return; } diff --git a/app/src/pages/adminRoutesPage/AdminRoutesPage.tsx b/app/src/pages/adminRoutesPage/AdminRoutesPage.tsx index 303cec71b..69daec7c7 100644 --- a/app/src/pages/adminRoutesPage/AdminRoutesPage.tsx +++ b/app/src/pages/adminRoutesPage/AdminRoutesPage.tsx @@ -15,6 +15,7 @@ import ReviewsDetailsStage from '../../components/blocks/_admin/reviewsDetailsSt import ReviewDetailsPatientSearchStage from '../../components/blocks/_admin/reviewDetailsPatientSearchStage/ReviewDetailsPatientSearchStage'; import { ReviewsPage } from '../../components/blocks/_admin/reviewsPage/ReviewsPage'; import PatientVerifyPage from '../../components/blocks/generic/patientVerifyPage/PatientVerifyPage'; +import DocumentSelectFileErrorsPage from '../../components/blocks/_documentUpload/documentSelectFileErrorsPage/DocumentSelectFileErrorsPage'; import useConfig from '../../helpers/hooks/useConfig'; import { getLastURLPath } from '../../helpers/utils/urlManipulations'; import { routeChildren, routes } from '../../types/generic/routes'; @@ -177,6 +178,10 @@ const AdminRoutesPage = (): JSX.Element => { /> } /> + } + /> } diff --git a/app/src/types/generic/routes.ts b/app/src/types/generic/routes.ts index aed587d44..cc118920d 100644 --- a/app/src/types/generic/routes.ts +++ b/app/src/types/generic/routes.ts @@ -83,6 +83,7 @@ export enum routeChildren { ADMIN_REVIEW_REMOVE_ALL = '/admin/reviews/:reviewId/remove-all', ADMIN_REVIEW_UPLOAD_FILE_ORDER = '/admin/reviews/:reviewId/upload-file-order', ADMIN_REVIEW_UPLOAD = '/admin/reviews/:reviewId/upload', + ADMIN_REVIEW_FILE_ERRORS = '/admin/reviews/:reviewId/file-errors', REVIEWS = 'reviews/*', COOKIES_POLICY_UPDATED = '/cookies-policy/confirmation', From 93b8de48d00f0d6b5e38df49fab8bc782a486a91 Mon Sep 17 00:00:00 2001 From: Lillie Dae Date: Thu, 12 Feb 2026 16:00:50 +0000 Subject: [PATCH 2/2] code comment changes --- .../ReviewDetailsDocumentSelectStage.test.tsx | 166 ++++-------------- 1 file changed, 35 insertions(+), 131 deletions(-) diff --git a/app/src/components/blocks/_admin/reviewDetailsDocumentSelectStage/ReviewDetailsDocumentSelectStage.test.tsx b/app/src/components/blocks/_admin/reviewDetailsDocumentSelectStage/ReviewDetailsDocumentSelectStage.test.tsx index e095873f4..a66c96d61 100644 --- a/app/src/components/blocks/_admin/reviewDetailsDocumentSelectStage/ReviewDetailsDocumentSelectStage.test.tsx +++ b/app/src/components/blocks/_admin/reviewDetailsDocumentSelectStage/ReviewDetailsDocumentSelectStage.test.tsx @@ -1,6 +1,6 @@ // need to use happy-dom for this test file as jsdom doesn't support DOMMatrix and scrollIntoView // @vitest-environment happy-dom -import { render, screen, waitFor, fireEvent } from '@testing-library/react'; +import { render, screen, waitFor, fireEvent, RenderResult } from '@testing-library/react'; import { describe, expect, it, vi, beforeEach } from 'vitest'; import userEvent from '@testing-library/user-event'; import ReviewDetailsDocumentSelectStage from './ReviewDetailsDocumentSelectStage'; @@ -53,7 +53,7 @@ vi.mock('react-router-dom', () => ({ })); describe('ReviewDetailsDocumentSelectStage', () => { - const testReviewSnoMed: DOCUMENT_TYPE = DOCUMENT_TYPE.LLOYD_GEORGE; + const testReviewSnomed: DOCUMENT_TYPE = DOCUMENT_TYPE.LLOYD_GEORGE; let mockReviewData: ReviewDetails; let mockDocuments: UploadDocument[]; @@ -64,7 +64,7 @@ describe('ReviewDetailsDocumentSelectStage', () => { mockReviewData = new ReviewDetails( 'test-review-id', - testReviewSnoMed, + testReviewSnomed, '2024-01-01T12:00:00Z', 'Test Uploader', '2024-01-01T12:00:00Z', @@ -78,39 +78,35 @@ describe('ReviewDetailsDocumentSelectStage', () => { mockSetDocuments = vi.fn() as SetUploadDocuments; }); + const renderApp = (props?: { + reviewData?: ReviewDetails | null; + documents?: UploadDocument[]; + setDocuments?: SetUploadDocuments; + }): RenderResult => { + const defaultProps = { + reviewData: mockReviewData, + documents: mockDocuments, + setDocuments: mockSetDocuments, + }; + + return render(); + }; + describe('Rendering', () => { it('shows spinner when reviewData is null', () => { - render( - , - ); + renderApp({ reviewData: null }); expect(screen.getByTestId('mock-spinner')).toBeInTheDocument(); }); it('shows spinner when files is null', () => { - render( - , - ); + renderApp({ reviewData: { ...mockReviewData, files: null } as any }); expect(screen.getByTestId('mock-spinner')).toBeInTheDocument(); }); it('shows spinner when documents are not initialised', () => { - render( - , - ); + renderApp(); expect(screen.getByTestId('mock-spinner')).toBeInTheDocument(); }); @@ -124,22 +120,14 @@ describe('ReviewDetailsDocumentSelectStage', () => { file: new File(['test'], 'test.pdf', { type: 'application/pdf' }), state: DOCUMENT_UPLOAD_STATE.SELECTED, progress: 0, - docType: testReviewSnoMed, + docType: testReviewSnomed, attempts: 0, numPages: 1, validated: false, }, ]; - const { rerender } = render( - , - ); - - rerender( + render( { }), state: DOCUMENT_UPLOAD_STATE.SELECTED, progress: 0, - docType: testReviewSnoMed, + docType: testReviewSnomed, attempts: 0, numPages: 1, validated: false, }, ]; - const { rerender } = render( - , - ); - - rerender( - , - ); + renderApp({ documents: testDocuments }); await waitFor(() => { expect(screen.queryByTestId('mock-spinner')).not.toBeInTheDocument(); @@ -204,28 +178,14 @@ describe('ReviewDetailsDocumentSelectStage', () => { file: new File(['test'], 'test.pdf', { type: 'application/pdf' }), state: DOCUMENT_UPLOAD_STATE.SELECTED, progress: 0, - docType: testReviewSnoMed, + docType: testReviewSnomed, attempts: 0, numPages: 1, validated: false, }, ]; - const { rerender } = render( - , - ); - - rerender( - , - ); + renderApp({ documents: testDocuments }); await waitFor(() => { expect(screen.queryByTestId('mock-spinner')).not.toBeInTheDocument(); @@ -247,28 +207,14 @@ describe('ReviewDetailsDocumentSelectStage', () => { file: new File(['test'], 'test.pdf', { type: 'application/pdf' }), state: DOCUMENT_UPLOAD_STATE.SELECTED, progress: 0, - docType: testReviewSnoMed, + docType: testReviewSnomed, attempts: 0, numPages: 1, validated: false, }, ]; - const { rerender } = render( - , - ); - - rerender( - , - ); + renderApp({ documents: testDocuments }); await waitFor(() => { expect(screen.queryByTestId('mock-spinner')).not.toBeInTheDocument(); @@ -291,7 +237,7 @@ describe('ReviewDetailsDocumentSelectStage', () => { const user = userEvent.setup(); const customReviewData = new ReviewDetails( 'custom-id', - testReviewSnoMed, + testReviewSnomed, '2024-01-01T12:00:00Z', 'Test Uploader', '2024-01-01T12:00:00Z', @@ -307,28 +253,14 @@ describe('ReviewDetailsDocumentSelectStage', () => { file: new File(['test'], 'test.pdf', { type: 'application/pdf' }), state: DOCUMENT_UPLOAD_STATE.SELECTED, progress: 0, - docType: testReviewSnoMed, + docType: testReviewSnomed, attempts: 0, numPages: 1, validated: false, }, ]; - const { rerender } = render( - , - ); - - rerender( - , - ); + renderApp({ reviewData: customReviewData, documents: testDocuments }); await waitFor(() => { expect(screen.queryByTestId('mock-spinner')).not.toBeInTheDocument(); @@ -359,28 +291,14 @@ describe('ReviewDetailsDocumentSelectStage', () => { file: new File(['test'], 'test.pdf', { type: 'application/pdf' }), state: DOCUMENT_UPLOAD_STATE.SELECTED, progress: 0, - docType: testReviewSnoMed, + docType: testReviewSnomed, attempts: 0, numPages: 1, validated: false, }, ]; - const { rerender } = render( - , - ); - - rerender( - , - ); + renderApp({ documents: testDocuments }); await waitFor(() => { expect(screen.queryByTestId('mock-spinner')).not.toBeInTheDocument(); @@ -415,28 +333,14 @@ describe('ReviewDetailsDocumentSelectStage', () => { file: new File(['test'], 'test.pdf', { type: 'application/pdf' }), state: DOCUMENT_UPLOAD_STATE.SELECTED, progress: 0, - docType: testReviewSnoMed, + docType: testReviewSnomed, attempts: 0, numPages: 1, validated: false, }, ]; - const { rerender } = render( - , - ); - - rerender( - , - ); + renderApp({ documents: testDocuments }); await waitFor(() => { expect(screen.queryByTestId('mock-spinner')).not.toBeInTheDocument();