CCM-14758: Preview approved letter template#885
CCM-14758: Preview approved letter template#885chris-elliott-nhsd wants to merge 3 commits intomainfrom
Conversation
chris-elliott-nhsd
force-pushed
the
feature/CCM-14758_approved-letter-template
branch
from
448013d to
e05c63d
Compare
chris-elliott-nhsd
force-pushed
the
feature/CCM-14758_approved-letter-template
branch
from
2a38e87 to
45da131
Compare
| title: content.pageTitle, | ||
| }; | ||
|
|
||
| const getPreviewURL = (template: TemplateDto) => { |
There was a problem hiding this comment.
Could this function be moved into utils?
There was a problem hiding this comment.
might also be looking at alongside / utilising the ones in enum
export const previewTemplatePages = (type: TemplateType) =>
`preview-${legacyTemplateTypeToUrlTextMappings(type)}-template`;
export const previewSubmittedTemplatePages = (type: TemplateType) =>
`preview-submitted-${legacyTemplateTypeToUrlTextMappings(type)}-template`;
so we dont have this sort of thing spread about the place :)
| await expect(page).toHaveURL(`${baseURL}/templates/invalid-template`); | ||
| }); | ||
|
|
||
| test('when user visits page with a fake template, then an invalid template error is displayed', async ({ |
| 'VALIDATION_FAILED' | ||
| ); | ||
|
|
||
| const letterProofApproved = TemplateFactory.createAuthoringLetterTemplate( |
There was a problem hiding this comment.
this should have long and short renders, and a letter variant
| @@ -0,0 +1,60 @@ | |||
| 'use client'; | |||
| language: 'en', | ||
| updatedAt: '2021-02-01T00:00:00.000Z', | ||
| lockNumber: 1, | ||
| files: { |
There was a problem hiding this comment.
might be worth adding campaignId, letterVariantId, files.shortFormRender and files.longFormRender, just to make the example more coherent
| import { AuthoringLetterTemplate } from 'nhs-notify-web-template-management-utils'; | ||
|
|
||
| const mockTemplate: AuthoringLetterTemplate = { | ||
| ...AUTHORING_LETTER_TEMPLATE, |
There was a problem hiding this comment.
This isn't realistic for an approved template, because it's lacking personalised renders and campaign.
| > | ||
| <iframe | ||
| aria-label="PDF preview of letter template with long example personalisation data" | ||
| src="/templates/files/undefined/renders/authoring-letter-template-id/render.pdf" |
There was a problem hiding this comment.
client ID is missing - it's being made required in Michael's PR
There was a problem hiding this comment.
This should also be the long form render filename, not the initial render
| }); | ||
|
|
||
| describe('previewApprovedTemplatePages', () => { | ||
| test('EMAIL', () => { |
There was a problem hiding this comment.
why is this test called 'EMAIL' ?
There was a problem hiding this comment.
should there be a test.each here?
| { template: templates.EMAIL, expectedStatus: 'Draft' }, | ||
| { template: templates.SMS, expectedStatus: 'Draft' }, | ||
| { template: templates.LETTER, expectedStatus: 'Proof approved' }, | ||
| { template: templates.LETTER, expectedStatus: 'Approved' }, |
There was a problem hiding this comment.
have we got e2e tests that are running with authoring off or not?
|
|
||
| export function LetterRender({ | ||
| template, | ||
| hideEditActions, |
There was a problem hiding this comment.
opinionated, feel free to ignore
editable?
| const messageTemplatesContent = content.pages.messageTemplates; | ||
|
|
||
| const generateViewTemplateLink = (template: TemplateDto): string => { | ||
| if ( |
There was a problem hiding this comment.
out of interest, if these links went to the normal preview page, would it then redirect to the correct approved/submitted page anyway?
There was a problem hiding this comment.
I think preview-letter page should redirect to preview-approved
frontend/src/components/organisms/PreviewPdfLetterTemplate/PreviewPdfLetterTemplate.tsx
Show resolved
Hide resolved
| const template = await getTemplate(templateId); | ||
|
|
||
| const validatedTemplate = validateSubmittedLetterTemplate(template); | ||
| const validatedTemplate = zodValidate( |
There was a problem hiding this comment.
do we need to update the zod validators so that they differentiate between versions of letters? or is the pdf stuff temporary enough its not worth it?
| @@ -23,6 +24,14 @@ import { useFeatureFlags } from '@providers/client-config-provider'; | |||
| const messageTemplatesContent = content.pages.messageTemplates; | |||
|
|
|||
| const generateViewTemplateLink = (template: TemplateDto): string => { | |||
There was a problem hiding this comment.
is this not the same as your getPreviewURL elsewhere?
| title: content.pageTitle, | ||
| }; | ||
|
|
||
| const getPreviewURL = (template: TemplateDto) => { |
There was a problem hiding this comment.
same comment as the other page
| ); | ||
| }); | ||
|
|
||
| test('redirects to preview submitted template page if template is submitted', async ({ |
There was a problem hiding this comment.
| test('redirects to preview approved template page if template is submitted', async ({ |
| ); | ||
| }); | ||
|
|
||
| test('redirects to preview submitted template page if template is submitted', async ({ |
There was a problem hiding this comment.
| test('redirects to preview approved template page if template is submitted', async ({ |
4 participants
Description
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.