Add unsubscribe link to email update #321
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request updates multiple modules related to email and token management. It disables the weekly statistics email by commenting out its send call and modularizes summary email sending with a new function. A new API endpoint is added to handle email unsubscriptions with input validation and concurrent database updates. Additionally, the Prisma schema and migration now support an Changes
Possibly related issues
Possibly related PRs
Poem
Tip 🌐 Web search-backed reviews and chat
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
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: 5
🧹 Nitpick comments (5)
apps/web/app/api/unsubscribe/route.ts (2)
12-14: Enforce consistent error structure for better maintainability.Currently, different errors are returned with somewhat inconsistent JSON structures (e.g.,
{ error: "Invalid token" },{ error: "Token expired" }). Consider centralizing these responses into a consistent format (e.g., returning{ error: { message: "..." } }or another common schema) to improve maintainability and clarity for API consumers.
22-28: Validate error codes for security and user clarity.Sending a 400 status code is correct for invalid/expired tokens. However, you might consider returning different error codes (e.g., 404 for a nonexistent token or 410 for an expired token) if it helps clarify the cause for the client.
apps/web/utils/api-key.ts (1)
4-6: Consider URL-safe tokens for broader usage.While Base64-encoded tokens are acceptable, if this token might be used in query parameters or non-encoded URLs, you might benefit from a URL-safe variant (e.g., replacing special characters like “+” and “/”). Doing so can prevent potential issues when passing tokens around in different parts of the application stack.
apps/web/utils/unsubscribe.ts (1)
5-18: Potential collision handling for token creation.Although unlikely, random token collisions can occur (albeit rarely). If you wish to handle that edge case, you might implement a retry-on-collision approach or ensure the database enforces uniqueness and handle the conflict.
apps/web/app/api/resend/summary/route.ts (1)
181-199: Refactor nested function for better maintainability.Consider moving the
sendEmailfunction outside its parent function to:
- Improve testability
- Make the code more modular
- Reduce complexity
- async function sendEmail(userId: string) { - const token = await createUnsubscribeToken(userId); - - return sendSummaryEmail({ - to: email, - emailProps: { - baseUrl: env.NEXT_PUBLIC_BASE_URL, - coldEmailers, - pendingCount, - needsReplyCount: typeCounts[ThreadTrackerType.NEEDS_REPLY], - awaitingReplyCount: typeCounts[ThreadTrackerType.AWAITING], - needsActionCount: typeCounts[ThreadTrackerType.NEEDS_ACTION], - needsReply: recentNeedsReply, - awaitingReply: recentAwaitingReply, - needsAction: recentNeedsAction, - unsubscribeToken: token, - }, - }); - }Move it to a separate function with explicit parameters:
async function sendSummaryEmailWithUnsubscribe({ userId, email, baseUrl, stats, }: { userId: string; email: string; baseUrl: string; stats: { coldEmailers: any[]; pendingCount: number; typeCounts: Record<ThreadTrackerType, number>; recentNeedsReply: any[]; recentAwaitingReply: any[]; recentNeedsAction: any[]; }; }) { const token = await createUnsubscribeToken(userId); return sendSummaryEmail({ to: email, emailProps: { baseUrl, coldEmailers: stats.coldEmailers, pendingCount: stats.pendingCount, needsReplyCount: stats.typeCounts[ThreadTrackerType.NEEDS_REPLY], awaitingReplyCount: stats.typeCounts[ThreadTrackerType.AWAITING], needsActionCount: stats.typeCounts[ThreadTrackerType.NEEDS_ACTION], needsReply: stats.recentNeedsReply, awaitingReply: stats.recentAwaitingReply, needsAction: stats.recentNeedsAction, unsubscribeToken: token, }, }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/web/app/api/resend/route.ts(1 hunks)apps/web/app/api/resend/summary/route.ts(2 hunks)apps/web/app/api/unsubscribe/route.ts(1 hunks)apps/web/prisma/migrations/20250204162638_email_token/migration.sql(1 hunks)apps/web/prisma/schema.prisma(1 hunks)apps/web/utils/actions/api-key.ts(2 hunks)apps/web/utils/api-key.ts(1 hunks)apps/web/utils/unsubscribe.ts(1 hunks)packages/resend/emails/summary.tsx(6 hunks)packages/resend/src/send.tsx(4 hunks)
🔇 Additional comments (10)
apps/web/app/api/unsubscribe/route.ts (1)
30-44: Assess partial failure risk with Promise.allSettled.Using
Promise.allSettledmeans one operation can fail while the other succeeds, yet the endpoint still returns success. This may create a discrepancy if the user update fails but the token is deleted, removing the chance to retry unsubscribing. Consider usingPromise.allif you’d like both operations to succeed or revert changes upon failure, or handle partial failures more explicitly (such as returning an error to the user).apps/web/utils/unsubscribe.ts (1)
1-1: Confirm the correctness of subpath imports in date-fns.You are importing
addDaysfrom"date-fns/addDays". Though valid in many setups, ensure your bundler/tooling fully supports subpath imports to avoid runtime errors or bundling issues. If not required for tree shaking, consider importing from"date-fns"directly for consistency.apps/web/utils/actions/api-key.ts (1)
14-14: Verify token generation security for different use cases.The
generateSecureTokenfunction is now used for both API keys and unsubscribe tokens. While reusing secure token generation is good practice, ensure that:
- The token length (32 bytes) provides sufficient security for both use cases
- Token storage and validation are appropriately scoped for each use case
Also applies to: 32-32
✅ Verification successful
Token Generation Security Appears Sufficient
- The
generateSecureTokenfunction uses Node.js’scrypto.randomBytes(32)combined with base64 encoding, providing 256 bits of entropy. This level of randomness is generally secure for both API keys and unsubscribe tokens.- Ensure that downstream handling (storage, validation, and scoping) distinguishes between API key usage and unsubscribe token usage to avoid potential mix-ups.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check token generation implementation ast-grep --pattern 'function generateSecureToken() { $$$ }'Length of output: 63
Script:
#!/bin/bash # Search for any reference to generateSecureToken in the repository, # including its declaration, export, or usage. rg -n "generateSecureToken"Length of output: 441
Script:
#!/bin/bash # Display the content of the generateSecureToken function implementation along with its surrounding context. sed -n '1,100p' apps/web/utils/api-key.tsLength of output: 471
packages/resend/src/send.tsx (1)
38-41: LGTM! Proper implementation of one-click unsubscribe headers.The implementation follows Google's bulk sender requirements by including both List-Unsubscribe and List-Unsubscribe-Post headers.
packages/resend/emails/summary.tsx (1)
35-35: LGTM! Good addition of the required unsubscribeToken field.The type safety ensures the token is always provided when using the component.
apps/web/prisma/migrations/20250204162638_email_token/migration.sql (3)
1-11: LGTM! Well-designed table schema with proper constraints.The table design includes all necessary fields and constraints for secure token management:
- Primary key for unique identification
- Creation timestamp with default
- Required fields for token and action
- Expiration timestamp for security
13-14: LGTM! Good addition of unique index on token.The unique constraint on the token field prevents token collisions and enhances security.
16-17: LGTM! Proper foreign key constraint with cascade.The foreign key with cascade delete/update maintains referential integrity with the User table.
apps/web/prisma/schema.prisma (2)
101-110: LGTM! Well-structured EmailToken model.The model includes all necessary fields and relationships:
- Unique token field for security
- Action field with clear documentation
- Proper user relation with cascade delete
- Expiration timestamp for token lifecycle
16-16: Verify the impact of adding unique constraint to userId.The comment suggests this constraint might be removed in the future. Consider if this change could affect existing data or cause issues with multiple accounts per user.
✅ Verification successful
Unique Constraint on Account.userId – Confirmed with Schema Changes
The review change adds a unique constraint to the
userIdfield in the Account model, enforcing a one-to-one relationship between Account and User. The retrieved shell output confirms that the code inapps/web/prisma/schema.prismaincludes the constraint along with a comment noting its experimental nature.
- The unique constraint will prevent a single user from associating with multiple accounts.
- This change has implications only if the current or future business logic requires supporting multiple accounts per user.
- The inline comment already indicates a potential reversion if needed.
Overall, the schema change is consistent, and the impact is as described—ensure that production data conforms to this constraint or update the schema if multiple accounts are needed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if there are any users with multiple accounts that would violate the new constraint rg -A 5 "model Account" | grep -A 10 "userId"Length of output: 266
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/resend/emails/summary.tsx (1)
162-162: Consider using a more realistic token format in preview props.The current preview token "123" is overly simplistic. Consider using a more realistic format to better represent production data.
- unsubscribeToken: "123", + unsubscribeToken: "usr_123e4567-e89b-12d3-a456-426614174000",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/app/api/unsubscribe/route.ts(1 hunks)packages/resend/emails/summary.tsx(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/app/api/unsubscribe/route.ts
🔇 Additional comments (2)
packages/resend/emails/summary.tsx (2)
338-344: LGTM! Well-implemented unsubscribe functionality.The Footer component correctly implements the unsubscribe link with proper URL encoding and clear user messaging.
Also applies to: 359-362
35-35: LGTM! Clean interface extension.The addition of the unsubscribe token to the props interface and its usage in the component is clean and follows the existing patterns.
Also applies to: 49-49
Summary by CodeRabbit
New Features
Refactor
Chores