Add template support and enhanced error handling to email sending API#813
Add template support and enhanced error handling to email sending API#813
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis change refactors and extends the email sending system to support template- and theme-based emails with dynamic variables and unsubscribe link handling. It updates backend APIs, type declarations, and client interfaces, removes deprecated admin-only methods, and introduces new server-side email-sending interfaces and result types to enable more flexible and granular email delivery. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ServerAPI
participant EmailRenderer
participant ThemeSelector
participant TemplateStore
participant User
Client->>ServerAPI: POST /emails/send-email (userIds, templateId, variables, themeId, etc.)
ServerAPI->>ThemeSelector: Get theme by themeId (if provided)
ServerAPI->>TemplateStore: Get template by templateId or use raw HTML
loop For each user
ServerAPI->>EmailRenderer: Render email (template, theme, variables, unsubscribeLink)
EmailRenderer-->>ServerAPI: Rendered email, notificationCategory
ServerAPI->>User: Check notificationCategory enabled
alt Notifications enabled
ServerAPI->>User: Send email
else Notifications disabled
ServerAPI-->>User: Skip email
end
end
ServerAPI-->>Client: Per-user result array (userId, email)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
✨ No issues found! Your code is sparkling clean! ✨ 🗒️ View all ignored comments in this repo
Need help? Join our Discord for support! |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Summary
This PR significantly enhances Stack Auth's email sending system by adding template support and improving error handling across the entire email infrastructure. The changes represent a major architectural shift from a simple HTML-based email system to a comprehensive template-based approach.
Key Changes:
-
New Email Sending API: Replaces the old
sendEmailmethod inStackAdminAppwith a more robust implementation inStackServerApp. The new API supports both HTML content and template-based emails with variables, themes, and proper error handling. -
Template System Enhancements: Updates email rendering to support unsubscribe links at the theme level rather than hardcoded in templates. This provides better customization and styling control for developers.
-
Type Safety Improvements: Adds comprehensive TypeScript definitions including
SendEmailOptions,SendEmailResult, andThemePropstypes to support the new functionality with proper IntelliSense. -
Enhanced Error Handling: Implements
Resulttypes with specific known errors (RequiresCustomEmailServer,SchemaError) to provide better error reporting and handling. -
Bulk Email Support: The new API can send emails to multiple users simultaneously and returns per-user results, making it suitable for batch operations.
-
Unsubscribe Compliance: Adds standardized unsubscribe link functionality to both Light and Dark email themes, supporting regulatory compliance requirements.
The changes maintain backward compatibility while providing a foundation for more sophisticated email campaigns and better developer experience through improved type safety and error reporting.
PR Description Notes:
- The PR body is empty and could benefit from a detailed description of the changes and their impact
Confidence score: 3/5
- This PR introduces significant architectural changes that should work well but has some performance and complexity concerns
- The dual rendering approach for template emails could impact performance, and there are edge cases in error handling that need attention
- Files that need more attention:
apps/backend/src/app/api/latest/emails/send-email/route.tsxfor performance optimization and error handling improvements
14 files reviewed, 4 comments
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-templates/page-client.tsx
Show resolved
Hide resolved
Documentation Changes Requireddocs/templates/sdk/objects/stack-app.mdx
Note on Other FilesNo changes are required for other documentation files, as the new types |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-templates/page-client.tsx (1)
64-64: Path correction improves consistency.The updated path
email-templates/${sharedSmtpWarningDialogOpen}now aligns with the routing pattern used elsewhere in the component (line 47), providing better consistency.However, as noted in a previous review, consider using tagged template literals for URL construction to ensure proper encoding:
- router.push(`email-templates/${sharedSmtpWarningDialogOpen}`); + router.push(urlString`email-templates/${sharedSmtpWarningDialogOpen}`);packages/template/src/lib/stack-app/email/index.ts (1)
17-17: UseRecord<string, unknown>for better type safety.Using
Record<string, any>for variables could lead to runtime type errors. Consider usingRecord<string, unknown>for better type safety.- variables?: Record<string, any>, + variables?: Record<string, unknown>,packages/stack-shared/src/interface/server-interface.ts (1)
832-837: Define proper interface for API response structure.Using
anytype for result mapping loses type safety. Consider defining a proper interface for the API response structure instead of casting toany.- const results = data.results.map((result: any) => ({ + interface ApiEmailResult { + user_id: string; + user_email?: string; + success: boolean; + error?: string; + } + const results = data.results.map((result: ApiEmailResult) => ({packages/stack-shared/src/helpers/emails.ts (1)
101-101: Remove trailing space.Extra trailing space after
ThemeProps.-} satisfies Partial<ThemeProps> +} satisfies Partial<ThemeProps>apps/backend/src/lib/email-rendering.tsx (1)
101-101: Clarify JSX interpolation syntax.The nested template literal and JSX expression syntax is unclear and hard to read. Consider refactoring for better clarity.
- {${previewMode ? "EmailTheme.PreviewProps?.children ?? " : ""} EmailTemplateWithProps} + {previewMode ? (EmailTheme.PreviewProps?.children ?? EmailTemplateWithProps) : EmailTemplateWithProps}
🧹 Nitpick comments (3)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/emails/page-client.tsx (1)
348-348: LGTM! Property name updated to match new API.The change from
contenttohtmlcorrectly aligns with the updated backend email sending API that now supports HTML content and template-based emails.Consider updating the form field name from
contenttohtmlfor consistency:-const handleSend = async (formData: { subject?: string, content?: string, notificationCategoryName?: string }) => { +const handleSend = async (formData: { subject?: string, html?: string, notificationCategoryName?: string }) => { if (!formData.subject || !formData.content || !formData.notificationCategoryName) {And update the corresponding form field name in the render function (line 505).
apps/backend/src/app/api/latest/emails/send-email/route.tsx (2)
100-122: Consider optimizing the double-rendering pattern.The current implementation renders template-based emails twice - once to extract the notification category and again for the final email. This could impact performance when sending bulk emails.
Consider refactoring to render once and extract all necessary data, or implement a lighter-weight method to determine the notification category from templates without full rendering.
166-172: Consider preserving error details for debugging.The subject fallback logic is good. However, the catch block discards valuable error information that could help with debugging email delivery issues.
Consider logging the actual error or including more specific error messages:
- } catch { + } catch (error) { + console.error(`Failed to send email to user ${userId}:`, error); userSendErrors.set(userId, "Failed to send email"); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/backend/src/app/api/latest/emails/render-email/route.tsx(1 hunks)apps/backend/src/app/api/latest/emails/send-email/route.tsx(6 hunks)apps/backend/src/lib/email-rendering.tsx(3 hunks)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-templates/page-client.tsx(1 hunks)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/emails/page-client.tsx(1 hunks)apps/dashboard/src/components/vibe-coding/code-editor.tsx(1 hunks)packages/stack-shared/src/helpers/emails.ts(4 hunks)packages/stack-shared/src/interface/admin-interface.ts(0 hunks)packages/stack-shared/src/interface/server-interface.ts(1 hunks)packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts(0 hunks)packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts(2 hunks)packages/template/src/lib/stack-app/apps/interfaces/admin-app.ts(0 hunks)packages/template/src/lib/stack-app/apps/interfaces/server-app.ts(2 hunks)packages/template/src/lib/stack-app/email/index.ts(1 hunks)
💤 Files with no reviewable changes (3)
- packages/template/src/lib/stack-app/apps/interfaces/admin-app.ts
- packages/stack-shared/src/interface/admin-interface.ts
- packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{ts,tsx}: TypeScript with strict types, prefertypeoverinterface
Avoid casting toany; Prefer making changes to the API so thatanycasts are unnecessary to access a property or method
Files:
apps/backend/src/app/api/latest/emails/render-email/route.tsxapps/dashboard/src/components/vibe-coding/code-editor.tsxpackages/template/src/lib/stack-app/email/index.tsapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-templates/page-client.tsxpackages/template/src/lib/stack-app/apps/interfaces/server-app.tsapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/emails/page-client.tsxpackages/template/src/lib/stack-app/apps/implementations/server-app-impl.tspackages/stack-shared/src/interface/server-interface.tsapps/backend/src/lib/email-rendering.tsxpackages/stack-shared/src/helpers/emails.tsapps/backend/src/app/api/latest/emails/send-email/route.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{js,jsx,ts,tsx}: 2-space indentation, spaces in braces, semicolons required
Return promises withreturn await, no floating promises
Proper error handling for async code with try/catch
Use helper functions:yupXyz()for validation,getPublicEnvVar()for env
Switch cases must use blocks
Files:
apps/backend/src/app/api/latest/emails/render-email/route.tsxapps/dashboard/src/components/vibe-coding/code-editor.tsxpackages/template/src/lib/stack-app/email/index.tsapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-templates/page-client.tsxpackages/template/src/lib/stack-app/apps/interfaces/server-app.tsapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/emails/page-client.tsxpackages/template/src/lib/stack-app/apps/implementations/server-app-impl.tspackages/stack-shared/src/interface/server-interface.tsapps/backend/src/lib/email-rendering.tsxpackages/stack-shared/src/helpers/emails.tsapps/backend/src/app/api/latest/emails/send-email/route.tsx
**/*.{jsx,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{jsx,tsx}: React Server Components preferred where applicable
No direct 'use' imports from React (use React.use instead)
Files:
apps/backend/src/app/api/latest/emails/render-email/route.tsxapps/dashboard/src/components/vibe-coding/code-editor.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-templates/page-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/emails/page-client.tsxapps/backend/src/lib/email-rendering.tsxapps/backend/src/app/api/latest/emails/send-email/route.tsx
🧠 Learnings (8)
apps/backend/src/app/api/latest/emails/render-email/route.tsx (4)
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{ts,tsx} : Avoid casting to any; Prefer making changes to the API so that any casts are unnecessary to access a property or method
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to apps/e2e/**/*.test.{ts,tsx} : Import test utilities from /apps/e2e/test/helpers.ts
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{jsx,tsx} : No direct 'use' imports from React (use React.use instead)
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Proper error handling for async code with try/catch
apps/dashboard/src/components/vibe-coding/code-editor.tsx (3)
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{ts,tsx} : Avoid casting to any; Prefer making changes to the API so that any casts are unnecessary to access a property or method
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{ts,tsx} : TypeScript with strict types, prefer type over interface
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{jsx,tsx} : No direct 'use' imports from React (use React.use instead)
packages/template/src/lib/stack-app/email/index.ts (2)
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{ts,tsx} : Avoid casting to any; Prefer making changes to the API so that any casts are unnecessary to access a property or method
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{ts,tsx} : TypeScript with strict types, prefer type over interface
packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts (1)
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{jsx,tsx} : React Server Components preferred where applicable
packages/stack-shared/src/interface/server-interface.ts (2)
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{ts,tsx} : Avoid casting to any; Prefer making changes to the API so that any casts are unnecessary to access a property or method
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{ts,tsx} : TypeScript with strict types, prefer type over interface
apps/backend/src/lib/email-rendering.tsx (3)
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{jsx,tsx} : React Server Components preferred where applicable
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{jsx,tsx} : No direct 'use' imports from React (use React.use instead)
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Use helper functions: yupXyz() for validation, getPublicEnvVar() for env
packages/stack-shared/src/helpers/emails.ts (3)
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : 2-space indentation, spaces in braces, semicolons required
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{jsx,tsx} : No direct 'use' imports from React (use React.use instead)
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{jsx,tsx} : React Server Components preferred where applicable
apps/backend/src/app/api/latest/emails/send-email/route.tsx (1)
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Use helper functions: yupXyz() for validation, getPublicEnvVar() for env
🧬 Code Graph Analysis (3)
packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts (2)
packages/template/src/lib/stack-app/email/index.ts (2)
SendEmailOptions(10-18)SendEmailResult(20-25)packages/stack-shared/src/known-errors.tsx (2)
KnownErrors(1418-1420)KnownErrors(1422-1532)
packages/stack-shared/src/interface/server-interface.ts (1)
packages/stack-shared/src/known-errors.tsx (2)
KnownErrors(1418-1420)KnownErrors(1422-1532)
apps/backend/src/lib/email-rendering.tsx (2)
packages/stack-shared/src/utils/html.tsx (1)
html(30-32)packages/stack-shared/src/utils/strings.tsx (1)
deindent(235-238)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: check_prisma_migrations (22.x)
- GitHub Check: all-good
- GitHub Check: docker
- GitHub Check: lint_and_build (latest)
- GitHub Check: setup-tests
- GitHub Check: restart-dev-and-test
- GitHub Check: build (22.x)
- GitHub Check: build (22.x)
- GitHub Check: Security Check
🔇 Additional comments (18)
apps/backend/src/app/api/latest/emails/render-email/route.tsx (1)
5-5: LGTM! Clean removal of unused imports.The unused
StackAssertionErrorandcaptureErrorimports were properly removed, leaving only the actively usedStatusErrorimport.apps/dashboard/src/components/vibe-coding/code-editor.tsx (1)
93-96: LGTM! Well-structured type definition for email themes.The
ThemePropstype properly defines the interface for email theme components, with requiredchildrenand optionalunsubscribeLinkproperties. This supports the enhanced email theming system with unsubscribe functionality.packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts (2)
31-31: Proper import for email functionality.The import statement correctly brings in the required types for the new email sending capability.
945-947: Well-implemented email sending method.The
sendEmailmethod follows established patterns in the codebase:
- Proper async/await with Result type for error handling
- Clean delegation to the interface layer
- Type-safe with
SendEmailOptionsandSendEmailResult[]- Includes appropriate error types (
RequiresCustomEmailServer,SchemaError)This implementation supports the enhanced email system with template and theme capabilities while maintaining consistency with the existing server app architecture.
packages/template/src/lib/stack-app/apps/interfaces/server-app.ts (1)
1-1: LGTM! Well-structured interface additions.The new imports and
sendEmailmethod are properly typed with explicit error handling using theResultpattern. The method signature clearly defines the expected input/output types and specific error cases.Also applies to: 7-8, 52-52
packages/template/src/lib/stack-app/email/index.ts (1)
20-25: LGTM! Well-structured result type.The
SendEmailResulttype appropriately captures the result of email sending operations with proper optional fields and clear property types.packages/stack-shared/src/helpers/emails.ts (2)
4-4: LGTM! Proper unsubscribe link implementation.The changes correctly add unsubscribe link support to the light email theme with proper conditional rendering and preview props for development.
Also applies to: 45-46, 48-48, 57-62, 69-71
75-76: LGTM! Consistent dark theme implementation.The dark theme changes properly mirror the light theme implementation with correct unsubscribe link support.
Also applies to: 78-78, 87-92, 99-100
apps/backend/src/lib/email-rendering.tsx (3)
34-43: LGTM! Good separation of concerns.Simplifying
createTemplateComponentFromHtmlby removing the unsubscribeLink parameter and moving that responsibility to the theme component level is a clean refactor.
52-52: LGTM! Proper parameter addition.Adding
unsubscribeLinkto the options parameter correctly supports the new theme-based unsubscribe link handling.
98-100: LGTM! Correct prop-based implementation.The logic correctly passes
unsubscribeLinkas a prop toEmailThemeand handles both preview and normal modes appropriately.Also applies to: 102-102
apps/backend/src/app/api/latest/emails/send-email/route.tsx (7)
1-1: LGTM! Import changes align with new template functionality.The new imports properly support the template-based email sending feature.
Also applies to: 6-6, 9-9, 11-11
22-23: Clear and accurate API documentation.The updated description properly documents both HTML and template-based email options.
32-37: Well-structured schema for dual email modes.The schema properly supports both HTML and template-based emails with appropriate optional fields.
58-58: Improved error handling with standardized error types.Good use of
KnownErrorsfor consistent error responses. The validation for eitherhtmlortemplate_idis properly placed.Regarding the past review comment about line 64: The use of
throwErrhere is appropriate since this is for the default notification category (not per-user), and a missing category represents a configuration error that should fail fast.Also applies to: 60-62, 64-64
63-69: Clean template and theme resolution logic.The code properly handles both template-based and HTML-based emails with appropriate error handling for missing templates.
124-145: Proper notification preferences and unsubscribe handling.The code correctly checks user notification preferences and generates unsubscribe links only when the category allows disabling.
147-160: Well-structured email rendering with proper context.The email rendering includes all necessary data and handles rendering errors appropriately.
N2D4
left a comment
N2D4
left a comment
There was a problem hiding this comment.
can you add some E2E tests for the new behavior (both JS library and the API)? otherwise looks great
packages/template/src/lib/stack-app/apps/interfaces/server-app.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/e2e/tests/js/email.test.ts (1)
202-202: Remove trailing empty line.There appears to be an extra line at the end of the file that should be cleaned up for better code formatting.
-
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/backend/src/app/api/latest/emails/render-email/route.tsx(1 hunks)apps/backend/src/app/api/latest/emails/send-email/route.tsx(6 hunks)apps/backend/src/lib/email-rendering.tsx(3 hunks)apps/e2e/tests/js/email.test.ts(1 hunks)packages/stack-shared/src/interface/admin-interface.ts(0 hunks)packages/stack-shared/src/interface/server-interface.ts(1 hunks)packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts(0 hunks)packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts(2 hunks)packages/template/src/lib/stack-app/apps/interfaces/server-app.ts(2 hunks)packages/template/src/lib/stack-app/email/index.ts(1 hunks)
💤 Files with no reviewable changes (2)
- packages/stack-shared/src/interface/admin-interface.ts
- packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts
✅ Files skipped from review due to trivial changes (1)
- apps/backend/src/app/api/latest/emails/render-email/route.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/template/src/lib/stack-app/email/index.ts
- packages/template/src/lib/stack-app/apps/interfaces/server-app.ts
- packages/stack-shared/src/interface/server-interface.ts
- apps/backend/src/app/api/latest/emails/send-email/route.tsx
- apps/backend/src/lib/email-rendering.tsx
- packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{ts,tsx}: TypeScript with strict types, prefertypeoverinterface
Avoid casting toany; Prefer making changes to the API so thatanycasts are unnecessary to access a property or method
Files:
apps/e2e/tests/js/email.test.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{js,jsx,ts,tsx}: 2-space indentation, spaces in braces, semicolons required
Return promises withreturn await, no floating promises
Proper error handling for async code with try/catch
Use helper functions:yupXyz()for validation,getPublicEnvVar()for env
Switch cases must use blocks
Files:
apps/e2e/tests/js/email.test.ts
apps/e2e/**/*.test.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Import test utilities from
/apps/e2e/test/helpers.ts
Files:
apps/e2e/tests/js/email.test.ts
**/*.test.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Prefer inline snapshot testing with
expect(response).toMatchInlineSnapshot(...)
Files:
apps/e2e/tests/js/email.test.ts
🧠 Learnings (1)
📚 Learning: applies to apps/e2e/**/*.test.{ts,tsx} : import test utilities from `/apps/e2e/test/helpers.ts`...
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to apps/e2e/**/*.test.{ts,tsx} : Import test utilities from `/apps/e2e/test/helpers.ts`
Applied to files:
apps/e2e/tests/js/email.test.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: build (22.x)
- GitHub Check: build (22.x)
- GitHub Check: docker
- GitHub Check: docker
- GitHub Check: restart-dev-and-test
- GitHub Check: all-good
- GitHub Check: setup-tests
- GitHub Check: lint_and_build (latest)
- GitHub Check: Security Check
🔇 Additional comments (9)
apps/e2e/tests/js/email.test.ts (9)
23-39: LGTM! Well-structured test for HTML email sending.The test properly sets up the environment, creates a user with verified email, sends an email with HTML content, and verifies the successful result. Good use of async/await and clear test structure.
41-57: LGTM! Good test coverage for template-based emails.The test effectively verifies the template-based email functionality using predefined template IDs. Follows the same solid test structure as the HTML test.
59-80: LGTM! Comprehensive test for bulk email functionality.The test properly validates sending emails to multiple users by creating two users and including both in the userIds array. Good coverage of the bulk email sending feature.
82-99: LGTM! Good test for email theme customization.The test effectively validates the theme functionality by using the DEFAULT_EMAIL_THEME_ID. Maintains consistent test structure and focuses on a specific feature.
101-118: LGTM! Good test for notification category functionality.The test properly validates the notification category feature by including the notificationCategoryName parameter. Consistent with other test structures and focuses on a specific feature.
120-138: LGTM! Excellent error handling test for missing email server configuration.The test properly validates the RequiresCustomEmailServer error case by skipping the email server setup. Good use of type narrowing to check the specific error instance.
141-157: LGTM! Thorough error handling test for non-existent users.The test effectively validates the UserIdDoesNotExist error case using a properly formatted UUID. Good use of inline snapshot testing as recommended in the coding guidelines, and proper type narrowing for error handling.
159-178: LGTM! Good schema validation error test.The test properly validates the SchemaError case when required email content is missing. The use of
as anyis acceptable here since it's intentionally testing invalid input that would be caught at compile time. Good use of inline snapshot testing and proper error type checking.
180-201: LGTM! Good test for mutually exclusive parameters validation.The test effectively validates the SchemaError case when both html and templateId are provided simultaneously, which should be mutually exclusive. Proper error type checking and inline snapshot testing usage.
<!-- Make sure you've read the CONTRIBUTING.md guidelines: https://github.com/stack-auth/stack-auth/blob/dev/CONTRIBUTING.md --> <!-- ELLIPSIS_HIDDEN --> ---- > [!IMPORTANT] > Enhance email functionality by updating mock server setup, email templates, and tests, and switching Freestyle mock to Bun. > > - **Behavior**: > - Removed mock API key handling in `renderEmailWithTemplate()` in `email-rendering.tsx`. > - Updated `Freestyle` class in `freestyle.tsx` to set `baseUrl` for mock environments. > - Changed port for Freestyle mock from 8119 to 8122 in `docker.compose.yaml` and `index.html`. > - **Docker**: > - Updated `freestyle-mock/Dockerfile` to use Bun instead of Deno. > - Updated `docker.compose.yaml` to reflect new Freestyle mock setup. > - **Tests**: > - Updated email-related tests in `send-email.test.ts`, `render-email.test.ts`, and others to reflect changes in email handling. > - Added tests for handling invalid user IDs and missing email content. > - **Misc**: > - Updated `esbuild.tsx` to use a new URL for `esbuild.wasm`. > - Minor updates to `server-app-impl.ts` for email sending logic. > > <sup>This description was created by </sup>[<img alt="Ellipsis" src="https://img.shields.io/badge/Ellipsis-blue?color=175173">](https://www.ellipsis.dev?ref=stack-auth%2Fstack-auth&utm_source=github&utm_medium=referral)<sup> for 4137933. You can [customize](https://app.ellipsis.dev/stack-auth/settings/summaries) this summary. It will automatically update as commits are pushed.</sup> <!-- ELLIPSIS_HIDDEN --> --------- Co-authored-by: Konsti Wohlwend <n2d4xc@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
apps/e2e/tests/backend/endpoints/api/v1/send-email.test.ts (2)
52-87: Test name doesn't match the test behaviorThe test is named "should return 200 with user not found error in results" but it actually creates a valid user and successfully sends an email. The response shows a successful email delivery, not a user not found error.
Consider renaming this test to reflect what it actually tests, such as "should return 200 and successfully send email to user with Marketing category".
162-219: Test name doesn't reflect actual behaviorThe test is named "should return 200 with disabled notifications error in results" but the response shows a successful result without any error indication. If the system is supposed to skip sending emails to users who have disabled notifications, the response structure should indicate this somehow (e.g., with an error field or status).
Either:
- The test name should be updated to reflect that emails are sent regardless of notification preferences
- Or the implementation should be updated to properly handle disabled notifications
🧹 Nitpick comments (5)
docker/dependencies/docker.compose.yaml (1)
163-163: Update the comment to reflect the new port mapping.The port mapping was changed from 8119 to 8122, but the comment still references the old port.
- - "8122:8080" # POST http://localhost:8119/execute/v1/script + - "8122:8080" # POST http://localhost:8122/execute/v1/scriptapps/e2e/tests/backend/endpoints/api/v1/send-email.test.ts (1)
221-255: Consider adding explicit error indication in responseThe test name mentions "no primary email error" but the response only shows
user_idwithout an explicit error field. For better API clarity, consider adding an error field to indicate why the email wasn't sent (e.g.,"error": "no_primary_email").apps/e2e/tests/backend/endpoints/api/v1/internal/email-templates.test.ts (3)
18-28: LGTM! Realistic TSX source improves test quality.The replacement of the placeholder with a full TSX source string significantly improves the test by using a realistic email template component structure with proper imports and React patterns.
Consider enhancing the
variablesSchemafunction for more comprehensive testing:- export const variablesSchema = (v) => v; + export const variablesSchema = (v) => ({ + ...v, + // Add test-specific variable validations if needed + });
69-79: Consider extracting the duplicated TSX source to reduce maintenance burden.The TSX source is identical between both test cases, which creates duplication that could lead to maintenance issues if the template structure needs to change.
Extract the TSX source to a constant:
+const MOCK_TSX_SOURCE = ` + import { Subject, NotificationCategory } from '@stackframe/emails'; + export const variablesSchema = (v) => v; + export function EmailTemplate() { + return <> + <Subject value="Test Subject" /> + <NotificationCategory value="Transactional" /> + <div>Mock email template</div> + </>; + } +`; it("should not allow updating email templates when using shared email config", async ({ expect }) => { // ... existing code ... body: { - tsx_source: ` - import { Subject, NotificationCategory } from '@stackframe/emails'; - export const variablesSchema = (v) => v; - export function EmailTemplate() { - return <> - <Subject value="Test Subject" /> - <NotificationCategory value="Transactional" /> - <div>Mock email template</div> - </>; - } - `, + tsx_source: MOCK_TSX_SOURCE, }, }); // Apply similar change to the second test case
86-86: Snapshot updated correctly but consider brittleness.The rendered HTML snapshot correctly reflects the new template structure and includes the unsubscribe link as expected from the email rendering changes.
The inline snapshot is quite lengthy and may be brittle to minor HTML formatting changes. Consider using a separate snapshot file or extracting key assertions:
-expect(updateResponse).toMatchInlineSnapshot(`...very long HTML...`); +expect(updateResponse.status).toBe(200); +expect(updateResponse.body.rendered_html).toContain('Mock email template'); +expect(updateResponse.body.rendered_html).toContain('unsubscribe'); +expect(updateResponse.body.rendered_html).toContain('DOCTYPE html');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
apps/backend/src/app/api/latest/emails/send-email/route.tsx(6 hunks)apps/backend/src/lib/email-rendering.tsx(4 hunks)apps/backend/src/lib/freestyle.tsx(1 hunks)apps/dev-launchpad/public/index.html(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/auth/otp/send-sign-in-code.test.ts(2 hunks)apps/e2e/tests/backend/endpoints/api/v1/auth/otp/sign-in.test.ts(2 hunks)apps/e2e/tests/backend/endpoints/api/v1/auth/password/reset.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/auth/password/send-reset-code.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/auth/password/sign-up.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/contact-channels/legacy-send-verification-code.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/contact-channels/send-verification-code.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/contact-channels/verify.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/email-themes.test.ts(2 hunks)apps/e2e/tests/backend/endpoints/api/v1/internal/email-templates.test.ts(2 hunks)apps/e2e/tests/backend/endpoints/api/v1/internal/email.test.ts(3 hunks)apps/e2e/tests/backend/endpoints/api/v1/render-email.test.ts(5 hunks)apps/e2e/tests/backend/endpoints/api/v1/send-email.test.ts(9 hunks)apps/e2e/tests/backend/endpoints/api/v1/unsubscribe-link.test.ts(1 hunks)apps/e2e/tests/js/email.test.ts(1 hunks)docker/dependencies/docker.compose.yaml(1 hunks)docker/dependencies/freestyle-mock/Dockerfile(1 hunks)packages/stack-shared/src/helpers/emails.ts(4 hunks)packages/stack-shared/src/utils/esbuild.tsx(1 hunks)packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts(2 hunks)
✅ Files skipped from review due to trivial changes (8)
- apps/dev-launchpad/public/index.html
- apps/e2e/tests/backend/endpoints/api/v1/auth/password/sign-up.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/contact-channels/legacy-send-verification-code.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/auth/password/send-reset-code.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/contact-channels/verify.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/contact-channels/send-verification-code.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/internal/email.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/auth/otp/sign-in.test.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts
- apps/e2e/tests/js/email.test.ts
- apps/backend/src/app/api/latest/emails/send-email/route.tsx
- apps/backend/src/lib/email-rendering.tsx
- packages/stack-shared/src/helpers/emails.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{test,spec}.{ts,js,tsx,jsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Testing is done using Vitest
Files:
apps/e2e/tests/backend/endpoints/api/v1/auth/password/reset.test.tsapps/e2e/tests/backend/endpoints/api/v1/unsubscribe-link.test.tsapps/e2e/tests/backend/endpoints/api/v1/auth/otp/send-sign-in-code.test.tsapps/e2e/tests/backend/endpoints/api/v1/render-email.test.tsapps/e2e/tests/backend/endpoints/api/v1/internal/email-templates.test.tsapps/e2e/tests/backend/endpoints/api/v1/send-email.test.tsapps/e2e/tests/backend/endpoints/api/v1/email-themes.test.ts
🧠 Learnings (2)
📚 Learning: applies to **/*.{test,spec}.{ts,js,tsx,jsx} : testing is done using vitest...
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-02T01:22:10.098Z
Learning: Applies to **/*.{test,spec}.{ts,js,tsx,jsx} : Testing is done using Vitest
Applied to files:
apps/e2e/tests/backend/endpoints/api/v1/unsubscribe-link.test.tsapps/e2e/tests/backend/endpoints/api/v1/render-email.test.tsapps/e2e/tests/backend/endpoints/api/v1/internal/email-templates.test.ts
📚 Learning: styling is done using tailwind css...
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-02T01:22:10.098Z
Learning: Styling is done using Tailwind CSS
Applied to files:
apps/e2e/tests/backend/endpoints/api/v1/render-email.test.tsapps/e2e/tests/backend/endpoints/api/v1/email-themes.test.ts
🧬 Code Graph Analysis (2)
packages/stack-shared/src/utils/esbuild.tsx (2)
packages/stack-shared/src/utils/telemetry.tsx (1)
withTraceSpan(7-11)packages/stack-shared/src/utils/env.tsx (1)
isBrowserLike(4-6)
apps/backend/src/lib/freestyle.tsx (1)
packages/stack-shared/src/utils/env.tsx (1)
getNodeEnvironment(65-67)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: build (22.x)
- GitHub Check: restart-dev-and-test
- GitHub Check: docker
- GitHub Check: docker
- GitHub Check: all-good
- GitHub Check: build (22.x)
- GitHub Check: setup-tests
- GitHub Check: lint_and_build (latest)
- GitHub Check: Security Check
🔇 Additional comments (22)
apps/backend/src/lib/freestyle.tsx (2)
1-2: LGTM! Clean import organization.The import changes improve code organization by using local telemetry utilities and adding the necessary environment detection.
10-17: Excellent conditional configuration for mock environment.The implementation correctly handles the mock freestyle service configuration by:
- Only setting baseUrl in development/test environments
- Properly checking for the mock API key
- Using the updated port 8122 that aligns with the Docker compose changes
- Keeping baseUrl undefined for production scenarios
docker/dependencies/freestyle-mock/Dockerfile (4)
1-1: LGTM! Runtime migration from Deno to Bun.The base image change from
denoland/deno:1.46.1tooven/bun:1aligns with the server implementation rewrite and provides better compatibility with the Node.js ecosystem.
6-19: Well-structured global dependency management.The package.json creation and global dependency installation is properly implemented. The choice of global dependencies (arktype, react, @react-email/components) aligns well with the PR's email functionality enhancements.
30-170: Excellent Bun server implementation with comprehensive script execution.The server implementation demonstrates several strong qualities:
- Proper isolation: Each script execution uses a unique temp directory
- Dependency management: Installs user-specified nodeModules via bun install
- Log capture: Proxies console methods to collect execution logs
- Error handling: Comprehensive try-catch with proper JSON error responses
- Resource cleanup: Ensures temp directories are removed in finally block
- Security: Uses crypto.randomUUID() for temp directory names
The migration from Deno's serve API to Bun's serve API is correctly implemented with the proper
{ port, async fetch(req) }structure.
177-177: LGTM! Updated command for Bun runtime.The CMD properly uses
bun run server.tsto execute the server with the new Bun runtime.packages/stack-shared/src/utils/esbuild.tsx (4)
8-9: Good refactoring: centralized WASM URL configuration.Extracting the esbuild WASM URL into a constant eliminates duplication and improves maintainability. Using
esbuild.versionin the template literal ensures version consistency.
18-18: LGTM: consistent usage of the centralized constant.Both references now use the
esbuildWasmUrlconstant, eliminating the previous duplication and maintaining consistency.Also applies to: 21-21
29-30: LGTM: correctly returns the initialization promise.The function now returns the
esbuildInitializePromisedirectly, which is consistent with the non-async function signature. This maintains the singleton initialization pattern while allowing callers more flexibility in handling the promise.
14-14: No external references toinitializeEsbuild– change is transparentI searched the entire codebase for calls to
initializeEsbuild(and only found the internal invocation withinpackages/stack-shared/src/utils/esbuild.tsx. Switching from anasyncfunction to one that directly returnsPromise<void>is transparent to callers and does not break any existing usage.• Only usage found:
– packages/stack-shared/src/utils/esbuild.tsx: internalawait initializeEsbuild()No further action required.
apps/e2e/tests/backend/endpoints/api/v1/auth/password/reset.test.ts (2)
25-32: LGTM: Improved email message identificationThe change from relying on the last message to matching by subject string is more robust and reliable. The concrete subject "Reset your password at Stack Dashboard" provides better test stability than template-based matching.
33-33: Consistent email matching logicGood use of the same subject matching pattern for extracting the reset code URL. This maintains consistency with the assertion above.
apps/e2e/tests/backend/endpoints/api/v1/unsubscribe-link.test.ts (1)
146-146: LGTM: Updated snapshot reflects actual email renderingThe snapshot update correctly shows the fully rendered HTML email output instead of JSX-like component code. This aligns with the email rendering system changes and provides better test coverage of the actual email content sent to users.
apps/e2e/tests/backend/endpoints/api/v1/auth/otp/send-sign-in-code.test.ts (2)
11-11: LGTM: Standardized email subject formatThe concrete subject string "Sign in to Stack Dashboard: Your code is " provides better test reliability than template-based matching.
103-103: LGTM: Updated OTP extraction for HTML contentThe regex pattern correctly matches the 6-character OTP code within HTML paragraph tags, which aligns with the actual email rendering format.
apps/e2e/tests/backend/endpoints/api/v1/render-email.test.ts (3)
11-11: LGTM: Updated template structure with arktype integrationThe template TSX source now correctly includes arktype import and variablesSchema export, which aligns with the new email template system requirements.
Also applies to: 63-63
30-30: LGTM: Consistent named export pattern for themesUsing named exports for EmailTheme functions provides better consistency and aligns with the updated email system architecture.
Also applies to: 82-82
75-112: LGTM: Comprehensive test for successful email renderingThe new test case properly validates the happy path for email rendering with both valid theme and template TSX sources. The snapshot correctly shows the fully rendered HTML output.
apps/e2e/tests/backend/endpoints/api/v1/email-themes.test.ts (4)
94-95: LGTM: Added necessary imports for unsubscribe functionalityThe Link component and ThemeProps type imports are correctly added to support the new unsubscribe link feature in email themes.
97-97: LGTM: Updated function signature for unsubscribe supportThe EmailTheme function now correctly accepts ThemeProps which includes the optional unsubscribeLink parameter, enabling dynamic unsubscribe link rendering.
106-112: LGTM: Proper conditional rendering of unsubscribe linkThe unsubscribe footer is correctly rendered only when unsubscribeLink is provided, with appropriate styling and link text. This provides a good user experience for email recipients.
118-121: LGTM: Preview props for theme testingThe EmailTheme.PreviewProps provides a sensible default unsubscribe link for theme preview purposes, which is helpful for testing and development workflows.
2 participants
Important
Enhances email sending API with template support, improved error handling, and new interfaces, while adding comprehensive tests and updating rendering logic.
send-email/route.tsx.send-email/route.tsxfor missing content, non-existent user IDs, and shared email server configurations.KnownErrorsfor specific error cases.server-interface.tsandadmin-interface.ts.email.test.tsfor various email sending scenarios, including HTML content, templates, and error cases.email-rendering.tsxto handle new template and theme options.This description was created by
for 1d5a056. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Improvements
API Changes
Bug Fixes
Chores
Tests