Feat/send qr codes#23
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR adds two new QR-code check-in email templates ( Key changes:
Issues found:
Confidence Score: 5/5Safe to merge; all remaining findings are style and performance suggestions with no correctness or data-integrity impact. The core QR logic — base64 normalisation, size guard, CID attachment, pre-flight missing-QR check — is correct. The non-QR batch path is unchanged. All open findings are P2: sequential sending (performance, not correctness), dead code, a duplicate guard, and a clarification question about the logo removal. None block functionality. lib/templates/components/EmailLayout.tsx — confirm the logo removal from the shared layout is intentional before merging, as it silently changes every existing email template. Important Files Changed
|
| } else if (recipientType === "status") { | ||
| if (!isParticipantStatus(status)) { | ||
| return NextResponse.json( | ||
| { error: "Invalid participant status" }, | ||
| { status: 400 }, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Duplicate
isParticipantStatus validation
The isParticipantStatus(status) guard at lines 143–148 already returns a 400 before any database work begins, so the identical check inside the DB try block (lines 195–200) can never be reached. The inner check is dead code and can be removed.
| } else if (recipientType === "status") { | |
| if (!isParticipantStatus(status)) { | |
| return NextResponse.json( | |
| { error: "Invalid participant status" }, | |
| { status: 400 }, | |
| ); | |
| } | |
| } else if (recipientType === "status") { | |
| const statusParticipants = await db |
| if (isQrTemplate) { | ||
| for (const email of recipients) { | ||
| const participant = participantMap.get(email); | ||
| const qrInline = participant?.qrInline; | ||
| const vars = recipientVariables[email] ?? { firstName: "Hacker" }; | ||
|
|
||
| if (!qrInline) { | ||
| results.push({ | ||
| recipient: email, | ||
| success: false, | ||
| error: "Missing QR image data", | ||
| }); | ||
| continue; | ||
| } | ||
|
|
||
| const [html, text] = await Promise.all([ | ||
| renderTemplateToHtml(templateId, { | ||
| firstName: vars.firstName, | ||
| qrImageSrc: qrInline.src, | ||
| }), | ||
| renderTemplateToText(templateId, { | ||
| firstName: vars.firstName, | ||
| qrImageSrc: qrInline.src, | ||
| }), | ||
| ]); | ||
|
|
||
| if (!html || !text) { | ||
| results.push({ | ||
| recipient: email, | ||
| success: false, | ||
| error: "Failed to render email template", | ||
| }); | ||
| continue; | ||
| } | ||
|
|
||
| try { | ||
| const response = await mg.messages.create(mailgunDomain, { | ||
| from: fromEmail, | ||
| to: email, | ||
| subject: emailSubject, | ||
| html, | ||
| text, | ||
| inline: [ | ||
| { | ||
| filename: qrInline.filename, | ||
| data: qrInline.data, | ||
| }, | ||
| ], | ||
| }); | ||
|
|
||
| try { | ||
| const response = await mg.messages.create(mailgunDomain, { | ||
| from: fromEmail, | ||
| to: batch, | ||
| subject: emailSubject, | ||
| html: html, | ||
| text: text, | ||
| "recipient-variables": JSON.stringify(batchVars), | ||
| }); | ||
|
|
||
| console.log( | ||
| `Batch ${Math.floor(i / BATCH_SIZE) + 1} sent (${batch.length} recipients):`, | ||
| response.id, | ||
| console.log(`QR email sent to ${email}:`, response.id); | ||
| results.push({ | ||
| recipient: email, | ||
| success: true, | ||
| messageId: response.id, | ||
| }); | ||
| } catch (error) { | ||
| console.error(`QR email send failed for ${email}:`, error); | ||
| results.push({ | ||
| recipient: email, | ||
| success: false, | ||
| error: | ||
| error instanceof Error | ||
| ? error.message | ||
| : "Unknown error", | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Sequential QR sends may be slow for large recipient lists
Each iteration awaits both a template render and a Mailgun API call before moving to the next recipient. For a hackathon with hundreds of confirmed attendees, this loop runs entirely in series and could exceed serverless function timeouts (e.g., 60 s on Vercel Pro) or take several minutes even if it completes.
Consider sending a bounded number of emails concurrently — for example, processing recipients in small parallel batches using Promise.allSettled:
const CONCURRENCY = 10;
for (let i = 0; i < recipients.length; i += CONCURRENCY) {
const slice = recipients.slice(i, i + CONCURRENCY);
await Promise.allSettled(slice.map(async (email) => { /* send logic */ }));
}This keeps throughput up while avoiding hammering Mailgun's rate limits.
Description
Checklist when merging to main
oxfmtorprettier