Show drafts in thread #324
Conversation
|
Warning Rate limit exceeded@elie222 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 16 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThis pull request updates how email threads are handled across components. The Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
apps/web/components/email-list/EmailPanel.tsx (2)
204-210: Fix potential memory leak in useEffect.The setTimeout should be cleaned up to prevent memory leaks.
useEffect(() => { if (defaultShowReply && replyRef.current) { - setTimeout(() => { + const timer = setTimeout(() => { replyRef.current?.scrollIntoView({ behavior: "smooth" }); }, 300); + return () => clearTimeout(timer); } }, [defaultShowReply]);
317-317: Fix onClick handler placement.The onClick handler is incorrectly placed on the icon instead of the button.
- <ForwardIcon className="h-4 w-4" onClick={onForward} /> + <ForwardIcon className="h-4 w-4" /> - <Button variant="ghost" size="icon"> + <Button variant="ghost" size="icon" onClick={onForward}>
🧹 Nitpick comments (3)
apps/web/hooks/useThread.ts (1)
7-14: Consider enhancing type safety for options parameter.The implementation looks good, but we can improve type safety by:
- Defining a dedicated type for the options parameter
- Using URLSearchParams more efficiently
+type ThreadOptions = { + includeDrafts?: boolean; +}; + export function useThread( { id }: ThreadQuery, - options?: { includeDrafts?: boolean }, + options?: ThreadOptions, ) { - const searchParams = new URLSearchParams(); - if (options?.includeDrafts) searchParams.set("includeDrafts", "true"); - const url = `/api/google/threads/${id}?${searchParams.toString()}`; + const searchParams = options?.includeDrafts ? "?includeDrafts=true" : ""; + const url = `/api/google/threads/${id}${searchParams}`; return useSWR<ThreadResponse>(url); }apps/web/components/email-list/EmailPanel.tsx (2)
141-163: Consider optimizing draft organization logic.The current implementation could be improved for better performance and error handling:
- Consider early return for empty messages array
- Add error handling for malformed message headers
- Use Set for better lookup performance
const organizedMessages = useMemo(() => { + if (!messages?.length) return []; + const drafts = new Map<string, EmailMessage>(); const regularMessages: EmailMessage[] = []; messages?.forEach((message) => { if (message.labelIds?.includes("DRAFT")) { - const parentId = - message.headers.references?.split(" ").pop() || - message.headers["in-reply-to"]; + try { + const parentId = + message.headers.references?.split(" ").pop() || + message.headers["in-reply-to"]; + if (parentId) { + drafts.set(parentId, message); + } + } catch (error) { + console.error("Error processing draft message:", error); + } - if (parentId) { - drafts.set(parentId, message); - } } else { regularMessages.push(message); } });
441-467: Consider using a Map for MIME type mapping.The switch statement could be replaced with a Map for better maintainability and performance.
+const mimeTypeMap = new Map([ + ["application/pdf", "PDF"], + ["application/zip", "ZIP"], + ["image/png", "PNG"], + ["image/jpeg", "JPEG"], + ["application/vnd.openxmlformats-officedocument.wordprocessingml.document", "DOCX"], + ["application/vnd.openxmlformats-officedocument.spreadsheetml.sheet", "XLSX"], + ["application/vnd.openxmlformats-officedocument.presentationml.presentation", "PPTX"], + ["application/vnd.ms-excel", "XLS"], + ["application/vnd.ms-powerpoint", "PPT"], + ["application/vnd.ms-word", "DOC"], +]); + function mimeTypeToString(mimeType: string): string { - switch (mimeType) { - case "application/pdf": - return "PDF"; - // ... other cases - default: - return mimeType; - } + return mimeTypeMap.get(mimeType) ?? mimeType; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/components/EmailViewer.tsx(1 hunks)apps/web/components/email-list/EmailPanel.tsx(4 hunks)apps/web/hooks/useThread.ts(1 hunks)
🔇 Additional comments (2)
apps/web/components/EmailViewer.tsx (1)
52-57: LGTM! Clean implementation of draft inclusion.The hook usage is correct and aligns with the new functionality.
apps/web/components/email-list/EmailPanel.tsx (1)
34-34: LGTM! Good type alias introduction.The EmailMessage type alias improves code readability and maintainability.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
apps/web/utils/parse/extract-reply.client.ts (1)
8-11: Consider supporting additional email client formats.The current implementation only handles Gmail's quote format. Consider extending support for other email clients (e.g., Outlook, Apple Mail).
// Find the first gmail_quote container const quoteContainer = doc.querySelector( - ".gmail_quote_container, .gmail_quote", + ".gmail_quote_container, .gmail_quote, .ms-outlook-quote, .apple-mail-quote", );apps/web/utils/parse/extract-reply.client.test.ts (2)
7-9: Improve test environment setup and cleanup.Consider using beforeAll/afterAll hooks for better test isolation.
// Setup JSDOM -const dom = new JSDOM(); -global.DOMParser = dom.window.DOMParser; +let dom: JSDOM; + +beforeAll(() => { + dom = new JSDOM(); + global.DOMParser = dom.window.DOMParser; +}); + +afterAll(() => { + dom.window.close(); +});
11-94: Add test cases for error scenarios and enhance test data organization.The test suite should include error cases and separate test data for better maintainability.
Consider adding these test cases:
// In a separate file: extract-reply.test.data.ts export const TEST_CASES = { gmailQuoteContainer: { input: `<div dir="ltr">...</div>`, expected: { latestReply: '...', originalThread: '...' } }, // ... other test cases }; // In the test file: it("handles malformed HTML", () => { const html = "<div>unclosed div"; const result = extractEmailReply(html); expect(result.latestReply).toBe("<div>unclosed div"); expect(result.originalThread).toBe(""); }); it("handles null/undefined input", () => { // @ts-expect-error testing null input expect(extractEmailReply(null)).toEqual({ latestReply: "", originalThread: "" }); // @ts-expect-error testing undefined input expect(extractEmailReply(undefined)).toEqual({ latestReply: "", originalThread: "" }); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
apps/web/components/email-list/EmailPanel.tsx(4 hunks)apps/web/package.json(1 hunks)apps/web/utils/parse/extract-reply.client.test.ts(1 hunks)apps/web/utils/parse/extract-reply.client.ts(1 hunks)package.json(1 hunks)turbo.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/components/email-list/EmailPanel.tsx
🔇 Additional comments (3)
package.json (1)
1-10: New Test Script AdditionThe addition of the
"test": "turbo run test --filter=./apps/web"script is a clear and concise way to target tests within the./apps/webdirectory using Turbo. Please ensure that the specified path accurately reflects your project’s structure and that the usage is documented accordingly for other team members.turbo.json (1)
103-105: Introduction of the Test Task with Environment ConfigurationThe new
"test"task—which sets the environment variableRUN_AI_TESTS—provides a targeted configuration for test runs. This approach is particularly useful if your test logic conditionally depends on this variable (for example, when handling draft messages in threads). It would be good to verify that the test runner correctly interprets this setting and that the tests behave as expected.apps/web/package.json (1)
144-144: Addition of jsdom Type DefinitionsAdding
"@types/jsdom": "21.1.7"to thedevDependenciesis a beneficial update to ensure that you have the appropriate TypeScript typings for jsdom, which can aid in testing environments that simulate browser behavior. Confirm that this specific version aligns with your current jsdom version and overall project requirements.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
apps/web/components/email-list/EmailPanel.tsx (1)
247-251: Fix forward button click handler placement.The onClick handler is incorrectly placed on the ForwardIcon instead of the Button component.
Apply this fix:
-<Button variant="ghost" size="icon"> - <ForwardIcon className="h-4 w-4" onClick={onForward} /> +<Button variant="ghost" size="icon" onClick={onForward}> + <ForwardIcon className="h-4 w-4" /> <span className="sr-only">Forward</span> </Button>
🧹 Nitpick comments (2)
apps/web/components/email-list/EmailPanel.tsx (2)
137-159: Well-structured draft organization logic!The memoized function efficiently organizes draft messages as replies to their parent messages using a Map for lookups.
Consider adding error handling for malformed messages:
const parentId = message.headers.references?.split(" ").pop() || message.headers["in-reply-to"]; -if (parentId) { +if (parentId && message.headers["message-id"]) { drafts.set(parentId, message); +} else { + console.warn("Draft message missing required headers:", message.id); }
400-448: Add TypeScript return type annotations.The email preparation functions would benefit from explicit return type annotations for better type safety and documentation.
Consider adding return types:
-const prepareReplyingToEmail = (message: ParsedMessage) => { +const prepareReplyingToEmail = (message: ParsedMessage): ReplyEmail => { -const prepareForwardingEmail = (message: ParsedMessage) => ({ +const prepareForwardingEmail = (message: ParsedMessage): ForwardEmail => ({ -function prepareDraftReplyEmail(message: ParsedMessage) { +function prepareDraftReplyEmail(message: ParsedMessage): DraftReplyEmail {You'll need to define these types at the top of the file:
type BaseEmail = { to: string; subject: string; headerMessageId: string; threadId: string; cc: string; references: string; messageText: string; messageHtml: string; }; type ReplyEmail = BaseEmail & { bcc: string; }; type ForwardEmail = BaseEmail; type DraftReplyEmail = ReplyEmail & { draftId: string; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/components/EmailViewer.tsx(1 hunks)apps/web/components/email-list/EmailList.tsx(0 hunks)apps/web/components/email-list/EmailPanel.tsx(5 hunks)
💤 Files with no reviewable changes (1)
- apps/web/components/email-list/EmailList.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/components/EmailViewer.tsx
🔇 Additional comments (1)
apps/web/components/email-list/EmailPanel.tsx (1)
33-35: LGTM! Good type definition.The new type alias improves code readability and type safety.
…us messages by default
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
apps/web/utils/gmail/mail.ts (1)
137-154: Remove duplicate utility functions.The
detectTextDirectionandformatEmailDatefunctions are now duplicated as they have been moved toreply.ts. Remove these functions and use the ones fromreply.tsinstead.-function detectTextDirection(text: string): "ltr" | "rtl" { - // Basic RTL detection - checks for RTL characters at the start of the text - const rtlRegex = - /[\u0591-\u07FF\u200F\u202B\u202E\uFB1D-\uFDFD\uFE70-\uFEFC]/; - return rtlRegex.test(text.trim().charAt(0)) ? "rtl" : "ltr"; -} - -function formatEmailDate(date: Date): string { - return date.toLocaleString("en-US", { - weekday: "short", - year: "numeric", - month: "short", - day: "numeric", - hour: "numeric", - minute: "numeric", - hour12: true, - }); -}And update any remaining usages to import these functions from
reply.ts.
♻️ Duplicate comments (3)
apps/web/utils/parse/extract-reply.client.ts (3)
9-16:⚠️ Potential issueSanitize HTML content to prevent XSS vulnerabilities.
Direct use of innerHTML could expose the application to XSS attacks.
Consider using DOMPurify or a similar library to sanitize the HTML content:
+import DOMPurify from 'dompurify'; + const doc = parser.parseFromString(html, "text/html"); if (doc.body.innerHTML === "null") { console.warn("Failed to parse HTML - received null content"); - return { draftHtml: html, originalHtml: "" }; + return { draftHtml: DOMPurify.sanitize(html), originalHtml: "" }; }
23-38:⚠️ Potential issueSanitize extracted reply content to prevent XSS vulnerabilities.
Direct use of innerHTML/outerHTML could expose the application to XSS attacks.
Consider using DOMPurify to sanitize the extracted content:
+import DOMPurify from 'dompurify'; + const latestReplyHtml = firstDiv?.innerHTML || ""; return { - draftHtml: `<div dir="ltr">${latestReplyHtml}</div>`, - originalHtml: quoteContainer.outerHTML, + draftHtml: DOMPurify.sanitize(`<div dir="ltr">${latestReplyHtml}</div>`), + originalHtml: DOMPurify.sanitize(quoteContainer.outerHTML), };
40-44:⚠️ Potential issueSanitize fallback content to prevent XSS vulnerabilities.
Direct use of unsanitized HTML could expose the application to XSS attacks.
Consider using DOMPurify to sanitize the fallback content:
+import DOMPurify from 'dompurify'; + -return { draftHtml: html, originalHtml: "" }; +return { draftHtml: DOMPurify.sanitize(html), originalHtml: "" };
🧹 Nitpick comments (5)
apps/web/app/(app)/compose/ComposeEmailForm.tsx (1)
73-85: Consider handling messageText on the server side.The TODO comment and commented-out code suggest messageText handling should be moved to the server. This would be a good improvement for maintainability.
apps/web/components/Tiptap.tsx (2)
27-27: Consider using proper type for StarterKit.The
as anytype assertion suppresses TypeScript checks. Consider using the proper type from the package.- extensions: [StarterKit as any], + extensions: [StarterKit],
62-72: Consider accessibility and layout improvements.The "more" button implementation has a few potential issues:
- The "•••" text should be more semantic for screen readers
- Absolute positioning might cause overlap with editor content
- <button - className="rounded-tr-md px-4 py-1 text-muted-foreground transition-transform hover:translate-x-1" + <button + className="rounded-tr-md px-4 py-1 text-muted-foreground transition-transform hover:translate-x-1" + aria-label="Show more options" type="button" onClick={onMoreClick} > - ••• + <span aria-hidden="true">•••</span> </button>apps/web/utils/parse/extract-reply.client.ts (1)
18-21: Consider making the quote container selection more robust.The current selectors are specific to Gmail's HTML structure. Consider adding support for other email clients' quote formats.
const quoteContainer = doc.querySelector( - ".gmail_quote_container, .gmail_quote", + ".gmail_quote_container, .gmail_quote, .quote-container, [data-quote], blockquote.reply", );apps/web/utils/gmail/reply.ts (1)
43-48: Enhance text direction detection logic.The current implementation only checks the first character for RTL detection, which might not be reliable for mixed-direction content. Consider scanning more characters or using a dedicated i18n library.
function detectTextDirection(text: string): "ltr" | "rtl" { - // Basic RTL detection - checks for RTL characters at the start of the text + // Enhanced RTL detection - checks first few characters const rtlRegex = /[\u0591-\u07FF\u200F\u202B\u202E\uFB1D-\uFDFD\uFE70-\uFEFC]/; - return rtlRegex.test(text.trim().charAt(0)) ? "rtl" : "ltr"; + const firstFewChars = text.trim().slice(0, 10); + const rtlCount = (firstFewChars.match(rtlRegex) || []).length; + return rtlCount > firstFewChars.length / 3 ? "rtl" : "ltr"; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/web/app/(app)/compose/ComposeEmailForm.tsx(6 hunks)apps/web/app/(app)/reply-tracker/ReplyTrackerEmails.tsx(1 hunks)apps/web/components/Tiptap.tsx(2 hunks)apps/web/components/email-list/EmailPanel.tsx(8 hunks)apps/web/providers/ComposeModalProvider.tsx(2 hunks)apps/web/utils/gmail/forward.ts(1 hunks)apps/web/utils/gmail/mail.ts(2 hunks)apps/web/utils/gmail/reply.ts(1 hunks)apps/web/utils/parse/extract-reply.client.test.ts(1 hunks)apps/web/utils/parse/extract-reply.client.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/utils/parse/extract-reply.client.test.ts
🔇 Additional comments (17)
apps/web/app/(app)/reply-tracker/ReplyTrackerEmails.tsx (1)
101-101: LGTM! Panel size adjustments improve content visibility.The changes to ResizablePanel sizes provide better space allocation:
- More space for email content in the right panel
- Reasonable minimum sizes to maintain readability
Please verify that these panel sizes work well across different screen sizes and with varying email content lengths.
Also applies to: 105-105
apps/web/providers/ComposeModalProvider.tsx (1)
4-11: LGTM! Clean migration to Dialog component.The migration from Modal to Dialog component improves standardization and likely enhances accessibility.
Also applies to: 29-36
apps/web/app/(app)/compose/ComposeEmailForm.tsx (2)
29-41: LGTM! Enhanced type definitions for draft support.The ReplyingToEmail type has been properly extended to support draft functionality with clear separation between draft and quoted content.
148-159: LGTM! Clean implementation of expanded content handling.The showExpandedContent implementation properly manages the editor state and content appending.
apps/web/components/email-list/EmailPanel.tsx (3)
34-34: LGTM! Improved type safety with EmailMessage alias.The type alias enhances code readability and maintainability.
136-158: LGTM! Well-implemented draft organization logic.The organizedMessages implementation efficiently:
- Uses Map for O(1) draft lookup
- Properly handles message references
- Maintains clear parent-child relationships
431-485: LGTM! Clean separation of email preparation logic.The email preparation functions are well-organized with:
- Clear separation of concerns
- Proper handling of sent vs received emails
- Consistent structure across different preparation types
apps/web/components/Tiptap.tsx (4)
9-11: LGTM! Clean type definition.The
TiptapHandletype provides a clear and focused API for external content manipulation.
13-25: LGTM! Well-structured component signature.The component is properly typed with
forwardRefand includes clear prop definitions with appropriate defaults.
40-40: Verify the impact of reduced min-height.The min-height has been reduced from 150px to 120px. Please ensure this doesn't negatively impact the user experience, especially in email composition where vertical space is important.
47-57: LGTM! Robust ref implementation.The
appendContentimplementation is well-structured with:
- Proper null checks
- Clear comments
- Correct document position handling
apps/web/utils/parse/extract-reply.client.ts (2)
1-4: LGTM!The function signature and return type are well-defined and descriptive.
5-7: LGTM!The input validation is robust and handles empty or whitespace-only input appropriately.
apps/web/utils/gmail/forward.ts (1)
15-15: LGTM! Improved email formatting.The addition of a line break enhances readability by adding proper spacing between the content and the forwarded message header.
apps/web/utils/gmail/reply.ts (2)
3-41: LGTM! Well-structured reply content creation.The function effectively handles both plain text and HTML formatting with proper type safety using Pick.
50-60: LGTM! Proper date formatting.The date formatting follows locale standards and includes all necessary components for email headers.
apps/web/utils/gmail/mail.ts (1)
115-118: LGTM! Clean integration of createReplyContent.The replyToEmail function now uses the centralized reply content creation logic effectively.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
apps/web/utils/ai/types.ts (1)
15-25: Consider adding JSDoc comments for better documentation.The
ActionItemtype looks good, but adding JSDoc comments would help explain the purpose of each field and when they're expected to be present/absent.+/** + * Represents an action item with properties derived from ExecutedAction. + * Only id and type are required, other fields are optional based on the action type. + */ export type ActionItem = { id: ExecutedAction["id"]; type: ExecutedAction["type"]; label?: ExecutedAction["label"]; subject?: ExecutedAction["subject"]; content?: ExecutedAction["content"]; to?: ExecutedAction["to"]; cc?: ExecutedAction["cc"]; bcc?: ExecutedAction["bcc"]; url?: ExecutedAction["url"]; };apps/web/utils/gmail/reply.ts (2)
3-41: Consider handling edge cases in reply content creation.The
createReplyContentfunction looks good but could be more robust:
- Handle potential undefined values in message.headers
- Consider adding max length for quoted content
export const createReplyContent = ({ plainContent, message, }: { plainContent: string; message: Pick<ParsedMessage, "headers" | "textPlain" | "textHtml">; }) => { - const quotedDate = formatEmailDate(new Date(message.headers.date)); - const quotedHeader = `On ${quotedDate}, ${message.headers.from} wrote:`; + const quotedDate = message.headers?.date + ? formatEmailDate(new Date(message.headers.date)) + : formatEmailDate(new Date()); + const quotedHeader = `On ${quotedDate}, ${message.headers?.from || 'unknown'} wrote:`; // Detect text direction from original message const textDirection = detectTextDirection(message.textPlain || ""); const dirAttribute = `dir="${textDirection}"`; // Format plain text version with proper quoting const quotedContent = message.textPlain ?.split("\n") + .slice(0, 100) // Limit quoted content to 100 lines .map((line) => `> ${line}`) .join("\n");
43-48: Consider expanding RTL detection for better accuracy.The current RTL detection only checks the first character. Consider:
- Checking a larger sample of text
- Adding more RTL Unicode ranges
function detectTextDirection(text: string): "ltr" | "rtl" { - // Basic RTL detection - checks for RTL characters at the start of the text + // Enhanced RTL detection - checks first 100 characters const rtlRegex = - /[\u0591-\u07FF\u200F\u202B\u202E\uFB1D-\uFDFD\uFE70-\uFEFC]/; + /[\u0591-\u07FF\u200F\u202B\u202E\uFB1D-\uFDFD\uFE70-\uFEFC\u10800-\u10FFF\u1E800-\u1EFFF]/; - return rtlRegex.test(text.trim().charAt(0)) ? "rtl" : "ltr"; + const sample = text.trim().slice(0, 100); + const rtlCount = (sample.match(rtlRegex) || []).length; + return rtlCount > sample.length * 0.4 ? "rtl" : "ltr"; }apps/web/utils/mail.ts (1)
131-162: Consider caching conversion results for performance.The HTML to text conversion is well-implemented with comprehensive formatting options. However, for large HTML content, consider:
- Caching conversion results
- Adding a maximum size limit
+const htmlTextCache = new Map<string, string>(); +const MAX_HTML_SIZE = 1000000; // 1MB export function convertEmailHtmlToText({ htmlText, }: { htmlText: string; }): string { + if (htmlText.length > MAX_HTML_SIZE) { + throw new Error('HTML content too large'); + } + + const cached = htmlTextCache.get(htmlText); + if (cached) return cached; const plainText = convert(htmlText, { // ... existing options ... }); + htmlTextCache.set(htmlText, plainText); return plainText; }apps/web/utils/gmail/mail.ts (1)
224-269: Consider splitting the function as suggested in the comment.The TODO comment on line 224 suggests splitting this into separate functions for replies and regular drafts. This would improve code clarity and maintainability.
Consider refactoring into two separate functions:
-// Handles both replies and regular drafts. May want to split that out into two functions -export async function draftEmail( +export async function draftReply( + gmail: gmail_v1.Gmail, + originalEmail: EmailForAction, + args: { + content: string; + attachments?: Attachment[]; + }, +) { + const { text, html } = createReplyContent({ + plainContent: args.content, + message: originalEmail, + }); + + const raw = await createRawMailMessage({ + to: originalEmail.headers["reply-to"] || originalEmail.headers.from, + cc: originalEmail.headers.cc, + bcc: originalEmail.headers.bcc, + subject: originalEmail.headers.subject, + messageHtml: html, + messageText: text, + attachments: args.attachments, + replyToEmail: { + threadId: originalEmail.threadId, + headerMessageId: originalEmail.headers["message-id"] || "", + references: originalEmail.headers.references, + }, + }); + + return gmail.users.drafts.create({ + userId: "me", + requestBody: { + message: { + threadId: originalEmail.threadId, + raw, + }, + }, + }); +} + +export async function draftNewEmail( + gmail: gmail_v1.Gmail, + args: { + to: string; + subject: string; + content: string; + attachments?: Attachment[]; + }, +) { + const { text, html } = createReplyContent({ + plainContent: args.content, + message: null, + }); + + const raw = await createRawMailMessage({ + to: args.to, + subject: args.subject, + messageHtml: html, + messageText: text, + attachments: args.attachments, + }); + + return gmail.users.drafts.create({ + userId: "me", + requestBody: { + message: { raw }, + }, + }); +}apps/web/components/email-list/EmailPanel.tsx (1)
132-154: Add documentation for draft organization logic.While the implementation is solid, consider adding JSDoc comments to explain:
- How drafts are associated with parent messages
- The significance of message references and in-reply-to headers
- The structure of the returned organized messages
+/** + * Organizes messages by associating draft replies with their parent messages. + * Uses message references or in-reply-to headers to establish the relationship. + * @returns Array of objects containing regular messages and their associated draft replies + */ const organizedMessages = useMemo(() => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
apps/web/app/(app)/automation/RuleForm.tsx(1 hunks)apps/web/app/(app)/compose/ComposeEmailForm.tsx(6 hunks)apps/web/app/api/google/draft/route.ts(0 hunks)apps/web/app/api/google/messages/send/route.ts(0 hunks)apps/web/components/email-list/EmailPanel.tsx(7 hunks)apps/web/utils/actions/mail.ts(2 hunks)apps/web/utils/ai/actions.ts(4 hunks)apps/web/utils/ai/choose-rule/execute.ts(1 hunks)apps/web/utils/ai/types.ts(1 hunks)apps/web/utils/assistant/process-assistant-email.ts(2 hunks)apps/web/utils/gmail/mail.ts(10 hunks)apps/web/utils/gmail/reply.ts(1 hunks)apps/web/utils/mail.ts(1 hunks)
💤 Files with no reviewable changes (2)
- apps/web/app/api/google/messages/send/route.ts
- apps/web/app/api/google/draft/route.ts
✅ Files skipped from review due to trivial changes (2)
- apps/web/app/(app)/automation/RuleForm.tsx
- apps/web/utils/ai/choose-rule/execute.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/app/(app)/compose/ComposeEmailForm.tsx
🔇 Additional comments (13)
apps/web/utils/ai/types.ts (2)
4-13: LGTM! Well-structured type definition.The
EmailForActiontype efficiently picks only the necessary fields fromParsedMessage, following TypeScript best practices.
27-33: LGTM! Type-safe property mapping.The
Propertiestype effectively usesPartialRecordto ensure type safety for email property mappings.apps/web/utils/gmail/reply.ts (1)
50-60: LGTM! Consistent date formatting.The
formatEmailDatefunction usestoLocaleStringwith appropriate options for consistent formatting.apps/web/utils/actions/mail.ts (1)
24-27: LGTM! Clean migration to HTML-aware email sending.The change from
sendEmailtosendEmailWithHtmlis straightforward and maintains the existing error handling.Also applies to: 238-238
apps/web/utils/gmail/mail.ts (4)
1-34: LGTM! Import changes and schema updates align with new email content handling.The changes correctly support the new approach of handling both HTML and plain text content in emails.
96-113: LGTM! Improved email content format handling.The function now properly handles both HTML and plain text versions of the email content, which is essential for email client compatibility.
124-162: LGTM! Good refactoring of reply content generation.Moving reply content creation to
createReplyContentimproves code organization and maintainability.
271-285: LGTM! Good conversion to function declaration.Converting from a variable to a function declaration improves code organization and hoisting behavior.
apps/web/utils/assistant/process-assistant-email.ts (1)
1-13: LGTM! Import cleanup aligns with broader refactoring.The removal of unused imports and reorganization improves code cleanliness.
apps/web/utils/ai/actions.ts (2)
290-291: LGTM! Simplified draft function call.The simplified call to
draftEmailimproves code readability while maintaining functionality.
305-313: LGTM! Updated to use new email sending function.The change to use
sendEmailWithPlainTextaligns with the new email content handling approach.apps/web/components/email-list/EmailPanel.tsx (2)
14-30: LGTM! Good type organization.The new type alias
EmailMessageand the added imports improve code organization and type safety.
32-118: LGTM! Component structure is well-maintained.The EmailPanel component effectively manages thread display and actions.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
.cursorrules (1)
222-225: LGTM! Clear and concise testing guidelines.The new testing section provides clear guidance on:
- Using Vitest as the testing framework
- Test file colocation strategy
- Consistent naming conventions
Consider enhancing these guidelines with examples of:
- Basic test structure
- Common testing patterns
- Mocking strategies
apps/web/app/(app)/compose/ComposeEmailForm.tsx (3)
29-39: Consider adding JSDoc comments to document the type properties.The type changes are well-structured, with a clear separation between draft and quoted content. Consider adding JSDoc comments to document the purpose and format of each property, especially for
draftHtmlandquotedContentHtml.export type ReplyingToEmail = { + /** The unique identifier for the email thread */ threadId: string; + /** The message ID from the email headers */ headerMessageId: string; + /** References to previous messages in the thread */ references?: string; + /** The email subject */ subject: string; + /** The recipient's email address */ to: string; + /** Carbon copy recipients */ cc?: string; + /** Blind carbon copy recipients */ bcc?: string; + /** The draft content being written/edited by the user */ draftHtml?: string | undefined; + /** The original message content being quoted/replied to */ quotedContentHtml?: string | undefined; };
70-84: Consider making the email template more generic.The current implementation uses Gmail-specific classes which might not be ideal for all email clients.
Consider extracting the template to a configurable constant:
+const REPLY_TEMPLATE = { + DEFAULT: (content: string, quotedContent?: string) => ` + <div> + ${content ?? ""} + ${quotedContent ? ` + <br/> + <br/> + <blockquote> + ${quotedContent} + </blockquote> + ` : ""} + </div> + `, + GMAIL: (content: string, quotedContent?: string) => ` + <div dir="ltr"> + ${content ?? ""} + <br/> + <br/> + <div class="gmail_quote"> + ${quotedContent ?? ""} + </div> + </div> + ` +}; const onSubmit: SubmitHandler<SendEmailBody> = useCallback( async (data) => { const enrichedData = { ...data, messageHtml: showFullContent ? data.messageHtml - : `<div dir="ltr"> - ${data.messageHtml ?? ""} - <br/> - <br/> - <div class="gmail_quote"> - ${replyingToEmail?.quotedContentHtml ?? ""} - </div> -</div>`, + : REPLY_TEMPLATE.DEFAULT(data.messageHtml, replyingToEmail?.quotedContentHtml), };
307-313: Consider enhancing accessibility for the editor controls.The UI changes look good, but consider adding ARIA labels and tooltips for better accessibility.
<Tiptap ref={editorRef} initialContent={replyingToEmail?.draftHtml} onChange={handleEditorChange} className="min-h-[200px]" + aria-label="Email content editor" onMoreClick={showFullContent ? undefined : showExpandedContent} /> <div className="flex items-center justify-between"> <Button type="submit" disabled={isSubmitting} + aria-label="Send email" > {isSubmitting && <ButtonLoader />} Send </Button> {onDiscard && ( <Button type="button" variant="secondary" size="icon" disabled={isSubmitting} onClick={onDiscard} + title="Discard draft" > <TrashIcon className="h-4 w-4" /> <span className="sr-only">Discard</span> </Button> )} </div>Also applies to: 315-333
apps/web/utils/gmail/mail.ts (2)
106-112: Consider improving error handling for HTML to text conversion.The current implementation swallows the error and falls back to using HTML as plain text. While this ensures the email is sent, it might hide issues and result in poorly formatted plain text content.
Consider one of these alternatives:
- Log the error but throw it to let the caller handle it
- Add a proper plain text fallback instead of using HTML
- try { - messageText = convertEmailHtmlToText({ htmlText: body.messageHtml }); - } catch (error) { - logger.error("Error converting email html to text", { error }); - messageText = body.messageHtml; - // throw error; - } + try { + messageText = convertEmailHtmlToText({ htmlText: body.messageHtml }); + } catch (error) { + logger.error("Error converting email html to text", { error }); + // Simple fallback: Strip HTML tags and use as plain text + messageText = body.messageHtml.replace(/<[^>]*>/g, ''); + }
234-244: Consider splitting draft email functionality.As noted in the comment, this function handles both replies and regular drafts. Consider splitting it into two separate functions for better separation of concerns:
draftReplyfor reply draftsdraftNewEmailfor new email draftsWould you like me to propose an implementation for this split?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.cursorrules(1 hunks)apps/web/app/(app)/compose/ComposeEmailForm.tsx(6 hunks)apps/web/utils/gmail/mail.ts(9 hunks)apps/web/utils/mail.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/utils/mail.ts
🔇 Additional comments (9)
.cursorrules (1)
220-220: LGTM!The minor edit to remove redundant punctuation improves readability while maintaining the same meaning.
apps/web/app/(app)/compose/ComposeEmailForm.tsx (3)
10-10: LGTM! Import changes are well-organized.The new imports support the added functionality for draft handling and editor manipulation.
Also applies to: 25-27
52-52: LGTM! Form state initialization looks good.The state management for full content display and default form values are properly implemented.
Also applies to: 66-66
145-155: Consider handling edge cases in content expansion.The content expansion logic looks good, but consider handling these edge cases:
- Multiple clicks on expand
- Content already containing quoted text
Consider adding checks:
const showExpandedContent = useCallback(() => { if (!showFullContent) { + const currentContent = editorRef.current?.getHTML() ?? ""; + const quotedContent = replyingToEmail?.quotedContentHtml ?? ""; + + // Avoid duplicate quoted content + if (!currentContent.includes(quotedContent)) { // Append the quoted content at the end editorRef.current?.appendContent( replyingToEmail?.quotedContentHtml ?? "", ); + } } setShowFullContent(true); }, [showFullContent, replyingToEmail?.quotedContentHtml]);apps/web/utils/gmail/mail.ts (5)
7-19: LGTM! Good improvements to imports and logging.The addition of scoped logging and well-organized imports enhances the maintainability and debuggability of the code.
125-132: LGTM! Good addition of plain text email support.The new
sendEmailWithPlainTextfunction provides a clean API for sending plain text emails while maintaining HTML compatibility.
143-146: LGTM! Good separation of concerns.Moving reply content generation to
createReplyContentimproves code organization and reusability.
281-295: LGTM! Good conversion to function declaration.Converting
convertTextToHtmlParagraphsto a proper function declaration improves code organization and maintainability.
34-34: Verify the impact of makingmessageHtmlrequired.Making
messageHtmla required field insendEmailBodyschema could be a breaking change. Ensure all callers of this API are updated to provide HTML content.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/utils/gmail/reply.test.ts (1)
6-18: Enhance test data for better coverage.The test data setup is good but could be improved:
- There's an inconsistency in the date between setup (line 10: "Sun, Feb 1, 2025") and assertion (line 30: "Sat, Feb 1, 2025")
- Consider using more realistic multi-line content to better test formatting
- textPlain: "Original message", - textHtml: "<div>Original message</div>", + textPlain: "Hello,\n\nThis is the original message.\nBest regards,\nSender", + textHtml: "<div>Hello,<br><br>This is the original message.<br>Best regards,<br>Sender</div>",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.cursorrules(1 hunks)apps/web/utils/gmail/reply.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .cursorrules
🔇 Additional comments (1)
apps/web/utils/gmail/reply.test.ts (1)
1-5: LGTM! Clear and focused test suite setup.The imports and test suite organization follow best practices.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
apps/web/utils/gmail/reply.test.ts (1)
5-38: Add more test cases for comprehensive coverage.While the current test is good, consider adding test cases for:
- RTL text direction handling
- Plain text output verification
- Different reply scenarios (e.g., longer threads, special characters)
Example additional test:
it("handles RTL text correctly", () => { const textContent = "מה שלומך"; // Hebrew text const message = { headers: { date: new Date().toISOString(), from: "[email protected]", subject: "Test subject", to: "[email protected]", "message-id": "<[email protected]>", }, textPlain: "שלום", textHtml: "<div>שלום</div>", }; const { html } = createReplyContent({ textContent, message, }); expect(html).toContain('dir="rtl"'); }); it("formats plain text output correctly", () => { const textContent = "This is my reply"; const message = { headers: { date: new Date().toISOString(), from: "[email protected]", subject: "Test subject", to: "[email protected]", "message-id": "<[email protected]>", }, textPlain: "Original message", textHtml: "<div>Original message</div>", }; const { text } = createReplyContent({ textContent, message, }); expect(text).toContain("On "); expect(text).toContain("> Original message"); });🧰 Tools
🪛 GitHub Check: test
[failure] 26-26: utils/gmail/reply.test.ts > email formatting > formats reply email like Gmail
AssertionError: expected 'This is my reply…' to be 'This is my reply…' // Object.is equality
- Expected
Received
This is my reply
❯ utils/gmail/reply.test.ts:26:18
🪛 GitHub Actions: Run Tests
[error] 26-26: AssertionError: expected '
This is my reply…' to be 'This is my reply…' // Object.is equalityapps/web/utils/gmail/reply.ts (1)
53-58: Consider using a more comprehensive RTL detection approach.The current RTL detection only checks the first character. This might miss RTL content that starts with neutral characters (numbers, punctuation).
Consider using a more robust approach:
-function detectTextDirection(text: string): "ltr" | "rtl" { - // Basic RTL detection - checks for RTL characters at the start of the text - const rtlRegex = - /[\u0591-\u07FF\u200F\u202B\u202E\uFB1D-\uFDFD\uFE70-\uFEFC]/; - return rtlRegex.test(text.trim().charAt(0)) ? "rtl" : "ltr"; -} +function detectTextDirection(text: string): "ltr" | "rtl" { + // Count RTL characters in the first few words + const rtlRegex = /[\u0591-\u07FF\u200F\u202B\u202E\uFB1D-\uFDFD\uFE70-\uFEFC]/g; + const firstFewChars = text.trim().slice(0, 20); + const rtlCount = (firstFewChars.match(rtlRegex) || []).length; + const threshold = firstFewChars.length * 0.3; // 30% threshold + return rtlCount > threshold ? "rtl" : "ltr"; +}apps/web/utils/gmail/mail.ts (1)
234-279: Add documentation for the draftEmail function.The comment "Handles both replies and regular drafts" suggests this function might be doing too much.
Consider splitting into two specialized functions:
export async function draftReply( gmail: gmail_v1.Gmail, originalEmail: EmailForAction, content: string, ) { // Handle reply drafts } export async function draftNewEmail( gmail: gmail_v1.Gmail, args: { to: string; subject: string; content: string; attachments?: Attachment[]; }, ) { // Handle new email drafts }apps/web/app/(app)/compose/ComposeEmailForm.tsx (1)
72-111: Extract email data preparation logic from onSubmit.The email data preparation logic in onSubmit makes the function complex and harder to test.
Consider extracting the logic:
const prepareEmailData = (data: SendEmailBody, showFullContent: boolean, replyingToEmail?: ReplyingToEmail) => { if (!showFullContent) { return { ...data, messageHtml: createReplyContent({ htmlContent: data.messageHtml ?? "", message: { headers: { date: replyingToEmail?.date ?? new Date().toISOString(), from: replyingToEmail?.to ?? "", subject: replyingToEmail?.subject ?? "", to: replyingToEmail?.to ?? "", "message-id": replyingToEmail?.headerMessageId ?? "", }, textPlain: replyingToEmail?.quotedContentHtml ?? "", textHtml: replyingToEmail?.quotedContentHtml ?? "", }, }).html, }; } return data; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/web/app/(app)/compose/ComposeEmailForm.tsx(6 hunks)apps/web/app/(app)/reply-tracker/ReplyTrackerEmails.tsx(3 hunks)apps/web/components/email-list/EmailPanel.tsx(7 hunks)apps/web/utils/gmail/mail.ts(9 hunks)apps/web/utils/gmail/reply.test.ts(1 hunks)apps/web/utils/gmail/reply.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/app/(app)/reply-tracker/ReplyTrackerEmails.tsx
- apps/web/components/email-list/EmailPanel.tsx
🧰 Additional context used
🪛 GitHub Check: test
apps/web/utils/gmail/reply.test.ts
[failure] 26-26: utils/gmail/reply.test.ts > email formatting > formats reply email like Gmail
AssertionError: expected '
- Expected
-
Received
This is my reply
-
On Thu, 6 Feb 2025 at 23:23, John Doe wrote:
-
On Thu, 6 Feb 2025 at 21:23, John Doe wrote:Original message content
❯ utils/gmail/reply.test.ts:26:18
🪛 GitHub Actions: Run Tests
apps/web/utils/gmail/reply.test.ts
[error] 26-26: AssertionError: expected '
🔇 Additional comments (1)
apps/web/utils/gmail/reply.ts (1)
60-70:⚠️ Potential issueAdd padding for single-digit minutes in date formatting.
The current implementation doesn't pad single-digit minutes with leading zeros.
Apply this diff to fix the minute formatting:
- return `${weekday}, ${day} ${month} ${year} at ${hour}:${minute.toString().padStart(2, "0")}`; + return `${weekday}, ${day} ${month} ${year} at ${hour.toString().padStart(2, "0")}:${minute.toString().padStart(2, "0")}`;Likely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/web/app/(app)/compose/ComposeEmailForm.tsx (2)
53-68: Consider adding a comment explaining the showFullContent state.The state variable controls an important UI behavior but its purpose isn't immediately clear from the code.
Add a comment above the state declaration:
+ // Controls whether to show the full quoted content in the editor const [showFullContent, setShowFullContent] = React.useState(false);
71-97: Enhance visual separation between draft and quoted content.When
showFullContentis false, the current implementation uses a simple<br>tag to separate draft content from quoted content. This might not provide enough visual distinction.Consider using a more distinctive separator:
- : `${data.messageHtml}<br>${replyingToEmail?.quotedContentHtml}`, + : `${data.messageHtml}<div class="quoted-content-separator" style="border-top: 1px solid #e5e7eb; margin: 1em 0;"></div>${replyingToEmail?.quotedContentHtml}`,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/app/(app)/compose/ComposeEmailForm.tsx(6 hunks)
🔇 Additional comments (3)
apps/web/app/(app)/compose/ComposeEmailForm.tsx (3)
29-40: LGTM! Well-structured type definition.The
ReplyingToEmailtype has been thoughtfully extended to support draft functionality with clear separation between draft content (draftHtml) and quoted content (quotedContentHtml).
141-149: Add error handling for appendContent operation.The showExpandedContent function doesn't handle potential errors when appending content.
301-311: LGTM! Clean implementation of expandable content.The conditional rendering of the
onMoreClickhandler is well implemented, providing a good user experience for viewing full content.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
apps/web/utils/gmail/forward.ts (1)
17-26:⚠️ Potential issuePotential XSS Vulnerability in HTML Generation
The HTML content is being directly injected into the template string without sanitization, which could lead to XSS attacks if the input content contains malicious code.
Consider using DOMPurify or a similar library to sanitize the HTML content before injection:
+ import DOMPurify from 'dompurify'; export const forwardEmailHtml = ({ content, message, }: { content: string; message: ParsedMessage; }) => { const quotedDate = formatEmailDate(new Date(message.headers.date)); - return `<div dir="ltr">${content}<br><br> + const sanitizedContent = DOMPurify.sanitize(content); + return `<div dir="ltr">${sanitizedContent}<br><br>apps/web/utils/gmail/reply.ts (1)
33-35:⚠️ Potential issuePotential XSS Vulnerability in Content Handling
Direct conversion of text to HTML without sanitization could lead to XSS vulnerabilities.
Consider sanitizing the content before HTML conversion:
+ import DOMPurify from 'dompurify'; - const contentHtml = htmlContent || textContent?.replace(/\n/g, "<br>") || ""; + const sanitizedHtml = htmlContent ? DOMPurify.sanitize(htmlContent) : + textContent ? DOMPurify.sanitize(textContent.replace(/\n/g, "<br>")) : "";
🧹 Nitpick comments (3)
apps/web/utils/gmail/forward.ts (1)
47-55: Improve email parsing robustnessThe email parsing logic could be more robust to handle various email formats.
Consider using a dedicated email parsing library or enhancing the regex to handle more edge cases:
const formatFromEmailWithName = (emailHeader: string) => { - const match = emailHeader.match(/(.*?)\s*<([^>]+)>/); + const match = emailHeader.match(/^(?:"?([^"]*)"?\s)?(?:<?(.+@[^>]+)>?)$/); if (!match) return emailHeader; const [, name, email] = match; const trimmedName = name?.trim() || email;apps/web/utils/gmail/reply.ts (1)
60-70: Improve date formatting precisionThe current date formatting could be enhanced for better precision and localization.
Consider using Intl.DateTimeFormat for more consistent formatting:
export function formatEmailDate(date: Date): string { - const weekday = date.toLocaleString("en-US", { weekday: "short" }); - const month = date.toLocaleString("en-US", { month: "short" }); - const day = date.getDate(); - const year = date.getFullYear(); - const hour = date.getHours(); - const minute = date.getMinutes(); + return new Intl.DateTimeFormat('en-US', { + weekday: 'short', + day: 'numeric', + month: 'short', + year: 'numeric', + hour: '2-digit', + minute: '2-digit', + hour12: false + }).format(date); - return `${weekday}, ${day} ${month} ${year} at ${hour}:${minute.toString().padStart(2, "0")}`; }apps/web/utils/gmail/forward.test.ts (1)
54-85: Enhance test coverage with edge casesThe current test suite only covers the happy path. Consider adding tests for edge cases.
Add tests for:
- Malformed email addresses
- Missing headers
- HTML content with special characters
- Different date formats
Example:
it("handles malformed email addresses", () => { const message = { headers: { from: "malformed-email", // ... other headers }, // ... other properties }; const html = forwardEmailHtml({ content: "test", message: message as ParsedMessage }); expect(html).toContain("malformed-email"); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/utils/gmail/forward.test.ts(1 hunks)apps/web/utils/gmail/forward.ts(3 hunks)apps/web/utils/gmail/reply.ts(1 hunks)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/web/utils/gmail/reply.ts (1)
3-51:⚠️ Potential issuePotential XSS Vulnerability — Missing Sanitization of User-Provided Content
The function directly injects user-provided content into HTML without sanitization, which could lead to XSS attacks.
Consider using DOMPurify or a similar library to sanitize the following user inputs before injection:
textContenthtmlContentmessage.textHtmlmessage.textPlainExample implementation:
+import DOMPurify from 'dompurify'; export const createReplyContent = ({ textContent, htmlContent, message, }: { textContent?: string; htmlContent?: string; message: Pick<ParsedMessage, "headers" | "textPlain" | "textHtml">; }): { html: string; text: string; } => { const quotedDate = formatEmailDate(new Date(message.headers.date)); - const quotedHeader = `On ${quotedDate}, ${message.headers.from} wrote:`; + const quotedHeader = `On ${quotedDate}, ${DOMPurify.sanitize(message.headers.from)} wrote:`; // Detect text direction from original message const textDirection = detectTextDirection(textContent || ""); const dirAttribute = `dir="${textDirection}"`; // Format plain text version with proper quoting const quotedContent = message.textPlain ?.split("\n") .map((line) => `> ${line}`) .join("\n"); const plainText = `${textContent}\n\n${quotedHeader}\n\n${quotedContent}`; // Get the message content, preserving any existing quotes const messageContent = - message.textHtml || message.textPlain?.replace(/\n/g, "<br>") || ""; + DOMPurify.sanitize(message.textHtml || message.textPlain?.replace(/\n/g, "<br>") || ""); // Use htmlContent if provided, otherwise convert textContent to HTML - const contentHtml = htmlContent || textContent?.replace(/\n/g, "<br>") || ""; + const contentHtml = DOMPurify.sanitize(htmlContent || textContent?.replace(/\n/g, "<br>") || "");
🧹 Nitpick comments (3)
apps/web/utils/gmail/reply.ts (1)
60-70: Consider adding timezone support.The current implementation uses local time, which could lead to inconsistencies across different timezones.
Consider using UTC or allowing timezone specification:
export function formatEmailDate(date: Date): string { + const options = { + weekday: "short" as const, + month: "short" as const, + day: "numeric" as const, + year: "numeric" as const, + hour: "2-digit" as const, + minute: "2-digit" as const, + hour12: false as const, + timeZone: "UTC" as const, + }; + return date.toLocaleString("en-US", options); }apps/web/utils/gmail/reply.test.ts (2)
54-85: Add more test assertions for comprehensive coverage.While the basic functionality is tested, consider adding assertions for:
- Plain text output
- Quoted content formatting
- Special characters handling
Add these assertions:
const { html } = createReplyContent({ textContent, htmlContent: "", message, }); expect(html).toBe(/* ... */); + + const { text } = createReplyContent({ + textContent, + htmlContent: "", + message, + }); + + expect(text).toContain("This is my reply"); + expect(text).toContain("> Original message content"); + expect(text).toMatch(/On .+, John Doe <[email protected]> wrote:/);
87-119: Add assertions for RTL-specific behavior.The RTL test could be enhanced with additional assertions to verify RTL-specific formatting.
Add these assertions:
expect(html).toBe(/* ... */); + expect(html).toContain('dir="rtl"'); + expect(html).not.toContain('dir="ltr"'); + + const { text } = createReplyContent({ + textContent, + htmlContent: "", + message, + }); + + expect(text).toContain("שלום, מה שלומך?"); + expect(text).toContain("> תוכן ההודעה המקורית");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/utils/gmail/reply.test.ts(1 hunks)apps/web/utils/gmail/reply.ts(1 hunks)
🔇 Additional comments (2)
apps/web/utils/gmail/reply.ts (1)
53-58: LGTM! Robust RTL detection implementation.The function uses a comprehensive regex pattern to detect RTL characters, which is a good approach for handling bidirectional text.
apps/web/utils/gmail/reply.test.ts (1)
5-52: LGTM! Comprehensive test setup with proper timezone mocking.The test setup is thorough with:
- Fixed UTC timestamp
- Mocked date methods
- Proper cleanup
Summary by CodeRabbit
New Features
Refactor
Tests
Chores