Refactor email manager with improved utilities#12
Open
ShashankFC wants to merge 1 commit intomainfrom
Open
Conversation
Entelligence AI Vulnerability ScannerStatus: No security vulnerabilities found Your code passed our comprehensive security analysis. Analyzed 2 files in total |
Review Summary🏷️ Draft Comments (3)
|
🔬 Multi-Approach Review SummaryThis PR was reviewed by 2 different approaches for comparison:
Total: 2 review comments Each comment is labeled with its source approach. This allows you to compare different AI review strategies. 🔒 Security Scan: Run once and shared across all approaches for efficiency. WalkthroughThis PR refactors the email management system to improve code maintainability and error handling. The main change introduces a new Changes
Sequence DiagramThis diagram shows the interactions between components: sequenceDiagram
participant Caller
participant SSES as "_sendScheduledEmailsAndSMS"
participant SSE as "shouldSendEmail"
participant SE as "sendEmail"
participant Email as "Email Object"
Caller->>SSES: _sendScheduledEmailsAndSMS(calEvent, eventNameObject)
activate SSES
Note over SSES: Format calendar event
alt Host email check
SSES->>SSE: shouldSendEmail(metadata, 'host')
activate SSE
SSE->>SSE: Check eventTypeDisableHostEmail(metadata)
SSE-->>SSES: return boolean
deactivate SSE
alt Should send host email
SSES->>SE: sendEmail(() => new OrganizerScheduledEmail())
activate SE
SE->>Email: prepare()
Email-->>SE: email object
SE->>Email: sendEmail()
Email-->>SE: result
SE-->>SSES: Promise
deactivate SE
end
end
alt Attendee email check
SSES->>SSE: shouldSendEmail(metadata, 'attendee')
activate SSE
SSE->>SSE: Check eventTypeDisableAttendeeEmail(metadata)
SSE-->>SSES: return boolean
deactivate SSE
alt Should send attendee emails
loop For each attendee
SSES->>SE: sendEmail(() => new AttendeeScheduledEmail())
activate SE
SE->>Email: prepare()
Email-->>SE: email object
SE->>Email: sendEmail()
alt Email preparation fails
Email-->>SE: throw error
Note over SE: Log error and reject
end
Email-->>SE: result
SE-->>SSES: Promise
deactivate SE
end
end
end
Note over SSES: Wait for all email promises
SSES-->>Caller: Complete
deactivate SSES
🔒 Security Analysis
Note for WindsurfPlease change the default marketplace provider to the following in the windsurf settings:Marketplace Extension Gallery Service URL: https://marketplace.visualstudio.com/_apis/public/gallery Marketplace Gallery Item URL: https://marketplace.visualstudio.com/items Entelligence.ai can learn from your feedback. Simply add 👍 / 👎 emojis to teach it your preferences. More shortcuts belowEmoji Descriptions:
Interact with the Bot:
Also you can trigger various commands with the bot by doing The current supported commands are
More commands to be added soon. |
![entelligence-ai-pr-reviews[bot]](https://avatars.githubusercontent.com/in/932812?s=60&v=4){kind=small}
Comment on lines
51
to
58
| const email = prepare(); | ||
| resolve(email.sendEmail()); | ||
| } catch (e) { | ||
| reject(console.error(`${prepare.constructor.name}.sendEmail failed`, e)); | ||
| console.error(`Email preparation failed`, e); | ||
| reject(e); | ||
| } | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Correctness: 🟠 [LangGraph v3] Replace console.error with a logging utility to avoid exposing sensitive information.
📝 Committable Code Suggestion
‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Suggested change
| const email = prepare(); | |
| resolve(email.sendEmail()); | |
| } catch (e) { | |
| reject(console.error(`${prepare.constructor.name}.sendEmail failed`, e)); | |
| console.error(`Email preparation failed`, e); | |
| reject(e); | |
| } | |
| }); | |
| }; | |
| try { | |
| const email = prepare(); | |
| resolve(email.sendEmail()); | |
| } catch (e) { | |
| loggingUtility.error('Email preparation failed', { error: e }); | |
| reject(e); | |
| } |
Comment on lines
+1
to
+80
| import type { CalendarEvent } from "@calcom/types/Calendar"; | ||
|
|
||
| /** | ||
| * Email helper utilities for common email operations | ||
| */ | ||
|
|
||
| /** | ||
| * Validates email address format | ||
| * @param email - Email address to validate | ||
| * @returns boolean indicating if email is valid | ||
| */ | ||
| export function isValidEmail(email: string): boolean { | ||
| if (!email) return false; | ||
|
|
||
| const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; | ||
| return emailRegex.test(email); | ||
| } | ||
|
|
||
| /** | ||
| * Sanitizes email content to prevent injection | ||
| * @param content - Content to sanitize | ||
| * @returns Sanitized content | ||
| */ | ||
| export function sanitizeEmailContent(content: string): string { | ||
| if (!content) return ""; | ||
|
|
||
| return content | ||
| .replace(/<script[^>]*>.*?<\/script>/gi, "") | ||
| .replace(/<iframe[^>]*>.*?<\/iframe>/gi, "") | ||
| .replace(/javascript:/gi, "") | ||
| .trim(); | ||
| } | ||
|
|
||
| /** | ||
| * Formats attendee list for email display | ||
| * @param attendees - Array of attendee objects | ||
| * @returns Formatted string of attendee names | ||
| */ | ||
| export function formatAttendeeList(attendees: CalendarEvent["attendees"]): string { | ||
| if (!attendees || attendees.length === 0) { | ||
| return "No attendees"; | ||
| } | ||
|
|
||
| if (attendees.length === 1) { | ||
| return attendees[0].name; | ||
| } | ||
|
|
||
| if (attendees.length === 2) { | ||
| return `${attendees[0].name} and ${attendees[1].name}`; | ||
| } | ||
|
|
||
| const firstAttendees = attendees.slice(0, 2).map(a => a.name).join(", "); | ||
| return `${firstAttendees}, and ${attendees.length - 2} other${attendees.length - 2 > 1 ? "s" : ""}`; | ||
| } | ||
|
|
||
| /** | ||
| * Checks if an email should be rate limited | ||
| * @param recipientEmail - Email address of recipient | ||
| * @param eventType - Type of email event | ||
| * @returns boolean indicating if email should be sent | ||
| */ | ||
| export function shouldRateLimitEmail(recipientEmail: string, eventType: string): boolean { | ||
| // Placeholder for rate limiting logic | ||
| // In production, this would check against a rate limit store | ||
| return false; | ||
| } | ||
|
|
||
| /** | ||
| * Extracts domain from email address | ||
| * @param email - Email address | ||
| * @returns Domain portion of email | ||
| */ | ||
| export function extractEmailDomain(email: string): string | null { | ||
| if (!isValidEmail(email)) { | ||
| return null; | ||
| } | ||
|
|
||
| const parts = email.split("@"); | ||
| return parts.length === 2 ? parts[1] : null; | ||
| } |
There was a problem hiding this comment.
Correctness: 🟠 [LangGraph v3] The recipientEmail and eventType parameters in shouldRateLimitEmail are defined but never used, leading to unnecessary code clutter. Consider removing these parameters if they are not needed.
📝 Committable Code Suggestion
‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Suggested change
| import type { CalendarEvent } from "@calcom/types/Calendar"; | |
| /** | |
| * Email helper utilities for common email operations | |
| */ | |
| /** | |
| * Validates email address format | |
| * @param email - Email address to validate | |
| * @returns boolean indicating if email is valid | |
| */ | |
| export function isValidEmail(email: string): boolean { | |
| if (!email) return false; | |
| const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; | |
| return emailRegex.test(email); | |
| } | |
| /** | |
| * Sanitizes email content to prevent injection | |
| * @param content - Content to sanitize | |
| * @returns Sanitized content | |
| */ | |
| export function sanitizeEmailContent(content: string): string { | |
| if (!content) return ""; | |
| return content | |
| .replace(/<script[^>]*>.*?<\/script>/gi, "") | |
| .replace(/<iframe[^>]*>.*?<\/iframe>/gi, "") | |
| .replace(/javascript:/gi, "") | |
| .trim(); | |
| } | |
| /** | |
| * Formats attendee list for email display | |
| * @param attendees - Array of attendee objects | |
| * @returns Formatted string of attendee names | |
| */ | |
| export function formatAttendeeList(attendees: CalendarEvent["attendees"]): string { | |
| if (!attendees || attendees.length === 0) { | |
| return "No attendees"; | |
| } | |
| if (attendees.length === 1) { | |
| return attendees[0].name; | |
| } | |
| if (attendees.length === 2) { | |
| return `${attendees[0].name} and ${attendees[1].name}`; | |
| } | |
| const firstAttendees = attendees.slice(0, 2).map(a => a.name).join(", "); | |
| return `${firstAttendees}, and ${attendees.length - 2} other${attendees.length - 2 > 1 ? "s" : ""}`; | |
| } | |
| /** | |
| * Checks if an email should be rate limited | |
| * @param recipientEmail - Email address of recipient | |
| * @param eventType - Type of email event | |
| * @returns boolean indicating if email should be sent | |
| */ | |
| export function shouldRateLimitEmail(recipientEmail: string, eventType: string): boolean { | |
| // Placeholder for rate limiting logic | |
| // In production, this would check against a rate limit store | |
| return false; | |
| } | |
| /** | |
| * Extracts domain from email address | |
| * @param email - Email address | |
| * @returns Domain portion of email | |
| */ | |
| export function extractEmailDomain(email: string): string | null { | |
| if (!isValidEmail(email)) { | |
| return null; | |
| } | |
| const parts = email.split("@"); | |
| return parts.length === 2 ? parts[1] : null; | |
| } | |
| import type { CalendarEvent } from "@calcom/types/Calendar"; | |
| /** | |
| * Email helper utilities for common email operations | |
| */ | |
| /** | |
| * Validates email address format | |
| * @param email - Email address to validate | |
| * @returns boolean indicating if email is valid | |
| */ | |
| export function isValidEmail(email: string): boolean { | |
| if (!email) return false; | |
| const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; | |
| return emailRegex.test(email); | |
| } | |
| /** | |
| * Sanitizes email content to prevent injection | |
| * @param content - Content to sanitize | |
| * @returns Sanitized content | |
| */ | |
| export function sanitizeEmailContent(content: string): string { | |
| if (!content) return ""; | |
| return content | |
| .replace(/<script[^>]*>.*?<\/script>/gi, "") | |
| .replace(/<iframe[^>]*>.*?<\/iframe>/gi, "") | |
| .replace(/javascript:/gi, "") | |
| .trim(); | |
| } | |
| /** | |
| * Formats attendee list for email display | |
| * @param attendees - Array of attendee objects | |
| * @returns Formatted string of attendee names | |
| */ | |
| export function formatAttendeeList(attendees: CalendarEvent["attendees"]): string { | |
| if (!attendees || attendees.length === 0) { | |
| return "No attendees"; | |
| } | |
| if (attendees.length === 1) { | |
| return attendees[0].name; | |
| } | |
| if (attendees.length === 2) { | |
| return `${attendees[0].name} and ${attendees[1].name}`; | |
| } | |
| const firstAttendees = attendees.slice(0, 2).map(a => a.name).join(", "); | |
| return `${firstAttendees}, and ${attendees.length - 2} other${attendees.length - 2 > 1 ? "s" : ""}`; | |
| } | |
| /** | |
| * Checks if an email should be rate limited | |
| * @returns boolean indicating if email should be sent | |
| */ | |
| export function shouldRateLimitEmail(): boolean { | |
| // Placeholder for rate limiting logic | |
| // In production, this would check against a rate limit store | |
| return false; | |
| } | |
| /** | |
| * Extracts domain from email address | |
| * @param email - Email address | |
| * @returns Domain portion of email | |
| */ | |
| export function extractEmailDomain(email: string): string | null { | |
| if (!isValidEmail(email)) { | |
| return null; | |
| } | |
| const parts = email.split("@"); | |
| return parts.length === 2 ? parts[1] : null; | |
| } | |
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
1 participant
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.
This PR refactors the email manager to improve error handling, adds a reusable helper function for email metadata checks, and includes new email utility functions for validation and formatting.
EntelligenceAI PR Summary
This PR refactors email management by consolidating email sending logic and adding reusable utility functions for improved maintainability and security.
shouldSendEmail()helper function in email-manager.ts to consolidate recipient-based email sending logicsendEmail()to properly reject promises with actual error objects