Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis PR implements an email-based upgrade intent system that tracks upgrade modal clicks, detects upgrade interest patterns, and sends targeted abandoned-checkout and upgrade-intent emails. It includes new email components, sending utilities, scheduled task definitions, and API endpoints for tracking clicks and orchestrating email delivery. Changes
Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@lib/emails/send-abandoned-checkout.ts`:
- Around line 10-21: The code currently casts params.user.email to string and
calls sendEmail without checking for null/undefined — before calling sendEmail
in the AbandonedCheckout flow, validate params.user.email (from
CreateUserEmailProps) is present and a non-empty string (e.g., if (!email)
return or log and bail), and only then call sendEmail with the validated email;
ensure AbandonedCheckoutEmail still receives the derived firstName and handle
the missing-email case by early return or appropriate error/logging so sendEmail
is never called with a nullish address.
In `@lib/emails/send-upgrade-intent.ts`:
- Around line 5-11: The SendUpgradeIntentEmailProps interface allows user.email
to be null/undefined but the code later casts it to string; update the code that
sends the email (the function handling SendUpgradeIntentEmailProps — e.g.,
sendUpgradeIntentEmail or whatever uses SendUpgradeIntentEmailProps) to validate
user.email is present and a non-empty string before using it (return/throw or
skip sending if missing), and update the type to reuse or extend
CreateUserEmailProps (replace the inline user type with "extends
CreateUserEmailProps" and keep optional triggers) so callers must provide a
validated email; ensure any places that currently cast user.email to string are
changed to use the validated value.
In `@pages/api/teams/`[teamId]/billing/track-upgrade-click.ts:
- Around line 110-136: The current get → set race around emailScheduledKey can
cause duplicate scheduled emails; replace the read-then-write with an atomic SET
NX call on emailScheduledKey (use redis.set(emailScheduledKey, "1", { ex:
THREE_DAYS_IN_SECONDS, nx: true }) which returns "OK" when the key was set or
null if it already exists) and only proceed to read upgradeTriggersKey, call
sendUpgradeIntentEmailTask.trigger and waitUntil when the SET returned "OK";
keep REQUIRED_CLICKS, upgradeTriggersKey, sendUpgradeIntentEmailTask.trigger,
waitUntil and THREE_DAYS_IN_SECONDS as the referenced symbols.
In `@pages/api/teams/`[teamId]/billing/upgrade.ts:
- Around line 186-197: Guard against null/undefined session.user fields before
scheduling the task: before calling
waitUntil/sendAbandonedCheckoutEmailTask.trigger, read session.user.email and
session.user.name into userEmail/userName, validate they are non-empty strings
(not null/undefined/"null"/"undefined"), and only call
sendAbandonedCheckoutEmailTask.trigger when both are valid; otherwise skip
scheduling (or log/return an error) so the task never receives invalid string
values. Use the existing symbols sendAbandonedCheckoutEmailTask.trigger,
waitUntil, session.user, userEmail and userName to locate and update the code.
🧹 Nitpick comments (1)
lib/trigger/send-scheduled-email.ts (1)
196-202: Consider extracting duplicatepaidPlansarray to a shared constant.The
paidPlansarray is duplicated insendAbandonedCheckoutEmailTask(lines 196-202),sendUpgradeIntentEmailTask(lines 247-253), and also appears in existing tasks likesendDataroomTrialExpiredEmailTaskandsendUpgradeOneMonthCheckinEmailTask.♻️ Proposed refactor to extract shared constant
Add at the top of the file:
const PAID_PLANS = [ "pro", "business", "datarooms", "datarooms-plus", "datarooms-premium", ] as const;Then use it in each task:
- const paidPlans = [ - "pro", - "business", - "datarooms", - "datarooms-plus", - "datarooms-premium", - ]; - const isPaidPlan = paidPlans.some((plan) => team.plan.includes(plan)); + const isPaidPlan = PAID_PLANS.some((plan) => team.plan.includes(plan));Also applies to: 247-253
| const { name, email } = params.user; | ||
|
|
||
| // Get the first name from the full name | ||
| const firstName = name ? name.split(" ")[0] : null; | ||
|
|
||
| const emailTemplate = AbandonedCheckoutEmail({ | ||
| name: firstName, | ||
| }); | ||
|
|
||
| try { | ||
| await sendEmail({ | ||
| to: email as string, |
There was a problem hiding this comment.
Add validation for null/undefined email before sending.
The email field can be null | undefined per CreateUserEmailProps, but it's cast directly to string on line 21. If email is nullish, this will either throw an error in sendEmail or attempt to send to an invalid address.
🛡️ Proposed fix to add email validation
export const sendAbandonedCheckoutEmail = async (
params: CreateUserEmailProps,
) => {
const { name, email } = params.user;
+ if (!email) {
+ console.error("Cannot send abandoned checkout email: no email address");
+ return;
+ }
+
// Get the first name from the full name
const firstName = name ? name.split(" ")[0] : null;
const emailTemplate = AbandonedCheckoutEmail({
name: firstName,
});
try {
await sendEmail({
- to: email as string,
+ to: email,
subject: "Did something block checkout?",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { name, email } = params.user; | |
| // Get the first name from the full name | |
| const firstName = name ? name.split(" ")[0] : null; | |
| const emailTemplate = AbandonedCheckoutEmail({ | |
| name: firstName, | |
| }); | |
| try { | |
| await sendEmail({ | |
| to: email as string, | |
| const { name, email } = params.user; | |
| if (!email) { | |
| console.error("Cannot send abandoned checkout email: no email address"); | |
| return; | |
| } | |
| // Get the first name from the full name | |
| const firstName = name ? name.split(" ")[0] : null; | |
| const emailTemplate = AbandonedCheckoutEmail({ | |
| name: firstName, | |
| }); | |
| try { | |
| await sendEmail({ | |
| to: email, |
🤖 Prompt for AI Agents
In `@lib/emails/send-abandoned-checkout.ts` around lines 10 - 21, The code
currently casts params.user.email to string and calls sendEmail without checking
for null/undefined — before calling sendEmail in the AbandonedCheckout flow,
validate params.user.email (from CreateUserEmailProps) is present and a
non-empty string (e.g., if (!email) return or log and bail), and only then call
sendEmail with the validated email; ensure AbandonedCheckoutEmail still receives
the derived firstName and handle the missing-email case by early return or
appropriate error/logging so sendEmail is never called with a nullish address.
| interface SendUpgradeIntentEmailProps { | ||
| user: { | ||
| name: string | null | undefined; | ||
| email: string | null | undefined; | ||
| }; | ||
| triggers?: string[]; | ||
| } |
There was a problem hiding this comment.
Add validation for null/undefined email before sending.
Same issue as send-abandoned-checkout.ts: the email field can be null | undefined, but it's cast to string without validation.
🛡️ Proposed fix to add email validation
export const sendUpgradeIntentEmail = async (
params: SendUpgradeIntentEmailProps,
) => {
const { name, email } = params.user;
const triggers = params.triggers || [];
+ if (!email) {
+ console.error("Cannot send upgrade intent email: no email address");
+ return;
+ }
+
// Get the first name from the full name
const firstName = name ? name.split(" ")[0] : null;Optional: Consider reusing CreateUserEmailProps from lib/types.ts and extending it with triggers:
import { CreateUserEmailProps } from "../types";
export interface SendUpgradeIntentEmailProps extends CreateUserEmailProps {
triggers?: string[];
}Also applies to: 16-29
🤖 Prompt for AI Agents
In `@lib/emails/send-upgrade-intent.ts` around lines 5 - 11, The
SendUpgradeIntentEmailProps interface allows user.email to be null/undefined but
the code later casts it to string; update the code that sends the email (the
function handling SendUpgradeIntentEmailProps — e.g., sendUpgradeIntentEmail or
whatever uses SendUpgradeIntentEmailProps) to validate user.email is present and
a non-empty string before using it (return/throw or skip sending if missing),
and update the type to reuse or extend CreateUserEmailProps (replace the inline
user type with "extends CreateUserEmailProps" and keep optional triggers) so
callers must provide a validated email; ensure any places that currently cast
user.email to string are changed to use the validated value.
| // If this is the 3rd click (or more) within 3 days, schedule the email | ||
| if (totalRecentClicks >= REQUIRED_CLICKS) { | ||
| // Check if we already scheduled an email for this session | ||
| const emailScheduledKey = `upgrade:email-scheduled:${teamId}`; | ||
| const alreadyScheduled = await redis.get(emailScheduledKey); | ||
|
|
||
| if (!alreadyScheduled) { | ||
| // Get all triggers the user clicked on | ||
| const triggers = (await redis.smembers(upgradeTriggersKey)) as string[]; | ||
|
|
||
| // Mark as scheduled | ||
| await redis.set(emailScheduledKey, "1", { ex: THREE_DAYS_IN_SECONDS }); | ||
|
|
||
| // Schedule email for 1 day after the last click | ||
| waitUntil( | ||
| sendUpgradeIntentEmailTask.trigger( | ||
| { | ||
| to: userEmail as string, | ||
| name: userName as string, | ||
| teamId, | ||
| triggers, | ||
| }, | ||
| { | ||
| delay: "1d", | ||
| }, | ||
| ), | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "redis" --path -e ts -e js -e jsonRepository: mfts/papermark
Length of output: 291
🏁 Script executed:
find . -type f -name "*redis*" | head -20Repository: mfts/papermark
Length of output: 129
🏁 Script executed:
rg -l "redis" --type ts --type js | grep -E "(lib|config)" | head -10Repository: mfts/papermark
Length of output: 337
🏁 Script executed:
cat -n lib/redis.tsRepository: mfts/papermark
Length of output: 1093
🏁 Script executed:
wc -l pages/api/teams/[teamId]/billing/track-upgrade-click.tsRepository: mfts/papermark
Length of output: 117
🏁 Script executed:
cat -n pages/api/teams/[teamId]/billing/track-upgrade-click.tsRepository: mfts/papermark
Length of output: 5615
🌐 Web query:
@upstash/redis SET nx option atomic support
💡 Result:
Yes—@upstash/redis supports Redis SET with the NX option (and XX, GET, EX, PX, EXAT, PXAT, keepTtl) via the opts object, e.g.:
await redis.set("lock:key", "token", { nx: true, ex: 30 });This maps to Redis SET key value [EX|PX|...] [NX|XX] options, where NX is part of the atomic SET command itself. In Redis, SET ... NX (optionally with EX/PX) is a single command and is atomic. [1][2]
If you’re combining multiple commands (e.g., SETNX then EXPIRE as two calls), that’s not atomic unless you use a transaction (redis.multi() ... exec()). Pipelines are explicitly not atomic in Upstash. [3][4]
Sources: Upstash SET command docs (options incl. nx) [1], Redis SET command reference [2], Upstash pipeline/transaction atomicity docs [3], Upstash REST API pipeline atomicity note [4].
🌐 Web query:
@upstash/redis SET nx return value true false null
💡 Result:
In @upstash/redis (TypeScript), redis.set() does not return true/false for nx.
await redis.set(key, value, { nx: true })returns:"OK"if the key was setnullif the key already existed (so nothing was written) — because Redis returns a nil reply forSET ... NXwhen it doesn’t set, and Upstash’s REST API represents nil as JSONnull. [1][2]
Also:
{ get: true }changes the return value to the previous value (string) ornullif it didn’t exist. [1]- Redis does not support combining
NXandGETonSET(it’s a known limitation discussed upstream), so expect that combination to fail. [3]
Example
const r1 = await redis.set("k", "v", { nx: true }); // "OK" | null
const wasSet = r1 === "OK";Sources: [1] Upstash TS SET docs, [2] Upstash REST API response format, [3] Redis issue about SET + NX + GET.
Prevent duplicate upgrade-intent emails under concurrent clicks.
The get → set sequence on upgrade:email-scheduled:* is racy. Between the get check (line 114) and set (line 121), concurrent requests can both pass the check and schedule duplicate emails. Use atomic SET ... NX, which returns "OK" if set or null if the key already exists—@upstash/redis supports this.
🛠️ Proposed fix (atomic scheduling)
- const emailScheduledKey = `upgrade:email-scheduled:${teamId}`;
- const alreadyScheduled = await redis.get(emailScheduledKey);
-
- if (!alreadyScheduled) {
- // Get all triggers the user clicked on
- const triggers = (await redis.smembers(upgradeTriggersKey)) as string[];
-
- // Mark as scheduled
- await redis.set(emailScheduledKey, "1", { ex: THREE_DAYS_IN_SECONDS });
-
- // Schedule email for 1 day after the last click
- waitUntil(
- sendUpgradeIntentEmailTask.trigger(
- {
- to: userEmail as string,
- name: userName as string,
- teamId,
- triggers,
- },
- {
- delay: "1d",
- },
- ),
- );
-
- return res.status(200).json({
- tracked: true,
- clicks: totalRecentClicks,
- triggers,
- emailScheduled: true,
- });
- }
+ const emailScheduledKey = `upgrade:email-scheduled:${teamId}`;
+ const scheduled = await redis.set(emailScheduledKey, "1", {
+ nx: true,
+ ex: THREE_DAYS_IN_SECONDS,
+ });
+
+ if (scheduled) {
+ const triggers = (await redis.smembers(upgradeTriggersKey)) as string[];
+
+ waitUntil(
+ sendUpgradeIntentEmailTask.trigger(
+ {
+ to: userEmail as string,
+ name: userName as string,
+ teamId,
+ triggers,
+ },
+ { delay: "1d" },
+ ),
+ );
+
+ return res.status(200).json({
+ tracked: true,
+ clicks: totalRecentClicks,
+ triggers,
+ emailScheduled: true,
+ });
+ }🤖 Prompt for AI Agents
In `@pages/api/teams/`[teamId]/billing/track-upgrade-click.ts around lines 110 -
136, The current get → set race around emailScheduledKey can cause duplicate
scheduled emails; replace the read-then-write with an atomic SET NX call on
emailScheduledKey (use redis.set(emailScheduledKey, "1", { ex:
THREE_DAYS_IN_SECONDS, nx: true }) which returns "OK" when the key was set or
null if it already exists) and only proceed to read upgradeTriggersKey, call
sendUpgradeIntentEmailTask.trigger and waitUntil when the SET returned "OK";
keep REQUIRED_CLICKS, upgradeTriggersKey, sendUpgradeIntentEmailTask.trigger,
waitUntil and THREE_DAYS_IN_SECONDS as the referenced symbols.
| waitUntil( | ||
| sendAbandonedCheckoutEmailTask.trigger( | ||
| { | ||
| to: userEmail as string, | ||
| name: userName as string, | ||
| teamId, | ||
| }, | ||
| { | ||
| delay: "1h", | ||
| }, | ||
| ), | ||
| ); |
There was a problem hiding this comment.
Guard against null userEmail or userName before scheduling the task.
session.user fields (email, name) may be null or undefined depending on the authentication provider. Casting them directly to string could result in the string "null" or "undefined" being passed to the task.
🛡️ Proposed fix to add null checks
// Schedule abandoned checkout email to be sent in 1 hour
// The task will check if the team has already upgraded before sending
+ if (userEmail) {
waitUntil(
sendAbandonedCheckoutEmailTask.trigger(
{
- to: userEmail as string,
- name: userName as string,
+ to: userEmail,
+ name: userName ?? "",
teamId,
},
{
delay: "1h",
},
),
);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| waitUntil( | |
| sendAbandonedCheckoutEmailTask.trigger( | |
| { | |
| to: userEmail as string, | |
| name: userName as string, | |
| teamId, | |
| }, | |
| { | |
| delay: "1h", | |
| }, | |
| ), | |
| ); | |
| if (userEmail) { | |
| waitUntil( | |
| sendAbandonedCheckoutEmailTask.trigger( | |
| { | |
| to: userEmail, | |
| name: userName ?? "", | |
| teamId, | |
| }, | |
| { | |
| delay: "1h", | |
| }, | |
| ), | |
| ); | |
| } |
🤖 Prompt for AI Agents
In `@pages/api/teams/`[teamId]/billing/upgrade.ts around lines 186 - 197, Guard
against null/undefined session.user fields before scheduling the task: before
calling waitUntil/sendAbandonedCheckoutEmailTask.trigger, read
session.user.email and session.user.name into userEmail/userName, validate they
are non-empty strings (not null/undefined/"null"/"undefined"), and only call
sendAbandonedCheckoutEmailTask.trigger when both are valid; otherwise skip
scheduling (or log/return an error) so the task never receives invalid string
values. Use the existing symbols sendAbandonedCheckoutEmailTask.trigger,
waitUntil, session.user, userEmail and userName to locate and update the code.
1 participant
Here two emails, one after 1h when person was already in checkout, and other after 3 times opened the
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.