improve test coverage and remove some comments

Author: lillie-dae

app/src/components/blocks/_admin/reviewDetailsDocumentSelectOrderStage/ReviewDetailsDocumentSelectOrderStage.test.tsx

@@ -7,7 +7,6 @@ import { ReviewDetails } from '../../../../types/generic/reviews';
 import { UploadDocument } from '../../../../types/pages/UploadDocumentsPage/types';
 import userEvent from '@testing-library/user-event';
 
-// Mock DocumentSelectOrderStage component
 vi.mock('../../_documentUpload/documentSelectOrderStage/DocumentSelectOrderStage', () => ({
     default: vi.fn(
         ({ documents, setDocuments, setMergedPdfBlob, existingDocuments, onSuccess }) => (
@@ -71,7 +70,7 @@ describe('ReviewDetailsDocumentSelectOrderStage', () => {
         '1',
         '1234567890',
     );
-    // Set files to empty array to avoid spinner due to null check
+
     mockReviewData.files = [];
 
     const mockDocument = {
@@ -100,7 +99,7 @@ describe('ReviewDetailsDocumentSelectOrderStage', () => {
             <MemoryRouter>
                 <ReviewDetailsDocumentSelectOrderStage
                     reviewData={reviewData}
-                    documents={documents as any} // TODO fix
+                    documents={documents}
                     setDocuments={mockSetDocuments}
                     existingDocuments={existingDocuments!}
                 />
@@ -128,7 +127,6 @@ describe('ReviewDetailsDocumentSelectOrderStage', () => {
         it('renders DocumentSelectOrderStage when documents are initialized', async () => {
             renderComponent(mockReviewData, [], [mockDocument]);
 
-            // Wait for documentsInitialised state to update
             await waitFor(() => {
                 expect(screen.getByTestId('mock-document-select-order-stage')).toBeInTheDocument();
             });
@@ -234,7 +232,6 @@ describe('ReviewDetailsDocumentSelectOrderStage', () => {
                 expect(mockSetDocuments).toHaveBeenCalled();
             });
 
-            // Navigation is mocked via MemoryRouter, just verify setDocuments was called
             expect(mockSetDocuments).toHaveBeenCalled();
         });
 
@@ -266,12 +263,10 @@ describe('ReviewDetailsDocumentSelectOrderStage', () => {
         it('resets documentsInitialised when review changes', async () => {
             const { rerender } = renderComponent(mockReviewData, [], [mockDocument]);
 
-            // Component should render DocumentSelectOrderStage
             await waitFor(() => {
                 expect(screen.getByTestId('mock-document-select-order-stage')).toBeInTheDocument();
             });
 
-            // Change review data
             const newReviewData = new ReviewDetails(
                 'new-id',
                 testReviewSnoMed,
@@ -294,17 +289,14 @@ describe('ReviewDetailsDocumentSelectOrderStage', () => {
                 </MemoryRouter>,
             );
 
-            // Should show spinner because documentsInitialised was reset and documents is empty
             expect(screen.getByText('Loading')).toBeInTheDocument();
         });
 
         it('initializes documents when documents length changes from 0 to > 0', async () => {
             const { rerender } = renderComponent(mockReviewData, [], []);
 
-            // Initially should show spinner
             expect(screen.getByText('Loading')).toBeInTheDocument();
 
-            // Update with documents
             rerender(
                 <MemoryRouter>
                     <ReviewDetailsDocumentSelectOrderStage
@@ -316,7 +308,6 @@ describe('ReviewDetailsDocumentSelectOrderStage', () => {
                 </MemoryRouter>,
             );
 
-            // Should now show the DocumentSelectOrderStage
             await waitFor(() => {
                 expect(screen.getByTestId('mock-document-select-order-stage')).toBeInTheDocument();
             });
@@ -347,7 +338,6 @@ describe('ReviewDetailsDocumentSelectOrderStage', () => {
                 expect(screen.getByTestId('mock-document-select-order-stage')).toBeInTheDocument();
             });
 
-            // The component should be rendered (verified by presence of the mock)
             expect(screen.getByTestId('mock-document-select-order-stage')).toBeInTheDocument();
         });
     });

app/src/components/blocks/_admin/reviewDetailsDocumentSelectStage/ReviewDetailsDocumentSelectStage.test.tsx

@@ -8,7 +8,6 @@ import {
     UploadDocument,
 } from '../../../../types/pages/UploadDocumentsPage/types';
 
-// Mock DocumentSelectStage component we are testing just ReviewDetailsDocumentSelectStage component
 vi.mock('../../_documentUpload/documentSelectStage/DocumentSelectStage', () => ({
     default: vi.fn(({ documents, setDocuments, documentType, filesErrorRef }) => (
         <div data-testid="mock-document-select-stage">
@@ -40,12 +39,10 @@ vi.mock('../../_documentUpload/documentSelectStage/DocumentSelectStage', () => (
     )),
 }));
 
-// Mock Spinner component
 vi.mock('../../../generic/spinner/Spinner', () => ({
     default: vi.fn(() => <div data-testid="mock-spinner">Loading...</div>),
 }));
 
-// Mock react-router-dom
 vi.mock('react-router-dom', () => ({
     useNavigate: vi.fn(() => vi.fn()),
 }));
@@ -62,7 +59,7 @@ describe('ReviewDetailsDocumentSelectStage', () => {
         '1',
         '1234567890',
     );
-    // Set files to an empty array so component doesn't show spinner
+
     mockReviewData.files = [];
 
     const mockDocuments: UploadDocument[] = [];
@@ -129,7 +126,6 @@ describe('ReviewDetailsDocumentSelectStage', () => {
                 />,
             );
 
-            // Rerender with documents to trigger documentsInitialised state change
             rerender(
                 <ReviewDetailsDocumentSelectStage
                     reviewData={mockReviewData}
@@ -166,7 +162,6 @@ describe('ReviewDetailsDocumentSelectStage', () => {
                 />,
             );
 
-            // Rerender with documents to trigger documentsInitialised state change
             rerender(
                 <ReviewDetailsDocumentSelectStage
                     reviewData={mockReviewData}

app/src/components/blocks/_admin/reviewDetailsDocumentUploadingStage/ReviewDetailsDocumentUploadingStage.tsx

@@ -58,7 +58,6 @@ const ReviewDetailsDocumentUploadingStage = ({
     const virusReference = useRef(false);
     const interval = useRef<number>(0);
 
-    // Normalise document state when entering this page so the UI starts from a known state.
     const [hasNormalisedOnEntry, setHasNormalisedOnEntry] = useState(false);
     const [isLoading, setIsLoading] = useState(true);
     const hasNormalisedOnEntryRef = useRef(false);

app/src/components/blocks/_admin/reviewDetailsDownloadChoiceStage/ReviewDetailsDownloadChoiceStage.tsx

@@ -29,7 +29,6 @@ const ReviewDetailsDownloadChoiceStage: React.FC<ReviewDetailsDownloadChoiceProp
     );
 
     useEffect(() => {
-        // If no unselected files in state, navigate back to file select page
         if (!unselectedFiles.length) {
             if (reviewId) {
                 const path = routeChildren.ADMIN_REVIEW_CHOOSE_WHICH_FILES.replace(

app/src/components/blocks/_admin/reviewDetailsFileSelectStage/ReviewDetailsFileSelectStage.test.tsx

@@ -71,7 +71,6 @@ describe('ReviewDetailsFileSelectStage', () => {
             displayName: 'lloyd george record',
         });
 
-        // jsdom doesn’t provide these in all environments.
         (globalThis.URL as any).createObjectURL = vi.fn(() => 'blob:mock-url');
         (HTMLElement.prototype as any).scrollIntoView = vi.fn();
     });

app/src/components/blocks/_admin/reviewDetailsNoFilesChoiceStage/ReviewDetailsNoFilesChoiceStage.test.tsx

@@ -159,15 +159,13 @@ describe('ReviewDetailsNoFilesChoicePage', () => {
         it('clears error when checkbox is checked', async () => {
             render(<ReviewDetailsNoFilesChoiceStage reviewData={mockReviewData} />);
 
-            // First trigger error
             const continueButton = screen.getByRole('button', { name: 'Continue' });
             await userEvent.click(continueButton);
 
             await waitFor(() => {
                 expect(screen.getByText('There is a problem')).toBeInTheDocument();
             });
 
-            // Then check the checkbox
             const checkbox = screen.getByRole('checkbox', {
                 name: /I don't need to add any of these files/i,
             });
@@ -200,7 +198,6 @@ describe('ReviewDetailsNoFilesChoicePage', () => {
                 name: /I don't need to add any of these files/i,
             });
 
-            // Check then uncheck
             await userEvent.click(checkbox);
             await waitFor(() => {
                 expect(checkbox).toBeChecked();

app/src/components/blocks/_admin/reviewDetailsPatientSearchStage/ReviewDetailsPatientSearchStage.test.tsx

@@ -12,7 +12,6 @@ import { AxiosError } from 'axios';
 import { JSX } from 'react/jsx-runtime';
 import { ReviewDetails } from '../../../../types/generic/reviews';
 
-// Mock dependencies
 const mockNavigate = vi.fn();
 const mockUseParams = vi.fn();
 const mockUseBaseAPIUrl = vi.fn();
@@ -410,15 +409,13 @@ describe('ReviewDetailsPatientSearchPage', () => {
                 />,
             );
 
-            // First submit with error
             await userEvent.click(screen.getByTestId('continue-button'));
 
             await waitFor(() => {
                 const errorMessages = screen.getAllByText(incorrectFormatMessage);
                 expect(errorMessages.length).toBeGreaterThan(0);
             });
 
-            // Then submit with valid input
             const input = screen.getByTestId('nhs-number-input');
             await userEvent.clear(input);
             await userEvent.type(input, '9000000009');
@@ -649,7 +646,6 @@ describe('ReviewDetailsPatientSearchPage', () => {
                 />,
             );
 
-            // Component renders successfully with prop
             expect(screen.getByRole('heading')).toBeInTheDocument();
         });
     });

app/src/components/blocks/_admin/reviewsDetailsStage/ReviewsDetailsStage.test.tsx

@@ -81,7 +81,6 @@ const renderComponent = (reviewData?: ReviewDetails, reviewSnoMed?: DOCUMENT_TYP
             '9691914948',
         );
 
-    // Prevent the page from refetching review details in tests unless a test explicitly populates files.
     if (currentReviewData.files === null) {
         currentReviewData.files = [];
     }
@@ -129,10 +128,8 @@ describe('ReviewDetailsPage', () => {
         mockUsePatientDetailsContext.mockReturnValue([mockPatientDetails, mockSetPatientDetails]);
         mockUseSessionContext.mockReturnValue([mockSession, vi.fn()]);
 
-        // Mock isLocal to return false by default
         vi.spyOn(isLocalModule, 'isLocal', 'get').mockReturnValue(false);
 
-        // Mock getPdfObjectUrl
         vi.spyOn(getPdfObjectUrlModule, 'getPdfObjectUrl').mockImplementation(
             (url, setPdfUrl, setStage) => {
                 setPdfUrl('blob:mock-pdf-url');
@@ -229,7 +226,6 @@ describe('ReviewDetailsPage', () => {
             renderComponent(mockReviewData);
 
             await waitFor(() => {
-                // The name should be formatted as "LastName, FirstName"
                 expect(screen.getByText('Dae, Lillie')).toBeInTheDocument();
             });
         });
@@ -238,7 +234,6 @@ describe('ReviewDetailsPage', () => {
             renderComponent(mockReviewData);
 
             await waitFor(() => {
-                // NHS number should be formatted with spaces
                 expect(screen.getByText('969 191 4948')).toBeInTheDocument();
             });
         });
@@ -247,7 +242,6 @@ describe('ReviewDetailsPage', () => {
             renderComponent(mockReviewData);
 
             await waitFor(() => {
-                // Date should be formatted as "3 June 2002"
                 expect(screen.getByText('3 June 2002')).toBeInTheDocument();
             });
         });
@@ -565,17 +559,14 @@ describe('ReviewDetailsPage', () => {
                 expect(screen.getByRole('button', { name: 'Continue' })).toBeInTheDocument();
             });
 
-            // Trigger error
             await user.click(screen.getByRole('button', { name: 'Continue' }));
             expect(screen.getByText('There is a problem')).toBeInTheDocument();
 
-            // Select option - error should remain until form is submitted again
             const yesRadio = screen.getByRole('radio', {
                 name: 'Yes, the details match and I want to accept this document',
             });
             await user.click(yesRadio);
 
-            // Error is still visible until Continue is clicked again
             expect(screen.getByText('There is a problem')).toBeInTheDocument();
         });
     });
@@ -638,12 +629,10 @@ describe('ReviewDetailsPage', () => {
 
     describe('Navigation - Missing Review Data', () => {
         it('renders content when review data is provided', async () => {
-            // Mock isLocal to false so review data won't be loaded
             vi.spyOn(isLocalModule, 'isLocal', 'get').mockReturnValue(false);
 
             renderComponent(mockReviewData);
 
-            // Wait for loading to complete and verify content is rendered
             await waitFor(
                 () => {
                     expect(
@@ -860,7 +849,6 @@ describe('ReviewDetailsPage', () => {
             await waitFor(() => {
                 const pdfCard = screen.getByTestId('pdf-card');
                 expect(pdfCard).toBeInTheDocument();
-                // Check for the flex container wrapper
                 expect(pdfCard.closest('.lloydgeorge_record-stage_flex')).toBeInTheDocument();
             });
         });

app/src/components/blocks/_admin/reviewsPage/ReviewsPage.test.tsx

@@ -250,8 +250,6 @@ describe('ReviewsPage', () => {
                 screen.getByRole('columnheader', { name: 'Sender ODS code' }),
             ).toBeInTheDocument();
             expect(screen.getByRole('columnheader', { name: 'Date uploaded' })).toBeInTheDocument();
-            // Review reason column is commented out in the component
-            // expect(screen.getByRole('columnheader', { name: 'Review reason' })).toBeInTheDocument();
             expect(screen.getByRole('columnheader', { name: 'View' })).toBeInTheDocument();
         });
 
@@ -287,9 +285,6 @@ describe('ReviewsPage', () => {
             expect(screen.getByText('Electronic Health Record')).toBeInTheDocument();
             expect(screen.getByText('Y12345')).toBeInTheDocument();
             expect(screen.getByText('Y67890')).toBeInTheDocument();
-            // Review reason is not displayed as the column is commented out
-            // expect(screen.getByText('Missing metadata')).toBeInTheDocument();
-            // expect(screen.getByText('Duplicate record')).toBeInTheDocument();
         });
 
         it('displays formatted NHS numbers', async () => {

app/src/components/blocks/_admin/reviewsPage/ReviewsPage.tsx

@@ -121,7 +121,6 @@ export const ReviewsPage = ({ setReviewData }: ReviewsPageProps): React.JSX.Elem
     const [failedLoading, setFailedLoading] = useState(false);
     const [count, setCount] = useState(0);
 
-    // Store page tokens: index is page number - 1, value is the startKey for that page
     const [pageTokens, setPageTokens] = useState<string[]>(['']);
 
     const isLastPage = (): boolean => !nextPageToken || count < pageLimit;
@@ -153,7 +152,6 @@ export const ReviewsPage = ({ setReviewData }: ReviewsPageProps): React.JSX.Elem
             setCount(response.count);
             setCurrentPage(pageNumber);
 
-            // If we got a nextPageToken and don't have it stored yet, add it to our history
             const hasNextPage = nextPageToken.includes(response.nextPageToken || '');
             if (!hasNextPage || (response.nextPageToken && !pageTokens[pageNumber])) {
                 setPageTokens((prev) => {
@@ -174,7 +172,6 @@ export const ReviewsPage = ({ setReviewData }: ReviewsPageProps): React.JSX.Elem
     };
 
     const handleSearch = async (): Promise<void> => {
-        // Reset pagination when searching
         setIsLoading(true);
         setSearchValue(inputValue);
         setCurrentPage(1);