implement email service with zod and nodemailer#13
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a complete email notification feature for attendance updates. Dependencies are added (nodemailer, zod, type definitions), constants define email types and Hebrew content, the service layer handles Zod validation and Nodemailer dispatch, helpers build attendance email HTML/text, and the HTTP layer exposes the feature via a POST endpoint with Swagger documentation. ChangesEmail Notification Feature
Sequence DiagramsequenceDiagram
participant Client
participant Controller
participant Service
participant Nodemailer
participant Gmail
Client->>Controller: POST /api/email {emailType, payload}
Controller->>Service: sendEmail(data)
Service->>Service: Validate with Zod schema
Service->>Service: Check SMTP env vars
Service->>Service: Build email content
Service->>Nodemailer: transporter.sendMail({to, subject, text, html})
Nodemailer->>Gmail: SMTP send
Gmail-->>Nodemailer: Success
Nodemailer-->>Service: Promise resolve
Service-->>Controller: Promise resolve
Controller-->>Client: 200 OK {message: emailSentSuccessfully}
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 docstrings
🧪 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: 3
🧹 Nitpick comments (1)
package.json (1)
24-24: Remove redundant@types/nodemailer:nodemailerships its own typings.
nodemailer@8.0.10exposes TypeScript types viaindex.d.ts, so@types/nodemaileris likely unnecessary and can be dropped to avoid redundancy/version drift for fresh installs and lockfile updates.🤖 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 `@package.json` at line 24, Remove the redundant dev dependency entry "`@types/nodemailer`" from package.json; nodemailer (>=8.0.10) includes its own TypeScript types, so delete the "`@types/nodemailer`" dependency line and run the package manager's install/update to refresh the lockfile (e.g., npm/yarn/pnpm install) to ensure no stale typings remain.
🤖 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 `@src/constants/email/email.constants.ts`:
- Around line 23-24: The file has formatting issues for the exported constant
rescheduleMessage (and another at the same file around line 56); run Prettier or
reformat the string literals so they are wrapped consistently and ensure the
file ends with a single trailing newline, then re-run lint/format checks;
specifically update the rescheduleMessage value in email.constants.ts to match
project Prettier rules (line-wrapping/quotes) and add the missing final newline.
In `@src/routes/email.routes.ts`:
- Line 97: The POST /email route (router.post('/', sendEmail)) is unprotected;
wrap this route with the same authentication and staff-authorization middleware
used for staff-only actions (apply the existing authenticate and authorizeStaff
middleware functions in the same order as other protected routes) so only
authorized staff can send mail, and ensure the route handler still receives the
request after middleware. Also update the Swagger/OpenAPI docs for this endpoint
to include 401 and 403 responses and any required security schemes so clients
know authentication/authorization is required.
In `@src/services/email/email.service.ts`:
- Around line 23-29: The Nodemailer transporter created in createTransport lacks
SMTP timeouts causing sendMail (which runs on the request path) to potentially
hang; update the transporter creation (the const transporter / createTransport
call in email.service.ts) to include connectionTimeout, greetingTimeout, and
socketTimeout (e.g., read from env vars like SMTP_CONN_TIMEOUT,
SMTP_GREETING_TIMEOUT, SMTP_SOCKET_TIMEOUT or use sensible millisecond defaults)
so the SMTP client fails fast when Gmail is slow/unreachable.
---
Nitpick comments:
In `@package.json`:
- Line 24: Remove the redundant dev dependency entry "`@types/nodemailer`" from
package.json; nodemailer (>=8.0.10) includes its own TypeScript types, so delete
the "`@types/nodemailer`" dependency line and run the package manager's
install/update to refresh the lockfile (e.g., npm/yarn/pnpm install) to ensure
no stale typings remain.
🪄 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: f13fa9dc-f178-401e-98ea-346451f44642
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
package.jsonsrc/app.tssrc/constants/email/email.constants.tssrc/controllers/email/email.controller.tssrc/routes/email.routes.tssrc/services/email/email.service.helpers.tssrc/services/email/email.service.ts
| rescheduleMessage: | ||
| 'אנא פנה למטופל לקביעת מועד חדש ועדכן במערכת כי התפנה כיסא.', |
There was a problem hiding this comment.
Run Prettier on this file before merge.
These spots already trip prettier/prettier, so lint will stay red until the wrapped string and trailing newline are normalized.
Also applies to: 56-56
🧰 Tools
🪛 ESLint
[error] 23-24: Delete ⏎···
(prettier/prettier)
🤖 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 `@src/constants/email/email.constants.ts` around lines 23 - 24, The file has
formatting issues for the exported constant rescheduleMessage (and another at
the same file around line 56); run Prettier or reformat the string literals so
they are wrapped consistently and ensure the file ends with a single trailing
newline, then re-run lint/format checks; specifically update the
rescheduleMessage value in email.constants.ts to match project Prettier rules
(line-wrapping/quotes) and add the missing final newline.
| * type: string | ||
| * example: Email was not sent | ||
| */ | ||
| router.post('/', sendEmail); |
There was a problem hiding this comment.
Protect this endpoint before exposing it publicly.
src/app.ts mounts emailRoutes at /api/email after only express.json() and logger, and this route accepts arbitrary recipient addresses. Right now anyone who can reach the API can use your Gmail account as a mail relay. Add the same auth/authorization middleware used for staff-only actions here, and update the Swagger docs with 401/403 once it is in place.
🤖 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 `@src/routes/email.routes.ts` at line 97, The POST /email route
(router.post('/', sendEmail)) is unprotected; wrap this route with the same
authentication and staff-authorization middleware used for staff-only actions
(apply the existing authenticate and authorizeStaff middleware functions in the
same order as other protected routes) so only authorized staff can send mail,
and ensure the route handler still receives the request after middleware. Also
update the Swagger/OpenAPI docs for this endpoint to include 401 and 403
responses and any required security schemes so clients know
authentication/authorization is required.
| const transporter = nodemailer.createTransport({ | ||
| service: 'gmail', | ||
| auth: { | ||
| user: process.env.SMTP_USER, | ||
| pass: process.env.SMTP_PASS, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What timeout options does Nodemailer createTransport support for SMTP transports, and are connectionTimeout, greetingTimeout, and socketTimeout valid in the currently used Nodemailer major version?
💡 Result:
Nodemailer's SMTP transport supports connectionTimeout, greetingTimeout, and socketTimeout as valid configuration options [1]. These options are part of the core SMTP transport configuration used in the current version of Nodemailer [1][2]. The following timeout options are supported: - connectionTimeout: Specifies the maximum time in milliseconds to wait for the TCP connection to be established. The default value is 120,000 ms (2 minutes) [3][1]. - greetingTimeout: Specifies the maximum time in milliseconds to wait for the server's initial greeting after the connection is established. The default value is 30,000 ms (30 seconds) [3][1]. - socketTimeout: Specifies the maximum time in milliseconds of inactivity allowed before the connection is automatically closed. The default value is 600,000 ms (10 minutes) [3][1]. Additionally, Nodemailer supports dnsTimeout (maximum time to wait for DNS resolution, defaulting to 30,000 ms) and dnsTtl (time to cache DNS lookup results, defaulting to 300,000 ms) [3][1]. These options are standard and have been consistently supported across major versions of Nodemailer, including earlier iterations [4][5].
Citations:
- 1: https://nodemailer.com/smtp
- 2: https://nodemailer.com/errors
- 3: https://nodemailer.com/extras/smtp-connection
- 4: https://github.com/nodemailer/nodemailer/tree/v2
- 5: https://npmx.dev/package/nodemailer-smtp-transport/v/2.7.2
Add SMTP timeouts to the Nodemailer transporter (avoid request hang).
sendMail() runs on the request path and the current createTransport() config sets no SMTP timeouts. Nodemailer’s SMTP transport supports connectionTimeout, greetingTimeout, and socketTimeout, so add them to fail fast when Gmail is slow/unreachable.
Suggested change
const transporter = nodemailer.createTransport({
service: 'gmail',
+ connectionTimeout: 10_000,
+ greetingTimeout: 10_000,
+ socketTimeout: 15_000,
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({ | |
| service: 'gmail', | |
| auth: { | |
| user: process.env.SMTP_USER, | |
| pass: process.env.SMTP_PASS, | |
| }, | |
| }); | |
| const transporter = nodemailer.createTransport({ | |
| service: 'gmail', | |
| connectionTimeout: 10_000, | |
| greetingTimeout: 10_000, | |
| socketTimeout: 15_000, | |
| 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 `@src/services/email/email.service.ts` around lines 23 - 29, The Nodemailer
transporter created in createTransport lacks SMTP timeouts causing sendMail
(which runs on the request path) to potentially hang; update the transporter
creation (the const transporter / createTransport call in email.service.ts) to
include connectionTimeout, greetingTimeout, and socketTimeout (e.g., read from
env vars like SMTP_CONN_TIMEOUT, SMTP_GREETING_TIMEOUT, SMTP_SOCKET_TIMEOUT or
use sensible millisecond defaults) so the SMTP client fails fast when Gmail is
slow/unreachable.
GilHeller
left a comment
GilHeller
left a comment
There was a problem hiding this comment.
Great job. Please see my comments
There was a problem hiding this comment.
add logs. when you catch and throw an error we might swallow the original error and that will cause the debugging to be significantly harder. (add a console.error(..,) before throwing an error)
| const attendanceEmailSchema = z.object({ | ||
| emailType: z.literal(EMAIL_TYPE.ATTENDANCE_UPDATE), | ||
| email: z.string().trim().email(), | ||
| payload: z.object({ | ||
| patientName: z.string().trim().min(1), | ||
| patientNumber: z.string().trim().min(1), | ||
| attendanceStatus: z.enum([ATTENDANCE_STATUS.COMING, ATTENDANCE_STATUS.NOT_COMING]), | ||
| time: z.string().trim().min(1), | ||
| cell: z.string().trim().min(1), | ||
| building: z.string().trim().min(1), | ||
| }), | ||
| }); |
There was a problem hiding this comment.
Consider moving this email schema definition to src/schemas/email/attendance-email.schema.ts or similar.
|
|
||
| const sendEmailSchema = z.discriminatedUnion('emailType', [attendanceEmailSchema]); | ||
|
|
||
| type SendEmailData = z.infer<typeof sendEmailSchema>; |
There was a problem hiding this comment.
Consider moving this type definition to the same file as attendanceEmailSchema (like src/schemas/email/attendance-email.schema.ts)
| const buildEmailContent = (data: SendEmailData): EmailContent => { | ||
| switch (data.emailType) { | ||
| case EMAIL_TYPE.ATTENDANCE_UPDATE: | ||
| return buildAttendanceEmailContent(data.payload); | ||
|
|
||
| default: | ||
| throw new Error(EMAIL_ERRORS.unsupportedEmailType); | ||
| } | ||
| }; |
There was a problem hiding this comment.
consider moving if into src/schemas/email/email.service.helpers.ts
2 participants
Description
Please include a summary of the changes and the related issue.
Related Issue(s)
Implemented a generic backend email service that validates requests with Zod, routes by emailType, builds the attendance email content, and sends it with Nodemailer.
The email content was separated into helpers/constants so the service stays clean and future email types can be added easily.
For Gmail, SMTP_PASS must be a Google App Password, and HTML values are escaped to prevent injected HTML/JS from being rendered.
Fixes # (issue number)
Checklist:
Screenshots (if appropriate):
Summary by CodeRabbit
Release Notes
New Features
Chores