implement email service with zod and nodemailer#13
Conversation
📝 WalkthroughWalkthroughThis pull request adds an attendance email feature: new dependencies (nodemailer, zod, types), Hebrew email constants, Zod validation schemas, email content builders (text + RTL HTML), a Gmail Nodemailer-backed sendEmail service, an Express POST /api/email route with Swagger docs, and app registration under /api/email. ChangesEmail Notification Feature
Sequence DiagramsequenceDiagram
participant Client
participant EmailController
participant EmailService
participant Nodemailer
participant Gmail
Client->>EmailController: POST /api/email { emailType, payload }
EmailController->>EmailService: sendEmail(data)
EmailService->>EmailService: Validate with Zod (sendEmailSchema)
EmailService->>EmailService: Check SMTP env vars (SMTP_USER/SMTP_PASS)
EmailService->>EmailService: buildEmailContent(data)
EmailService->>Nodemailer: transporter.sendMail({ from, to, subject, text, html })
Nodemailer->>Gmail: SMTP send
Gmail-->>Nodemailer: SMTP response
Nodemailer-->>EmailService: send result Promise
EmailService-->>EmailController: resolve/reject
EmailController-->>Client: 200/400/500 with message
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
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/schemas/email/attendance-email.schema.ts`:
- Line 2: attendance-email.schema.ts currently imports sendEmailSchema solely to
reuse the SendEmailData type, creating unnecessary coupling with email.service;
instead derive SendEmailData from attendanceEmailSchema itself. Remove the
import of sendEmailSchema, export or declare a type like SendEmailData =
z.infer<typeof attendanceEmailSchema> (reference attendanceEmailSchema and
SendEmailData) so email.service.ts can import the type from
attendance-email.schema.ts without creating a schema↔service import cycle;
update any usages that referenced the old sendEmailSchema type to use the new
derived SendEmailData.
🪄 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: 8637b5a3-5d19-476b-8a82-fda58dc1c090
📒 Files selected for processing (3)
src/schemas/email/attendance-email.schema.tssrc/schemas/email/email.service.helpers.tssrc/services/email/email.service.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/services/email/email.service.ts
| @@ -0,0 +1,18 @@ | |||
| import { ATTENDANCE_STATUS, EMAIL_TYPE } from '@/constants/email/email.constants'; | |||
| import { sendEmailSchema } from '@/services/email/email.service'; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether TS config preserves non-type imports and confirm dependency cycle edges.
fd -i 'tsconfig*.json' .
rg -n --no-heading -e 'verbatimModuleSyntax|preserveValueImports|importsNotUsedAsValues' $(fd -i 'tsconfig*.json' .)
# Confirm cross-imports between schema and service files.
rg -n --no-heading -e "from '`@/services/email/email.service`'" -e "from '`@/schemas/email/attendance-email.schema`'" src/services/email/email.service.ts src/schemas/email/attendance-email.schema.tsRepository: ColmanDevClubORG/Sagol360ManagementServer
Length of output: 333
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== tsconfig.json (relevant compilerOptions) ==="
cat -n tsconfig.json | sed -n '1,220p' | rg -n "compilerOptions|verbatimModuleSyntax|preserveValueImports|importsNotUsedAsValues|module|target|moduleResolution|esModuleInterop|isolatedModules|useDefineForClassFields" || true
echo
echo "=== attendance-email.schema.ts (imports + SendEmailData) ==="
nl -ba src/schemas/email/attendance-email.schema.ts | sed -n '1,120p'
echo
echo "=== email.service.ts (imports + attendanceEmailSchema usage) ==="
nl -ba src/services/email/email.service.ts | sed -n '1,160p'Repository: ColmanDevClubORG/Sagol360ManagementServer
Length of output: 440
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== tsconfig.json (compilerOptions excerpt) ==="
# Print the compilerOptions block only (small)
python3 - <<'PY'
import json
p='tsconfig.json'
with open(p,'r',encoding='utf-8') as f: data=json.load(f)
co=data.get('compilerOptions',{})
keys=[
'verbatimModuleSyntax',
'preserveValueImports',
'importHelpers',
'importsNotUsedAsValues',
'isolatedModules',
'module',
'moduleResolution',
'target',
'esModuleInterop',
'useUnknownInCatchVariables',
'strict',
'requireEmitForEachFunction',
'importsNotUsedAsValues'
]
print(json.dumps({k:co.get(k) for k in keys if k in co or k in ['module','moduleResolution','target','esModuleInterop']},indent=2))
PY
echo
echo "=== attendance-email.schema.ts (first 80 lines) ==="
cat -n src/schemas/email/attendance-email.schema.ts | sed -n '1,120p'
echo
echo "=== email.service.ts (first 140 lines) ==="
cat -n src/services/email/email.service.ts | sed -n '1,160p'Repository: ColmanDevClubORG/Sagol360ManagementServer
Length of output: 3089
Remove the schema↔service import coupling by deriving SendEmailData from attendanceEmailSchema
attendance-email.schema.ts imports sendEmailSchema only for SendEmailData typing, while email.service.ts uses sendEmailSchema at runtime and imports attendanceEmailSchema back—creating a circular dependency. With the current tsconfig (NodeNext; no verbatimModuleSyntax/preserveValueImports/importsNotUsedAsValues overrides shown), this is likely not a runtime-cycle issue, but it’s still unnecessary coupling; derive the type directly from attendanceEmailSchema.
Suggested change
import { ATTENDANCE_STATUS, EMAIL_TYPE } from '`@/constants/email/email.constants`';
-import { sendEmailSchema } from '`@/services/email/email.service`';
import { z } from 'zod';
@@
-export type SendEmailData = z.infer<typeof sendEmailSchema>;
+export type SendEmailData = z.infer<typeof attendanceEmailSchema>;🤖 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/schemas/email/attendance-email.schema.ts` at line 2,
attendance-email.schema.ts currently imports sendEmailSchema solely to reuse the
SendEmailData type, creating unnecessary coupling with email.service; instead
derive SendEmailData from attendanceEmailSchema itself. Remove the import of
sendEmailSchema, export or declare a type like SendEmailData = z.infer<typeof
attendanceEmailSchema> (reference attendanceEmailSchema and SendEmailData) so
email.service.ts can import the type from attendance-email.schema.ts without
creating a schema↔service import cycle; update any usages that referenced the
old sendEmailSchema type to use the new derived SendEmailData.
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
New Features
Chores