diff --git a/frontend/src/__tests__/app/edit-template-name/[templateId]/__snapshots__/page.test.tsx.snap b/frontend/src/__tests__/app/edit-template-name/[templateId]/__snapshots__/page.test.tsx.snap index fe3d1460eb..5408e70090 100644 --- a/frontend/src/__tests__/app/edit-template-name/[templateId]/__snapshots__/page.test.tsx.snap +++ b/frontend/src/__tests__/app/edit-template-name/[templateId]/__snapshots__/page.test.tsx.snap @@ -266,7 +266,7 @@ exports[`valid template renders errors when blank form is submitted and error st Enter a template name { beforeEach(() => { jest.resetAllMocks(); + getTemplateMock.mockResolvedValue(TEMPLATE_WITH_BOTH_RENDERS); }); it('should redirect to get-ready-to-approve-letter-template page with valid form data', async () => { const formData = new FormData(); - formData.append('templateId', 'template-123'); + formData.append('templateId', AUTHORING_LETTER_TEMPLATE.id); formData.append('lockNumber', '1'); await submitAuthoringLetterAction({}, formData); expect(redirectMock).toHaveBeenCalledWith( - '/get-ready-to-approve-letter-template/template-123?lockNumber=1' + `/get-ready-to-approve-letter-template/${AUTHORING_LETTER_TEMPLATE.id}?lockNumber=1` ); }); @@ -37,7 +68,7 @@ describe('submitAuthoringLetterAction', () => { it('should return error state when lockNumber is missing', async () => { const formData = new FormData(); - formData.append('templateId', 'template-123'); + formData.append('templateId', AUTHORING_LETTER_TEMPLATE.id); const result = await submitAuthoringLetterAction({}, formData); @@ -47,7 +78,7 @@ describe('submitAuthoringLetterAction', () => { it('should return error state when lockNumber is invalid', async () => { const formData = new FormData(); - formData.append('templateId', 'template-123'); + formData.append('templateId', AUTHORING_LETTER_TEMPLATE.id); formData.append('lockNumber', 'not-a-number'); const result = await submitAuthoringLetterAction({}, formData); @@ -66,4 +97,147 @@ describe('submitAuthoringLetterAction', () => { expect(result).toHaveProperty('errorState'); expect(redirectMock).not.toHaveBeenCalled(); }); + + it('should redirect to invalid-template page when template validation fails', async () => { + getTemplateMock.mockResolvedValueOnce({ + id: 'template-id', + } as unknown as TemplateDto); + + const formData = getMockFormData({ + templateId: '992fe769-f8b3-43a9-84f1-6e10d0480bb6', + lockNumber: '300', + }); + + await submitAuthoringLetterAction({}, formData); + + expect(redirectMock).toHaveBeenCalledWith('/invalid-template', 'replace'); + }); + + it('should return error when short example has not been generated', async () => { + getTemplateMock.mockResolvedValue({ + ...AUTHORING_LETTER_TEMPLATE, + files: { + ...AUTHORING_LETTER_TEMPLATE.files, + shortFormRender: undefined, + longFormRender: { + status: 'RENDERED' as const, + fileName: 'long.pdf', + currentVersion: 'v1', + pageCount: 2, + }, + }, + }); + + const formData = new FormData(); + formData.append('templateId', AUTHORING_LETTER_TEMPLATE.id); + formData.append('lockNumber', '1'); + + const result = await submitAuthoringLetterAction({}, formData); + + expect(result.errorState?.fieldErrors?.['tab-short']).toContain( + approveErrors.shortExampleRequired + ); + expect(result.errorState?.fieldErrors?.['tab-long']).toBeUndefined(); + expect(redirectMock).not.toHaveBeenCalled(); + }); + + it('should return error when long example has not been generated', async () => { + getTemplateMock.mockResolvedValue({ + ...AUTHORING_LETTER_TEMPLATE, + files: { + ...AUTHORING_LETTER_TEMPLATE.files, + shortFormRender: { + status: 'RENDERED' as const, + fileName: 'short.pdf', + currentVersion: 'v1', + pageCount: 2, + }, + longFormRender: undefined, + }, + }); + + const formData = new FormData(); + formData.append('templateId', AUTHORING_LETTER_TEMPLATE.id); + formData.append('lockNumber', '1'); + + const result = await submitAuthoringLetterAction({}, formData); + + expect(result.errorState?.fieldErrors?.['tab-long']).toContain( + approveErrors.longExampleRequired + ); + expect(result.errorState?.fieldErrors?.['tab-short']).toBeUndefined(); + expect(redirectMock).not.toHaveBeenCalled(); + }); + + it('should return errors for both missing examples', async () => { + getTemplateMock.mockResolvedValue(AUTHORING_LETTER_TEMPLATE); + + const formData = new FormData(); + formData.append('templateId', AUTHORING_LETTER_TEMPLATE.id); + formData.append('lockNumber', '1'); + + const result = await submitAuthoringLetterAction({}, formData); + + expect(result.errorState?.fieldErrors?.['tab-short']).toContain( + approveErrors.shortExampleRequired + ); + expect(result.errorState?.fieldErrors?.['tab-long']).toContain( + approveErrors.longExampleRequired + ); + expect(redirectMock).not.toHaveBeenCalled(); + }); + + it('should return error when short render exists but is not RENDERED status', async () => { + getTemplateMock.mockResolvedValue({ + ...AUTHORING_LETTER_TEMPLATE, + files: { + ...AUTHORING_LETTER_TEMPLATE.files, + shortFormRender: { status: 'FAILED' as const }, + longFormRender: { + status: 'RENDERED' as const, + fileName: 'long.pdf', + currentVersion: 'v1', + pageCount: 2, + }, + }, + }); + + const formData = new FormData(); + formData.append('templateId', AUTHORING_LETTER_TEMPLATE.id); + formData.append('lockNumber', '1'); + + const result = await submitAuthoringLetterAction({}, formData); + + expect(result.errorState?.fieldErrors?.['tab-short']).toContain( + approveErrors.shortExampleRequired + ); + expect(redirectMock).not.toHaveBeenCalled(); + }); + + it('should return error when long render exists but is not RENDERED status', async () => { + getTemplateMock.mockResolvedValue({ + ...AUTHORING_LETTER_TEMPLATE, + files: { + ...AUTHORING_LETTER_TEMPLATE.files, + shortFormRender: { + status: 'RENDERED' as const, + fileName: 'short.pdf', + currentVersion: 'v1', + pageCount: 2, + }, + longFormRender: { status: 'FAILED' as const }, + }, + }); + + const formData = new FormData(); + formData.append('templateId', AUTHORING_LETTER_TEMPLATE.id); + formData.append('lockNumber', '1'); + + const result = await submitAuthoringLetterAction({}, formData); + + expect(result.errorState?.fieldErrors?.['tab-long']).toContain( + approveErrors.longExampleRequired + ); + expect(redirectMock).not.toHaveBeenCalled(); + }); }); diff --git a/frontend/src/__tests__/app/upload-british-sign-language-letter-template/__snapshots__/page.test.tsx.snap b/frontend/src/__tests__/app/upload-british-sign-language-letter-template/__snapshots__/page.test.tsx.snap index f30d523247..63b722176f 100644 --- a/frontend/src/__tests__/app/upload-british-sign-language-letter-template/__snapshots__/page.test.tsx.snap +++ b/frontend/src/__tests__/app/upload-british-sign-language-letter-template/__snapshots__/page.test.tsx.snap @@ -391,7 +391,7 @@ exports[`client has multiple campaign ids renders errors when blank form is subm Enter a template name ({ + useLetterRenderError: jest.fn(() => ({ + parentErrorState: undefined, + setParentErrorState: jest.fn(), + letterRenderErrorState: undefined, + setLetterRenderErrorState: jest.fn(), + })), +})); + jest.mock('next/navigation'); jest.mock('@utils/get-base-path', () => ({ diff --git a/frontend/src/__tests__/components/molecules/LetterRender/LetterRenderForm.test.tsx b/frontend/src/__tests__/components/molecules/LetterRender/LetterRenderForm.test.tsx index 9f63287ccc..cdb160172f 100644 --- a/frontend/src/__tests__/components/molecules/LetterRender/LetterRenderForm.test.tsx +++ b/frontend/src/__tests__/components/molecules/LetterRender/LetterRenderForm.test.tsx @@ -20,6 +20,15 @@ jest.mock('@providers/letter-render-polling-provider', () => { }; }); +jest.mock('@providers/letter-render-error-provider', () => ({ + useLetterRenderError: jest.fn().mockReturnValue({ + parentErrorState: undefined, + setParentErrorState: jest.fn(), + letterRenderErrorState: undefined, + setLetterRenderErrorState: jest.fn(), + }), +})); + const baseTemplate: AuthoringLetterTemplate = { id: 'template-123', clientId: 'client-123', @@ -262,12 +271,14 @@ describe('LetterRenderForm', () => { }); describe('error display', () => { - it('displays error message and applies error styling when systemPersonalisationPackId has validation error', () => { + it('displays error message and applies error styling when recipient has validation error (short tab)', () => { const initialState = createInitialFormState({ errorState: { formErrors: [], fieldErrors: { - systemPersonalisationPackId: ['Select an example recipient'], + 'system-personalisation-pack-id-shortFormRender': [ + 'Choose example recipient', + ], }, }, }); @@ -277,12 +288,10 @@ describe('LetterRenderForm', () => { initialState ); - expect( - screen.getByText('Select an example recipient') - ).toBeInTheDocument(); + expect(screen.getByText('Choose example recipient')).toBeInTheDocument(); const formGroup = screen - .getByText('Select an example recipient') + .getByText('Choose example recipient') .closest('.nhsuk-form-group'); expect(formGroup).toHaveClass('nhsuk-form-group--error'); @@ -293,6 +302,74 @@ describe('LetterRenderForm', () => { expect(select).toHaveClass('nhsuk-select--error'); }); + + it('displays error message and applies error styling when recipient has validation error (long tab)', () => { + const initialState = createInitialFormState({ + errorState: { + formErrors: [], + fieldErrors: { + 'system-personalisation-pack-id-longFormRender': [ + 'Choose example recipient', + ], + }, + }, + }); + + renderWithProvider( + , + initialState + ); + + expect(screen.getByText('Choose example recipient')).toBeInTheDocument(); + + const formGroup = screen + .getByText('Choose example recipient') + .closest('.nhsuk-form-group'); + + expect(formGroup).toHaveClass('nhsuk-form-group--error'); + + const select = screen.getByRole('combobox', { + name: 'Example recipient', + }); + + expect(select).toHaveClass('nhsuk-select--error'); + }); + + it('displays inline error and applies error styling when a custom personalisation field is empty', () => { + const templateWithCustom: AuthoringLetterTemplate = { + ...baseTemplate, + customPersonalisation: ['appointmentDate'], + }; + + const initialState = createInitialFormState({ + errorState: { + formErrors: [], + fieldErrors: { + 'custom-appointmentDate-shortFormRender': [ + 'Enter example data for appointmentDate', + ], + }, + }, + }); + + renderWithProvider( + , + initialState + ); + + expect( + screen.getByText('Enter example data for appointmentDate') + ).toBeInTheDocument(); + + const formGroup = screen + .getByText('Enter example data for appointmentDate') + .closest('.nhsuk-form-group'); + + expect(formGroup).toHaveClass('nhsuk-form-group--error'); + }); }); describe('snapshots', () => { @@ -335,7 +412,9 @@ describe('LetterRenderForm', () => { errorState: { formErrors: [], fieldErrors: { - systemPersonalisationPackId: ['Select an example recipient'], + 'system-personalisation-pack-id-shortFormRender': [ + 'Choose example recipient', + ], }, }, }); diff --git a/frontend/src/__tests__/components/molecules/LetterRender/LetterRenderTab.test.tsx b/frontend/src/__tests__/components/molecules/LetterRender/LetterRenderTab.test.tsx index 857fdee2a4..b476ee5837 100644 --- a/frontend/src/__tests__/components/molecules/LetterRender/LetterRenderTab.test.tsx +++ b/frontend/src/__tests__/components/molecules/LetterRender/LetterRenderTab.test.tsx @@ -2,6 +2,7 @@ import { render, screen, fireEvent, waitFor } from '@testing-library/react'; import { LetterRenderTab } from '@molecules/LetterRender/LetterRenderTab'; import { updateLetterPreview } from '@molecules/LetterRender/server-action'; import { LetterRenderPollingProvider } from '@providers/letter-render-polling-provider'; +import { useLetterRenderError } from '@providers/letter-render-error-provider'; import type { AuthoringLetterTemplate, FormState, @@ -14,6 +15,10 @@ jest.mocked(verifyFormCsrfToken).mockResolvedValue(true); jest.mock('@molecules/LetterRender/server-action'); +jest.mock('@providers/letter-render-error-provider', () => ({ + useLetterRenderError: jest.fn(), +})); + jest.mock('next/navigation'); jest.mock('@utils/get-base-path', () => ({ @@ -22,6 +27,17 @@ jest.mock('@utils/get-base-path', () => ({ const mockUpdateLetterPreview = jest.mocked(updateLetterPreview); +const mockSetLetterRenderErrorState = jest.fn(); + +beforeEach(() => { + jest.mocked(useLetterRenderError).mockReturnValue({ + parentErrorState: undefined, + setParentErrorState: jest.fn(), + letterRenderErrorState: undefined, + setLetterRenderErrorState: mockSetLetterRenderErrorState, + }); +}); + function Provider({ children }: PropsWithChildren) { return {children}; } @@ -72,7 +88,7 @@ describe('LetterRenderTab', () => { mockUpdateLetterPreview.mockResolvedValue(createMockState()); }); - describe('buildPdfUrl', () => { + describe('derivePdfUrl', () => { it('builds URL from initialRender when no variant render exists', () => { render( , @@ -248,7 +264,7 @@ describe('LetterRenderTab', () => { }); }); - describe('getInitialState', () => { + describe('deriveFormState', () => { it('uses initial empty state when no variant render exists', () => { render( , @@ -521,7 +537,9 @@ describe('LetterRenderTab', () => { errorState: { formErrors: [], fieldErrors: { - systemPersonalisationPackId: ['Select an example recipient'], + 'system-personalisation-pack-id-shortFormRender': [ + 'Select an example recipient', + ], }, }, }) @@ -547,6 +565,32 @@ describe('LetterRenderTab', () => { await screen.findByText('Select an example recipient') ).toBeInTheDocument(); }); + + it('forwards error state to LetterRenderErrorProvider after submission', async () => { + const errorState = { + formErrors: [], + fieldErrors: { + 'system-personalisation-pack-id-shortFormRender': [ + 'Select an example recipient', + ], + }, + }; + + mockUpdateLetterPreview.mockResolvedValue( + createMockState({ errorState }) + ); + + render( + , + { wrapper: Provider } + ); + + fireEvent.click(screen.getByRole('button', { name: 'Update preview' })); + + await waitFor(() => { + expect(mockSetLetterRenderErrorState).toHaveBeenCalledWith(errorState); + }); + }); }); describe('snapshots', () => { @@ -568,4 +612,61 @@ describe('LetterRenderTab', () => { expect(asFragment()).toMatchSnapshot(); }); }); + + describe('hideEditActions', () => { + it('renders details panel instead of form', () => { + render( + , + { wrapper: Provider } + ); + + expect( + screen.getByTitle('Letter preview - short examples') + ).toBeInTheDocument(); + expect( + screen.getByText('PDS personalisation fields') + ).toBeInTheDocument(); + expect( + screen.queryByRole('combobox', { name: 'Example recipient' }) + ).not.toBeInTheDocument(); + expect( + screen.queryByRole('button', { name: 'Update preview' }) + ).not.toBeInTheDocument(); + }); + + it('renders iframe with correct URL', () => { + render( + , + { wrapper: Provider } + ); + + expect( + screen.getByTitle('Letter preview - short examples') + ).toHaveAttribute( + 'src', + '/templates/files/client-456/renders/template-123/initial.pdf' + ); + }); + + it('does not call useLetterRenderError', () => { + render( + , + { wrapper: Provider } + ); + + expect(useLetterRenderError).not.toHaveBeenCalled(); + }); + }); }); diff --git a/frontend/src/__tests__/components/molecules/LetterRender/LetterSubmitButton.test.tsx b/frontend/src/__tests__/components/molecules/LetterRender/LetterSubmitButton.test.tsx index 3f0def04e4..33bcbe5b7a 100644 --- a/frontend/src/__tests__/components/molecules/LetterRender/LetterSubmitButton.test.tsx +++ b/frontend/src/__tests__/components/molecules/LetterRender/LetterSubmitButton.test.tsx @@ -1,12 +1,20 @@ import { render, screen } from '@testing-library/react'; import { LetterSubmitButton } from '@molecules/LetterRender/LetterSubmitButton'; import { useLetterRenderPolling } from '@providers/letter-render-polling-provider'; +import { useLetterRenderError } from '@providers/letter-render-error-provider'; jest.mock('@providers/letter-render-polling-provider'); +jest.mock('@providers/letter-render-error-provider'); describe('LetterSubmitButton', () => { beforeEach(() => { jest.resetAllMocks(); + jest.mocked(useLetterRenderError).mockReturnValue({ + parentErrorState: undefined, + setParentErrorState: jest.fn(), + letterRenderErrorState: undefined, + setLetterRenderErrorState: jest.fn(), + }); }); it('renders an enabled submit button when no tab is polling', () => { diff --git a/frontend/src/__tests__/components/molecules/LetterRender/__snapshots__/LetterRenderForm.test.tsx.snap b/frontend/src/__tests__/components/molecules/LetterRender/__snapshots__/LetterRenderForm.test.tsx.snap index e03fd2c117..2a49804b5c 100644 --- a/frontend/src/__tests__/components/molecules/LetterRender/__snapshots__/LetterRenderForm.test.tsx.snap +++ b/frontend/src/__tests__/components/molecules/LetterRender/__snapshots__/LetterRenderForm.test.tsx.snap @@ -366,7 +366,7 @@ exports[`LetterRenderForm snapshots matches snapshot with validation error 1`] = > Error: - Select an example recipient + Choose example recipient - - {approveButtonText} - - )} -

- - {backLinkText} - -

+ + {backLinkText} + - - + + + +
+
+ +
+
+ + {showRenderer && } + + {showSubmitForm && ( + + + + + {approveButtonText} + + + )} +

+ + {backLinkText} + +

+ + + + diff --git a/frontend/src/app/preview-letter-template/[templateId]/server-action.ts b/frontend/src/app/preview-letter-template/[templateId]/server-action.ts index 20b8079858..8b070680fe 100644 --- a/frontend/src/app/preview-letter-template/[templateId]/server-action.ts +++ b/frontend/src/app/preview-letter-template/[templateId]/server-action.ts @@ -3,7 +3,12 @@ import { z } from 'zod/v4'; import { $LockNumber } from 'nhs-notify-backend-client/schemas'; import type { FormState } from 'nhs-notify-web-template-management-utils'; -import { redirect } from 'next/navigation'; +import { validateLetterTemplate } from 'nhs-notify-web-template-management-utils'; +import { redirect, RedirectType } from 'next/navigation'; +import { getTemplate } from '@utils/form-actions'; +import content from '@content/content'; + +const { approveErrors } = content.pages.previewLetterTemplate; const $FormSchema = z.object({ templateId: z.string().nonempty(), @@ -24,6 +29,27 @@ export async function submitAuthoringLetterAction( const { templateId, lockNumber } = result.data; + const template = await getTemplate(templateId); + const validatedTemplate = validateLetterTemplate(template); + + if (!validatedTemplate || validatedTemplate.letterVersion !== 'AUTHORING') { + return redirect('/invalid-template', RedirectType.replace); + } + + const fieldErrors: Record = {}; + + if (validatedTemplate.files.shortFormRender?.status !== 'RENDERED') { + fieldErrors['tab-short'] = [approveErrors.shortExampleRequired]; + } + + if (validatedTemplate.files.longFormRender?.status !== 'RENDERED') { + fieldErrors['tab-long'] = [approveErrors.longExampleRequired]; + } + + if (Object.keys(fieldErrors).length > 0) { + return { errorState: { fieldErrors } }; + } + redirect( `/get-ready-to-approve-letter-template/${templateId}?lockNumber=${lockNumber}` ); diff --git a/frontend/src/components/atoms/FileUpload/FileUpload.tsx b/frontend/src/components/atoms/FileUpload/FileUpload.tsx index eb44d3c1f6..d93c1c2386 100644 --- a/frontend/src/components/atoms/FileUpload/FileUpload.tsx +++ b/frontend/src/components/atoms/FileUpload/FileUpload.tsx @@ -2,19 +2,32 @@ import classNames from 'classnames'; import { ErrorMessage, HintText, Label } from 'nhsuk-react-components'; import React, { HTMLProps } from 'react'; import styles from './FileUpload.module.scss'; +import { useNHSNotifyForm } from '@providers/form-provider'; interface FileUploadProps extends HTMLProps { + id: string; error?: string; hint?: string; } export function FileUploadInput({ className, + id, ...props -}: Omit, 'type'>) { +}: Omit, 'type'> & { id: string; name: string }) { + const [state] = useNHSNotifyForm(); + + const error = Boolean(state.errorState?.fieldErrors?.[id]?.length); + return ( diff --git a/frontend/src/components/atoms/NHSNotifyForm/Input.tsx b/frontend/src/components/atoms/NHSNotifyForm/Input.tsx index 5b07c17874..2771028f0e 100644 --- a/frontend/src/components/atoms/NHSNotifyForm/Input.tsx +++ b/frontend/src/components/atoms/NHSNotifyForm/Input.tsx @@ -6,16 +6,27 @@ import { useNHSNotifyForm } from '@providers/form-provider'; export function NHSNotifyFormInput({ className, + id, name, ...props -}: Omit, 'defaultValue'>) { +}: Omit, 'defaultValue'> & { + id: string; + name: string; +}) { const [state] = useNHSNotifyForm(); + const error = Boolean(state.errorState?.fieldErrors?.[id]?.length); + return ( ); diff --git a/frontend/src/components/atoms/NHSNotifyForm/Select.tsx b/frontend/src/components/atoms/NHSNotifyForm/Select.tsx index 66bc80ced7..80b5c198a4 100644 --- a/frontend/src/components/atoms/NHSNotifyForm/Select.tsx +++ b/frontend/src/components/atoms/NHSNotifyForm/Select.tsx @@ -7,12 +7,16 @@ import { useNHSNotifyForm } from '@providers/form-provider'; export function NHSNotifyFormSelect({ children, className, + id, name, ...props -}: Omit, 'defaultValue'>) { +}: Omit, 'defaultValue'> & { + id: string; + name: string; +}) { const [state] = useNHSNotifyForm(); - const error = Boolean(name && state.errorState?.fieldErrors?.[name]?.length); + const error = Boolean(state.errorState?.fieldErrors?.[id]?.length); return ( ); diff --git a/frontend/src/components/molecules/LetterRender/CombinedLetterErrorSummary.tsx b/frontend/src/components/molecules/LetterRender/CombinedLetterErrorSummary.tsx new file mode 100644 index 0000000000..b7cf4ff7b7 --- /dev/null +++ b/frontend/src/components/molecules/LetterRender/CombinedLetterErrorSummary.tsx @@ -0,0 +1,30 @@ +'use client'; + +import { useEffect } from 'react'; +import { useNHSNotifyForm } from '@providers/form-provider'; +import { useLetterRenderError } from '@providers/letter-render-error-provider'; +import { NhsNotifyErrorSummary } from '@molecules/NhsNotifyErrorSummary/NhsNotifyErrorSummary'; + +/** + * Displays validation errors for both the letter rendering component and a parent form. + * Error state is stored in LetterRenderErrorProvider so it can be cleared when either form is submitted. + * + * Must be rendered inside NHSNotifyFormProvider (for the parent action state) + * and LetterRenderErrorProvider. + */ +export function CombinedLetterErrorSummary() { + const [state] = useNHSNotifyForm(); + + const { setParentErrorState, parentErrorState, letterRenderErrorState } = + useLetterRenderError(); + + useEffect(() => { + setParentErrorState(state.errorState); + }, [state, setParentErrorState]); + + return ( + + ); +} diff --git a/frontend/src/components/molecules/LetterRender/LetterRenderForm.tsx b/frontend/src/components/molecules/LetterRender/LetterRenderForm.tsx index 8bbca6e03f..8f2116592a 100644 --- a/frontend/src/components/molecules/LetterRender/LetterRenderForm.tsx +++ b/frontend/src/components/molecules/LetterRender/LetterRenderForm.tsx @@ -10,6 +10,7 @@ import { import { NHSNotifyButton } from '@atoms/NHSNotifyButton/NHSNotifyButton'; import * as NHSNotifyForm from '@atoms/NHSNotifyForm'; import { useLetterRenderPolling } from '@providers/letter-render-polling-provider'; +import { useLetterRenderError } from '@providers/letter-render-error-provider'; import type { PersonalisedRenderKey } from '@utils/types'; import styles from './LetterRenderForm.module.scss'; import { PERSONALISATION_FORMDATA_PREFIX } from '@utils/constants'; @@ -22,6 +23,7 @@ type LetterRenderFormProps = { export function LetterRenderForm({ template, tab }: LetterRenderFormProps) { const { letterRender: copy } = content.components; const { isAnyTabPolling } = useLetterRenderPolling(); + const { setParentErrorState } = useLetterRenderError(); const exampleRecipients = tab === 'shortFormRender' @@ -36,11 +38,15 @@ export function LetterRenderForm({ template, tab }: LetterRenderFormProps) {

{copy.pdsSection.heading}

{copy.pdsSection.hint}

- + - + + + setParentErrorState(undefined)} > {copy.updatePreviewButton} diff --git a/frontend/src/components/molecules/LetterRender/LetterRenderTab.tsx b/frontend/src/components/molecules/LetterRender/LetterRenderTab.tsx index cb00e21e18..3f03c981ef 100644 --- a/frontend/src/components/molecules/LetterRender/LetterRenderTab.tsx +++ b/frontend/src/components/molecules/LetterRender/LetterRenderTab.tsx @@ -19,6 +19,8 @@ import { PollLetterRender } from '@molecules/PollLetterRender/PollLetterRender'; import { PERSONALISATION_FORMDATA_PREFIX } from '@utils/constants'; import content from '@content/content'; import { buildLetterRenderUrl } from '@utils/letter-render-url'; +import { useLetterRenderError } from '@providers/letter-render-error-provider'; +import { useEffect, type ReactNode } from 'react'; const { loadingText } = content.components.letterRender; @@ -67,54 +69,64 @@ function deriveFormState( }; } -function LetterRenderTabContent({ +function LetterRenderTabLayout({ + leftColumn, + rightColumn, +}: { + leftColumn: ReactNode; + rightColumn: ReactNode; +}) { + return ( +
+
{leftColumn}
+
+ {rightColumn} +
+
+ ); +} + +function LetterRenderTabFormInner({ template, tab, pdfUrl, - hideEditActions, }: { template: AuthoringLetterTemplate; tab: PersonalisedRenderKey; pdfUrl: string | null; - hideEditActions?: boolean; }) { - const [_state, _dispatch, isPending] = useNHSNotifyForm(); + const [state, _dispatch, isPending] = useNHSNotifyForm(); + const { setLetterRenderErrorState } = useLetterRenderError(); - return ( -
-
- {hideEditActions ? ( - - ) : ( - - )} -
+ useEffect(() => { + setLetterRenderErrorState(state.errorState); + }, [state, setLetterRenderErrorState]); -
- {hideEditActions ? ( + return ( + } + rightColumn={ + {loadingText}

} + forcePolling={isPending} + > - ) : ( - {loadingText}

} - forcePolling={isPending} - > - - - )} -
-
+ + } + /> ); } -export function LetterRenderTab({ +function LetterRenderTabForm({ template, tab, - hideEditActions, -}: LetterRenderTabProps) { +}: { + template: AuthoringLetterTemplate; + tab: PersonalisedRenderKey; +}) { const personalisedRender = template.files[tab]; - const formState = deriveFormState(template, personalisedRender); const pdfUrl = derivePdfUrl(template, personalisedRender); @@ -123,12 +135,36 @@ export function LetterRenderTab({ initialState={formState} serverAction={updateLetterPreview} > - + ); } + +function LetterRenderTabReadOnly({ + template, + tab, +}: { + template: AuthoringLetterTemplate; + tab: PersonalisedRenderKey; +}) { + const pdfUrl = derivePdfUrl(template, template.files[tab]); + + return ( + } + rightColumn={} + /> + ); +} + +export function LetterRenderTab({ + template, + tab, + hideEditActions, +}: LetterRenderTabProps) { + if (hideEditActions) { + return ; + } + + return ; +} diff --git a/frontend/src/components/molecules/LetterRender/LetterSubmitButton.tsx b/frontend/src/components/molecules/LetterRender/LetterSubmitButton.tsx index f348c3ba50..8b53ab9bfd 100644 --- a/frontend/src/components/molecules/LetterRender/LetterSubmitButton.tsx +++ b/frontend/src/components/molecules/LetterRender/LetterSubmitButton.tsx @@ -3,9 +3,11 @@ import type { PropsWithChildren } from 'react'; import { NHSNotifyButton } from '@atoms/NHSNotifyButton/NHSNotifyButton'; import { useLetterRenderPolling } from '@providers/letter-render-polling-provider'; +import { useLetterRenderError } from '@providers/letter-render-error-provider'; export function LetterSubmitButton({ children }: PropsWithChildren) { const { isAnyTabPolling } = useLetterRenderPolling(); + const { setLetterRenderErrorState } = useLetterRenderError(); return ( setLetterRenderErrorState(undefined)} > {children} diff --git a/frontend/src/components/molecules/LetterRender/server-action.ts b/frontend/src/components/molecules/LetterRender/server-action.ts index 7045591de3..26f20194e6 100644 --- a/frontend/src/components/molecules/LetterRender/server-action.ts +++ b/frontend/src/components/molecules/LetterRender/server-action.ts @@ -14,8 +14,9 @@ import type { LetterProofRequest } from 'nhs-notify-web-template-management-type import { PERSONALISATION_FORMDATA_PREFIX } from '@utils/constants'; import { format as formatDate } from 'date-fns'; import { $LockNumber } from 'nhs-notify-backend-client/schemas'; +import { interpolate } from '@utils/interpolate'; -const { pdsSection } = copy.components.letterRender; +const { pdsSection, customSection } = copy.components.letterRender; const $FormSchema = z.object({ systemPersonalisationPackId: z.enum(EXAMPLE_RECIPIENT_IDS, { @@ -26,6 +27,9 @@ const $FormSchema = z.object({ tab: z.enum(['longFormRender', 'shortFormRender']), }); +const systemPackIdErrorKey = (tab?: string) => + `system-personalisation-pack-id-${tab}`; + export async function updateLetterPreview( _: FormState, formData: FormData @@ -33,16 +37,50 @@ export async function updateLetterPreview( const result = $FormSchema.safeParse(Object.fromEntries(formData.entries())); const fields = formDataToFormStateFields(formData); + const { tab } = fields; + + const personalisationFieldErrors: Record = {}; + + for (const [key, value] of Object.entries(fields)) { + if (key.startsWith(PERSONALISATION_FORMDATA_PREFIX) && !value) { + const fieldName = key.slice(PERSONALISATION_FORMDATA_PREFIX.length); + personalisationFieldErrors[`custom-${fieldName}-${tab}`] = [ + interpolate(customSection.error.required, { field: fieldName }), + ]; + } + } if (result.error) { + const baseError = z.flattenError(result.error); + const { systemPersonalisationPackId, ...otherFieldErrors } = + baseError.fieldErrors; + + return { + errorState: { + ...baseError, + fieldErrors: { + // need to convert to field id so summary link works + ...(systemPersonalisationPackId && { + [systemPackIdErrorKey(tab)]: systemPersonalisationPackId, + }), + ...otherFieldErrors, + ...personalisationFieldErrors, + }, + }, + fields, + }; + } + + if (Object.keys(personalisationFieldErrors).length > 0) { return { - errorState: z.flattenError(result.error), + errorState: { + fieldErrors: personalisationFieldErrors, + }, fields, }; } - const { templateId, systemPersonalisationPackId, tab, lockNumber } = - result.data; + const { templateId, systemPersonalisationPackId, lockNumber } = result.data; const customPersonalisation = Object.fromEntries( Object.entries(fields).flatMap(([k, v]) => @@ -62,7 +100,7 @@ export async function updateLetterPreview( return { errorState: { fieldErrors: { - systemPersonalisationPackId: [pdsSection.error.invalid], + [systemPackIdErrorKey(tab)]: [pdsSection.error.invalid], }, }, fields, diff --git a/frontend/src/components/providers/form-provider.tsx b/frontend/src/components/providers/form-provider.tsx index 591fc72e58..9d53d602a6 100644 --- a/frontend/src/components/providers/form-provider.tsx +++ b/frontend/src/components/providers/form-provider.tsx @@ -12,7 +12,7 @@ type NHSNotifyFormActionState = ReturnType< typeof useActionState >; -const FormContext = createContext(null); +export const FormContext = createContext(null); export function useNHSNotifyForm() { const context = useContext(FormContext); diff --git a/frontend/src/components/providers/letter-render-error-provider.tsx b/frontend/src/components/providers/letter-render-error-provider.tsx new file mode 100644 index 0000000000..48236c08d7 --- /dev/null +++ b/frontend/src/components/providers/letter-render-error-provider.tsx @@ -0,0 +1,53 @@ +'use client'; + +import type { ErrorState } from 'nhs-notify-web-template-management-utils'; +import { + createContext, + useContext, + useState, + type PropsWithChildren, +} from 'react'; + +type LetterRenderErrorContextValue = { + parentErrorState: ErrorState | undefined; + setParentErrorState: (state: ErrorState | undefined) => void; + letterRenderErrorState: ErrorState | undefined; + setLetterRenderErrorState: (state: ErrorState | undefined) => void; +}; + +const LetterRenderErrorContext = + createContext(null); + +export function useLetterRenderError() { + const context = useContext(LetterRenderErrorContext); + + if (!context) { + throw new Error( + 'useLetterRenderError must be used within LetterRenderErrorProvider' + ); + } + + return context; +} + +export function LetterRenderErrorProvider({ children }: PropsWithChildren) { + const [parentErrorState, setParentErrorState] = useState< + ErrorState | undefined + >(); + const [letterRenderErrorState, setLetterRenderErrorState] = useState< + ErrorState | undefined + >(); + + return ( + + {children} + + ); +} diff --git a/frontend/src/content/content.ts b/frontend/src/content/content.ts index 45de96c9da..d696088d62 100644 --- a/frontend/src/content/content.ts +++ b/frontend/src/content/content.ts @@ -559,6 +559,12 @@ const previewLetterTemplate = { 'After you submit your template, our service team will send you a proof by email instead.', text3: 'This email will tell you what to do next.', }, + approveErrors: { + shortExampleRequired: + "Enter short example data and select 'Update preview' to check how your personalisation fields will appear in your letter", + longExampleRequired: + "Enter long example data and select 'Update preview' to check how your personalisation fields will appear in your letter", + }, links: { messageTemplates: '/message-templates', submitLetterTemplate: @@ -591,11 +597,14 @@ const letterRender = { recipientLabel: 'Example recipient', recipientPlaceholder: 'Select a recipient', error: { - invalid: 'Select an example recipient', + invalid: 'Choose example recipient', }, }, customSection: { heading: 'Custom personalisation fields', + error: { + required: `Enter example data for {{field}}`, + }, }, updatePreviewButton: 'Update preview', }; diff --git a/tests/test-team/pages/letter/template-mgmt-preview-letter-page.ts b/tests/test-team/pages/letter/template-mgmt-preview-letter-page.ts index d87acd8416..9787401e5c 100644 --- a/tests/test-team/pages/letter/template-mgmt-preview-letter-page.ts +++ b/tests/test-team/pages/letter/template-mgmt-preview-letter-page.ts @@ -11,6 +11,7 @@ export class TemplateMgmtPreviewLetterPage extends TemplateMgmtPreviewBasePage { ); public readonly errorSummary: Locator; + public readonly errorSummaryLinks: Locator; public readonly continueButton: Locator; public readonly statusTag: Locator; @@ -35,6 +36,7 @@ export class TemplateMgmtPreviewLetterPage extends TemplateMgmtPreviewBasePage { super(page); this.errorSummary = page.locator('[class="nhsuk-error-summary"]'); + this.errorSummaryLinks = this.errorSummary.locator('a'); this.continueButton = page.locator('[id="preview-letter-template-cta"]'); this.statusTag = page.getByTestId('status-tag'); @@ -86,6 +88,13 @@ export class TemplateMgmtPreviewLetterPage extends TemplateMgmtPreviewBasePage { tabSpinner, getCustomFieldInput: (fieldName: string): Locator => panel.locator(`input[id="custom-${fieldName}-${variant}"]`), + getInlineError: (fieldId: string): Locator => + panel + .locator(`[id="${fieldId}"]`) + .locator('xpath=..') + .locator('.nhsuk-error-message'), + getFormGroupForField: (fieldId: string): Locator => + panel.locator(`[id="${fieldId}"]`).locator('xpath=..'), getRecipientOptions: (): Locator => recipientSelect.locator('option'), async clickTab() { diff --git a/tests/test-team/template-mgmt-component-tests/letter/template-mgmt-preview-letter-page.component.spec.ts b/tests/test-team/template-mgmt-component-tests/letter/template-mgmt-preview-letter-page.component.spec.ts index e2f99b32e8..2aae3382e5 100644 --- a/tests/test-team/template-mgmt-component-tests/letter/template-mgmt-preview-letter-page.component.spec.ts +++ b/tests/test-team/template-mgmt-component-tests/letter/template-mgmt-preview-letter-page.component.spec.ts @@ -1,4 +1,5 @@ import { test, expect } from '@playwright/test'; +import { randomUUID } from 'node:crypto'; import { TemplateStorageHelper } from '../../helpers/db/template-storage-helper'; import { TemplateFactory } from '../../helpers/factories/template-factory'; import { makeLetterVariant } from '../../helpers/factories/letter-variant-factory'; @@ -153,6 +154,28 @@ function createTemplates( { letterVariantId: variants.doubleSided.id, initialRender: { pageCount: 4 }, + shortFormRender: { + fileName: 'short-personalised.pdf', + currentVersion: 'v1-short', + pageCount: 4, + systemPersonalisationPackId: 'short-1', + personalisationParameters: { + firstName: 'Jo', + lastName: 'Bloggs', + appointmentDate: '2025-03-15', + }, + }, + longFormRender: { + fileName: 'long-personalised.pdf', + currentVersion: 'v1-long', + pageCount: 4, + systemPersonalisationPackId: 'long-1', + personalisationParameters: { + firstName: 'Jo', + lastName: 'Bloggs', + appointmentDate: '2025-03-15', + }, + }, } ), authoringNoCampaign: TemplateFactory.createAuthoringLetterTemplate( @@ -306,6 +329,78 @@ function createTemplates( }, } ), + authoringForUpdatePreviewErrors: + TemplateFactory.createAuthoringLetterTemplate( + randomUUID(), + user, + 'authoring-update-preview-errors', + 'NOT_YET_SUBMITTED', + { + letterVariantId: variants.doubleSided.id, + customPersonalisation: ['appointmentDate', 'clinicName'], + initialRender: { + fileName: 'initial-render.pdf', + currentVersion: 'v1-initial', + pageCount: 4, + }, + } + ), + authoringNoExamplesGenerated: TemplateFactory.createAuthoringLetterTemplate( + randomUUID(), + user, + 'authoring-no-examples-generated', + 'NOT_YET_SUBMITTED', + { + letterVariantId: variants.doubleSided.id, + initialRender: { + fileName: 'initial-render.pdf', + currentVersion: 'v1-initial', + pageCount: 4, + }, + } + ), + authoringOnlyShortExampleGenerated: + TemplateFactory.createAuthoringLetterTemplate( + randomUUID(), + user, + 'authoring-only-short-example', + 'NOT_YET_SUBMITTED', + { + letterVariantId: variants.doubleSided.id, + initialRender: { + fileName: 'initial-render.pdf', + currentVersion: 'v1-initial', + pageCount: 4, + }, + shortFormRender: { + fileName: 'short-render.pdf', + currentVersion: 'v1-short', + pageCount: 4, + systemPersonalisationPackId: 'short-1', + }, + } + ), + authoringOnlyLongExampleGenerated: + TemplateFactory.createAuthoringLetterTemplate( + randomUUID(), + user, + 'authoring-only-long-example', + 'NOT_YET_SUBMITTED', + { + letterVariantId: variants.doubleSided.id, + initialRender: { + fileName: 'initial-render.pdf', + currentVersion: 'v1-initial', + pageCount: 4, + }, + longFormRender: { + fileName: 'long-render.pdf', + currentVersion: 'v1-long', + pageCount: 4, + systemPersonalisationPackId: 'long-1', + }, + } + ), authoringSingleSided: TemplateFactory.createAuthoringLetterTemplate( 'AABB1122-3344-5566-7788-99AABBCCDDEE', user, @@ -1391,5 +1486,288 @@ test.describe('Preview Letter template Page', () => { await expect(previewPage.summaryRowValue('Total pages')).toHaveText('4'); await expect(previewPage.summaryRowValue('Sheets')).toHaveText('4'); }); + + test.describe('Update preview validation errors', () => { + test('shows error summary and inline error when Update preview is clicked with no recipient selected (short tab)', async ({ + page, + }) => { + const previewPage = new TemplateMgmtPreviewLetterPage( + page + ).setPathParam( + 'templateId', + templates.authoringForUpdatePreviewErrors.id + ); + + await previewPage.loadPage(); + + await previewPage.shortTab.clickUpdatePreview(); + + await expect(previewPage.errorSummary).toBeVisible(); + await expect(previewPage.errorSummary).toContainText( + 'Choose example recipient' + ); + + const summaryLink = previewPage.errorSummaryLinks.filter({ + hasText: 'Choose example recipient', + }); + await expect(summaryLink).toHaveAttribute( + 'href', + '#system-personalisation-pack-id-shortFormRender' + ); + + await expect( + previewPage.shortTab.getInlineError( + 'system-personalisation-pack-id-shortFormRender' + ) + ).toBeVisible(); + await expect( + previewPage.shortTab.getInlineError( + 'system-personalisation-pack-id-shortFormRender' + ) + ).toContainText('Choose example recipient'); + }); + + test('shows error summary and inline error when Update preview is clicked with no recipient selected (long tab)', async ({ + page, + }) => { + const previewPage = new TemplateMgmtPreviewLetterPage( + page + ).setPathParam( + 'templateId', + templates.authoringForUpdatePreviewErrors.id + ); + + await previewPage.loadPage(); + + await previewPage.longTab.clickTab(); + await previewPage.longTab.clickUpdatePreview(); + + await expect(previewPage.errorSummary).toBeVisible(); + await expect(previewPage.errorSummary).toContainText( + 'Choose example recipient' + ); + + const summaryLink = previewPage.errorSummaryLinks.filter({ + hasText: 'Choose example recipient', + }); + await expect(summaryLink).toHaveAttribute( + 'href', + '#system-personalisation-pack-id-longFormRender' + ); + + await expect( + previewPage.longTab.getInlineError( + 'system-personalisation-pack-id-longFormRender' + ) + ).toBeVisible(); + await expect( + previewPage.longTab.getInlineError( + 'system-personalisation-pack-id-longFormRender' + ) + ).toContainText('Choose example recipient'); + }); + + test('shows error summary and inline error for each empty custom field when Update preview is clicked (short tab)', async ({ + page, + }) => { + const previewPage = new TemplateMgmtPreviewLetterPage( + page + ).setPathParam( + 'templateId', + templates.authoringForUpdatePreviewErrors.id + ); + + await previewPage.loadPage(); + + await previewPage.shortTab.selectRecipient({ index: 1 }); + // leave both custom fields empty + await previewPage.shortTab.clickUpdatePreview(); + + await expect(previewPage.errorSummary).toBeVisible(); + await expect(previewPage.errorSummary).toContainText( + 'Enter example data for appointmentDate' + ); + await expect(previewPage.errorSummary).toContainText( + 'Enter example data for clinicName' + ); + + await expect( + previewPage.shortTab.getInlineError( + 'custom-appointmentDate-shortFormRender' + ) + ).toBeVisible(); + await expect( + previewPage.shortTab.getInlineError( + 'custom-clinicName-shortFormRender' + ) + ).toBeVisible(); + }); + + test('shows error summary and inline error for empty custom field when Update preview is clicked (long tab)', async ({ + page, + }) => { + const previewPage = new TemplateMgmtPreviewLetterPage( + page + ).setPathParam( + 'templateId', + templates.authoringForUpdatePreviewErrors.id + ); + + await previewPage.loadPage(); + + await previewPage.longTab.clickTab(); + await previewPage.longTab.selectRecipient({ index: 1 }); + // leave custom fields empty + await previewPage.longTab.clickUpdatePreview(); + + await expect(previewPage.errorSummary).toBeVisible(); + await expect(previewPage.errorSummary).toContainText( + 'Enter example data for appointmentDate' + ); + + await expect( + previewPage.longTab.getInlineError( + 'custom-appointmentDate-longFormRender' + ) + ).toBeVisible(); + }); + + test('errors from one tab do not appear on the other tab', async ({ + page, + }) => { + const previewPage = new TemplateMgmtPreviewLetterPage( + page + ).setPathParam( + 'templateId', + templates.authoringForUpdatePreviewErrors.id + ); + + await previewPage.loadPage(); + + await previewPage.shortTab.clickUpdatePreview(); + + await expect(previewPage.errorSummary).toBeVisible(); + + // switch tab - errors should not be present in long tab + await previewPage.longTab.clickTab(); + + await expect( + previewPage.longTab.getInlineError( + 'system-personalisation-pack-id-longFormRender' + ) + ).toBeHidden(); + }); + }); + + test.describe('Approve template validation errors', () => { + test('shows error in summary for both missing examples when Approve template is clicked', async ({ + page, + }) => { + const previewPage = new TemplateMgmtPreviewLetterPage( + page + ).setPathParam('templateId', templates.authoringNoExamplesGenerated.id); + + await previewPage.loadPage(); + + await previewPage.clickContinueButton(); + + await expect(previewPage.errorSummary).toBeVisible(); + + const shortErrorLink = previewPage.errorSummaryLinks.filter({ + hasText: /Enter short example data/, + }); + await expect(shortErrorLink).toBeVisible(); + await expect(shortErrorLink).toHaveAttribute('href', '#tab-short'); + + const longErrorLink = previewPage.errorSummaryLinks.filter({ + hasText: /Enter long example data/, + }); + await expect(longErrorLink).toBeVisible(); + await expect(longErrorLink).toHaveAttribute('href', '#tab-long'); + }); + + test('shows only the short example error when long example is already generated', async ({ + page, + }) => { + const previewPage = new TemplateMgmtPreviewLetterPage( + page + ).setPathParam( + 'templateId', + templates.authoringOnlyLongExampleGenerated.id + ); + + await previewPage.loadPage(); + + await previewPage.clickContinueButton(); + + await expect(previewPage.errorSummary).toBeVisible(); + + const shortErrorLink = previewPage.errorSummaryLinks.filter({ + hasText: /Enter short example data/, + }); + await expect(shortErrorLink).toBeVisible(); + await expect(shortErrorLink).toHaveAttribute('href', '#tab-short'); + + const longErrorLink = previewPage.errorSummaryLinks.filter({ + hasText: /Enter long example data/, + }); + await expect(longErrorLink).toBeHidden(); + }); + + test('shows only the long example error when short example is already generated', async ({ + page, + }) => { + const previewPage = new TemplateMgmtPreviewLetterPage( + page + ).setPathParam( + 'templateId', + templates.authoringOnlyShortExampleGenerated.id + ); + + await previewPage.loadPage(); + + await previewPage.clickContinueButton(); + + await expect(previewPage.errorSummary).toBeVisible(); + + const longErrorLink = previewPage.errorSummaryLinks.filter({ + hasText: /Enter long example data/, + }); + await expect(longErrorLink).toBeVisible(); + await expect(longErrorLink).toHaveAttribute('href', '#tab-long'); + + const shortErrorLink = previewPage.errorSummaryLinks.filter({ + hasText: /Enter short example data/, + }); + await expect(shortErrorLink).toBeHidden(); + }); + + test('shows both short and long example errors when personalised renders have failed', async ({ + page, + }) => { + const previewPage = new TemplateMgmtPreviewLetterPage( + page + ).setPathParam( + 'templateId', + templates.authoringWithFailedPersonalisedRenders.id + ); + + await previewPage.loadPage(); + + await previewPage.clickContinueButton(); + + await expect(previewPage.errorSummary).toBeVisible(); + await expect( + previewPage.errorSummaryLinks.filter({ + hasText: /Enter short example data/, + }) + ).toBeVisible(); + await expect( + previewPage.errorSummaryLinks.filter({ + hasText: /Enter long example data/, + }) + ).toBeVisible(); + }); + }); }); });