Feature: add Mark as read action
#310
Conversation
|
@arndom is attempting to deploy a commit to the Inbox Zero Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe pull request introduces a new action type Changes
Assessment against linked issues
Possibly related PRs
Poem
Finishing Touches
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/utils/ai/actions.ts (1)
436-441: Add error handling and improve type safety.While the implementation is correct, consider these improvements:
- Add error handling to match other functions
- Use a more specific type instead of
any-const mark_read: ActionFunction<any> = async ( +const mark_read: ActionFunction<Record<string, never>> = async ( gmail: gmail_v1.Gmail, email: EmailForAction, ) => { - return await markReadThread({ gmail, threadId: email.threadId, read: true }); + try { + return await markReadThread({ gmail, threadId: email.threadId, read: true }); + } catch (error) { + logger.error("Failed to mark thread as read", { error, threadId: email.threadId }); + throw error; + } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/web/components/PlanBadge.tsx(3 hunks)apps/web/prisma/migrations/20250116101856_mark_read_action/migration.sql(1 hunks)apps/web/prisma/schema.prisma(2 hunks)apps/web/utils/actionType.ts(1 hunks)apps/web/utils/actions/validation.ts(1 hunks)apps/web/utils/ai/actions.ts(5 hunks)
🔇 Additional comments (9)
apps/web/utils/actionType.ts (1)
119-119: Implementation looks good!The MARK_READ action is correctly implemented with no input fields, consistent with other similar actions like ARCHIVE and MARK_SPAM.
apps/web/components/PlanBadge.tsx (1)
123-124: UI implementation is consistent and well-designed!The MARK_READ action is properly integrated into the UI components:
- Badge component follows the same pattern as other actions
- Clear and user-friendly label "Mark as read"
- Consistent color scheme with similar actions (yellow, same as Archive)
Also applies to: 156-157, 189-189
apps/web/utils/actions/validation.ts (1)
39-39: Validation schema correctly updated!The MARK_READ action type is properly added to the zodActionType enum. No additional validation rules are needed since this action doesn't require any input fields.
apps/web/prisma/migrations/20250116101856_mark_read_action/migration.sql (1)
1-2: Database migration is properly implemented!The migration correctly adds the MARK_READ value to the ActionType enum using standard PostgreSQL syntax. The migration file is well-named with date and purpose.
apps/web/utils/ai/actions.ts (3)
13-13: LGTM! Import added correctly.The
markReadThreadimport is appropriately grouped with other Gmail label utilities.
252-261: LGTM! Action definition is well-structured.The
MARK_READaction definition follows the established pattern and correctly specifies no required parameters.
475-476: LGTM! Switch case added correctly.The
MARK_READcase is properly integrated into the action function dispatcher.apps/web/prisma/schema.prisma (2)
378-378: LGTM! Enum value added correctly.The
MARK_READaction type is properly added to the enum and follows the established naming convention.
Line range hint
17-17: Verify the impact of adding @unique constraint.Adding a unique constraint to
userIdis a breaking change that:
- Enforces one account per user
- Could fail if existing users have multiple accounts
Run this script to check for potential conflicts:
✅ Verification successful
The unique constraint on Account.userId is safe to add
The multi-account functionality in the codebase is implemented through the premium relation for sharing premium features, not through multiple OAuth accounts. The Account model is strictly used for authentication, and no code patterns were found that rely on multiple accounts per user. The migration warning is precautionary but not a concern in this case.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if any users have multiple accounts that would violate the new constraint # Use ast-grep to find the Account model definition and analyze its structure echo "Current Account model structure:" ast-grep --pattern 'model Account { $$$ }' # Use ripgrep to find any migration files that might have relevant history echo -e "\nRelevant migration history:" rg -l "Account" --glob "migrations/*.sql"Length of output: 383
Script:
#!/bin/bash # First, find all prisma schema files echo "Prisma schema files:" fd -e prisma # Find migration files in various possible locations echo -e "\nMigration files:" fd -e sql # Look for Account model definition and its usage echo -e "\nAccount model structure and usage:" rg -A 10 "model Account" --type-not sql # Check for any code that might be accessing multiple accounts per user echo -e "\nCode patterns accessing accounts:" rg "findMany.*where.*userId" --type typescriptLength of output: 5542
Script:
#!/bin/bash # Check content of migration that added unique constraint echo "Migration that added unique constraint:" cat "apps/web/prisma/migrations/20231027022923_unique_account/migration.sql" # Search for Account-related patterns in TypeScript/JavaScript files echo -e "\nCode patterns accessing accounts:" rg "Account.*findMany|findMany.*Account" --type js --type ts # Look for any imports or uses of Account model echo -e "\nAccount model usage:" rg "Account.*from.*prisma|import.*Account" --type js --type tsLength of output: 1132
Script:
#!/bin/bash # Check content of multi-account related files echo "Multi-account component content:" cat "apps/web/app/(app)/settings/MultiAccountSection.tsx" echo -e "\nMulti-account API route:" cat "apps/web/app/api/user/settings/multi-account/route.ts"Length of output: 8935
| -- AlterEnum | ||
| ALTER TYPE "ActionType" ADD VALUE 'MARK_READ'; |
There was a problem hiding this comment.
💡 Codebase verification
Test coverage missing for MARK_READ action
The verification confirms no test coverage exists for the new MARK_READ action. Based on the existing test patterns in apps/web/__tests__/, please add tests that cover:
- Action type validation in the action system
- Integration with existing action handling logic
- Any specific argument processing if applicable
Follow the established testing pattern using Vitest as shown in the existing action tests.
🔗 Analysis chain
Verify test coverage for the new action.
While the implementation looks good, please ensure adequate test coverage for the MARK_READ action. Consider adding:
- Unit tests for the action handler
- Integration tests for the UI components
- E2E tests for the complete workflow
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for test files covering the new MARK_READ action
echo "Searching for test coverage of MARK_READ action..."
rg -l "MARK_READ" "**/*test*" "**/*spec*"
Length of output: 252
Script:
#!/bin/bash
# Find test files and search for MARK_READ
echo "Searching in test files..."
fd -e test.ts -e test.tsx -e spec.ts -e spec.tsx | xargs rg -l "MARK_READ"
echo -e "\nSearching for action-related test files..."
fd -e test.ts -e test.tsx -e spec.ts -e spec.tsx | xargs rg -l "ActionType|action.type"
Length of output: 458
Script:
#!/bin/bash
# Examine existing action test structure
echo "Checking existing action test structure..."
rg -A 5 "describe|test\(" "apps/web/__tests__/ai-choose-rule.test.ts"
Length of output: 1369
Summary by CodeRabbit
Release Notes
New Features
Database Changes
Improvements