Sending new email templates bg #779

BilalG1 · 2025-07-18T00:54:09Z

Important

Refactor email system to use new templates and themes, updating rendering, error handling, and tests.

Email Rendering:
- Introduced renderEmailWithTemplate in email-templates.tsx for rendering emails with templates and themes.
- Updated renderEmailWithTheme in email-themes.tsx to return Result objects.
- Replaced inline error checks with result.status === "error" in route.tsx files.
Template and Theme Management:
- Added getNewEmailTemplate and getActiveEmailTheme functions for fetching templates and themes.
- Moved renderEmailWithTemplate from email-themes.tsx to email-templates.tsx.
- Updated DEFAULT_EMAIL_TEMPLATES and DEFAULT_EMAIL_THEMES in emails.ts and emails.tsx.
Error Handling:
- Replaced string error checks with result.status checks in route.tsx files.
- Updated error handling in _sendEmailWithoutRetries in emails.tsx.
Testing:
- Updated OTP tests in send-sign-in-code.test.ts and sign-in.test.ts to match new email format.
- Adjusted test cases to check for JSON formatted OTP codes.

^{This description was created by}^{for 8831ddd. You can customize this summary. It will automatically update as commits are pushed.}

vercel · 2025-07-18T00:54:13Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
stack-backend	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jul 18, 2025 1:28am
stack-dashboard	🛑 Canceled (Inspect)			Jul 18, 2025 1:28am
stack-demo	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jul 18, 2025 1:28am
stack-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jul 18, 2025 1:28am

greptile-apps

Greptile Summary

This PR implements a significant overhaul of the email template system with several key architectural improvements:

Introduces a new Result-based pattern for email rendering that provides better type safety and error handling
Implements proper theme support with light/dark variants and tenancy-aware theme selection
Moves template IDs into named constants and exports a DEFAULT_TEMPLATE_IDS mapping
Restructures how email templates are rendered by separating theme and template concerns
Updates OTP email format to use a more structured JSON-like format

The changes create a more maintainable and robust email system while improving type safety throughout the codebase. The separation of themes from templates and the introduction of proper Result types shows good architectural thinking.

Confidence score: 4/5

This PR is safe to merge with careful testing of email template rendering
High confidence due to comprehensive test updates and consistent error handling patterns, but email rendering changes need thorough verification
Key files needing attention:
- apps/backend/src/lib/email-templates.tsx
- apps/backend/src/lib/emails.tsx
- E2E tests for email rendering

_{12 files reviewed, 1 comment}
_{Edit PR Review Bot Settings | Greptile}

greptile-apps · 2025-07-18T00:54:45Z

apps/backend/src/lib/emails.tsx

