Feature: add Mark as read action#310
Feature: add Mark as read action#310elie222 merged 5 commits intoelie222:mainfrom arndom:feature/read-action
Mark as read action#310Conversation
|
@arndom is attempting to deploy a commit to the Inbox Zero Team on Vercel. A member of the Team first needs to authorize it. |
|
Warning Rate limit exceeded@elie222 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 11 minutes and 27 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 (1)
WalkthroughThis pull request introduces a new action type Changes
Assessment against linked issues
Possibly related PRs
Poem
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
apps/web/prisma/migrations/20250116101856_mark_read_action/migration.sql
Show resolved
Hide resolved
|
Thanks! Will try get this merged tomorrow. I caused a merge conflict here today |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/utils/ai/actions.ts (1)
437-442: Improve type safety by usingRecord<string, unknown>instead ofany.The
mark_readfunction implementation is correct, but it's using theanytype for the args parameter. For better type safety, useRecord<string, unknown>since this action doesn't require any parameters.Apply this diff:
-const mark_read: ActionFunction<any> = async ( +const mark_read: ActionFunction<Record<string, unknown>> = async ( gmail: gmail_v1.Gmail, email: EmailForAction, ) => { return await markReadThread({ gmail, threadId: email.threadId, read: true }); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/web/prisma/schema.prisma(2 hunks)apps/web/utils/action-item.ts(1 hunks)apps/web/utils/actions/validation.ts(1 hunks)apps/web/utils/ai/actions.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/utils/actions/validation.ts
- apps/web/prisma/schema.prisma
🔇 Additional comments (3)
apps/web/utils/ai/actions.ts (3)
265-274: LGTM!The
MARK_READaction function definition follows the same pattern as other no-parameter actions and has a clear description.
285-285: LGTM!The new action type is correctly added to the
actionFunctionDefsrecord.
476-477: LGTM!The new case is correctly added to the
runActionFunctionswitch statement.
| }, | ||
| ], | ||
| }, | ||
| [ActionType.MARK_READ]: { fields: [] }, |
There was a problem hiding this comment.
LGTM! But handle the new action type in sanitizeActionFields.
The MARK_READ action is correctly added to actionInputs. However, it needs to be handled in the sanitizeActionFields function's switch statement to maintain type safety.
Add this case before the default case:
case ActionType.CALL_WEBHOOK: {
return {
...base,
url: action.url ?? null,
};
}
+ case ActionType.MARK_READ:
+ return base;
default:
// biome-ignore lint/correctness/noSwitchDeclarations: intentional exhaustive check
const exhaustiveCheck: never = action.type;
return exhaustiveCheck;Committable suggestion skipped: line range outside the PR's diff.
|
Thanks! Merged! |
…oviders Root cause of React Error elie222#310 (Rendered more hooks than during the previous render): - GlobalProviders (root layout) wraps children in NuqsAdapter and ComposeModalProvider - AppProviders ((app) layout) was ALSO wrapping children in NuqsAdapter and ComposeModalProvider - This nested duplication caused inconsistent hook counts during hydration/navigation The fix: - Remove NuqsAdapter from AppProviders (already provided by GlobalProviders) - Remove ComposeModalProvider from AppProviders (already provided by GlobalProviders) - Sync ChatProvider with upstream (use useAccount() instead of useParams()) Provider hierarchy is now: Root Layout → GlobalProviders → NuqsAdapter → EmailAccountProvider → SWRProvider → ... └── (app) Layout → AppProviders → ThemeProvider → Jotai → ChatProvider → children This also includes the SSO fix from earlier commits: - handleLinkAccount skips EmailAccount creation for SSO providers - API filters to only valid email providers (Google/Microsoft) - Client-side filtering in EmailAccountProvider
2 participants
Summary by CodeRabbit
Release Notes
New Features
Database Changes
Improvements