Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughA new test suite was added for the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant draft (actions.ts)
participant draftEmail
participant handlePreviousDraftDeletion
participant Gmail
participant Logger
User->>draft: call draft(gmail, executedRule, logger, ...)
draft->>+draftEmail: create draft
draft->>+handlePreviousDraftDeletion: delete previous draft
handlePreviousDraftDeletion->>Gmail: fetch previous draft
handlePreviousDraftDeletion->>Logger: log actions/errors
handlePreviousDraftDeletion->>Gmail: delete draft (if applicable)
handlePreviousDraftDeletion->>Logger: log deletion/update
draftEmail-->>draft: draftId
handlePreviousDraftDeletion-->>draft: (done)
draft-->>User: draftId
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/web/__tests__/ai/choose-rule/draft-management.test.tsOops! Something went wrong! :( ESLint: 9.28.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by apps/web/utils/ai/actions.tsOops! Something went wrong! :( ESLint: 9.28.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by apps/web/utils/ai/choose-rule/draft-management.tsOops! Something went wrong! :( ESLint: 9.28.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by ✨ 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 (
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ BugBot reviewed your changes and found no bugs!
BugBot free trial expires on June 9, 2025
You have used $0.00 of your $50.00 spend limit so far. Manage your spend limit in the Cursor dashboard.
Was this report helpful? Give feedback by reacting with 👍 or 👎
![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
🧹 Nitpick comments (2)
apps/web/utils/ai/actions.ts (1)
124-141: Excellent optimization with parallel execution.Running draft creation and previous draft deletion concurrently is a smart optimization. The implementation correctly destructures only the first result since
handlePreviousDraftDeletiondoesn't return a meaningful value.Consider that if
handlePreviousDraftDeletionthrows an error, it will cause the entirePromise.allto reject and prevent the draft from being created. Given that draft deletion is not critical to the main flow, you might want to handle errors withinhandlePreviousDraftDeletionto ensure draft creation always succeeds.Looking at the
handlePreviousDraftDeletionfunction inapps/web/utils/ai/choose-rule/draft-management.ts(lines 106-109), I can see it already has proper error handling with try-catch and logging, so the current implementation should be robust.apps/web/__tests__/ai/choose-rule/draft-management.test.ts (1)
24-40: Consider adding tests for different quote header patterns.While the current test covers the most common quote pattern (
\n\nOn .* wrote:), the actual function supports multiple patterns. Consider adding tests for the other supported patterns to ensure robust quote stripping.Add test cases for other quote header patterns:
+ it("should handle different quote header patterns", async () => { + const testCases = [ + { + pattern: "----+ Original Message ----+", + content: "Hello, test\n\n-------- Original Message --------\n> Previous message", + expected: "Hello, test" + }, + { + pattern: ">+ On .*", + content: "Hello, test\n\n>> On Monday...\n> Previous message", + expected: "Hello, test" + }, + { + pattern: "From: .*", + content: "Hello, test\n\nFrom: sender@example.com\n> Previous message", + expected: "Hello, test" + } + ]; + + // Test each pattern... + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.cursor/rules/testing.mdc(1 hunks)apps/web/__tests__/ai/choose-rule/draft-management.test.ts(1 hunks)apps/web/utils/ai/actions.ts(2 hunks)apps/web/utils/ai/choose-rule/draft-management.ts(4 hunks)version.txt(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
apps/web/utils/ai/actions.ts (1)
apps/web/utils/ai/choose-rule/draft-management.ts (1)
handlePreviousDraftDeletion(11-110)
apps/web/utils/ai/choose-rule/draft-management.ts (1)
apps/web/utils/gmail/draft.ts (1)
deleteDraft(24-45)
apps/web/__tests__/ai/choose-rule/draft-management.test.ts (4)
apps/web/utils/logger.ts (1)
createScopedLogger(17-65)apps/web/utils/gmail/draft.ts (2)
getDraft(9-22)deleteDraft(24-45)apps/web/utils/types.ts (1)
ParsedMessage(47-58)apps/web/utils/ai/choose-rule/draft-management.ts (1)
handlePreviousDraftDeletion(11-110)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Static Code Analysis Js
- GitHub Check: Jit Security
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (14)
version.txt (1)
1-1: Version increment looks appropriate.The version bump from v1.3.20 to v1.3.21 is appropriate for the draft management enhancements in this PR.
.cursor/rules/testing.mdc (1)
44-45: Good addition to testing best practices.The guideline to avoid mocking the Logger is sensible and aligns with the code changes where logging behavior is important to preserve in tests.
apps/web/utils/ai/actions.ts (1)
22-22: Good addition of the import.The import for
handlePreviousDraftDeletionis correctly added to support the new parallel execution.apps/web/utils/ai/choose-rule/draft-management.ts (4)
25-31: Improved filtering logic for better draft selection.The explicit filtering by
threadIdandemailAccountIdensures precise targeting of drafts, and thedraftSendLog: nullcondition properly excludes already-sent drafts. The explicit exclusion of the currentexecutedRule.idprevents self-reference issues.
56-72: Enhanced content comparison with multiple quote patterns.The implementation of multiple quote header patterns significantly improves the accuracy of content comparison by handling different email client quote formats. The loop structure efficiently finds the first matching pattern and properly extracts the user's content.
The patterns cover common quote formats:
- Gmail/Outlook:
"On ... wrote:"- Some email clients:
"---- Original Message ----"- Reply prefixes:
"> On ..."- Forward headers:
"From: ..."
84-93: Excellent use of parallel operations for performance.The concurrent execution of
deleteDraftand database update usingPromise.allis a good optimization. Both operations are independent and can safely run in parallel, reducing the overall execution time.The error handling is properly managed by the outer try-catch block, and the logging provides good visibility into the operations.
127-127: Clean use of shorthand property notation.The simplification to use shorthand property notation (
{ draftId }instead of{ draftId: draftId }) improves code readability without changing functionality.apps/web/__tests__/ai/choose-rule/draft-management.test.ts (7)
1-22: Well-structured test setup with comprehensive mocking.The imports and mock setup are properly configured. The mocking strategy correctly isolates the function under test by mocking external dependencies (Prisma and Gmail draft functions).
42-104: Excellent test coverage for the main success scenario.This test correctly validates the core functionality:
- Proper database query with all expected filters
- Content comparison logic with quote stripping (the test content with "\n\nOn Monday wrote:" gets stripped correctly)
- Concurrent execution of draft deletion and database update
- All assertions verify the correct function calls and parameters
The mock data accurately represents real-world email draft scenarios.
106-143: Good test for preventing deletion of user-modified drafts.This test ensures that when users modify draft content, the system respects their changes and doesn't delete the draft. The content comparison ("test" vs "MODIFIED") correctly triggers the skip deletion logic.
145-156: Proper handling of the no previous draft scenario.This test correctly validates that when no previous draft exists, the function gracefully exits without making unnecessary Gmail API calls.
158-175: Good edge case coverage for Gmail API scenarios.This test handles the case where a draft record exists in the database but the actual draft has been deleted from Gmail (404 scenarios), ensuring the function doesn't crash.
177-189: Excellent error handling validation.The test correctly verifies that database errors are caught and logged without throwing, ensuring the function's resilience doesn't break the overall workflow.
191-226: Important edge case for drafts without plain text content.This test covers scenarios where drafts might only have HTML content or no readable text content, ensuring the function handles these gracefully without attempting deletion.
1 participant
Summary by CodeRabbit