feat: automatic emails #18
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds React-based email templates, mailer wrappers, and Azure Graph email sending (client + sendEmail). Adds env vars and dependencies for email tooling, a /test/welcome-email route, WebSocketServer rename + close(), graceful shutdown and global error handlers, and a MaybeArray type. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Backend
participant Renderer
participant GraphAPI
Client->>Backend: POST /test/welcome-email (toAddress, firstName, assocNumber)
Backend->>Renderer: Render WelcomeEmail -> HTML + plain text
Renderer-->>Backend: HTML + plain text
Backend->>Backend: Compose MIME via MailComposer, base64-encode
Backend->>GraphAPI: POST /users/{sender}/sendMail (base64 MIME) with TokenCredential
GraphAPI-->>Backend: Success / Error
Backend-->>Client: JSON result (success: boolean)
Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used🧬 Code graph analysis (1)backend/src/emails/templates/welcome.tsx (3)
🔇 Additional comments (3)
✏️ Tip: You can disable this entire section by setting 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 review |
✅ Actions performedReview triggered.
|
![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
♻️ Duplicate comments (2)
backend/src/server.ts (1)
48-65: Security: Remove hardcoded personal email and credentials from test route.This test endpoint contains a real email address and test credentials that shouldn't be in the codebase. Even for WIP code, this poses risks if accidentally deployed. The
forceProd: trueflag will send real emails via production SMTP.Suggested improvements
app.post("/test-email", async (c) => { - if (env.NODE_ENV === "production") return c.status(500) + if (env.NODE_ENV === "production") return c.text("Not available in production", 500) await sendEmail({ - to: "lorenzo.corallo@mail.polimi.it", + to: env.TEST_EMAIL_RECIPIENT ?? "test@example.com", subject: "Welcome to PoliNetwork", email: WelcomeEmail, - props: { email: "example@polinetwork.org", password: "R@123123123123as", assocNum: 12, firstName: "Lorenzo" }, - forceProd: true, + props: { email: "test@polinetwork.org", password: "TestPassword123!", assocNum: 1, firstName: "Test" }, }) - // await sendEmail({ - // to: "test@example.org", - // subject: "Welcome to PoliNetwork", - // email: WelcomeEmail, - // props: { email: "example@polinetwork.org", password: "R@123123123123as", assocNum: 12, firstName: "Lorenzo" }, - // }) return c.text("Success!") })Consider:
- Remove
forceProd: trueto use the Ethereal test transport in development- Use environment variables for test recipients
- Remove commented-out code
backend/src/emails/index.tsx (1)
52-52: Missing closing angle bracket in email address.The
fromaddress is malformed:<noreply@example.comshould be<noreply@example.com>.- from: "Test PoliNetwork APS <noreply@example.com", + from: "Test PoliNetwork APS <noreply@example.com>",
🧹 Nitpick comments (4)
backend/package.json (1)
44-44: Move@types/reactto devDependencies.Type definitions are only needed at build/compile time, not at runtime. Placing them in
dependenciesunnecessarily increases production bundle size.Suggested change
"devDependencies": { "@biomejs/biome": "^2.2.4", "@react-email/preview-server": "4.3.2", "@types/node": "^24.5.2", "@types/nodemailer": "^7.0.3", "@types/pg": "^8.15.5", + "@types/react": "^19.2.2", "drizzle-kit": "^0.31.4", ... }, "dependencies": { ... - "@types/react": "^19.2.2", ... }backend/src/env.ts (2)
42-42: Add minimum length validation forSMTP_PASS.Without a minimum length constraint, an effectively empty password could pass validation if
emptyStringAsUndefineddoesn't catch edge cases.- SMTP_PASS: z.string(), + SMTP_PASS: z.string().min(1),
43-43: Consider removing deprecated TLS versions.TLSv1 and TLSv1.1 are deprecated and considered insecure. If Azure Communication Services or your SMTP provider supports it, limiting to TLSv1.2+ would improve security posture.
- SMTP_TLS_VERSION: z.enum(["TLSv1.3", "TLSv1.2", "TLSv1.1", "TLSv1"]).default("TLSv1.2"), + SMTP_TLS_VERSION: z.enum(["TLSv1.3", "TLSv1.2"]).default("TLSv1.2"),backend/src/emails/templates/welcome.tsx (1)
36-36: Consider hosting the logo on a stable CDN.Using a GitHub raw URL (
raw.githubusercontent.com) for the logo could break if the repository structure changes or if GitHub changes their raw content delivery. Consider hosting on a dedicated CDN or using a versioned asset URL.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
backend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
backend/package.jsonbackend/src/emails/index.tsxbackend/src/emails/templates/welcome.tsxbackend/src/env.tsbackend/src/server.tsbackend/src/utils/types.tsbackend/src/websocket/index.tsbackend/tsconfig.json
🧰 Additional context used
🧬 Code graph analysis (3)
backend/src/emails/index.tsx (3)
backend/src/env.ts (1)
env(8-65)backend/src/utils/types.ts (1)
MaybeArray(1-1)backend/src/logger.ts (1)
logger(3-5)
backend/src/server.ts (2)
backend/src/env.ts (1)
env(8-65)backend/src/emails/index.tsx (1)
sendEmail(34-66)
backend/src/env.ts (1)
backend/src/constants.ts (1)
TRUSTED_ORIGINS(5-12)
🔇 Additional comments (7)
backend/package.json (1)
14-14: LGTM!The email dev script is well-configured, pointing to the correct directory for email templates.
backend/tsconfig.json (1)
5-5: LGTM!The TypeScript configuration changes correctly enable JSX support for the React Email templates. Using
"preserve"is appropriate here since the JSX transformation will be handled by the email rendering toolchain.Also applies to: 10-10
backend/src/utils/types.ts (1)
1-1: LGTM!Clean utility type that enables flexibility for single or multiple email recipients.
backend/src/websocket/index.ts (1)
14-14: LGTM!Good naming consistency improvement -
WebSocketServercorrectly follows PascalCase convention.backend/src/emails/index.tsx (1)
33-40: LGTM on the sendEmail API design.The generic typing for email props and flexible recipient handling with
MaybeArrayis well-designed.backend/src/env.ts (1)
38-41: LGTM on SMTP configuration structure.Good defaults for Azure Communication Services and proper use of
z.email()for the SMTP_USER validation.backend/src/emails/templates/welcome.tsx (1)
25-105: Well-structured email template.The template is clean with good use of Tailwind for styling, proper accessibility (alt text on images), and semantic structure. The zero-width non-joiner technique for email obfuscation is a nice touch for reducing spam scraping.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
backend/src/emails/templates/welcome.tsx (1)
108-119: Multi-part TLD issue remains unresolved.The
EmailWithoutHrefcomponent still splits the domain on the first.only, which causes multi-part TLDs like.co.ukto be handled incorrectly (e.g.,user@example.co.ukwould render with incomplete domain information).This issue was previously flagged with a suggested fix that uses
lastIndexOfto properly split the domain.
🧹 Nitpick comments (1)
backend/src/emails/templates/welcome.tsx (1)
58-63: Consider aligning the link URL with the link text.The link text says "Microsoft Login" but the href points to
https://outlook.office.com. While this works (Outlook redirects to the login page), it may be clearer to usehttps://login.microsoftonline.com/to match the user's expectation from the text, or change the text to "Outlook" to match the URL.🔎 Option 1: Update href to match "Microsoft Login" text
To activate your Office 365 account, using a PC open{" "} - <Link href="https://outlook.office.com" className="text-[#1156AE] underline"> + <Link href="https://login.microsoftonline.com/" className="text-[#1156AE] underline"> Microsoft Login </Link>🔎 Option 2: Update text to match Outlook URL
To activate your Office 365 account, using a PC open{" "} <Link href="https://outlook.office.com" className="text-[#1156AE] underline"> - Microsoft Login + Outlook </Link>
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
backend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
backend/package.jsonbackend/src/emails/index.tsxbackend/src/emails/templates/welcome.tsxbackend/src/server.tsbackend/src/websocket/index.tsbackend/tsconfig.json
🚧 Files skipped from review as they are similar to previous changes (5)
- backend/package.json
- backend/tsconfig.json
- backend/src/server.ts
- backend/src/websocket/index.ts
- backend/src/emails/index.tsx
🔇 Additional comments (9)
backend/src/emails/templates/welcome.tsx (9)
1-17: LGTM!The imports are correctly structured with type-only React import (appropriate for React 19) and all @react-email/components are used in the template.
19-24: LGTM!Props type definition is clear and appropriate for the welcome email template.
66-73: LGTM!The credentials section is well-structured with appropriate visual styling to highlight the sensitive information.
85-90: LGTM!The Telegram contact link is properly implemented with the correct href attribute.
33-43: LGTM!The logo section is properly structured with appropriate sizing and alt text for accessibility.
46-56: LGTM!The welcome message properly uses the provided props and has a clear, personalized greeting structure.
75-83: LGTM!The instructions and important notice are clearly formatted with appropriate emphasis (bold text and red color) to draw attention to critical information.
96-101: LGTM!The footer section is clean and appropriately styled with a working link to the organization's website.
129-129: LGTM!Standard default export for the email template component.
|
@toto04 dovremmo esserci |
|
Rimetto in draft perché voglio migrare a Microsoft Graph API che ci permette di avere limiti di invio molto superiori utilizzando un utente vero e proprio (licenza Microsoft Business 365 Basic tipo) |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@backend/src/emails/components/logo.tsx`:
- Around line 1-13: The Logo component currently uses an SVG which many major
email clients block; update the Logo() function to use a PNG as the primary
image source (replace the current SVG URL in the Img src prop with a PNG URL of
the same logo and size) and add a robust fallback strategy (include a srcSet or
additional src fallback that references the SVG only as a secondary option,
preserve alt text and sizing classes on the Img component, and ensure the
Section/Img remain unchanged otherwise); after making the change, test rendering
in Gmail, Outlook (desktop/web) and Apple Mail to confirm the PNG displays
everywhere.
In `@backend/src/emails/mailer.tsx`:
- Around line 10-26: sendWelcomeEmail currently embeds plaintext
accountCredentials.password in the Welcome component and email body; stop
sending raw passwords. Change sendWelcomeEmail to generate a time-limited
setup/reset token (e.g., createPasswordSetupToken(accountCredentials.email) or
reuse existing reset flow), pass a secure setup link (not the password) to
sendEmail/Welcome, remove the password prop from the Welcome component, and
ensure backend enforces one-time token validation and forces password
creation/change on first use.
🧹 Nitpick comments (3)
backend/package.json (1)
25-33: Consider version pinning consistency and potential unused types.Two observations:
@types/nodemaileris added, but given the migration to Microsoft Graph API mentioned in the PR, this may become unused if nodemailer is fully replaced.Version pinning is inconsistent:
@react-email/preview-serverandreact-emailuse exact versions while@types/reactand@microsoft/microsoft-graph-typesuse caret ranges. Consider aligning the strategy for react-email ecosystem packages.backend/src/azure/client.ts (1)
19-28: Test request at module load has side effects on import.The fire-and-forget test request is useful for early credential validation, but be aware this executes whenever the module is imported (including during tests or when the email feature isn't needed).
Consider wrapping this in an initialization function that can be called explicitly from the application startup, or gating it behind a condition:
♻️ Optional: Extract to explicit initialization
+export async function testAzureConnection(): Promise<boolean> { + try { + const r: Organization = await client.api(`/organization/${env.AZURE_TENANT_ID}`).get() + logger.info({ orgName: r.displayName }, "[Azure Graph API] Client connected successfully") + return true + } catch (e) { + const error = e as GraphError + if (error.code === "AuthenticationRequiredError") + logger.error({ error: error.message }, "[Azure Graph API] Authentication failed, check credentials") + else logger.error({ error }, "[Azure Graph API] Error on TEST request") + return false + } +} + -// test request -- void because we do not want to wait -void client - .api(`/organization/${env.AZURE_TENANT_ID}`) - .get() - .then((r: Organization) => logger.info({ orgName: r.displayName }, "[Azure Graph API] Client connected successfully")) - .catch((e: GraphError) => { - if (e.code === "AuthenticationRequiredError") - logger.error({ error: e.message }, "[Azure Graph API] Authentication failed, check credentials") - else logger.error({ error: e }, "[Azure Graph API] Error on TEST request") - })backend/src/azure/functions.ts (1)
27-38: Consider validating thetoparameter before sending.The function accepts any string for
towithout validation. While the Graph API will reject invalid addresses, validating early provides better error messages and avoids unnecessary API calls.♻️ Suggested validation
export async function sendEmail(to: string, subject: string, component: JSX.Element) { + // Basic email format check before attempting to send + if (!to || !/^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(to)) { + logger.error({ to }, "[Azure Graph API] Invalid recipient email address") + return false + } + const html = await render(component)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
backend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
backend/package.jsonbackend/src/azure/client.tsbackend/src/azure/functions.tsbackend/src/emails/components/footer.tsxbackend/src/emails/components/logo.tsxbackend/src/emails/mailer.tsxbackend/src/emails/templates/otp.tsxbackend/src/emails/templates/welcome.tsxbackend/src/emails/types/index.tsbackend/src/env.tsbackend/src/server.tsbackend/src/websocket/index.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- backend/src/emails/templates/welcome.tsx
- backend/src/websocket/index.ts
- backend/src/server.ts
🧰 Additional context used
🧬 Code graph analysis (5)
backend/src/azure/functions.ts (3)
backend/src/env.ts (1)
env(8-64)backend/src/azure/client.ts (1)
client(17-17)backend/src/logger.ts (1)
logger(3-5)
backend/src/azure/client.ts (2)
backend/src/env.ts (1)
env(8-64)backend/src/logger.ts (1)
logger(3-5)
backend/src/emails/templates/otp.tsx (3)
backend/src/emails/types/index.ts (1)
EmailTemplate(1-1)backend/src/emails/components/logo.tsx (1)
Logo(3-13)backend/src/emails/components/footer.tsx (1)
Footer(3-17)
backend/src/emails/mailer.tsx (1)
backend/src/azure/functions.ts (1)
sendEmail(16-57)
backend/src/env.ts (1)
backend/src/constants.ts (1)
TRUSTED_ORIGINS(5-12)
🔇 Additional comments (13)
backend/package.json (3)
14-14: LGTM!The email preview script is correctly configured for react-email development workflow.
54-54:nodemailerdependency is correctly needed for MIME composition with MS Graph API.The dependency is actively used via
MailComposer(line 3 ofbackend/src/azure/functions.ts) to generate MIME-formatted email content, which is required by the Microsoft Graph API's/sendMailendpoint. The old SMTP-basedcreateTransportapproach is fully commented out. The migration to MS Graph is complete and the dependency serves a specific purpose in the new implementation.Likely an incorrect or invalid review comment.
57-58: No action required. React 19.2.0 and react-email 5.1.0 are compatible. React 19 support has been maintained through react-email's releases, including v5.x. While some@types/reactversion alignment issues may occur in edge cases, the current dependency versions in package.json are safe to use together.backend/src/env.ts (3)
13-17: TRUSTED_ORIGINS transformation is well-implemented.The pipeline correctly transforms a comma-separated string into a validated string array. The default value from
TRUSTED_ORIGINSconstant provides good development defaults.
21-25: ENCRYPTION_KEY validation looks appropriate.The hex string validation with 64-1024 character bounds is reasonable for encryption key material (representing 32-512 bytes of key data).
38-42: Azure configuration uses Zod 4's standalonez.email()correctly.The Azure environment variables are properly defined. Note that
AZURE_TENANT_ID,AZURE_CLIENT_ID, andAZURE_CLIENT_SECRETare required without defaults, so the application will fail to start if these are missing. Consider whether this is intentional for all environments (including development).Based on library documentation,
z.email()is the correct standalone format schema in Zod 4.backend/src/azure/client.ts (1)
8-17: Client credentials flow is correctly configured.The
ClientSecretCredentialwithTokenCredentialAuthenticationProviderusing.defaultscope is the appropriate pattern for daemon/service applications using the client credentials OAuth flow.backend/src/azure/functions.ts (2)
16-22: Email rendering and plain text conversion look correct.The
render()andtoPlainText()usage from@react-email/componentsis appropriate. The selector to skipdisplay:noneelements prevents duplicate text in the plain text version.
37-56: MIME-based Graph API email sending is correctly implemented.The approach of building MIME content with
MailComposer, base64 encoding, and posting to/users/{sender}/sendMailwithContent-Type: text/plainis the correct pattern for sending raw MIME messages via Microsoft Graph API.One consideration: the error object structure isn't preserved in the return value. If callers need to distinguish between different failure modes (auth issues vs. recipient issues vs. rate limits), consider returning a more detailed result type.
backend/src/emails/components/footer.tsx (1)
1-17: LGTM! Clean footer component with appropriate disclaimer.The footer provides good UX with the PoliNetwork link and clear "do not reply" messaging for automated emails. The component structure follows react-email patterns correctly.
backend/src/emails/types/index.ts (1)
1-1: LGTM!Clean type definition that enforces readonly props and provides optional preview props support for email templates.
backend/src/emails/mailer.tsx (1)
5-8: LGTM!Clean implementation for sending OTP emails.
backend/src/emails/templates/otp.tsx (1)
1-74: LGTM!Well-structured OTP email template with:
- Clean component architecture using React Email
- Proper conditional rendering for optional expiration notice
- Sensible preview props for development/testing
- Good accessibility practices (meta tag to disable phone detection)
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| import { Img, Section } from "@react-email/components" | ||
|
|
||
| export function Logo() { | ||
| return ( | ||
| <Section className="py-7 sm:py-10 text-center"> | ||
| <Img | ||
| src="https://raw.githubusercontent.com/PoliNetworkOrg/Logo/a1e02bf150ac0b3c734a629901eada79fb4fd762/Logo.svg" | ||
| alt="PoliNetwork APS Logo" | ||
| className="mx-auto block w-24 h-24 sm:w-32 sm:h-32" | ||
| /> | ||
| </Section> | ||
| ) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
email client SVG image support compatibility 2025
💡 Result:
Short summary (status as of 2025)
- Major web/desktop clients: Microsoft announced that Outlook (Outlook for Web and the new Outlook for Windows) stopped rendering inline SVG embedded in email bodies during the Sep–Oct 2025 rollout. Inline SVGs will appear blank; SVG attachments still open from the attachment well. [1][2]
- Gmail: historically and through 2024–2025 Gmail has not reliably displayed SVG images embedded in emails (many senders still avoid SVG for Gmail). [3]
- Apple Mail / native macOS & iOS mail: generally more permissive and have supported SVG rendering where the client uses the system WebKit renderer (but behavior can vary by OS/version). Use CanIEmail for per-client tests. [4]
- Other clients (Thunderbird, some webmail/UIs): mixed support; many ESPs and signature tools still warn that SVGs are not safe/compatible for mass email. [3][5]
Practical recommendations
- Don't rely on inline SVG for critical imagery (logos, CTAs). Use PNG (2x for retina) as a fallback or as the primary asset. [5]
- If you must use SVG, provide a robust fallback:
or server-side/templating fallbacks and include alt text. Test rendering across clients (use Litmus / Email on Acid / CanIEmail for up-to-date client checks). [4][5]
Sources
- Microsoft 365 Message Center – MC1130385 (Retirement of inline SVG images in Microsoft Outlook). [Microsoft change notice summary].
- Reporting on the Outlook change (TechRadar / industry posts summarizing the Sep–Oct 2025 rollout).
- Community/FAQ notes and long-standing issues: Gmail/other clients not reliably supporting SVG in email (StackOverflow, vendor docs).
- CanIEmail — email feature support tracker (SVG/image notes & client scoreboard).
- Vendor help articles (email platforms / signature tools) recommending PNG/JPG instead of SVG for email.
If you want, I can run a quick CanIEmail/ Litmus check now and produce a per-client table (Gmail web/mobile, Outlook desktop/web/mobile, Apple Mail macOS/iOS, Thunderbird, Yahoo, AOL).
SVG logo will fail to render in major email clients—use PNG with fallback.
Recent changes (Sept–Oct 2025) have made SVG incompatibility a critical issue: Microsoft Outlook (desktop and web) now blocks inline SVG rendering entirely, causing images to appear blank. Gmail also does not reliably display SVG images in email. For a logo—critical imagery—relying solely on SVG is unacceptable.
Replace the SVG URL with a PNG asset and provide a robust fallback strategy. Use srcset with both formats, test across clients (Gmail, Outlook desktop/web, Apple Mail), or serve PNG as the primary asset. Do not ship the component until this is resolved.
🤖 Prompt for AI Agents
In `@backend/src/emails/components/logo.tsx` around lines 1 - 13, The Logo
component currently uses an SVG which many major email clients block; update the
Logo() function to use a PNG as the primary image source (replace the current
SVG URL in the Img src prop with a PNG URL of the same logo and size) and add a
robust fallback strategy (include a srcSet or additional src fallback that
references the SVG only as a secondary option, preserve alt text and sizing
classes on the Img component, and ensure the Section/Img remain unchanged
otherwise); after making the change, test rendering in Gmail, Outlook
(desktop/web) and Apple Mail to confirm the PNG displays everywhere.
There was a problem hiding this comment.
@lorenzocorallo questa è una cosa effettivamente rilevante
posso switchare al PNG allo stesso url di github, ma ci sono due issue rilevanti
- il PNG è 3000x3000 e pesa 350kB che per l'amor di dio non è completamente folle ma è anche un po' inutile
- sarebbe meglio avere il più possibile incorporato nell'email dato che client tipo Mail di iOS spesso bloccano il caricamento di risorse esterne prima di consenso esplicito dall'utente (che è anche giusto dato che renderebbe l'apertura dell'email tracciabile)
che ne dici di incorporare l'email renderizzando un PNG a grandezza ridotta e piantandocelo dentro come base64? sarebbe anche carino poter importare direttamente il file .png come modulo e averlo convertito in base64 da un loader ma non penso che litigare con dei bundler tipo webpack per un singolo PNG sia la migliore delle scelte ahah
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
che ne dici di incorporare l'email renderizzando un PNG a grandezza ridotta e piantandocelo dentro come base64? sarebbe anche carino poter importare direttamente il file .png come modulo e averlo convertito in base64 da un loader ma non penso che litigare con dei bundler tipo webpack per un singolo PNG sia la migliore delle scelte ahah
Credo che sia per il momento sia la scelta ottimale, quantomeno per il logo che è un asset critico. @toto04
There was a problem hiding this comment.
Anche se probabilmente darei un occhio a come react-emails gestisce le immagini: https://react.email/docs/components/image
@toto04 magari è quello che cercavi per evitare di hard-codarde il base64 e far fare tutto al codice
There was a problem hiding this comment.
react-emails non sembra fare alcuna magia arcana per fare embedding delle immagini, quello che dai al src è quello che viene mandato, quindi base64 a mano sia
ho piantato il png 192x192, da quanto ho estrapolato w-24 in tailwind vuol dire 96px (default 1rem = 16px), e sembra essere la scelta migliore avere immagini PNG a 2x
andrebbe testato per assicurarci che sia effettivamente bello e non si spacchi niente
(even though there should be no usecase for this)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@backend/src/emails/templates/welcome.tsx`:
- Around line 95-107: The EmailWithoutHref component currently calls
email.split("@") and immediately destructures into user and domain, then calls
domain.split(".") which throws if email lacks "@"; update EmailWithoutHref to
first verify the presence of "@" (e.g., check email.includes("@") or
indexOf("@") !== -1) before splitting or handle the split result safely (guard
domain existence), and if the check fails return the fallback CodeInline with
the full email; ensure references to EmailWithoutHref, user, domain, domainName,
and domainTld remain but add the early guard so domain.split(".") is only
invoked when domain is defined.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/src/emails/templates/welcome.tsx
🔇 Additional comments (2)
backend/src/emails/templates/welcome.tsx (2)
27-93: LGTM!The WelcomeEmail component is well-structured, follows React Email patterns, and properly uses Tailwind styling. The Link components now include
hrefattributes (addressing previous feedback), and the email content is clear and professional.
109-114: LGTM!The
PreviewPropsassignment is clean with appropriate sample data, and the previous TypeScript typing issue has been resolved.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@backend/src/azure/functions.ts`:
- Line 30: The sender email is interpolated directly into the Graph API path
which breaks for special characters; URL-encode the AZURE_EMAIL_SENDER value
before constructing the API path used in client.api(`/users/${sender}/sendMail`)
(i.e., replace usage of sender with encodeURIComponent(sender)) so the call to
client.api(...) with header("Content-Type", "text/plain").post(base64Encoded) is
made with a safely encoded path; keep the same header and body variables
(base64Encoded) unchanged.
🧹 Nitpick comments (3)
backend/src/azure/functions.ts (3)
8-14: Consider adding input validation.The function accepts any string for
toandsubjectwithout validation. Consider validating the email format and ensuring the subject is non-empty to fail fast rather than making an API call that will fail anyway.💡 Optional validation example
export async function sendEmail(to: string, subject: string, component: JSX.Element) { + if (!to || !/^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(to)) { + logger.warn({ to }, "[Azure Graph API] Invalid recipient email") + return false + } + if (!subject.trim()) { + logger.warn("[Azure Graph API] Empty subject") + return false + } + const html = await render(component)
34-37: Consider exposing error context to callers.Returning
falseon failure doesn't let callers distinguish between transient issues (rate limits, network errors) and permanent failures (invalid credentials, bad recipient). This matters for retry logic or user feedback.💡 Alternative: throw or return a result object
// Option 1: Throw on failure (let caller handle) throw new Error(`Failed to send email: ${error instanceof Error ? error.message : "Unknown error"}`) // Option 2: Return a result object type EmailResult = { success: true } | { success: false; error: string }
29-31: The Graph API endpoint and MIME format are correct, but be aware of known limitations.The endpoint
/users/{sender}/sendMailwithContent-Type: text/plainand base64-encoded MIME body is the documented approach and is implemented correctly. However, be aware of these Microsoft Graph API limitations:
- Custom MIME headers (e.g., Return-Path, Auto-Submitted) may be removed or replaced by Graph.
- Known issues exist with certain attachment types, particularly text/calendar (ICS) files, where Graph may modify Content-Type parameters.
- MIME submissions always save to Sent Items (no option to override this behavior).
If your emails rely on specific custom headers or calendar-format attachments being preserved exactly as-is, consider testing thoroughly or using SMTP with OAuth2 for more granular control.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/src/azure/functions.ts
🧰 Additional context used
🧬 Code graph analysis (1)
backend/src/azure/functions.ts (3)
backend/src/env.ts (1)
env(8-64)backend/src/azure/client.ts (1)
client(17-17)backend/src/logger.ts (1)
logger(3-5)
🔇 Additional comments (2)
backend/src/azure/functions.ts (2)
1-6: LGTM!Imports are well-organized with appropriate type-only import for
JSX.
15-27: LGTM!Good use of MailComposer for MIME construction, and the comment clearly explains why raw MIME is needed for multipart emails with Graph API.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| import { Img, Section } from "@react-email/components" | ||
|
|
||
| export function Logo() { | ||
| return ( | ||
| <Section className="py-7 sm:py-10 text-center"> | ||
| <Img | ||
| src="https://raw.githubusercontent.com/PoliNetworkOrg/Logo/a1e02bf150ac0b3c734a629901eada79fb4fd762/Logo.svg" | ||
| alt="PoliNetwork APS Logo" | ||
| className="mx-auto block w-24 h-24 sm:w-32 sm:h-32" | ||
| /> | ||
| </Section> | ||
| ) | ||
| } |
There was a problem hiding this comment.
@lorenzocorallo questa è una cosa effettivamente rilevante
posso switchare al PNG allo stesso url di github, ma ci sono due issue rilevanti
- il PNG è 3000x3000 e pesa 350kB che per l'amor di dio non è completamente folle ma è anche un po' inutile
- sarebbe meglio avere il più possibile incorporato nell'email dato che client tipo Mail di iOS spesso bloccano il caricamento di risorse esterne prima di consenso esplicito dall'utente (che è anche giusto dato che renderebbe l'apertura dell'email tracciabile)
che ne dici di incorporare l'email renderizzando un PNG a grandezza ridotta e piantandocelo dentro come base64? sarebbe anche carino poter importare direttamente il file .png come modulo e averlo convertito in base64 da un loader ma non penso che litigare con dei bundler tipo webpack per un singolo PNG sia la migliore delle scelte ahah
…ontain the `@` symbol
No description provided.