Skip to content

fix: use sample data for email previews - #1941

Merged
GrantDLS merged 4 commits into
developfrom
email-preview-fix
Oct 30, 2026
Merged

GrantDLS merged 4 commits into
developfrom
email-preview-fix

Conversation

@GrantDLS

Copy link
Copy Markdown
Contributor

Description

After the addition of new data provided to email templates, preview functionality is now broken:

image

This is caused by the preview data using strings, so when, for example, an array function is called on it, an error is returned. This PR provides some sample data to the template preview for any values which can't just be substituted with strings.

Screenshot From 2026-10-29 09-57-46

@GrantDLS
GrantDLS requested a review from a team as a code owner October 29, 2026 10:00
@GrantDLS
GrantDLS requested review from TCMeldrum and removed request for a team October 29, 2026 10:00
@@ -0,0 +1,67 @@
import {
buildSubstitutionData,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think it would make more sense just to edit the buildSubstitutionData object instead of importing it like this. It isn't actually used anywhere in the codebase other than in EmailTemplateQueries so there is no need to use this object structure.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've moved things around now so that they make more sense. Everything to do with gathering sample data is in one file, and then the existing rendering code is in another. With all the extra sample data, I think having it in one file would be too large and not have proper separation of concerns.

TemplateVariable,
} from './emailTemplateRenderer';

const PROPOSAL_WORKFLOW_PREVIEW_DATA: Record<string, unknown> = {

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.

This covers proposals, instruments, technicalReviews and call, but the real proposal workflow email (emailActionHandler.ts) passes several other arrays that will still break the preview the same way:

  • coProposers (BasicUserDetails[])
  • techniques (Technique[])
  • samples (Answer[])
  • hazards (Answer[])
  • fapMeetingDecisions (FapMeetingDecision[])

For example, #{coProposers.map(u => u.firstName).join(', ')} or - const decision = fapMeetingDecisions.find(d => d.instrumentId === 1) will still fail with ... is not a function, because the placeholder is a string.

Could you add sample entries for these too, so the preview works for any template built from the standard email data?

@GrantDLS

Copy link
Copy Markdown
Contributor Author

@jekabs-karklins and @zacharyjhankin I've pushed changes addressing both of your comments.

I have a slight concern that the preview sample data is separate from the actual code that passes the real data to the templates (in emailActionHandler.ts), so these could become out of sync over time, but I can't think of a sensible way around this for now.

@jekabs-karklins
jekabs-karklins self-requested a review October 30, 2026 09:25
@zacharyjhankin

Copy link
Copy Markdown
Collaborator

Tested it locally and works

@zacharyjhankin
zacharyjhankin self-requested a review October 30, 2026 13:22

@zacharyjhankin zacharyjhankin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Good fix

@GrantDLS
GrantDLS merged commit b5c6212 into develop Oct 30, 2026
26 checks passed
@GrantDLS
GrantDLS deleted the email-preview-fix branch October 30, 2026 13:27
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