CCM-8409: Email send confirmation screen#971
Conversation
…CCM-8409_email-send-confirmation
| <ContentRenderer | ||
| content={bannerBody} | ||
| variables={{ | ||
| contactDetail: proofingRequest.contactDetailValue, |
There was a problem hiding this comment.
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
…CCM-8409_email-send-confirmation
dbef854 to
7e630fd
Compare
…CCM-8409_email-send-confirmation
| return redirect('/invalid-template', RedirectType.replace); | ||
| } | ||
|
|
||
| const backUrl = getPreviewURL(template); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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'> |
There was a problem hiding this comment.
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'); |
There was a problem hiding this comment.
check back link URL?
Description
Confirmation screen for email proof:
Context
Type of changes
Checklist
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.