feat: finally done with Auto-Save , Update and Delete Drafts#1718
feat: finally done with Auto-Save , Update and Delete Drafts#1718AnjanyKumarJaiswal wants to merge 12 commits intoMail-0:stagingfrom
Conversation
…yKumarJaiswal/mail0 into feat/autoSave_Email_drafts
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
Implements comprehensive draft management functionality across both frontend and backend components of the Zero mail application.
- Added auto-save feature in
apps/mail/components/create/email-composer.tsxwith 3-second debounce and toast notifications for draft operations - Implemented new draft management methods (
updateDraft,deleteDraft) in Microsoft and Google drivers (apps/server/src/lib/driver/*.ts) - Enhanced
MailManagerinterface inapps/server/src/lib/driver/types.tswith new draft management methods - Added TRPC procedures for draft operations in
apps/server/src/trpc/routes/drafts.tswith improved handling - Optimized draft fetching and updates in
apps/mail/components/create/create-email.tsxwith immediate composer display
7 files reviewed, 18 comments
Edit PR Review Bot Settings | Greptile
| updateDraft( | ||
| data: CreateDraftData, | ||
| ) : Promise<{id?: string | null, success?: boolean, error?: string}>; |
There was a problem hiding this comment.
style: Similar return type to createDraft, but consider consolidating into a shared type definition like DraftOperationResult to maintain consistency
| deleteDraft( | ||
| data: CreateDraftData | ||
| ) : Promise<void>; |
There was a problem hiding this comment.
logic: Delete operation using full CreateDraftData seems excessive. Consider using just the draft ID
| deleteDraft( | |
| data: CreateDraftData | |
| ) : Promise<void>; | |
| deleteDraft( | |
| id: string | |
| ): Promise<void>; |
| .mutation(async ({input, ctx}) =>{ | ||
| const {activeConnection} = ctx; | ||
| const agent = await getZeroAgent(activeConnection.id); | ||
| const res = agent.updateDraft(input) |
There was a problem hiding this comment.
style: Missing semicolon at line end. Keep consistent with rest of codebase style.
| const res = agent.updateDraft(input) | |
| const res = agent.updateDraft(input); |
| .input(createDraftData) | ||
| .mutation(async({input,ctx})=>{ |
There was a problem hiding this comment.
logic: Delete endpoint using createDraftData schema. Should use simpler schema with just ID.
| const { mutateAsync: sendEmail } = useMutation(trpc.mail.send.mutationOptions()); | ||
| const [isComposeOpen, setIsComposeOpen] = useQueryState('isComposeOpen'); | ||
| const [, setThreadId] = useQueryState('threadId'); | ||
| const [{ isFetching, refetch: refetchThreads }] = useThreads(); |
There was a problem hiding this comment.
syntax: isFetching is destructured but never used. Remove unused variable.
| const [{ isFetching, refetch: refetchThreads }] = useThreads(); | |
| const [{ refetch: refetchThreads }] = useThreads(); |
apps/server/src/routes/chat.ts
Outdated
| async deleteDraft(draftData: CreateDraftData){ | ||
| return await this.mainDo.deleteDraft(draftData); | ||
| } |
There was a problem hiding this comment.
style: Inconsistent spacing after function parameters - missing space before opening brace
| async deleteDraft(draftData: CreateDraftData){ | |
| return await this.mainDo.deleteDraft(draftData); | |
| } | |
| async deleteDraft(draftData: CreateDraftData) { | |
| return await this.mainDo.deleteDraft(draftData); | |
| } |
| if(!this.driver){ | ||
| throw new Error("No driver available") | ||
| } |
There was a problem hiding this comment.
style: Double quotes used instead of single quotes for string literal - inconsistent with codebase style
| if(!this.driver){ | |
| throw new Error("No driver available") | |
| } | |
| if(!this.driver){ | |
| throw new Error('No driver available') | |
| } |
| async updateDraft(draftData: CreateDraftData){ | ||
| if(!this.driver){ | ||
| throw new Error("No driver available") | ||
| } | ||
| return await this.driver.updateDraft(draftData); | ||
| } | ||
|
|
||
| async deleteDraft(draftData: CreateDraftData){ | ||
| if(!this.driver){ | ||
| throw new Error("No driver available") | ||
| } | ||
| return await this.driver.deleteDraft(draftData); | ||
| } |
There was a problem hiding this comment.
style: Error message format inconsistent with other driver check errors in file (missing period at end)
| useEffect(() => { | ||
| if (!hasUnsavedChanges) return; | ||
|
|
||
| const autoSaveTimer = setTimeout(() => { | ||
| console.log('Draft Save TimeOut'); | ||
| saveDraft(); | ||
| }, 3000); | ||
|
|
||
| return () => clearTimeout(autoSaveTimer); | ||
| }, [hasUnsavedChanges, saveDraft]); |
There was a problem hiding this comment.
style: Complex save logic inside useEffect. Consider moving to useMemo or useCallback to optimize performance.
| useEffect(() => { | |
| if (!hasUnsavedChanges) return; | |
| const autoSaveTimer = setTimeout(() => { | |
| console.log('Draft Save TimeOut'); | |
| saveDraft(); | |
| }, 3000); | |
| return () => clearTimeout(autoSaveTimer); | |
| }, [hasUnsavedChanges, saveDraft]); | |
| const saveDraftCallback = useCallback(async () => { | |
| const values = getValues(); | |
| if (!hasUnsavedChanges) return; | |
| const messageText = editor.getText(); | |
| if (messageText.trim() === initialMessage.trim()) return; | |
| if (editor.getHTML() === initialMessage.trim()) return; | |
| if (!values.to.length || !values.subject.length || !messageText.length) return; | |
| if (aiGeneratedMessage || aiIsLoading || isGeneratingSubject) return; | |
| try { | |
| setIsSavingDraft(true); | |
| const draftData = { | |
| to: values.to.join(', '), | |
| cc: values.cc?.join(', '), | |
| bcc: values.bcc?.join(', '), | |
| subject: values.subject, | |
| message: editor.getHTML(), | |
| attachments: await serializeFiles(values.attachments ?? []), | |
| id: draftId, | |
| threadId: threadId ? threadId : null, | |
| fromEmail: values.fromEmail ? values.fromEmail : null, | |
| }; | |
| if(draftId){ | |
| const response = await updateDraft(draftData); | |
| if(response?.id){ | |
| setDraftId(response?.id); | |
| onDraftUpdate?.(); | |
| toast.success("Your Draft has been Successfully Saved") | |
| } | |
| else{ | |
| const response = await createDraft(draftData); | |
| if(response?.id){ | |
| setDraftId(response?.id); | |
| toast.success("Your Draft has been Successfully Saved") | |
| } | |
| console.error("Failed Setting up Draft Id") | |
| toast.error("Failed Setting up Draft Id") | |
| } | |
| } else { | |
| const response = await createDraft(draftData); | |
| if(response?.id){ | |
| setDraftId(response?.id); | |
| toast.success("Your Draft has been Successfully Saved") | |
| } | |
| } | |
| } catch (error) { | |
| console.error('Error saving draft:', error); | |
| toast.error('Failed to save draft'); | |
| setIsSavingDraft(false); | |
| setHasUnsavedChanges(false); | |
| } finally { | |
| setIsSavingDraft(false); | |
| setHasUnsavedChanges(false); | |
| } | |
| }, [hasUnsavedChanges, getValues, editor, initialMessage, aiGeneratedMessage, aiIsLoading, isGeneratingSubject, draftId, threadId, updateDraft, createDraft, onDraftUpdate]); | |
| useEffect(() => { | |
| if (!hasUnsavedChanges) return; | |
| const autoSaveTimer = setTimeout(() => { | |
| console.log('Draft Save TimeOut'); | |
| saveDraftCallback(); | |
| }, 3000); | |
| return () => clearTimeout(autoSaveTimer); | |
| }, [hasUnsavedChanges, saveDraftCallback]); |
| const DeleteDraft = async () => { | ||
| const values = getValues(); | ||
| if (!draftId) { | ||
| toast.error('No draft Id available to delete any Draft.'); | ||
| return; | ||
| } | ||
| try { | ||
| const draftData = { | ||
| to: values.to.join(', '), | ||
| cc: values.cc?.join(', '), | ||
| bcc: values.bcc?.join(', '), | ||
| subject: values.subject, | ||
| message: editor.getHTML(), | ||
| attachments: await serializeFiles(values.attachments ?? []), | ||
| id: draftId, | ||
| threadId: threadId ? threadId : null, | ||
| fromEmail: values.fromEmail ? values.fromEmail : null, | ||
| }; | ||
|
|
||
| if(draftId){ | ||
| const response = await deleteDraft(draftData); | ||
| if(response === ''){ | ||
| setDraftId(null); | ||
| setIsComposeOpen(null); | ||
| setTimeout(() => { | ||
| // const currentUrl = new URL(window.location.href); | ||
| // window.location.href = currentUrl.toString(); | ||
| toast.success("Successfully Deleted Draft"); | ||
| refetchThreads(); | ||
| }, 500); | ||
| } | ||
| } | ||
| } catch (error) { | ||
| console.error('Failed to delete draft:', error); | ||
| toast.error('Failed to delete draft.'); | ||
| } finally { | ||
| setIsDeleteDraft(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
logic: DeleteDraft function is unnecessarily re-serializing attachment data when only draftId is needed for deletion.
WalkthroughThis change introduces full draft lifecycle management for email drafts, including explicit creation, updating, and deletion. It adds new API endpoints, server-side methods, and client-side hooks and handlers for updating and deleting drafts. The email composer now supports auto-saving, discarding, and immediate UI refreshes on draft updates or dialog closure. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CreateEmail
participant EmailComposer
participant ThreadsHook
participant DraftsAPI
User->>CreateEmail: Open compose dialog
CreateEmail->>EmailComposer: Render with draft data
EmailComposer->>DraftsAPI: Auto-save or update draft (on change)
DraftsAPI-->>EmailComposer: Save/update response
EmailComposer->>CreateEmail: onDraftUpdate callback
CreateEmail->>ThreadsHook: refetchThreads (on close)
User->>EmailComposer: Discard draft
EmailComposer->>DraftsAPI: deleteDraft
DraftsAPI-->>EmailComposer: Deletion response
EmailComposer->>CreateEmail: Close dialog
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (7)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| // } | ||
|
|
||
| //ths function is going to be used to delete drafts | ||
| const DeleteDraft = async () => { |
There was a problem hiding this comment.
Function name DeleteDraft uses PascalCase, which is typically reserved for class or component names in JavaScript/TypeScript. Consider renaming to deleteDraft to follow the standard camelCase convention for functions, maintaining consistency with other functions in this file like saveDraft and handleClose.
| const DeleteDraft = async () => { | |
| const deleteDraft = async () => { |
Spotted by Diamond (based on custom rules)
Is this helpful? React 👍 or 👎 to let us know.
| .mutation(async ({input, ctx}) =>{ | ||
| const {activeConnection} = ctx; | ||
| const agent = await getZeroAgent(activeConnection.id); | ||
| const res = agent.updateDraft(input) |
There was a problem hiding this comment.
The async function call to agent.updateDraft(input) is missing the await keyword. This could lead to unexpected behavior since the promise won't be properly resolved before assigning to res. The correct implementation should be:
const res = await agent.updateDraft(input);This ensures the promise is properly awaited before continuing execution.
| const res = agent.updateDraft(input) | |
| const res = await agent.updateDraft(input) |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
cubic found 5 issues across 7 files. Review them in cubic.dev
React with 👍 or 👎 to teach cubic. Tag @cubic-dev-ai to give specific feedback.
| const { mutateAsync: sendEmail } = useMutation(trpc.mail.send.mutationOptions()); | ||
| const [isComposeOpen, setIsComposeOpen] = useQueryState('isComposeOpen'); | ||
| const [, setThreadId] = useQueryState('threadId'); | ||
| const [{ isFetching, refetch: refetchThreads }] = useThreads(); |
There was a problem hiding this comment.
The isFetching variable is destructured but never used, which will trigger the no-unused-vars ESLint rule and can cause CI lint failures.
| const [{ isFetching, refetch: refetchThreads }] = useThreads(); | |
| const [{ refetch: refetchThreads }] = useThreads(); |
| toast.success("Your Draft has been Successfully Saved") | ||
| } | ||
| console.error("Failed Setting up Draft Id") | ||
| toast.error("Failed Setting up Draft Id") |
There was a problem hiding this comment.
Error toast is shown unconditionally, causing users to see a failure message even when the draft saved successfully.
| } catch (deleteError) { | ||
| console.error(`Failed to delete draft ${data.id}`, deleteError); | ||
| } | ||
| const message = await sanitizeTipTapHtml(data.message); |
There was a problem hiding this comment.
sanitizeTipTapHtml returns an object with an html property, but the entire object is being assigned to message and later treated as a string. This will serialize to "[object Object]" in the email body sent to Graph, corrupting the draft content.
| const message = await sanitizeTipTapHtml(data.message); | |
| const { html: message } = await sanitizeTipTapHtml(data.message); |
| return agent.createDraft(input); | ||
| }), | ||
| update: activeDriverProcedure | ||
| .input(createDraftData) |
There was a problem hiding this comment.
Using createDraftData for the delete mutation allows id to be null, so the router can be called without a valid draft id and will crash inside deleteDraft. Restrict the schema to require a non-nullable id.
| .input(createDraftData) | |
| .input(z.object({ id: z.string() })) |
| <div className="flex w-[750px] justify-start"> | ||
| <DialogClose asChild className="flex"> | ||
| <button className="dark:bg-panelDark flex items-center gap-1 rounded-lg bg-[#F0F0F0] px-2 py-1.5"> | ||
| <button onClick={() =>{ refetchThreads()}} className="dark:bg-panelDark flex items-center gap-1 rounded-lg bg-[#F0F0F0] px-2 py-1.5"> |
There was a problem hiding this comment.
Rule violated: Detect React performance bottlenecks and rule breaking
Inline anonymous functions are passed directly to JSX props, violating the performance rule that discourages defining functions inside JSX. This causes a new function instance on every render, leading to unnecessary re-renders.
![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
♻️ Duplicate comments (14)
apps/server/src/lib/driver/types.ts (1)
63-65: Consider simplifying deleteDraft parameterUsing the full
CreateDraftDataobject for deletion seems excessive when typically only an ID is needed for delete operations.apps/server/src/trpc/routes/drafts.ts (1)
21-28: Simplify delete endpoint and fix formattingThe delete endpoint uses an overly complex schema when it should just use an ID.
Apply this diff to fix formatting:
- delete: activeDriverProcedure - .input(createDraftData) - .mutation(async({input,ctx})=>{ - const {activeConnection} = ctx; - const agent = await getZeroAgent(activeConnection.id); - const res = agent.deleteDraft(input); - return res; - }), + delete: activeDriverProcedure + .input(createDraftData) + .mutation(async ({ input, ctx }) => { + const { activeConnection } = ctx; + const agent = await getZeroAgent(activeConnection.id); + const res = await agent.deleteDraft(input); + return res; + }),apps/mail/components/create/create-email.tsx (2)
66-66: Remove unused isFetching variableThe
isFetchingvariable is destructured but never used in the component.
177-177: Consider optimizing the refetch callThe inline function call could cause unnecessary re-renders. Consider using a useCallback hook.
apps/server/src/routes/chat.ts (3)
221-223: Fix spacing consistencyMissing space before opening brace - inconsistent with codebase style.
225-227: Fix spacing consistencyMissing space before opening brace - inconsistent with codebase style.
774-786: Fix quote consistency and error message formatThe implementation uses double quotes and inconsistent error message formatting.
Apply this diff to fix the issues:
- async updateDraft(draftData: CreateDraftData){ - if(!this.driver){ - throw new Error("No driver available") - } - return await this.driver.updateDraft(draftData); - } - async deleteDraft(draftData: CreateDraftData){ - if(!this.driver){ - throw new Error("No driver available") - } - return await this.driver.deleteDraft(draftData); - } + async updateDraft(draftData: CreateDraftData) { + if (!this.driver) { + throw new Error('No driver available'); + } + return await this.driver.updateDraft(draftData); + } + async deleteDraft(draftData: CreateDraftData) { + if (!this.driver) { + throw new Error('No driver available'); + } + return await this.driver.deleteDraft(draftData); + }apps/server/src/lib/driver/microsoft.ts (2)
724-724: Fix HTML and inline images destructuring.The sanitizeTipTapHtml function returns both HTML and inline images. This needs to be destructured properly.
Apply this diff:
- const message = await sanitizeTipTapHtml(data.message); + const { html: message, inlineImages } = await sanitizeTipTapHtml(data.message);
761-776: Add inline image handling to updateDraft.The updateDraft method is missing inline image handling that exists in createDraft. This could cause inline images to be lost when updating drafts.
Add inline image handling after line 760:
} + const allAttachments = []; + + if (inlineImages.length > 0) { + for (const image of inlineImages) { + allAttachments.push({ + '@odata.type': '#microsoft.graph.fileAttachment', + name: image.cid, + contentType: image.mimeType, + contentBytes: image.data, + contentId: image.cid, + isInline: true, + }); + } + } + if (data.attachments && data.attachments.length > 0) { - outlookMessage.attachments = await Promise.all( + const regularAttachments = await Promise.all( data.attachments.map(async (file) => { const arrayBuffer = await file.arrayBuffer(); const buffer = Buffer.from(arrayBuffer); const base64Content = buffer.toString('base64'); return { '@odata.type': '#microsoft.graph.fileAttachment', name: file.name, contentType: file.type || 'application/octet-stream', contentBytes: base64Content, }; }), ); + allAttachments.push(...regularAttachments); + } + + if (allAttachments.length > 0) { + outlookMessage.attachments = allAttachments; }apps/mail/components/create/email-composer.tsx (2)
564-632: Optimize saveDraft with useCallback to prevent unnecessary re-renders.The saveDraft function is used as a dependency in useEffect but isn't memoized, which can cause unnecessary effect re-runs.
Wrap the saveDraft function with useCallback:
- const saveDraft = async () => { + const saveDraft = useCallback(async () => { const values = getValues(); if (!hasUnsavedChanges) return; // ... rest of the function - }; + }, [hasUnsavedChanges, getValues, editor, initialMessage, aiGeneratedMessage, aiIsLoading, isGeneratingSubject, draftId, threadId, updateDraft, createDraft, onDraftUpdate]);
644-682: Remove unnecessary data serialization in DeleteDraft.The DeleteDraft function only needs the draftId but is creating and serializing the entire draft data including attachments.
Simplify the function to only use the required draftId:
const DeleteDraft = async () => { - const values = getValues(); if (!draftId) { toast.error('No draft Id available to delete any Draft.'); return; } try { - const draftData = { - to: values.to.join(', '), - cc: values.cc?.join(', '), - bcc: values.bcc?.join(', '), - subject: values.subject, - message: editor.getHTML(), - attachments: await serializeFiles(values.attachments ?? []), - id: draftId, - threadId: threadId ? threadId : null, - fromEmail: values.fromEmail ? values.fromEmail : null, - }; - - if(draftId){ - const response = await deleteDraft(draftData); + const response = await deleteDraft({ id: draftId }); if(response === ''){ setDraftId(null); setIsComposeOpen(null); setTimeout(() => { - // const currentUrl = new URL(window.location.href); - // window.location.href = currentUrl.toString(); toast.success("Successfully Deleted Draft"); refetchThreads(); }, 500); } - } } catch (error) {apps/server/src/lib/driver/google.ts (3)
607-611: Verify thatattachment.base64is a Promise before awaitingThe code awaits
attachment.base64, but this may not be a Promise if the attachment object already contains a base64 string.Run the following script to check how attachments are structured in the codebase:
#!/bin/bash # Description: Check attachment structure and base64 property usage # Search for attachment type definitions and interfaces ast-grep --pattern 'interface $_ { $$$ base64: $_; $$$ }' # Search for places where attachments are created or processed rg -A 5 "attachments.*base64" --type ts
722-735: Fix inconsistent spacing in function declarationThe function has spacing issues that affect code readability and consistency.
Apply this diff to fix the spacing:
- public deleteDraft(data: CreateDraftData){ + public deleteDraft(data: CreateDraftData) { return this.withErrorHandler( 'deleteDraft', - async () =>{ + async () => { if (!data.id) throw new Error('Missing draft ID to delete'); const res = await this.gmail.users.drafts.delete({ userId: 'me', id: data.id, - }) + }); return res.data; - } , {data} + }, { data } ); }
644-721: Extract common MIME message creation logic to reduce duplicationThe
updateDraftmethod duplicates most of the logic fromcreateDraft. This violates the DRY principle and makes maintenance difficult.Additionally, there's an inconsistency in attachment handling:
createDraftawaitsattachment.base64(line 611) whileupdateDraftdoesn't (line 688).Extract the common MIME message creation logic into a private helper method:
+ private async createMimeMessageForDraft(data: CreateDraftData) { + const { html: message, inlineImages } = await sanitizeTipTapHtml(data.message); + const msg = createMimeMessage(); + msg.setSender('me'); + + const to = data.to.split(', ').map((recipient: string) => { + if (recipient.includes('<')) { + const [name, email] = recipient.split('<'); + return { addr: email.replace('>', ''), name: name.replace('>', '') }; + } + return { addr: recipient }; + }); + + msg.setTo(to); + if (data.cc) msg.setCc(data.cc.split(', ').map((addr) => ({ addr }))); + if (data.bcc) msg.setBcc(data.bcc.split(', ').map((addr) => ({ addr }))); + msg.setSubject(data.subject); + + msg.addMessage({ + contentType: 'text/html', + data: message || '', + }); + + if (inlineImages.length > 0) { + for (const image of inlineImages) { + msg.addAttachment({ + inline: true, + filename: `${image.cid}`, + contentType: image.mimeType, + data: image.data, + headers: { + 'Content-ID': `<${image.cid}>`, + 'Content-Disposition': 'inline', + }, + }); + } + } + + if (data.attachments?.length) { + for (const attachment of data.attachments) { + const base64Data = await attachment.base64; + msg.addAttachment({ + filename: attachment.name, + contentType: attachment.type, + data: base64Data, + }); + } + } + + const mimeMessage = msg.asRaw(); + const encodedMessage = Buffer.from(mimeMessage) + .toString('base64') + .replace(/\+/g, '-') + .replace(/\//g, '_') + .replace(/=+$/, ''); + + return { + raw: encodedMessage, + threadId: data.threadId, + }; + }Then simplify both methods:
public createDraft(data: CreateDraftData) { return this.withErrorHandler( 'createDraft', async () => { - // ... all the duplicated code ... + const { raw, threadId } = await this.createMimeMessageForDraft(data); + const res = await this.gmail.users.drafts.create({ + userId: 'me', + requestBody: { + message: { raw, threadId }, + }, + }); + return res.data; }, { data } ); } public updateDraft(data: CreateDraftData) { return this.withErrorHandler( 'updateDraft', async () => { if (!data.id) throw new Error('Missing draft ID for update'); - // ... all the duplicated code ... + const { raw, threadId } = await this.createMimeMessageForDraft(data); + const res = await this.gmail.users.drafts.update({ + userId: 'me', + id: data.id, + requestBody: { + message: { raw, threadId }, + }, + }); + return res.data; }, { data } ); }
🧹 Nitpick comments (1)
apps/server/src/trpc/routes/drafts.ts (1)
13-20: Fix missing semicolon and formatting inconsistenciesThe implementation is correct but has style issues that need to be addressed.
Apply this diff to fix the styling:
- update: activeDriverProcedure - .input(createDraftData) - .mutation(async ({input, ctx}) =>{ - const {activeConnection} = ctx; - const agent = await getZeroAgent(activeConnection.id); - const res = agent.updateDraft(input) - return res; - }), + update: activeDriverProcedure + .input(createDraftData) + .mutation(async ({ input, ctx }) => { + const { activeConnection } = ctx; + const agent = await getZeroAgent(activeConnection.id); + const res = await agent.updateDraft(input); + return res; + }),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/mail/components/create/create-email.tsx(5 hunks)apps/mail/components/create/email-composer.tsx(13 hunks)apps/server/src/lib/driver/google.ts(4 hunks)apps/server/src/lib/driver/microsoft.ts(1 hunks)apps/server/src/lib/driver/types.ts(1 hunks)apps/server/src/routes/chat.ts(2 hunks)apps/server/src/trpc/routes/drafts.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (4)
apps/server/src/lib/driver/types.ts (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1468
File: apps/server/src/trpc/routes/mail.ts:386-391
Timestamp: 2025-06-27T04:59:29.731Z
Learning: In apps/server/src/trpc/routes/mail.ts, the attachment processing logic conditionally handles mixed attachment types - it preserves existing File-like objects with arrayBuffer methods while only converting serialized attachments that need processing through toAttachmentFiles.
apps/mail/components/create/create-email.tsx (2)
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
Learnt from: retrogtx
PR: Mail-0/Zero#1354
File: apps/mail/components/ui/prompts-dialog.tsx:85-88
Timestamp: 2025-06-20T05:03:16.944Z
Learning: In React Hook Form, avoid using useEffect for form state synchronization when the values prop can handle reactive updates automatically. The values prop is specifically designed for this purpose and is more optimal than manual useEffect-based synchronization.
apps/server/src/lib/driver/google.ts (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1468
File: apps/server/src/trpc/routes/mail.ts:386-391
Timestamp: 2025-06-27T04:59:29.731Z
Learning: In apps/server/src/trpc/routes/mail.ts, the attachment processing logic conditionally handles mixed attachment types - it preserves existing File-like objects with arrayBuffer methods while only converting serialized attachments that need processing through toAttachmentFiles.
apps/mail/components/create/email-composer.tsx (2)
Learnt from: retrogtx
PR: Mail-0/Zero#1354
File: apps/mail/components/ui/prompts-dialog.tsx:85-88
Timestamp: 2025-06-20T05:03:16.944Z
Learning: In React Hook Form, avoid using useEffect for form state synchronization when the values prop can handle reactive updates automatically. The values prop is specifically designed for this purpose and is more optimal than manual useEffect-based synchronization.
Learnt from: retrogtx
PR: Mail-0/Zero#1468
File: apps/server/src/trpc/routes/mail.ts:386-391
Timestamp: 2025-06-27T04:59:29.731Z
Learning: In apps/server/src/trpc/routes/mail.ts, the attachment processing logic conditionally handles mixed attachment types - it preserves existing File-like objects with arrayBuffer methods while only converting serialized attachments that need processing through toAttachmentFiles.
🧬 Code Graph Analysis (3)
apps/server/src/lib/driver/types.ts (1)
apps/server/src/lib/schemas.ts (1)
CreateDraftData(37-37)
apps/mail/components/create/create-email.tsx (2)
apps/mail/hooks/use-drafts.ts (1)
useDraft(5-15)apps/mail/hooks/use-threads.ts (1)
useThreads(13-61)
apps/server/src/routes/chat.ts (1)
apps/server/src/lib/schemas.ts (1)
CreateDraftData(37-37)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: cubic · AI code reviewer
🔇 Additional comments (5)
apps/server/src/lib/driver/types.ts (1)
60-62: LGTM: Consistent return type with createDraftThe return type structure is consistent with the existing
createDraftmethod, maintaining API consistency.apps/mail/components/create/create-email.tsx (1)
208-210: Well-integrated draft update callbackThe
onDraftUpdateprop properly connects the draft update functionality with the UI refetch logic, ensuring data consistency.apps/server/src/routes/chat.ts (1)
767-786: Well-implemented draft lifecycle methodsThe methods properly implement the draft update and delete functionality with appropriate error handling and delegation patterns consistent with the existing codebase.
apps/server/src/lib/driver/microsoft.ts (1)
709-717: LGTM! Good practice to explicitly return the response.The explicit return of the draft creation response improves code clarity and aligns with the updateDraft method pattern.
apps/mail/components/create/email-composer.tsx (1)
685-693: Good implementation of auto-save on close.The handleClose function properly saves the draft before showing the leave confirmation dialog, ensuring user data is preserved.
| //ths function is going to be used to delete drafts | ||
| const DeleteDraft = async () => { |
There was a problem hiding this comment.
The function name DeleteDraft uses PascalCase instead of the standard camelCase convention for JavaScript/TypeScript functions. Consider renaming it to deleteDraft to maintain consistent naming conventions throughout the codebase. This will improve code readability and align with the existing style where other functions like saveDraft and handleClose follow camelCase.
| //ths function is going to be used to delete drafts | |
| const DeleteDraft = async () => { | |
| //this function is going to be used to delete drafts | |
| const deleteDraft = async () => { |
Spotted by Diamond (based on custom rules)
Is this helpful? React 👍 or 👎 to let us know.
| if(draftId){ | ||
| const response = await updateDraft(draftData); | ||
| if(response?.id){ | ||
| setDraftId(response?.id); | ||
| onDraftUpdate?.(); | ||
| toast.success("Your Draft has been Successfully Saved") | ||
| } | ||
| else{ | ||
| const response = await createDraft(draftData); | ||
| if(response?.id){ | ||
| setDraftId(response?.id); | ||
| toast.success("Your Draft has been Successfully Saved") | ||
| } | ||
| console.error("Failed Setting up Draft Id") | ||
| toast.error("Failed Setting up Draft Id") | ||
| } |
There was a problem hiding this comment.
The error handling logic in this section has a structural issue. When updateDraft fails, the fallback to createDraft is nested inside the else block, making it inaccessible when response?.id is falsy. Consider restructuring to:
if (draftId) {
try {
const response = await updateDraft(draftData);
if (response?.id) {
setDraftId(response.id);
onDraftUpdate?.();
toast.success("Your Draft has been Successfully Saved");
} else {
throw new Error("Update failed - missing ID in response");
}
} catch (error) {
// Fall back to creating a new draft if update fails
const response = await createDraft(draftData);
if (response?.id) {
setDraftId(response.id);
toast.success("Your Draft has been Successfully Saved");
} else {
console.error("Failed Setting up Draft Id");
toast.error("Failed Setting up Draft Id");
}
}
} else {
// No existing draft ID, create new
const response = await createDraft(draftData);
// ...rest of create logic
}This ensures proper error handling and fallback behavior when updating drafts fails.
| if(draftId){ | |
| const response = await updateDraft(draftData); | |
| if(response?.id){ | |
| setDraftId(response?.id); | |
| onDraftUpdate?.(); | |
| toast.success("Your Draft has been Successfully Saved") | |
| } | |
| else{ | |
| const response = await createDraft(draftData); | |
| if(response?.id){ | |
| setDraftId(response?.id); | |
| toast.success("Your Draft has been Successfully Saved") | |
| } | |
| console.error("Failed Setting up Draft Id") | |
| toast.error("Failed Setting up Draft Id") | |
| } | |
| if(draftId){ | |
| try { | |
| const response = await updateDraft(draftData); | |
| if(response?.id){ | |
| setDraftId(response.id); | |
| onDraftUpdate?.(); | |
| toast.success("Your Draft has been Successfully Saved") | |
| } else { | |
| throw new Error("Update failed - missing ID in response"); | |
| } | |
| } catch (error) { | |
| // Fall back to creating a new draft if update fails | |
| const response = await createDraft(draftData); | |
| if(response?.id){ | |
| setDraftId(response.id); | |
| toast.success("Your Draft has been Successfully Saved") | |
| } else { | |
| console.error("Failed Setting up Draft Id") | |
| toast.error("Failed Setting up Draft Id") | |
| } | |
| } | |
| } else { | |
| // No existing draft ID, create new | |
| const response = await createDraft(draftData); | |
| if(response?.id){ | |
| setDraftId(response.id); | |
| toast.success("Your Draft has been Successfully Saved") | |
| } else { | |
| console.error("Failed Setting up Draft Id") | |
| toast.error("Failed Setting up Draft Id") | |
| } | |
| } |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
| .mutation(async({input,ctx})=>{ | ||
| const {activeConnection} = ctx; | ||
| const agent = await getZeroAgent(activeConnection.id); | ||
| const res = agent.deleteDraft(input); |
There was a problem hiding this comment.
The deleteDraft call is missing the await keyword, which means this function will return a Promise rather than the resolved value. This could lead to unexpected behavior when the caller tries to use the return value. Consider updating to:
const res = await agent.deleteDraft(input);This ensures the operation completes before returning the result to the caller.
| const res = agent.deleteDraft(input); | |
| const res = await agent.deleteDraft(input); |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
…yKumarJaiswal/mail0 into feat/autoSave_Email_drafts
| .mutation(async ({input, ctx}) =>{ | ||
| const {activeConnection} = ctx; | ||
| const agent = await getZeroAgent(activeConnection.id); | ||
| const res = agent.updateDraft(input); |
There was a problem hiding this comment.
The updateDraft call is missing the await keyword. Since this is an asynchronous operation, without await it will return a Promise rather than the resolved value:
const res = await agent.updateDraft(input);This ensures the response contains the actual data rather than a Promise object, which would cause issues when the client tries to access properties like id from the response.
| const res = agent.updateDraft(input); | |
| const res = await agent.updateDraft(input); |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
1 participant
Auto-Save , Auto-Update and Delete Draft
Remove bullet points that are not relevant.
PLEASE REFRAIN FROM USING AI TO WRITE YOUR CODE AND PR DESCRIPTION. IF YOU DO USE AI TO WRITE YOUR CODE PLEASE PROVIDE A DESCRIPTION AND REVIEW IT CAREFULLY. MAKE SURE YOU UNDERSTAND THE CODE YOU ARE SUBMITTING USING AI.
Description
Please provide a clear description of your changes.
Type of Change
Please delete options that are not relevant.
Areas Affected
Please check all that apply:
Testing Done
Describe the tests you've done:
Security Considerations
For changes involving data or authentication:
Checklist
Additional Notes
Add any other context about the pull request here.
Screenshots/Recordings
Add screenshots or recordings here if applicable.
By submitting this pull request, I confirm that my contribution is made under the terms of the project's license.
Summary by cubic
Added auto-save, update, and delete features for email drafts, allowing users to save changes automatically and remove drafts as needed.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor