Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes introduce a modularized implementation of the Changes
Sequence Diagram(s)sequenceDiagram
participant CronJob as Cron Job (POST)
participant API as API Route
participant Logic as disableUnusedAutoDrafts Module
participant DB as Database
CronJob->>API: POST /reply-tracker/disable-unused-auto-draft
API->>Logic: disableUnusedAutoDrafts()
Logic->>DB: Query users with auto-draft enabled
loop For each user
Logic->>DB: Fetch last 10 drafts older than 1 day
alt All drafts unused
Logic->>DB: Delete DRAFT_EMAIL actions
else At least one draft used
Logic->>Logic: Skip disabling
end
end
Logic-->>API: {usersChecked, usersDisabled, errors}
API-->>CronJob: JSON response
Possibly related PRs
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/app/api/reply-tracker/disable-unused-auto-draft/disable-unused-auto-drafts.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/app/api/reply-tracker/disable-unused-auto-draft/disable-unused-auto-drafts.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/app/api/reply-tracker/disable-unused-auto-draft/route.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 (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/web/app/api/reply-tracker/disable-unused-auto-draft/disable-unused-auto-drafts.ts (3)
20-20: Address the efficiency TODO or create a tracking issue.The TODO comment indicates potential performance concerns with the current query strategy. Consider either optimizing the query or creating a tracking issue to address this in the future.
Would you like me to help optimize the query or create an issue to track this performance concern?
112-121: Clarify thecontent: nullfilter in the deletion query.The deletion query specifically targets actions with
content: null. This seems intentional but could benefit from a comment explaining why only null content actions are deleted.Consider adding a comment to explain why the deletion is limited to actions with null content:
// Delete the DRAFT_EMAIL actions from all TO_REPLY rules + // Only delete actions with null content as these are auto-generated draft actions await prisma.action.deleteMany({ where: { rule: { emailAccountId, systemType: SystemType.TO_REPLY, }, type: ActionType.DRAFT_EMAIL, content: null, }, });
7-7: Consider making the draft threshold configurable.The hard-coded
MAX_DRAFTS_TO_CHECK = 10could benefit from being configurable, especially as usage patterns may vary across different user bases.Consider making this configurable through environment variables or function parameters for better flexibility.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/web/app/api/reply-tracker/disable-unused-auto-draft/disable-unused-auto-drafts.test.ts(1 hunks)apps/web/app/api/reply-tracker/disable-unused-auto-draft/disable-unused-auto-drafts.ts(1 hunks)apps/web/app/api/reply-tracker/disable-unused-auto-draft/route.ts(1 hunks)version.txt(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (7)
version.txt (1)
1-1: LGTM! Version bump aligns with the new functionality.The version increment is appropriate for the addition of comprehensive test coverage and code refactoring.
apps/web/app/api/reply-tracker/disable-unused-auto-draft/disable-unused-auto-drafts.ts (1)
15-134: LGTM! Well-structured function with comprehensive logic.The function is well-implemented with proper error handling, logging, and clear business logic. The extraction from the route handler improves code organization and testability.
apps/web/app/api/reply-tracker/disable-unused-auto-draft/route.ts (2)
5-5: LGTM! Clean refactoring improves separation of concerns.The extraction of business logic into a dedicated module while maintaining the same API contract is excellent. This improves testability and code organization.
17-18: LGTM! Preserved functionality with cleaner implementation.The route handler now focuses solely on HTTP concerns while delegating business logic to the appropriate module.
apps/web/app/api/reply-tracker/disable-unused-auto-draft/disable-unused-auto-drafts.test.ts (3)
1-249: Excellent comprehensive test coverage!The test suite thoroughly covers all the main scenarios:
- Users with unused drafts (should be disabled)
- Users with sent drafts (should not be disabled)
- Users with draft send logs (should not be disabled)
- Users with insufficient drafts (should be skipped)
- Multiple user scenarios
- Error handling
The tests are well-structured with clear descriptions and proper mocking. This significantly improves the reliability of the codebase.
46-51: Good test data generation pattern.Using
Array.fromwith a mapping function to generate test data is clean and readable. This pattern makes it easy to create realistic test scenarios.
182-209: Excellent use of mock chaining for complex scenarios.The use of
mockResolvedValueOnceto simulate different responses for multiple users is a great testing pattern that clearly demonstrates the expected behavior for each user type.
There was a problem hiding this comment.
Bug: Cron Job Data Stale After Config Change
The export const dynamic = "force-dynamic"; declaration was removed. This Next.js configuration is essential for cron job routes to ensure they run dynamically and fetch fresh database data on each execution, rather than being cached or statically optimized. Its removal could lead to stale data being used by the cron job logic.
apps/web/app/api/reply-tracker/disable-unused-auto-draft/route.ts#L1-L18
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 👎
Summary by CodeRabbit