Feature/mailgun integration templates final#43
Merged
MrImmortal09 merged 5 commits intoApr 11, 2026
Merged
Conversation
…and failed attempts
|
@DistantMyth is attempting to deploy a commit to the mrimmortal09's projects Team on Vercel. A member of the Team first needs to authorize it. |
3 tasks
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds Mailgun-based email sending: new env entries, Mailgun client and Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant EmailService
participant MailgunAPI
participant Database
Caller->>EmailService: sendEmail({ to, subject, text, html })
EmailService->>EmailService: validate MAILGUN_API_KEY & MAILGUN_DOMAIN
alt validation fails
EmailService-->>Caller: return false
else
EmailService->>MailgunAPI: mg.messages.create(message)
alt send success
MailgunAPI-->>EmailService: { id: messageId }
EmailService->>Database: logEmail(status: "sent", messageId)
Database-->>EmailService: record created
EmailService-->>Caller: return true
else send failure
MailgunAPI-->>EmailService: throws error
EmailService->>Database: logEmail(status: "failed", errorDetails)
Database-->>EmailService: record created
EmailService-->>Caller: return false
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 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 (5)
models/EmailLog.ts (1)
13-36: Consider adding an index oncreatedAtfor efficient log queries.As the email log grows, queries for recent emails or time-range searches will benefit from an index. This is optional but recommended for operational use.
📊 Suggested index addition
const EmailLogSchema: Schema<IEmailLog> = new Schema<IEmailLog>( { to: { type: String, required: [true, 'Recipient email is required'], + index: true, }, // ... other fields }, { timestamps: true } ); + +EmailLogSchema.index({ createdAt: -1 });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@models/EmailLog.ts` around lines 13 - 36, Add an index on the timestamp field to speed up time-range and recent-email queries: update the EmailLogSchema (Schema<IEmailLog>) to declare an index on createdAt (e.g., via EmailLogSchema.index({ createdAt: -1 }) or equivalent in schema options) after the schema definition so the createdAt timestamp generated by timestamps: true is indexed for efficient queries.lib/db.ts (1)
3-3: Remove the unused module-levelMONGODB_URIconstant.Line 3 declares
const MONGODB_URI = process.env.MONGODB_URIat module scope, but after adding the local declaration at Line 20, the module-level constant is now dead code. Consider removing Line 3 to avoid confusion.🧹 Proposed cleanup
import mongoose from "mongoose"; - -const MONGODB_URI = process.env.MONGODB_URI; declare global {Also applies to: 20-21
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/db.ts` at line 3, Remove the unused module-level constant MONGODB_URI by deleting the top-level declaration (const MONGODB_URI = process.env.MONGODB_URI) and rely on the existing local MONGODB_URI declaration used later; ensure there are no remaining references to the top-level symbol and keep only the local const MONGODB_URI in the function scope so the code and linting no longer report the dead variable.lib/email/index.ts (2)
61-65: Consider preserving the full error for debugging complex failures.The current error casting
as { message?: string }may lose valuable debugging information (e.g., Mailgun API status codes, response body). For production debugging, you might want to stringify or preserve more error context.🔍 Optional: Preserve more error context
} catch (error: unknown) { - const err = error as { message?: string }; - console.error('Mailgun error details:', err?.message); - await logEmail(to, subject, 'failed', undefined, err?.message); + const errorMessage = error instanceof Error + ? error.message + : JSON.stringify(error); + console.error('Mailgun error details:', error); + await logEmail(to, subject, 'failed', undefined, errorMessage); return false; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/email/index.ts` around lines 61 - 65, The catch block currently narrows the thrown value to { message?: string } losing rich Mailgun error details; update the catch in the email send function so you preserve and log the full error object (include stack, status, response body) by serializing the original error (e.g., JSON.stringify with a safe replacer or fallback to util.inspect) and pass that serialized payload into console.error and logEmail (keep calling logEmail(to, subject, 'failed', undefined, <fullErrorString>)); ensure you still extract message for quick reads but store the full serialized error string for debugging and include references to the existing variables/functions error, err, and logEmail to locate the change.
24-36: Singleton client won't reinitialize if initially missing env vars are later provided.Once
validateMailgunEnvVars()returnsfalse, the client remainsnullforever (until process restart). If this runs early during app startup before env vars are fully loaded, subsequent calls will always fail even after vars become available.This is likely fine for typical Next.js deployments where env vars are stable, but worth noting for edge cases.
♻️ Optional: Reset client on validation failure for retry capability
function getMailgunClient() { - if (!mailgunClient && validateMailgunEnvVars()) { + if (!mailgunClient) { + if (!validateMailgunEnvVars()) { + return null; + } const mailgun = new Mailgun(FormData); mailgunClient = mailgun.client({ username: 'api', key: process.env.MAILGUN_API_KEY || '', url: process.env.MAILGUN_URL || 'https://api.mailgun.net', }); } return mailgunClient; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/email/index.ts` around lines 24 - 36, The current getMailgunClient short-circuits client initialization when mailgunClient is falsy and validateMailgunEnvVars() is false, which can prevent later successful reinitialization; change getMailgunClient so it checks mailgunClient first and if missing runs validateMailgunEnvVars() at that time, returning null when validation fails but allowing creation once validation succeeds later—update the logic in getMailgunClient (and keep mailgunClient as the singleton) so the creation branch runs whenever mailgunClient is null and validateMailgunEnvVars() returns true, rather than locking out retries when validation first fails.package.json (1)
29-29: Clarify the purpose oftsxin the project.The
tsxpackage is added as a dev dependency, but there's no corresponding npm script using it. If this is intended for running TypeScript scripts directly (e.g., seeding, testing the email module), consider adding a script or documenting its usage.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 29, The package.json includes the devDependency "tsx" but there is no npm script referencing it; either add an npm script that uses tsx (for example a script name like "seed" or "dev:ts" that runs TypeScript scripts via tsx) so its purpose is clear, or remove the unused devDependency and instead document tsx usage in the README/contributing notes. Update the "scripts" section to include a descriptive script that invokes tsx for the intended task (e.g., seeding or ad-hoc TypeScript runs) and add a short README note referencing tsx and the new script name so developers know when to use it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.student-hub-ref:
- Line 1: The repository added a gitlink at .student-hub-ref but no .gitmodules
entry, so Git won’t initialize the submodule; create a .gitmodules file at repo
root with a submodule stanza for "student-hub" that sets path = .student-hub-ref
and url = <submodule-remote-url> (replace with the actual remote), commit that
file alongside the gitlink, then run git submodule init/update (or document it)
to ensure the submodule can be cloned with --recursive; reference the
.student-hub-ref gitlink and the .gitmodules filename when making the change.
---
Nitpick comments:
In `@lib/db.ts`:
- Line 3: Remove the unused module-level constant MONGODB_URI by deleting the
top-level declaration (const MONGODB_URI = process.env.MONGODB_URI) and rely on
the existing local MONGODB_URI declaration used later; ensure there are no
remaining references to the top-level symbol and keep only the local const
MONGODB_URI in the function scope so the code and linting no longer report the
dead variable.
In `@lib/email/index.ts`:
- Around line 61-65: The catch block currently narrows the thrown value to {
message?: string } losing rich Mailgun error details; update the catch in the
email send function so you preserve and log the full error object (include
stack, status, response body) by serializing the original error (e.g.,
JSON.stringify with a safe replacer or fallback to util.inspect) and pass that
serialized payload into console.error and logEmail (keep calling logEmail(to,
subject, 'failed', undefined, <fullErrorString>)); ensure you still extract
message for quick reads but store the full serialized error string for debugging
and include references to the existing variables/functions error, err, and
logEmail to locate the change.
- Around line 24-36: The current getMailgunClient short-circuits client
initialization when mailgunClient is falsy and validateMailgunEnvVars() is
false, which can prevent later successful reinitialization; change
getMailgunClient so it checks mailgunClient first and if missing runs
validateMailgunEnvVars() at that time, returning null when validation fails but
allowing creation once validation succeeds later—update the logic in
getMailgunClient (and keep mailgunClient as the singleton) so the creation
branch runs whenever mailgunClient is null and validateMailgunEnvVars() returns
true, rather than locking out retries when validation first fails.
In `@models/EmailLog.ts`:
- Around line 13-36: Add an index on the timestamp field to speed up time-range
and recent-email queries: update the EmailLogSchema (Schema<IEmailLog>) to
declare an index on createdAt (e.g., via EmailLogSchema.index({ createdAt: -1 })
or equivalent in schema options) after the schema definition so the createdAt
timestamp generated by timestamps: true is indexed for efficient queries.
In `@package.json`:
- Line 29: The package.json includes the devDependency "tsx" but there is no npm
script referencing it; either add an npm script that uses tsx (for example a
script name like "seed" or "dev:ts" that runs TypeScript scripts via tsx) so its
purpose is clear, or remove the unused devDependency and instead document tsx
usage in the README/contributing notes. Update the "scripts" section to include
a descriptive script that invokes tsx for the intended task (e.g., seeding or
ad-hoc TypeScript runs) and add a short README note referencing tsx and the new
script name so developers know when to use it.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 95af6326-0e8d-40ea-be29-b3a84139f9b2
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
.env.example.student-hub-reflib/db.tslib/email/index.tsmodels/EmailLog.tspackage.json
| @@ -0,0 +1 @@ | |||
| Subproject commit 3efe55acdda0315af54e19afe99b9f4836e24df6 | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
TARGET_PATH=".student-hub-ref"
TARGET_SHA="3efe55acdda0315af54e19afe99b9f4836e24df6"
echo "1) Verify gitlink recorded in current tree:"
git ls-tree HEAD "$TARGET_PATH"
echo
echo "2) Resolve submodule URL from .gitmodules:"
SUBMODULE_NAME="$(git config -f .gitmodules --get-regexp '^submodule\..*\.path$' | awk -v p="$TARGET_PATH" '$2==p {print $1}' | sed -E 's/^submodule\.([^.]+)\.path$/\1/')"
if [[ -z "${SUBMODULE_NAME:-}" ]]; then
echo "Submodule path '$TARGET_PATH' not found in .gitmodules" >&2
exit 1
fi
SUBMODULE_URL="$(git config -f .gitmodules --get "submodule.${SUBMODULE_NAME}.url")"
echo "Submodule name: $SUBMODULE_NAME"
echo "Submodule url: $SUBMODULE_URL"
echo
echo "3) Check whether target SHA is present on remote:"
if git ls-remote "$SUBMODULE_URL" "$TARGET_SHA" | grep -q "$TARGET_SHA"; then
echo "OK: target SHA exists on remote."
else
echo "ERROR: target SHA not found on remote refs (may need fetch/permissions)." >&2
exit 1
fi
echo
echo "4) If submodule is initialized locally, print checked out commit:"
if [[ -d "$TARGET_PATH" ]]; then
git -C "$TARGET_PATH" rev-parse HEAD || true
git -C "$TARGET_PATH" log --oneline -n 5 || true
fiRepository: iiitl/alumni
Length of output: 215
🏁 Script executed:
# Check if .gitmodules exists
git ls-files .gitmodules
# If it exists, display its contents
if git ls-files .gitmodules | grep -q .gitmodules; then
echo "=== .gitmodules file contents ==="
cat .gitmodules
fi
# Check git diff for any changes to .gitmodules in this PR
echo
echo "=== Changes to .gitmodules in this PR ==="
git diff HEAD~1 .gitmodules || echo "(no changes to .gitmodules in this commit)"Repository: iiitl/alumni
Length of output: 345
🏁 Script executed:
# Check total number of commits and current branch
echo "=== Repository state ==="
git rev-parse --short HEAD
git log --oneline | wc -l
# List all tracked files to understand repo structure
echo
echo "=== All tracked files (first 30) ==="
git ls-files | head -30
# Check if .student-hub-ref is actually a gitlink or directory
echo
echo "=== Detailed status of .student-hub-ref ==="
git ls-tree HEAD .student-hub-ref
ls -la .student-hub-ref 2>&1 || echo "(file check failed)"Repository: iiitl/alumni
Length of output: 951
Add .gitmodules configuration for the .student-hub-ref gitlink.
The PR introduces a gitlink to .student-hub-ref but does not register it in a .gitmodules file. Without this configuration, users cannot clone the repository with git clone --recursive and properly initialize the submodule. Add a .gitmodules entry specifying the submodule name, path, and URL:
[submodule "student-hub"]
path = .student-hub-ref
url = <submodule-remote-url>
This is required for Git to recognize and manage the submodule correctly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.student-hub-ref at line 1, The repository added a gitlink at
.student-hub-ref but no .gitmodules entry, so Git won’t initialize the
submodule; create a .gitmodules file at repo root with a submodule stanza for
"student-hub" that sets path = .student-hub-ref and url = <submodule-remote-url>
(replace with the actual remote), commit that file alongside the gitlink, then
run git submodule init/update (or document it) to ensure the submodule can be
cloned with --recursive; reference the .student-hub-ref gitlink and the
.gitmodules filename when making the change.
… utilities, and email service functions
Contributor
Author
|
@MrImmortal09 I think i tried to fix the issues |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/email/index.ts`:
- Around line 93-95: The code currently writes raw provider error text
(err?.message) into the DB via logEmail; create a small sanitizer and truncation
step before persistence and use that sanitized string in both places (the
failure path around the console.error/logEmail call and the other error handling
block at lines 109-124). Specifically, add a helper (e.g., sanitizeErrorDetails)
that strips control/PII-prone characters, collapses whitespace, and limits
length (e.g., 1000 characters) and then pass its output to logEmail instead of
err?.message; update any references to provider error text in this file
(including the console.error/logEmail usages) to use the sanitized, bounded
value.
- Around line 88-95: The sendEmail path currently awaits logEmail (in the
success and error branches) causing sendEmail latency to depend on DB; change
both occurrences so logging is best-effort and non-blocking: call logEmail(...)
without await (fire-and-forget) and attach a catch handler to capture/log any
logging errors to avoid unhandled rejections. Update the success branch (where
console.log and await logEmail(to, subject, 'sent', data.id) live) and the error
branch (await logEmail(to, subject, 'failed', undefined, err?.message)), and
apply the same change to the similar block around lines 109-128; keep function
names logEmail and sendEmail as the reference points.
In `@models/EmailLog.ts`:
- Around line 23-46: EmailLogSchema currently stores PII fields (to, subject)
and timestamps with no expiry; add configurable TTL support by creating an index
on a date field (e.g., createdAt) or adding an expiresAt field and applying a
TTL index using an env var like EMAIL_LOG_TTL_DAYS (convert days to seconds for
expireAfterSeconds) so records auto-delete after the configured retention
period; also consider limiting long-term persisted fields by redacting or
truncating subject/to before save (or storing a hashed recipient) in the
EmailLogSchema save flow to minimize stored PII.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 74b17803-b330-446e-b4b5-248d7be6ae6d
📒 Files selected for processing (3)
lib/db.tslib/email/index.tsmodels/EmailLog.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/db.ts
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
lib/email/index.ts (1)
93-95:⚠️ Potential issue | 🟠 Major
errorDetailsis truncated but still not sanitized before persistence.Line 93 bounds size, but control characters/unsafe formatting are still stored and logged raw. This was previously raised and is only partially fixed.
Suggested patch
+function sanitizeErrorDetails(message: string, maxLen = 1000): string { + return message + .replace(/[\u0000-\u001F\u007F]/g, ' ') + .replace(/\s+/g, ' ') + .trim() + .slice(0, maxLen); +} + } catch (error: unknown) { const err = error as { message?: string }; - const safeErrorDetails = (err?.message ?? '').slice(0, 1000); + const safeErrorDetails = sanitizeErrorDetails(err?.message ?? ''); console.error('Mailgun error details:', safeErrorDetails); void logEmail(to, subject, 'failed', undefined, safeErrorDetails); return false; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/email/index.ts` around lines 93 - 95, The code currently truncates err.message into safeErrorDetails but does not sanitize control or unsafe characters before logging and persisting; update the logic that builds safeErrorDetails (used in the console.error call and the logEmail invocation) to first strip or normalize non-printable/control characters (e.g., allow only printable UTF-8 characters, replace newlines/tabs with spaces, or use a sanitizer/escape routine), then truncate to the desired maximum length, and use that sanitized string for both console.error('Mailgun error details:', safeErrorDetails) and void logEmail(to, subject, 'failed', undefined, safeErrorDetails) so no raw/unsafe characters are logged or stored.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/email/index.ts`:
- Around line 71-74: The early-return branch that checks `mg` and
`process.env.MAILGUN_DOMAIN` should record a failed email attempt before
returning; update the block in lib/email/index.ts to create an `EmailLog` (or
call the existing email-logging routine) with status `"failed"` and include a
clear message/metadata indicating the Mailgun client or domain was missing
(reference `mg`, `process.env.MAILGUN_DOMAIN`, and the `EmailLog` creation code
paths), then return false as before so all initialization/config failures are
auditable.
---
Duplicate comments:
In `@lib/email/index.ts`:
- Around line 93-95: The code currently truncates err.message into
safeErrorDetails but does not sanitize control or unsafe characters before
logging and persisting; update the logic that builds safeErrorDetails (used in
the console.error call and the logEmail invocation) to first strip or normalize
non-printable/control characters (e.g., allow only printable UTF-8 characters,
replace newlines/tabs with spaces, or use a sanitizer/escape routine), then
truncate to the desired maximum length, and use that sanitized string for both
console.error('Mailgun error details:', safeErrorDetails) and void logEmail(to,
subject, 'failed', undefined, safeErrorDetails) so no raw/unsafe characters are
logged or stored.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8229f76e-cf0c-4b6e-98ce-ee9f99ab8029
📒 Files selected for processing (2)
.gitignorelib/email/index.ts
✅ Files skipped from review due to trivial changes (1)
- .gitignore
Contributor
Author
|
@MrImmortal09 Looks like it's fine now |
Collaborator
|
The PR looks fine to me but each file contains a large number of unnecessary comments which in my opinion should be removed. Keep the ones that are important for other to understand, else remove them. |
Collaborator
Agreed , but this can be merged for now, @DistantMyth take note from next time. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@MrImmortal09
Description
This PR implements a robust email delivery integration using Mailgun. It introduces the core email sending service and a database logging mechanism to keep track of all outbound email requests (both successful deliveries and failed attempts).
Key changes included:
.env.example.EmailLogMongoose model to record the success/failure status and metadata of sent emails.package.jsonandpnpm-lock.yaml) with required packages.Closes #22
Type of Change
Checklist
feat/,fix/,docs/, etc.)pnpm lintand there are no errorspnpm dev)Screenshots / Screen Recording
Summary by CodeRabbit
New Features
Configuration
Dependencies
Chores