Replace AWS SES with SMTP email provider#385
Conversation
|
|
📝 WalkthroughWalkthroughThe file ChangesSMTP Transport Migration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
worklenz-backend/src/shared/email.ts (1)
86-86: ⚡ Quick winRemove unused
charsetvariable.This variable was used by the AWS SES implementation but is no longer referenced after the migration to Nodemailer.
♻️ Proposed fix
- const charset = "UTF-8"; - await transporter.sendMail({🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@worklenz-backend/src/shared/email.ts` at line 86, The `charset` variable is no longer used after the migration from AWS SES to Nodemailer. Remove the unused variable declaration `const charset = "UTF-8";` from the email.ts file since no code references it anymore.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@worklenz-backend/src/shared/email.ts`:
- Around line 8-16: The nodemailer transporter configuration in the email.ts
file has `secure: false` hardcoded, which only works correctly for port 587
(STARTTLS) but fails for port 465 (SMTPS) which requires `secure: true`. Make
the `secure` property dynamic by checking the SMTP_PORT value: if the port is
465, set `secure: true`; otherwise set it to `false`. This ensures the
transporter configuration works correctly regardless of which SMTP port is
configured.
---
Nitpick comments:
In `@worklenz-backend/src/shared/email.ts`:
- Line 86: The `charset` variable is no longer used after the migration from AWS
SES to Nodemailer. Remove the unused variable declaration `const charset =
"UTF-8";` from the email.ts file since no code references it anymore.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 36634285-a7c5-4f4c-9f29-c1e5b9ced9d1
📒 Files selected for processing (1)
worklenz-backend/src/shared/email.ts
| const transporter = nodemailer.createTransport({ | ||
| host: process.env.SMTP_HOST, | ||
| port: Number(process.env.SMTP_PORT || 587), | ||
| secure: false, | ||
| auth: { | ||
| user: process.env.SMTP_USER, | ||
| pass: process.env.SMTP_PASS, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Hardcoded secure: false may break port 465 (SMTPS) configurations.
Port 587 uses STARTTLS (where secure: false is correct), but port 465 requires secure: true for implicit TLS. Users configuring SMTP_PORT=465 will experience connection failures.
🔧 Proposed fix to auto-detect secure mode
const transporter = nodemailer.createTransport({
host: process.env.SMTP_HOST,
port: Number(process.env.SMTP_PORT || 587),
- secure: false,
+ secure: Number(process.env.SMTP_PORT || 587) === 465,
auth: {
user: process.env.SMTP_USER,
pass: process.env.SMTP_PASS,
},
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const transporter = nodemailer.createTransport({ | |
| host: process.env.SMTP_HOST, | |
| port: Number(process.env.SMTP_PORT || 587), | |
| secure: false, | |
| auth: { | |
| user: process.env.SMTP_USER, | |
| pass: process.env.SMTP_PASS, | |
| }, | |
| }); | |
| const transporter = nodemailer.createTransport({ | |
| host: process.env.SMTP_HOST, | |
| port: Number(process.env.SMTP_PORT || 587), | |
| secure: Number(process.env.SMTP_PORT || 587) === 465, | |
| auth: { | |
| user: process.env.SMTP_USER, | |
| pass: process.env.SMTP_PASS, | |
| }, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@worklenz-backend/src/shared/email.ts` around lines 8 - 16, The nodemailer
transporter configuration in the email.ts file has `secure: false` hardcoded,
which only works correctly for port 587 (STARTTLS) but fails for port 465
(SMTPS) which requires `secure: true`. Make the `secure` property dynamic by
checking the SMTP_PORT value: if the port is 465, set `secure: true`; otherwise
set it to `false`. This ensures the transporter configuration works correctly
regardless of which SMTP port is configured.
2 participants
Summary
This PR replaces AWS SES email implementation with SMTP-based email delivery using Nodemailer.
Changes
Environment variables
SMTP_HOST=smtp.server.com
SMTP_PORT=587
SMTP_USER=your_user
SMTP_PASS=your_password
FROM_EMAIL=noreply@domain.com
ENABLE_EMAIL=true
Testing
Notes
This improves portability and removes dependency on AWS SES.
Deployment Notes
This backend is TypeScript-based and requires compilation after deployment changes.
Required production steps:
Install dependencies:
npm install
Build application:
npm run build
Restart backend service:
docker restart worklenz-backend
OR systemctl restart worklenz-backend
Important:
src/shared/email.tsonly take effect after build.Summary by CodeRabbit