@@ -318,22 +321,30 @@ export async function sendEmailFromTemplate(options: {
  extraVariables: Record<string, string | null>,
  version?: 1 | 2,
 }) {
-  const template = await getEmailTemplateWithDefault(options.tenancy.project.id, options.templateType, options.version);
-
+  const template = getNewEmailTemplate(options.tenancy, options.templateType);


style: getNewEmailTemplate is used but version param is still accepted. Consider removing unused version param if it's no longer needed.

recurseml · 2025-07-18T00:55:17Z

apps/backend/src/app/api/latest/emails/send-email/route.tsx

@@ -118,7 +118,7 @@ export const POST = createSmartRouteHandler({
        activeTheme.tsxSource,
        unsubscribeLink || undefined
      );
-      if ("error" in renderedEmail) {
+      if (renderedEmail.status === "error") {


Incorrect error checking pattern. The renderEmailWithTheme function returns a Result type where errors are detected using 'error in renderedEmail'. The new code checks for a non-existent 'status' property, which will fail to catch errors and lead to runtime errors when trying to access .data on error cases. Should keep the original 'if ("error" in renderedEmail)' check.

Suggested change

if (renderedEmail.status === "error") {

if ("error" in renderedEmail) {

React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)

recurseml · 2025-07-18T00:55:19Z

apps/backend/src/app/api/latest/emails/send-email/route.tsx

@@ -129,8 +129,8 @@ export const POST = createSmartRouteHandler({
          emailConfig,
          to: primaryEmail,
          subject: body.subject,
-          html: renderedEmail.html,
-          text: renderedEmail.text,
+          html: renderedEmail.data.html,


Invalid property access. The renderEmailWithTheme function returns success results in the format { html: string, text: string } directly, not nested under a 'data' property. This will cause undefined errors when trying to access renderedEmail.data.html. Should use renderedEmail.html directly as in the original code.

Suggested change

html: renderedEmail.data.html,

html: renderedEmail.html,

React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)

recurseml · 2025-07-18T00:55:21Z

😱 Found 2 issues. Time to roll up your sleeves! 😱

🗒️ View all ignored comments in this repo

The constraint 'TokenStoreType extends string' is too restrictive. It should likely be 'TokenStoreType extends string | object' to match the condition check in line 113 where TokenStoreType is checked against {}
Return type mismatch - the interface declares useUsers() returning ServerUser[] but the Team interface that this extends declares useUsers() returning TeamUser[]
There is a syntax error in the super constructor call due to the ellipsis operator used incorrectly. Objects aren't being merged correctly. This syntax usage can lead to runtime errors when trying to pass the merged object to 'super()'. Verify that the intended alterations to the object occur before or outside of the super() call if needed.
Throwing an error when no active span is found is too aggressive. The log function should gracefully fallback to console.log or another logging mechanism when there's no active span, since not all execution contexts will have an active span. This makes the code less resilient and could break functionality in non-traced environments.

📚 Relevant Docs

Function sets backendContext with a new configuration but doesn't pass 'defaultProjectKeys'. Since defaultProjectKeys is required in the type definition and cannot be updated (throws error if tried to set), this will cause a type error.
The schema is using array syntax for pick() which is incorrect for Yup schemas. The pick() method in Yup expects individual arguments, not an array. Should be changed to: emailConfigSchema.pick('type', 'host', 'port', 'username', 'sender_name', 'sender_email')

📚 Relevant Docs

Creating a refresh token with current timestamp as expiration means it expires immediately. Should set a future date for token expiration.
The 'tools' object is initialized as an empty object, even though 'tools' is presumably expected to contain tool definitions. This could cause the server capabilities to lack necessary tool configurations, thus potentially impacting functionalities that depend on certain tool setups.

📚 Relevant Docs

'STACK_SECRET_SERVER_KEY' is potentially being included in every request header without checking its existence again here. Although it's checked during initialization, this could lead to security issues as it's exposed in all communications where the header is logged or captured.

📚 Relevant Docs

When adding 'use client' directive at the beginning, it doesn't check if file.text already contains the 'use client' directive. This could lead to duplicate 'use client' directives if the file already has one.

📚 Relevant Docs

⚠️ Only 5 files were analyzed due to processing limits.

^{Need help? Join our Discord for support!
https://discord.gg/qEjHQk64Z9}

BilalG1 added 5 commits July 17, 2025 17:52

sending new templates

64aa260

add file

6a51da5

fix mock email templating

89db110

fix otp tests

09fee82

remove log

dbd944f

github-actions bot assigned BilalG1 Jul 18, 2025

greptile-apps bot reviewed Jul 18, 2025

View reviewed changes

recurseml bot reviewed Jul 18, 2025

View reviewed changes

BilalG1 added 2 commits July 17, 2025 18:01

fix missing export

8831ddd

merge

54502ca

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Sending new email templates bg #779

Sending new email templates bg #779

Uh oh!

BilalG1 commented Jul 18, 2025 •

edited by ellipsis-dev bot

Loading

Uh oh!

vercel bot commented Jul 18, 2025 •

edited

Loading

Uh oh!

greptile-apps bot left a comment

Uh oh!

greptile-apps bot Jul 18, 2025

Uh oh!

recurseml bot Jul 18, 2025

Uh oh!

recurseml bot Jul 18, 2025

Uh oh!

recurseml bot commented Jul 18, 2025

Uh oh!

Uh oh!

	if (renderedEmail.status === "error") {
	if ("error" in renderedEmail) {

Sending new email templates bg #779

Are you sure you want to change the base?

Sending new email templates bg #779

Uh oh!

Conversation

BilalG1 commented Jul 18, 2025 • edited by ellipsis-dev bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vercel bot commented Jul 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Greptile Summary

Confidence score: 4/5

Uh oh!

greptile-apps bot Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

recurseml bot Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

recurseml bot Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

recurseml bot commented Jul 18, 2025

Uh oh!

Uh oh!

BilalG1 commented Jul 18, 2025 •

edited by ellipsis-dev bot

Loading

vercel bot commented Jul 18, 2025 •

edited

Loading