Skip to content

CCM-8409: Email send confirmation screen#971

Open
ClareJonesBJSS wants to merge 7 commits into
mainfrom
feature/CCM-8409_email-send-confirmation
Open

CCM-8409: Email send confirmation screen#971
ClareJonesBJSS wants to merge 7 commits into
mainfrom
feature/CCM-8409_email-send-confirmation

Conversation

@ClareJonesBJSS
Copy link
Copy Markdown
Contributor

Description

Confirmation screen for email proof:

Screenshot 2026-05-22 at 15 02 50

Context

Type of changes

  • Refactoring (non-breaking change)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would change existing functionality)
  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • I am familiar with the contributing guidelines
  • I have followed the code style of the project
  • I have added tests to cover my changes
  • I have updated the documentation accordingly
  • This PR is a result of pair or mob programming

Sensitive Information Declaration

To ensure the utmost confidentiality and protect your and others privacy, we kindly ask you to NOT including PII (Personal Identifiable Information) / PID (Personal Identifiable Data) or any other sensitive data in this PR (Pull Request) and the codebase changes. We will remove any PR that do contain any sensitive information. We really appreciate your cooperation in this matter.

  • I confirm that neither PII/PID nor sensitive data are included in this PR and the codebase changes.

@ClareJonesBJSS ClareJonesBJSS requested a review from a team as a code owner May 22, 2026 15:00
<ContentRenderer
content={bannerBody}
variables={{
contactDetail: proofingRequest.contactDetailValue,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We chose to make the proof request's contactDetailValue the raw, rather than normalised value, so I think we need to strip leading/trailing whitespace here

Comment thread tests/test-team/template-mgmt-accessibility/email-templates.accessibility.spec.ts Outdated
@ClareJonesBJSS ClareJonesBJSS force-pushed the feature/CCM-8409_email-send-confirmation branch from dbef854 to 7e630fd Compare May 26, 2026 10:44
return redirect('/invalid-template', RedirectType.replace);
}

const backUrl = getPreviewURL(template);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

retrieving and validating the template with an extra API call feels like quite a lot of overhead just to generate a back link. not sure if there's a better way of doing it though.

  • back link going to message templates page feels less helpful for the user
  • just having a direct link to preview-email-template would present invalid options to the user if it was a submitted template (although maybe we could add a redirect if you go on preview-email-template with a submitted template?)
  • adding extra fields from the template to the proofing request API response so you can derive the back link without the extra API call might disproportionate extra effort on this ticket

what do you think?

Copy link
Copy Markdown
Contributor Author

@ClareJonesBJSS ClareJonesBJSS May 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmm I agree. I can't remember if preview-[x]-template validates that the target template is unsubmitted or not, and I think @nicki-nhs mentioned something about redirects last week when I asked about whether we were allowing proofs from already-submitted templates. It definitely feels to me like preview redirecting to preview submitted and vice versa would be a nice behaviour

import { faker } from '@faker-js/faker';

type ProofInput = Partial<
Omit<ProofRequestDbEntry, 'owner' | 'createdAt' | 'createdBy'>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we do without the omit? i've found being able to easily override any fields when generating test data is usually useful

);
await expect(page.getByText('NHS Notify - test')).toBeVisible();
await expect(sentPage.backToTemplateLink).toBeVisible();
await expect(sentPage.backToTemplateLink).toHaveText('Back to template');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check back link URL?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants