Skip to content

refactor(email): add default body fallback for test notification template#2654

Open
8emk10 wants to merge 1 commit intoseerr-team:developfrom
8emk10:pr1-test-email-template-default
Open

refactor(email): add default body fallback for test notification template#2654
8emk10 wants to merge 1 commit intoseerr-team:developfrom
8emk10:pr1-test-email-template-default

Conversation

@8emk10
Copy link

@8emk10 8emk10 commented Mar 7, 2026

Description

This PR adds a small fallback for the test email notification template.

Currently the template renders #{body} directly.
If the body variable is not provided, the email content may appear blank.

This change updates the template to:

#{body || 'Check check, 1, 2, 3. Are we coming in clear?'}

This ensures that a visible message is always rendered even if body is missing.

This change is intentionally minimal and only affects the test notification template.

It also helps prepare the template behavior for future improvements discussed around multi-language email templates, where template rendering may depend on localized content generation.

Modified file:

server/templates/email/test-email/html.pug

How Has This Been Tested?

  • Template compiled successfully with pug
  • Local development build succeeded
  • Container started successfully
  • Test email rendered correctly
  • Verified that the fallback message appears when body is undefined

Screenshots / Logs (if applicable)

Test email rendered locally showing the fallback message.

Checklist:

  • I have read and followed the contribution guidelines.
  • Disclosed any use of AI (see our policy).
  • I have updated the documentation accordingly.
  • All new and existing tests passed.
  • Successful build pnpm build
  • Translation keys pnpm i18n:extract
  • Database migration (if required)

Summary by CodeRabbit

  • Bug Fixes
    • Test email template now displays a default message when the body field is empty, ensuring test emails always contain visible content instead of appearing blank.

@8emk10 8emk10 requested a review from a team as a code owner March 7, 2026 19:11
@coderabbitai
Copy link

coderabbitai bot commented Mar 7, 2026

📝 Walkthrough

Walkthrough

This change adds a fallback message to the test email template's body field. When the body parameter is not provided or is falsy, the template now displays the default message "Check check, 1, 2, 3. Are we coming in clear?" instead of rendering an empty output.

Changes

Cohort / File(s) Summary
Email Template Fallback
server/templates/email/test-email/html.pug
Added default fallback message for the body field using logical OR operator to display a test message when body is falsy.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

A whisper through the template code,
When body's gone, a rabbit's ode:
"Check check, 1, 2, 3!" we hear,
The fallback makes our message clear! 🐰📧

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding a default body fallback to the test notification email template.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@8emk10 8emk10 changed the title email: add default body fallback for test notification template refactor(email): add default body fallback for test notification template Mar 7, 2026
Copy link
Contributor

@0xSysR3ll 0xSysR3ll left a comment

Choose a reason for hiding this comment

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

Can you update your PR to use our template please ? https://github.com/seerr-team/seerr/blob/develop/.github/PULL_REQUEST_TEMPLATE.md

@8emk10
Copy link
Author

8emk10 commented Mar 7, 2026

Updated the PR description to follow the project template. Thanks!

@8emk10 8emk10 requested a review from 0xSysR3ll March 7, 2026 20:30
@0xSysR3ll
Copy link
Contributor

0xSysR3ll commented Mar 8, 2026

I'm not sure I understand in which scenario body would be undefined.

In the current test notification flow the message is already hard-coded:

const sendTestNotification = async (agent: NotificationAgent, user: User) =>
  await agent.send(Notification.TEST_NOTIFICATION, {
    notifySystem: true,
    notifyAdmin: false,
    notifyUser: user,
    subject: 'Test Notification',
    message: 'Check check, 1, 2, 3. Are we coming in clear?',
  });

So the template should always receive a value for the body.

Could you clarify how body might end up being undefined in this case? I'm trying to understand the scenario this change is protecting against.

@8emk10
Copy link
Author

8emk10 commented Mar 8, 2026

Good point — you're right that in the current implementation the test notification flow always provides a message value:

message: 'Check check, 1, 2, 3. Are we coming in clear?'

So in the current code path the template should indeed always receive a body value.

The motivation for this change is mainly related to a longer-term goal around the email templates. Currently some email content is still partially defined in the notification sender. The idea is to gradually move email content fully into the templates so the templates become the single source of truth for rendering emails.

This is especially useful for future improvements around localization / multi-language email templates, where different template folders (e.g. en, de, etc.) would render the email content themselves.

In that scenario templates should ideally be able to render independently without relying on the sender always providing a message string.

That said, if you prefer to keep the template strictly dependent on the sender always providing message, I'm happy to remove the fallback. I mainly added it as a small defensive guard while experimenting with template-driven rendering.

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.

2 participants