Open
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a paid-plan 14-day trial system: trial-aware Stripe handling, workspace billing fields, trial-limit constants, trial marketing emails sent via a QStash-backed cron with pagination and dedupe, UI/UX for activating paid plans during trials, partner enrollment limits, and tests + backfill scripts. Changes
Sequence DiagramsequenceDiagram
participant Client as Client/User
participant App as App Server
participant Stripe as Stripe
participant DB as Database
participant QStash as QStash
participant Mail as Email Service
Client->>App: POST /billing/upgrade (creates checkout)
App->>Stripe: Create Checkout (maybe trial_period_days)
Stripe-->>App: Checkout session
App->>DB: Persist trial experiment metadata
Client->>Stripe: Complete Checkout
Stripe->>App: webhook checkout.session.completed
App->>Stripe: Fetch subscription
App->>DB: Update workspace (trialEndsAt, planPeriod, limits)
Note over App,DB: hourly cron triggers
App->>DB: Find projects with active trial (cursor/pagination)
DB-->>App: Eligible workspaces
loop per workspace
App->>App: determine due email types
App->>Mail: sendBatchEmail (batched, idempotent)
Mail-->>App: success
App->>DB: create sentEmail record
end
alt more pages
App->>QStash: publishJSON (POST /api/cron/trial-emails with startingAfter)
QStash-->>App: queued
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
|
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 16
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/ui/modals/upgraded-modal.tsx (1)
76-81:⚠️ Potential issue | 🟠 MajorDon’t persist
dotLinkOfferDismissedon every close.When
showDotLinkClaimUiis false, closing this modal still writes the dismissal flag. The links page only shows the deferred.linkoffer when that flag is unset, so trial workspaces can lose the claim flow permanently before they ever become eligible. Gate this write onshowDotLinkClaimUi, or store a separate “upgraded modal seen” flag instead.Suggested fix
const onClose = async () => { queryParams({ del: ["upgraded", "plan", "period"], }); - await setDotLinkOfferDismissed(new Date().toISOString()); - mutateWorkspace(); + if (showDotLinkClaimUi) { + await setDotLinkOfferDismissed(new Date().toISOString()); + mutateWorkspace(); + } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/ui/modals/upgraded-modal.tsx` around lines 76 - 81, The onClose handler currently always calls setDotLinkOfferDismissed and persists the dismissal; change it so the dismissal is only written when showDotLinkClaimUi is true (or alternatively write a separate "upgradedModalSeen" flag instead). In practice, update the onClose function to check the boolean showDotLinkClaimUi before calling setDotLinkOfferDismissed (keep the existing queryParams(...) and mutateWorkspace() behavior unchanged), or replace the call to setDotLinkOfferDismissed with a different setter like setUpgradedModalSeen when you want a non-dot-link-specific seen flag.
🟡 Minor comments (6)
apps/web/ui/modals/plan-change-confirmation-modal.tsx-47-49 (1)
47-49:⚠️ Potential issue | 🟡 MinorEdge case:
displayNamecould be empty string.When both
program?.nameandworkspaceNameare nullish,displayNamebecomes"", resulting in a logo URL like${OG_AVATAR_URL}without a name suffix. Verify thatOG_AVATAR_URLhandles this gracefully, or add a fallback.🛡️ Potential defensive fix
const displayName = program?.name ?? workspaceName ?? ""; const logoSrc = - program?.logo ?? workspaceLogo ?? `${OG_AVATAR_URL}${displayName}`; + program?.logo ?? workspaceLogo ?? (displayName ? `${OG_AVATAR_URL}${displayName}` : `${OG_AVATAR_URL}default`);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/ui/modals/plan-change-confirmation-modal.tsx` around lines 47 - 49, displayName can be empty when both program?.name and workspaceName are nullish, causing logoSrc to be just OG_AVATAR_URL; update the logic that computes displayName/logoSrc (references: displayName, logoSrc, OG_AVATAR_URL, program?.name, workspaceName, workspaceLogo) to provide a safe fallback string (e.g. "default" or "unknown") when displayName is empty or whitespace and/or use a true default image key instead of interpolating an empty name; ensure the fallback is used only when workspaceLogo and program?.logo are absent so logoSrc always yields a valid URL or a default avatar.apps/web/ui/modals/bulk-approve-partners-modal.tsx-46-56 (1)
46-56:⚠️ Potential issue | 🟡 MinorEmpty toast notification when
serverErroris missing or empty.If
error.serverErrorisundefinedor an empty string,toast.error("")will display a blank/empty toast notification, which provides no useful feedback to the user.🛠️ Proposed fix: Add fallback message
onError({ error }) { - const msg = error.serverError ?? ""; - toast.error(msg); + const msg = error.serverError || "Failed to approve partners."; if ( trialActive && - (String(msg).includes("free trial") || - String(msg).includes("enrolled partners")) + (msg.includes("free trial") || msg.includes("enrolled partners")) ) { openTrialLimitModal("partnerEnrollments"); + } else { + toast.error(msg); } },Note: This also avoids showing a toast when the trial modal is opened, preventing duplicate feedback.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/ui/modals/bulk-approve-partners-modal.tsx` around lines 46 - 56, The onError handler currently calls toast.error(msg) where msg = error.serverError ?? "" which can render an empty toast; change the logic in the onError callback to compute a fallback message (e.g., a generic "An error occurred" or error.message) when error.serverError is missing/empty, and only call toast.error with that fallback; additionally, if trialActive and the message triggers the trial condition (String(msg).includes("free trial") || String(msg).includes("enrolled partners")), call openTrialLimitModal("partnerEnrollments") and avoid showing the toast (prevent duplicate feedback) so the trial modal is shown without an empty or redundant toast.apps/web/playwright/workspaces/billing-mocks.ts-27-47 (1)
27-47:⚠️ Potential issue | 🟡 MinorReset cancellation metadata in the mock trial helper.
applyMockTrialToWorkspacenow simulates a freshtrialingsubscription, but it leavessubscriptionCanceledAtandbillingCycleEndsAtuntouched. If a seeded workspace ever carries stale cancellation state, the post-checkout UI will still look canceled even though this helper just activated a trial.Proposed fix
data: { plan: "pro", planTier: 1, planPeriod: "monthly", trialEndsAt, + subscriptionCanceledAt: null, + billingCycleEndsAt: null, billingCycleStart: new Date().getDate(), stripeId: `cus_e2e_mock_${slug}`, usageLimit: limits.clicks,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/playwright/workspaces/billing-mocks.ts` around lines 27 - 47, applyMockTrialToWorkspace currently updates plan fields via prisma.project.update but fails to reset cancellation metadata, so stale subscriptionCanceledAt and billingCycleEndsAt can make the UI show a canceled state; update the data payload in applyMockTrialToWorkspace (the prisma.project.update call) to explicitly set subscriptionCanceledAt to null and billingCycleEndsAt to null (or appropriate fresh values) when applying the mock trial so the workspace is treated as an active, non-cancelled trial.packages/utils/src/constants/pricing/trial-limits.ts-39-39 (1)
39-39:⚠️ Potential issue | 🟡 MinorTypo: "fee" should be "free".
The feature phrase reads "claim a fee .link domain" but should be "claim a free .link domain".
✏️ Fix typo
- dublink: "claim a fee .link domain", + dublink: "claim a free .link domain",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/utils/src/constants/pricing/trial-limits.ts` at line 39, Fix the typo in the trial-limits constant by changing the string value for the dublink entry from "claim a fee .link domain" to "claim a free .link domain"; locate the dublink constant in packages/utils/src/constants/pricing/trial-limits.ts and update only the text inside the quotes to correct "fee" → "free".packages/utils/src/constants/pricing/trial-limits.ts-46-59 (1)
46-59:⚠️ Potential issue | 🟡 MinorDefault fallback to "payouts" may be misleading when no flags are exceeded.
When all three boolean flags are
false, the function returns"payouts"as a default. This could display incorrect messaging to users who haven't actually exceeded any limit. Consider either:
- Adding a more explicit "general" resource type, or
- Throwing an error / returning
nullto signal invalid state to callers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/utils/src/constants/pricing/trial-limits.ts` around lines 46 - 59, getTrialLimitResourceForOverageBanner currently falls back to "payouts" when none of exceededEvents, exceededLinks, or exceededPayouts are true, which can mislead callers; change the function to return a clear invalid state instead of "payouts" — for example, update the return type of getTrialLimitResourceForOverageBanner/TrialLimitResource to allow either a distinct "general" resource value or null and then return that when no flags are set (or alternatively throw an Error), and update any callers to handle the new "general"/null/Error case accordingly.apps/web/ui/modals/trial-limit-activate-modal.tsx-50-79 (1)
50-79:⚠️ Potential issue | 🟡 MinorNetwork errors are caught but swallowed silently.
If
fetchthrows (e.g., network failure), the error is not handled and propagates, leavingisSubmittingstuck attruesincefinallywon't run if the promise rejects before reaching it. Actually, looking again,finallywill run, but the user won't see any feedback for network errors.Consider wrapping the entire try block to catch network-level errors and display a toast:
🛡️ Proposed fix
const handleConfirm = async () => { if (isSubmitting || !workspaceId) return; setIsSubmitting(true); try { const res = await fetch( `/api/workspaces/${workspaceId}/billing/end-trial`, { method: "POST" }, ); if (res.ok) { await mutate(); queryParams({ set: { upgraded: "true", plan: plan ?? "pro", }, replace: true, scroll: false, }); setShowModal(false); } else { const body = await res.json().catch(() => null); const message = body?.error?.message ?? "Could not end trial. Try again or use the billing portal."; toast.error(message); } - } finally { + } catch (error) { + toast.error("Network error. Please check your connection and try again."); + } finally { setIsSubmitting(false); } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/ui/modals/trial-limit-activate-modal.tsx` around lines 50 - 79, The handleConfirm function currently has a try/finally but no catch, so network-level fetch errors are not surfaced to the user; update handleConfirm to add a catch block around the async work (or convert to try/catch/finally) that catches fetch/network errors and shows a user-facing toast (use toast.error) with a helpful message, while preserving existing behavior: call setIsSubmitting(true) at start, call mutate(), queryParams(...), and setShowModal(false) on success, and always call setIsSubmitting(false) in the finally; reference handleConfirm, fetch(`/api/workspaces/${workspaceId}/billing/end-trial`), mutate, queryParams, setShowModal, and toast.error when making the change.
🧹 Nitpick comments (9)
apps/web/tests/plans/has-advanced-features.test.ts (1)
45-71: Consider adding a case-sensitivity test forleftAdvancedPlan.Since the implementation uses
.toLowerCase(), a test verifying case-insensitive behavior (e.g.,"ADVANCED"→"business") would document and protect this intended behavior.🧪 Optional test case
it("is false when staying on Advanced", () => { expect( leftAdvancedPlan({ currentPlan: "advanced", newPlan: "advanced", }), ).toBe(false); }); + + it("handles case-insensitive plan names", () => { + expect( + leftAdvancedPlan({ + currentPlan: "ADVANCED", + newPlan: "Business", + }), + ).toBe(true); + }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/tests/plans/has-advanced-features.test.ts` around lines 45 - 71, Add a test that verifies leftAdvancedPlan is case-insensitive by asserting it returns true when currentPlan is "ADVANCED" (or other mixed-case) and newPlan is "business"; update the tests around leftAdvancedPlan to include a new it block that calls leftAdvancedPlan({ currentPlan: "ADVANCED", newPlan: "business" }) and expects true so the .toLowerCase() behavior is covered and protected.apps/web/lib/actions/partners/accept-program-invite.ts (1)
24-25: Trim unused field fromprogramselect.
workspaceIdis fetched but not used in this action; removing it slightly reduces query payload noise.♻️ Suggested cleanup
select: { - workspaceId: true, workspace: { select: { trialEndsAt: true }, }, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/actions/partners/accept-program-invite.ts` around lines 24 - 25, The program select currently includes workspaceId but that field is unused in the accept-program-invite action; remove the workspaceId: true entry from the program select (located where the program is queried inside the acceptProgramInvite action) so the DB query omits that unused scalar and reduces payload; update any local references if present (e.g., variables named program or program.workspaceId) to avoid dead code.apps/web/lib/email/render-trial-email.tsx (1)
23-37: Render the email templates as React elements, not plain function calls.Calling
TrialStartedEmail(props)works only while these exports stay hook-free plain functions. Returning<TrialStartedEmail {...props} />(orcreateElement) preserves React component semantics if one of these templates later starts using hooks or context.Possible shape
- case TRIAL_EMAIL_TYPE.STARTED: - return TrialStartedEmail(props); + case TRIAL_EMAIL_TYPE.STARTED: + return <TrialStartedEmail {...props} />;Apply the same pattern to the other branches.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/email/render-trial-email.tsx` around lines 23 - 37, The switch in render-trial-email.tsx currently invokes templates as plain functions (e.g., TrialStartedEmail(props), TrialLinksFocusEmail(props), TrialPartnerFocusEmail(props), TrialSocialProofEmail(props), Trial7DaysRemainingEmail(props), Trial3DaysRemainingEmail(props), TrialEndsTodayEmail(props)); change each branch to return a React element instead (e.g., <TrialStartedEmail {...props} /> or React.createElement(TrialStartedEmail, props)) so the templates keep correct React semantics if they later use hooks or context.apps/web/lib/billing/trial-checkout-experiment.ts (1)
6-12: Mix the hash before taking a 2-way bucket.
Math.abs(h) % 2only inspects the low bit of this rolling hash. With a multiplier of31, that bit collapses to simple character-code parity, so the control/treatment split is much weaker than it looks. Add a final mixing step, or bucket off a digest byte, before mapping to the AB variant.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/billing/trial-checkout-experiment.ts` around lines 6 - 12, The bucket assignment in hashWorkspaceIdToBucket only uses Math.abs(h) % 2 which effectively examines the low bit of the rolling hash and can correlate with input parity; fix by applying a final mixing/avalanche to h before taking the bucket bit (for example, perform a couple of bitwise xors and multiplicative mixes and then extract a single bit from the mixed unsigned value) so the mapping is not just the low-bit of the 31-multiplier rolling hash; update hashWorkspaceIdToBucket to mix h (e.g., xor-shift and multiply) and then return the least significant bit of the mixed unsigned integer as 0 or 1.apps/web/tests/email/run-trial-email-cron.test.ts (1)
46-114: Add a failure-path test forsendBatchEmail.The suite covers the happy path, dedupe, and pagination well, but it never asserts what happens when
sendBatchEmailthrows or returns anerror. That’s the branch that decides whether aSentEmailrow gets written, so a regression there would silently suppress retries.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/tests/email/run-trial-email-cron.test.ts` around lines 46 - 114, Add a failing-path test that mocks sendBatchEmail to throw (e.g., vi.fn().mockRejectedValue(new Error("send failed"))) when calling runTrialEmailCron; assert sendBatchEmail was called but the sentEmail.create mock (the create passed into prisma.sentEmail) was not called, and assert that runTrialEmailCron propagates the failure by using await expect(runTrialEmailCron(...)).rejects.toThrow("send failed"). Use the same test scaffolding as the existing tests (mocks for project.findMany and sentEmail.create) and reference runTrialEmailCron, sendBatchEmail, and the sentEmail.create mock in the new test.apps/web/lib/email/trial-email-schedule.ts (2)
78-99: Repetitive pattern could be simplified with a loop.The four sequential if-statements checking
daysSinceStartagainst each milestone could be consolidated into a loop over theTRIAL_EMAIL_DAYS_FROM_STARTentries.♻️ Optional simplification
- if ( - daysSinceStart === TRIAL_EMAIL_DAYS_FROM_START[TRIAL_EMAIL_TYPE.STARTED] - ) { - tryAdd(TRIAL_EMAIL_TYPE.STARTED); - } - if ( - daysSinceStart === TRIAL_EMAIL_DAYS_FROM_START[TRIAL_EMAIL_TYPE.LINKS_FOCUS] - ) { - tryAdd(TRIAL_EMAIL_TYPE.LINKS_FOCUS); - } - if ( - daysSinceStart === - TRIAL_EMAIL_DAYS_FROM_START[TRIAL_EMAIL_TYPE.PARTNER_FOCUS] - ) { - tryAdd(TRIAL_EMAIL_TYPE.PARTNER_FOCUS); - } - if ( - daysSinceStart === - TRIAL_EMAIL_DAYS_FROM_START[TRIAL_EMAIL_TYPE.SOCIAL_PROOF] - ) { - tryAdd(TRIAL_EMAIL_TYPE.SOCIAL_PROOF); - } + for (const [type, day] of Object.entries(TRIAL_EMAIL_DAYS_FROM_START)) { + if (daysSinceStart === day) { + tryAdd(type as TrialEmailType); + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/email/trial-email-schedule.ts` around lines 78 - 99, The four repetitive if blocks comparing daysSinceStart to TRIAL_EMAIL_DAYS_FROM_START for each TRIAL_EMAIL_TYPE should be replaced by iterating the TRIAL_EMAIL_TYPE/entries of TRIAL_EMAIL_DAYS_FROM_START and calling tryAdd for any type whose configured day equals daysSinceStart; locate the checks using the symbols daysSinceStart, TRIAL_EMAIL_DAYS_FROM_START, TRIAL_EMAIL_TYPE and tryAdd, loop over the map or enum keys and perform a single compare-and-tryAdd inside the loop to remove duplication.
56-112: Consider potential overlap between "days from start" and "days until end" emails.For a 14-day trial, day 7 from start equals day 7 until end (since 14 - 7 = 7). This means
SOCIAL_PROOF(day 6) andSEVEN_DAYS_REMAINING(7 days left) could potentially trigger on adjacent days, which seems intentional.However, if
PARTNER_CHECKOUT_TRIAL_PERIOD_DAYSchanges, the milestone emails (days 0, 2, 4, 6 from start) might overlap with the countdown emails (7, 3, 0 days remaining). The current 14-day period avoids this, but it's worth documenting this coupling.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/email/trial-email-schedule.ts` around lines 56 - 112, Add an inline comment above the logic in getDueTrialEmailTypes documenting the coupling between TRIAL_EMAIL_DAYS_FROM_START and the countdown checks (daysUntilEnd) and that overlaps can occur when a value in TRIAL_EMAIL_DAYS_FROM_START equals PARTNER_CHECKOUT_TRIAL_PERIOD_DAYS minus a countdown day; then add a runtime sanity check (throw or processLogger.warn/assert) that iterates TRIAL_EMAIL_DAYS_FROM_START values and verifies none equal PARTNER_CHECKOUT_TRIAL_PERIOD_DAYS - X for X in [7,3,0] (or adjust list of countdown days) so maintainers are alerted if PARTNER_CHECKOUT_TRIAL_PERIOD_DAYS changes and creates unintended overlaps; reference symbols: getDueTrialEmailTypes, TRIAL_EMAIL_DAYS_FROM_START, TRIAL_EMAIL_TYPE, PARTNER_CHECKOUT_TRIAL_PERIOD_DAYS.apps/web/lib/email/run-trial-email-cron.ts (1)
137-137: Hardcoded email address should be extracted to a constant or configuration.The
replyToemail is hardcoded directly in the code. Consider extracting this to a shared constant or environment variable for maintainability and easier updates.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/email/run-trial-email-cron.ts` at line 137, The replyTo address is hardcoded; extract it to a configuration constant or environment variable (e.g., TRIAL_EMAIL_REPLY_TO) and update the place where replyTo is set (the replyTo assignment in apps/web/lib/email/run-trial-email-cron.ts) to use that constant with a sensible fallback; ensure the constant is exported/imported from a shared config module or read via process.env and update any tests or callers to reference the new config key.apps/web/ui/modals/trial-limit-activate-modal.tsx (1)
149-157: Creating a new component function on every render may cause unnecessary remounts.Wrapping
TrialLimitActivateModalInnerinuseCallbackstill creates a new function component identity whenshowModalorlimitResourcechanges. This can cause React to treat it as a different component, unmounting and remounting the modal's internal state.Consider either:
- Returning the JSX element directly instead of a component function, or
- Keeping
TrialLimitActivateModalas a stable reference and passing state as props from the parent.♻️ Suggested refactor returning JSX element
- const TrialLimitActivateModal = useCallback(() => { - return ( - <TrialLimitActivateModalInner - showModal={showModal} - setShowModal={setShowModal} - limitResource={limitResource} - /> - ); - }, [showModal, limitResource]); + const TrialLimitActivateModal = useMemo( + () => ( + <TrialLimitActivateModalInner + showModal={showModal} + setShowModal={setShowModal} + limitResource={limitResource} + /> + ), + [showModal, limitResource], + ); return useMemo( () => ({ openTrialLimitModal, setShowTrialLimitActivateModal, TrialLimitActivateModal, }), - [ - openTrialLimitModal, - setShowTrialLimitActivateModal, - TrialLimitActivateModal, - ], + [openTrialLimitModal, setShowTrialLimitActivateModal, TrialLimitActivateModal], );Then render as
{TrialLimitActivateModal}instead of<TrialLimitActivateModal />.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/ui/modals/trial-limit-activate-modal.tsx` around lines 149 - 157, The current useCallback wrapper creates a new component identity for TrialLimitActivateModal when showModal or limitResource change, causing remounts of TrialLimitActivateModalInner; fix this by returning the JSX element directly instead of a function component (i.e., make TrialLimitActivateModal a memoized JSX value or plain constant that evaluates to <TrialLimitActivateModalInner showModal={showModal} setShowModal={setShowModal} limitResource={limitResource} />) and update the parent render to use {TrialLimitActivateModal} (not <TrialLimitActivateModal />), or alternatively keep TrialLimitActivateModal as a stable reference (use useRef or move the function outside the render) and pass showModal/setShowModal/limitResource as props so the component identity of TrialLimitActivateModalInner does not change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/app/`(ee)/api/cron/trial-emails/route.ts:
- Around line 65-86: The POST handler in route.ts currently calls req.text()
directly and uses handleAndReturnErrorResponse, bypassing the standardized cron
wrapper; change the export async function POST to run inside the withCron
wrapper (use withCron to obtain the provided rawBody) instead of calling
req.text(), then pass that rawBody into verifyQstashSignature and into
JSON.parse/postBodySchema.parse and continue to call
executeTrialEmailCronPage(startingAfter); remove the direct
handleAndReturnErrorResponse path so errors and retries go through withCron’s
centralized logging and error handling.
In `@apps/web/app/`(ee)/api/stripe/webhook/checkout-session-completed.ts:
- Around line 24-37: The handler currently allows non-subscription sessions with
payment_status === "paid" to continue but later unconditionally uses
checkoutSession.subscription; update the logic so only checkout sessions where
checkoutSession.mode === "subscription" are allowed to proceed: explicitly
return/skip for any mode that is not "subscription" (after already handling
"setup"), and ensure you only access checkoutSession.subscription after
confirming mode === "subscription" (adjust the conditional around the existing
payment_status checks that reference checkoutSession.payment_status and the
later use of checkoutSession.subscription).
- Around line 152-164: The checkout-session-completed handler currently skips
setting defaultDomains.dublink for trialing subscriptions (the
prisma.defaultDomains.update call is gated by subscription.status !==
"trialing"), which leaves trials unable to get dub.link when they later convert;
modify the logic so that either checkout-session-completed sets
defaultDomains.dublink true even for trialing subscriptions or (preferably)
update the customer-subscription-updated flow to set defaultDomains.dublink when
the subscription transitions from trialing to active by adding handling in
updateWorkspacePlan (or its caller) to update prisma.defaultDomains.update({
where: { projectId: workspaceId }, data: { dublink: true } }) whenever the
subscription becomes active and the workspace doesn’t already have dublink
enabled.
In `@apps/web/app/`(ee)/api/stripe/webhook/customer-subscription-deleted.ts:
- Around line 248-279: The teardown and notification run even if the billing
downgrade failed because the earlier downgrade was executed with
Promise.allSettled; make the plan-change write authoritative by awaiting and
verifying the prisma.project.update result (or switch from Promise.allSettled to
a failing Promise.all) before running wouldLoseAdvancedRewardLogic,
stripAdvancedRewardModifiersForProgram,
pauseOrCancelCampaignsForProgramOnPlanDowngrade, and
sendAdvancedDowngradeNoticeEmailIfNeeded; specifically ensure the code that
performs the actual plan update returns/throws on failure and only then proceed
to call those helper functions so you don’t strip modifiers or send emails when
the workspace.plan was not actually changed.
- Around line 93-97: The webhook handler currently looks up fallback
subscriptions using only status "active" and unconditionally runs advanced-plan
teardown even if the workspace downgrade write fails; update the subscription
lookup used before calling updateWorkspacePlan to include both "active" and
"trialing" statuses (mirroring the pattern in billing/upgrade/route.ts) so
trialing subscriptions are considered, and change the teardown logic (calls to
stripAdvancedRewardModifiersForProgram and
pauseOrCancelCampaignsForProgramOnPlanDowngrade) to run only after confirming
the workspace downgrade write succeeded (e.g., check the result of
updateWorkspacePlan or the Promise.allSettled outcome and proceed with side
effects only on success).
In `@apps/web/app/api/workspaces/`[idOrSlug]/billing/upgrade/route.ts:
- Around line 162-175: The prisma.project.update call that persists
trialCheckoutExperiment after stripe.checkout.sessions.create is on the response
path and can turn a successful Stripe session into a 500; change it to
best-effort by moving the update off the critical path: wrap the
prisma.project.update (the call that updates project where id: workspace.id and
sets store.trialCheckoutExperiment) in a fire-and-forget background task or at
minimum surround it with try/catch and swallow/log errors (using your logger) so
failures don’t propagate to the client; alternatively enqueue a background job
to perform the update asynchronously to ensure stripe.checkout.sessions.create
success isn’t reverted by a DB error.
- Around line 93-101: The getFeatureFlags call on the checkout path must not
throw and block upgrades; wrap the getFeatureFlags(...) call in a try/catch (or
add a timeout) and on any error or timeout assign a safe default flags object
(e.g., empty/disabled) so shouldEnableStripeCheckoutTrial(flags, workspace.id)
will treat the trial as disabled and proceed to normal checkout; also log the
error/timeout for observability. Ensure this handling is applied where
getFeatureFlags, getTrialAbVariant, and shouldEnableStripeCheckoutTrial are used
in the route.
In
`@apps/web/app/app.dub.co/`(dashboard)/[slug]/(ee)/settings/billing/plan-usage.tsx:
- Around line 268-289: The onClick handler for the Resubscribe Button can leave
resubscribePortalLoading true on thrown or non-JSON errors; refactor the inline
handler to an async function that uses try/catch/finally, call
setResubscribePortalLoading(true) at start, await
fetch(`/api/workspaces/${workspaceId}/billing/manage`, { method: "POST" }),
handle non-ok responses by parsing JSON and calling toast.error(error.message)
inside the try or catch as appropriate, call router.push(url) on success, and
ensure setResubscribePortalLoading(false) always runs in finally; update
references to setResubscribePortalLoading, fetch, router.push, and toast.error
accordingly.
In `@apps/web/lib/api/campaigns/pause-campaigns-on-plan-downgrade.ts`:
- Around line 22-38: The update currently nulls campaign.qstashMessageId even if
qstash.messages.delete(campaign.qstashMessageId) fails, losing the only handle
to retry; change the flow so that prisma.campaign.update(...) sets
qstashMessageId: null only when the delete succeeded (e.g. track a
deletionSucceeded flag inside the try/catch around qstash.messages.delete), and
when the delete throws leave qstashMessageId unchanged while still setting
status: CampaignStatus.canceled; keep the existing warn log in the catch and
ensure you reference campaign.id and campaign.qstashMessageId when deciding
whether to clear the field.
In `@apps/web/lib/partners/throw-if-trial-program-enrollment-exceeded.ts`:
- Around line 25-32: The current helper throwIfTrialProgramEnrollmentExceeded
runs a standalone count via the root Prisma client allowing race conditions;
change the function throwIfTrialProgramEnrollmentExceeded to accept an optional
Prisma transaction client (e.g., tx: Prisma.TransactionClient) and use that
client for the count so callers can perform the count and subsequent approval
write inside the same transaction (prisma.$transaction) or lock; update callers
to pass the transaction client when performing approvals so the count+write
happen atomically.
In `@apps/web/lib/stripe/workspace-subscription-fields.ts`:
- Around line 34-41: Don't use new Date() for subscriptionCanceledAt because it
drifts; instead derive a stable timestamp from the Stripe subscription fields.
When cancelAtPeriodEnd is true, set subscriptionCanceledAt from
subscription.cancel_at (fallback to subscription.canceled_at) converted to a
Date (Number(...) * 1000) so the value remains constant across repeated webhook
processing; keep billingCycleEndsAt logic as-is using current_period_end.
In `@apps/web/scripts/migrations/backfill-workspace-stripe-billing-fields.ts`:
- Around line 115-118: The script's per-project catch blocks currently only log
and return, allowing the process to exit 0 despite partial failures; add failure
tracking to the BackfillCounters (e.g., add failed: number), increment that
counter in every per-project catch (and the other catch sites you noted around
lines 203-215, 247-250, 341-349), and after the main processing loop check if
counters.failed > 0 and throw a descriptive Error (or process.exit(1)) so the
script fails when any project backfill errored; update references to
BackfillCounters and any summary output to include the new failed count.
In `@apps/web/ui/modals/approve-partner-application-modal.tsx`:
- Around line 40-41: The hook useTrialLimitActivateModal returns both
openTrialLimitModal and the TrialLimitActivateModal component but only the
opener is used; render TrialLimitActivateModal inside the component's JSX so
calling openTrialLimitModal("partnerEnrollments") actually shows the modal.
Locate the destructuring const { openTrialLimitModal, TrialLimitActivateModal }
= useTrialLimitActivateModal() and add the TrialLimitActivateModal element
(mount the component) alongside the existing JSX (e.g., within the
modal/fragment that contains the approve partner UI) so the modal component is
mounted and can be opened on error.
In `@apps/web/ui/modals/start-paid-plan-modal.tsx`:
- Around line 48-76: handleConfirm currently only treats non-2xx fetch responses
as errors, but network/runtime rejections from fetch, mutate, or queryParams are
unhandled; wrap the await sequence inside a try/catch (or add a catch after the
try) so any thrown error from fetch, mutate, or queryParams is caught and
surfaces a user-friendly toast.error fallback (e.g., "Could not end trial. Try
again or use the billing portal.") and optionally log the error; keep the
existing finally that calls setIsSubmitting(false) and ensure
setShowModal(false) remains inside the success branch after res.ok.
In `@apps/web/ui/workspaces/upgrade-plan-button.tsx`:
- Around line 117-120: The button is currently disabled whenever isCurrentPlan
is true, which prevents handleClick from opening StartPaidPlanModal during an
active trial; update the disabled condition so the button stays enabled when
workspaceSlug exists and the workspace is on the current plan but trialing—i.e.,
change the disabled prop logic (used where disabled={!workspaceSlug ||
isCurrentPlan}) to account for isWorkspaceBillingTrialActive(trialEndsAt):
disable only when no workspaceSlug or when isCurrentPlan and NOT
isWorkspaceBillingTrialActive(trialEndsAt). Apply the same change to the other
button instance that uses the same disabled condition.
In `@packages/utils/src/constants/pricing/trial-limits.ts`:
- Around line 84-96: The returned limits object spreads planLimits but omits the
trial-only fields api and analyticsApi from TRIAL_LIMITS; update the return in
the function that builds PlanDetails["limits"] to also assign api:
TRIAL_LIMITS.api and analyticsApi: TRIAL_LIMITS.analyticsApi alongside the
existing overrides (links, clicks, payouts, users, domains, tags, folders,
groups, networkInvites, ai) so the trial values are applied.
---
Outside diff comments:
In `@apps/web/ui/modals/upgraded-modal.tsx`:
- Around line 76-81: The onClose handler currently always calls
setDotLinkOfferDismissed and persists the dismissal; change it so the dismissal
is only written when showDotLinkClaimUi is true (or alternatively write a
separate "upgradedModalSeen" flag instead). In practice, update the onClose
function to check the boolean showDotLinkClaimUi before calling
setDotLinkOfferDismissed (keep the existing queryParams(...) and
mutateWorkspace() behavior unchanged), or replace the call to
setDotLinkOfferDismissed with a different setter like setUpgradedModalSeen when
you want a non-dot-link-specific seen flag.
---
Minor comments:
In `@apps/web/playwright/workspaces/billing-mocks.ts`:
- Around line 27-47: applyMockTrialToWorkspace currently updates plan fields via
prisma.project.update but fails to reset cancellation metadata, so stale
subscriptionCanceledAt and billingCycleEndsAt can make the UI show a canceled
state; update the data payload in applyMockTrialToWorkspace (the
prisma.project.update call) to explicitly set subscriptionCanceledAt to null and
billingCycleEndsAt to null (or appropriate fresh values) when applying the mock
trial so the workspace is treated as an active, non-cancelled trial.
In `@apps/web/ui/modals/bulk-approve-partners-modal.tsx`:
- Around line 46-56: The onError handler currently calls toast.error(msg) where
msg = error.serverError ?? "" which can render an empty toast; change the logic
in the onError callback to compute a fallback message (e.g., a generic "An error
occurred" or error.message) when error.serverError is missing/empty, and only
call toast.error with that fallback; additionally, if trialActive and the
message triggers the trial condition (String(msg).includes("free trial") ||
String(msg).includes("enrolled partners")), call
openTrialLimitModal("partnerEnrollments") and avoid showing the toast (prevent
duplicate feedback) so the trial modal is shown without an empty or redundant
toast.
In `@apps/web/ui/modals/plan-change-confirmation-modal.tsx`:
- Around line 47-49: displayName can be empty when both program?.name and
workspaceName are nullish, causing logoSrc to be just OG_AVATAR_URL; update the
logic that computes displayName/logoSrc (references: displayName, logoSrc,
OG_AVATAR_URL, program?.name, workspaceName, workspaceLogo) to provide a safe
fallback string (e.g. "default" or "unknown") when displayName is empty or
whitespace and/or use a true default image key instead of interpolating an empty
name; ensure the fallback is used only when workspaceLogo and program?.logo are
absent so logoSrc always yields a valid URL or a default avatar.
In `@apps/web/ui/modals/trial-limit-activate-modal.tsx`:
- Around line 50-79: The handleConfirm function currently has a try/finally but
no catch, so network-level fetch errors are not surfaced to the user; update
handleConfirm to add a catch block around the async work (or convert to
try/catch/finally) that catches fetch/network errors and shows a user-facing
toast (use toast.error) with a helpful message, while preserving existing
behavior: call setIsSubmitting(true) at start, call mutate(), queryParams(...),
and setShowModal(false) on success, and always call setIsSubmitting(false) in
the finally; reference handleConfirm,
fetch(`/api/workspaces/${workspaceId}/billing/end-trial`), mutate, queryParams,
setShowModal, and toast.error when making the change.
In `@packages/utils/src/constants/pricing/trial-limits.ts`:
- Line 39: Fix the typo in the trial-limits constant by changing the string
value for the dublink entry from "claim a fee .link domain" to "claim a free
.link domain"; locate the dublink constant in
packages/utils/src/constants/pricing/trial-limits.ts and update only the text
inside the quotes to correct "fee" → "free".
- Around line 46-59: getTrialLimitResourceForOverageBanner currently falls back
to "payouts" when none of exceededEvents, exceededLinks, or exceededPayouts are
true, which can mislead callers; change the function to return a clear invalid
state instead of "payouts" — for example, update the return type of
getTrialLimitResourceForOverageBanner/TrialLimitResource to allow either a
distinct "general" resource value or null and then return that when no flags are
set (or alternatively throw an Error), and update any callers to handle the new
"general"/null/Error case accordingly.
---
Nitpick comments:
In `@apps/web/lib/actions/partners/accept-program-invite.ts`:
- Around line 24-25: The program select currently includes workspaceId but that
field is unused in the accept-program-invite action; remove the workspaceId:
true entry from the program select (located where the program is queried inside
the acceptProgramInvite action) so the DB query omits that unused scalar and
reduces payload; update any local references if present (e.g., variables named
program or program.workspaceId) to avoid dead code.
In `@apps/web/lib/billing/trial-checkout-experiment.ts`:
- Around line 6-12: The bucket assignment in hashWorkspaceIdToBucket only uses
Math.abs(h) % 2 which effectively examines the low bit of the rolling hash and
can correlate with input parity; fix by applying a final mixing/avalanche to h
before taking the bucket bit (for example, perform a couple of bitwise xors and
multiplicative mixes and then extract a single bit from the mixed unsigned
value) so the mapping is not just the low-bit of the 31-multiplier rolling hash;
update hashWorkspaceIdToBucket to mix h (e.g., xor-shift and multiply) and then
return the least significant bit of the mixed unsigned integer as 0 or 1.
In `@apps/web/lib/email/render-trial-email.tsx`:
- Around line 23-37: The switch in render-trial-email.tsx currently invokes
templates as plain functions (e.g., TrialStartedEmail(props),
TrialLinksFocusEmail(props), TrialPartnerFocusEmail(props),
TrialSocialProofEmail(props), Trial7DaysRemainingEmail(props),
Trial3DaysRemainingEmail(props), TrialEndsTodayEmail(props)); change each branch
to return a React element instead (e.g., <TrialStartedEmail {...props} /> or
React.createElement(TrialStartedEmail, props)) so the templates keep correct
React semantics if they later use hooks or context.
In `@apps/web/lib/email/run-trial-email-cron.ts`:
- Line 137: The replyTo address is hardcoded; extract it to a configuration
constant or environment variable (e.g., TRIAL_EMAIL_REPLY_TO) and update the
place where replyTo is set (the replyTo assignment in
apps/web/lib/email/run-trial-email-cron.ts) to use that constant with a sensible
fallback; ensure the constant is exported/imported from a shared config module
or read via process.env and update any tests or callers to reference the new
config key.
In `@apps/web/lib/email/trial-email-schedule.ts`:
- Around line 78-99: The four repetitive if blocks comparing daysSinceStart to
TRIAL_EMAIL_DAYS_FROM_START for each TRIAL_EMAIL_TYPE should be replaced by
iterating the TRIAL_EMAIL_TYPE/entries of TRIAL_EMAIL_DAYS_FROM_START and
calling tryAdd for any type whose configured day equals daysSinceStart; locate
the checks using the symbols daysSinceStart, TRIAL_EMAIL_DAYS_FROM_START,
TRIAL_EMAIL_TYPE and tryAdd, loop over the map or enum keys and perform a single
compare-and-tryAdd inside the loop to remove duplication.
- Around line 56-112: Add an inline comment above the logic in
getDueTrialEmailTypes documenting the coupling between
TRIAL_EMAIL_DAYS_FROM_START and the countdown checks (daysUntilEnd) and that
overlaps can occur when a value in TRIAL_EMAIL_DAYS_FROM_START equals
PARTNER_CHECKOUT_TRIAL_PERIOD_DAYS minus a countdown day; then add a runtime
sanity check (throw or processLogger.warn/assert) that iterates
TRIAL_EMAIL_DAYS_FROM_START values and verifies none equal
PARTNER_CHECKOUT_TRIAL_PERIOD_DAYS - X for X in [7,3,0] (or adjust list of
countdown days) so maintainers are alerted if PARTNER_CHECKOUT_TRIAL_PERIOD_DAYS
changes and creates unintended overlaps; reference symbols:
getDueTrialEmailTypes, TRIAL_EMAIL_DAYS_FROM_START, TRIAL_EMAIL_TYPE,
PARTNER_CHECKOUT_TRIAL_PERIOD_DAYS.
In `@apps/web/tests/email/run-trial-email-cron.test.ts`:
- Around line 46-114: Add a failing-path test that mocks sendBatchEmail to throw
(e.g., vi.fn().mockRejectedValue(new Error("send failed"))) when calling
runTrialEmailCron; assert sendBatchEmail was called but the sentEmail.create
mock (the create passed into prisma.sentEmail) was not called, and assert that
runTrialEmailCron propagates the failure by using await
expect(runTrialEmailCron(...)).rejects.toThrow("send failed"). Use the same test
scaffolding as the existing tests (mocks for project.findMany and
sentEmail.create) and reference runTrialEmailCron, sendBatchEmail, and the
sentEmail.create mock in the new test.
In `@apps/web/tests/plans/has-advanced-features.test.ts`:
- Around line 45-71: Add a test that verifies leftAdvancedPlan is
case-insensitive by asserting it returns true when currentPlan is "ADVANCED" (or
other mixed-case) and newPlan is "business"; update the tests around
leftAdvancedPlan to include a new it block that calls leftAdvancedPlan({
currentPlan: "ADVANCED", newPlan: "business" }) and expects true so the
.toLowerCase() behavior is covered and protected.
In `@apps/web/ui/modals/trial-limit-activate-modal.tsx`:
- Around line 149-157: The current useCallback wrapper creates a new component
identity for TrialLimitActivateModal when showModal or limitResource change,
causing remounts of TrialLimitActivateModalInner; fix this by returning the JSX
element directly instead of a function component (i.e., make
TrialLimitActivateModal a memoized JSX value or plain constant that evaluates to
<TrialLimitActivateModalInner showModal={showModal} setShowModal={setShowModal}
limitResource={limitResource} />) and update the parent render to use
{TrialLimitActivateModal} (not <TrialLimitActivateModal />), or alternatively
keep TrialLimitActivateModal as a stable reference (use useRef or move the
function outside the render) and pass showModal/setShowModal/limitResource as
props so the component identity of TrialLimitActivateModalInner does not change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c114ee3b-27f0-4565-81c3-8cf66eae6724
📒 Files selected for processing (78)
.github/workflows/playwright.yamlapps/web/app/(ee)/api/cron/trial-emails/route.tsapps/web/app/(ee)/api/cron/welcome-user/route.tsapps/web/app/(ee)/api/stripe/webhook/checkout-session-completed.tsapps/web/app/(ee)/api/stripe/webhook/customer-subscription-deleted.tsapps/web/app/(ee)/api/stripe/webhook/customer-subscription-updated.tsapps/web/app/(ee)/api/stripe/webhook/utils/stripe-plan-period.tsapps/web/app/(ee)/api/stripe/webhook/utils/update-workspace-plan.tsapps/web/app/api/workspaces/[idOrSlug]/billing/end-trial/route.tsapps/web/app/api/workspaces/[idOrSlug]/billing/upgrade/route.tsapps/web/app/app.dub.co/(auth)/side-panel.tsxapps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/settings/billing/plan-usage.tsxapps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/settings/billing/upgrade/page-client.tsxapps/web/app/app.dub.co/(dashboard)/[slug]/links/page-client.tsxapps/web/app/app.dub.co/(onboarding)/onboarding/(steps)/plan/plan-selector.tsxapps/web/app/app.dub.co/(onboarding)/onboarding/(steps)/products/product-selector.tsxapps/web/lib/actions/partners/accept-program-invite.tsapps/web/lib/actions/partners/bulk-approve-partners.tsapps/web/lib/api/campaigns/pause-campaigns-on-plan-downgrade.tsapps/web/lib/api/domains/claim-dot-link-domain.tsapps/web/lib/api/get-ratelimit-for-plan.tsapps/web/lib/api/partners/strip-advanced-reward-modifiers.tsapps/web/lib/auth/token-cache.tsapps/web/lib/auth/workspace.tsapps/web/lib/billing/trial-checkout-experiment.tsapps/web/lib/edge-config/get-feature-flags.tsapps/web/lib/email/render-trial-email.tsxapps/web/lib/email/run-trial-email-cron.tsapps/web/lib/email/send-advanced-downgrade-notice-email.tsapps/web/lib/email/trial-email-schedule.tsapps/web/lib/partners/approve-partner-enrollment.tsapps/web/lib/partners/throw-if-trial-program-enrollment-exceeded.tsapps/web/lib/plans/has-advanced-features.tsapps/web/lib/stripe/workspace-subscription-fields.tsapps/web/lib/types.tsapps/web/lib/zod/schemas/workspaces.tsapps/web/playwright.config.tsapps/web/playwright/README.mdapps/web/playwright/seed.tsapps/web/playwright/workspaces/auth.setup.tsapps/web/playwright/workspaces/billing-mocks.tsapps/web/playwright/workspaces/billing-trial.spec.tsapps/web/scripts/dev/seed.tsapps/web/scripts/migrations/backfill-workspace-stripe-billing-fields.tsapps/web/tests/email/run-trial-email-cron.test.tsapps/web/tests/email/trial-email-schedule.test.tsapps/web/tests/plans/has-advanced-features.test.tsapps/web/ui/domains/free-dot-link-banner.tsxapps/web/ui/layout/sidebar/app-sidebar-nav.tsxapps/web/ui/layout/sidebar/sidebar-usage.tsxapps/web/ui/layout/upgrade-banner.tsxapps/web/ui/modals/approve-partner-application-modal.tsxapps/web/ui/modals/bulk-approve-partners-modal.tsxapps/web/ui/modals/link-builder/index.tsxapps/web/ui/modals/plan-change-confirmation-modal.tsxapps/web/ui/modals/start-paid-plan-modal.tsxapps/web/ui/modals/trial-limit-activate-modal.tsxapps/web/ui/modals/upgraded-modal.tsxapps/web/ui/partners/overview/exceeded-events-limit.tsxapps/web/ui/partners/partner-network/network-partner-sheet.tsxapps/web/ui/workspaces/invite-teammates-form.tsxapps/web/ui/workspaces/subscription-menu.tsxapps/web/ui/workspaces/upgrade-plan-button.tsxapps/web/ui/workspaces/workspace-exceeded-events.tsxapps/web/vercel.jsonpackages/email/src/templates/advanced-plan-downgrade-notice.tsxpackages/email/src/templates/trial/trial-3-days-remaining.tsxpackages/email/src/templates/trial/trial-7-days-remaining.tsxpackages/email/src/templates/trial/trial-ends-today.tsxpackages/email/src/templates/trial/trial-links-focus.tsxpackages/email/src/templates/trial/trial-partner-focus.tsxpackages/email/src/templates/trial/trial-social-proof.tsxpackages/email/src/templates/trial/trial-started.tsxpackages/email/src/types/trial-marketing-email.tspackages/prisma/client.tspackages/prisma/schema/workspace.prismapackages/utils/src/constants/index.tspackages/utils/src/constants/pricing/trial-limits.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/ui/workspaces/upgrade-plan-button.tsx (1)
85-99:⚠️ Potential issue | 🟠 MajorCheck
res.okbefore firing analytics or redirecting.The codebase consistently uses
res.okchecks for fetch error handling (seesubscription-menu.tsxandmanage-subscription-button.tsxin the same directory). This code fires the"Opened Checkout"analytics event before validating the response, so a 4xx/5xx from/billing/upgradewill be logged as a successful checkout and then fail when parsing the error body. Gate this branch onres.ok, surface the error message to the user, and only emit the analytics event once the checkout session/portal URL is successfully obtained.🩹 Suggested change
}) .then(async (res) => { + if (!res.ok) { + const body = await res.json().catch(() => null); + throw new Error( + body?.error?.message ?? "Failed to start checkout.", + ); + } + plausible( checkoutTrialEnabled ? "Opened Checkout Trial" : "Opened Checkout No Trial", ); if (!stripeId || currentPlan === "free") { const data = await res.json(); const { id: sessionId } = data; const stripe = await getStripe(); stripe?.redirectToCheckout({ sessionId });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/ui/workspaces/upgrade-plan-button.tsx` around lines 85 - 99, In upgrade-plan-button.tsx, don’t call plausible or perform redirects before verifying the fetch response: check res.ok first, if false parse the error body (await res.json() or text()) and surface the message to the user (e.g., show a toast/error UI) and return; only after a successful response parse the session/portal data, emit the plausible event, and then call getStripe().redirectToCheckout({ sessionId }) or router.push(url) depending on stripeId/currentPlan to avoid logging failures as successes. Ensure you reference res.ok, res.json(), plausible, getStripe, stripeId, currentPlan, and router.push in the fix.
🧹 Nitpick comments (3)
apps/web/ui/modals/approve-partner-application-modal.tsx (1)
55-63: Let the server error drive this modal.This branch now depends on
trialActiveplus two English substrings. A stale workspace cache or a copy tweak inthrowIfTrialProgramEnrollmentLimitExceededwill silently fall back to the generic toast. Prefer a stable error code/identifier from the action and branch on that alone.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/ui/modals/approve-partner-application-modal.tsx` around lines 55 - 63, The onError handler in approve-partner-application-modal.tsx is matching English substrings on error.serverError to decide whether to call openTrialLimitModal("partnerEnrollments"); instead, read a stable error identifier from the server error (e.g., error.serverError.code or error.code emitted by throwIfTrialProgramEnrollmentLimitExceeded) and branch only on that specific code (for example 'TRIAL_PROGRAM_ENROLLMENT_LIMIT_EXCEEDED'), calling openTrialLimitModal("partnerEnrollments") when it matches; otherwise fall back to toast.error(msg || "Failed to approve partner."). Ensure you update the onError callback (the onError function) to check the server error code rather than using message.includes and keep existing toast.error fallback.packages/utils/src/constants/pricing/trial-limits.ts (1)
46-59: Consider the default return value when no overage is detected.The function returns
"payouts"as the default when none of theexceeded*flags are true (line 58). If this function is only called when an overage exists, the fallback is fine. However, if it can be called without any overage, the return value may be misleading for UI logic.Consider returning undefined or throwing for invalid state
export function getTrialLimitResourceForOverageBanner({ exceededEvents, exceededLinks, exceededPayouts, }: { exceededEvents: boolean; exceededLinks: boolean; exceededPayouts: boolean; -}): TrialLimitResource { +}): TrialLimitResource | undefined { if (exceededEvents) return "clicks"; if (exceededLinks) return "links"; if (exceededPayouts) return "payouts"; - return "payouts"; + return undefined; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/utils/src/constants/pricing/trial-limits.ts` around lines 46 - 59, The function getTrialLimitResourceForOverageBanner currently defaults to returning "payouts" when no exceeded* flags are true, which can mislead UI; update getTrialLimitResourceForOverageBanner to explicitly handle the no-overage case by returning undefined (or null) or by throwing an error for an invalid state, and update callers to handle that new return (or catch the thrown error). Locate the function by name and replace the final fallback return "payouts" with a clear invalid-state handling strategy (e.g., return undefined or throw new Error("no overage resource")), and adjust any code that calls getTrialLimitResourceForOverageBanner to respect the new contract.apps/web/scripts/migrations/backfill-workspace-stripe-billing-fields.ts (1)
157-162: Intentional but edge-case: unmappableplanPeriodpreserves stale values.When
getPlanPeriodFromStripeSubscriptionreturnsundefined(e.g., unsupported billing interval like "day"), the no-op check treats it as "equal" to any existing value, preserving potentially staleplanPerioddata. This matches the webhook behavior but could be surprising if a subscription legitimately changes to an unsupported interval.The dry-run output shows
proposed.planPeriodasundefinedin these cases, so operators can spot them manually.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/scripts/migrations/backfill-workspace-stripe-billing-fields.ts` around lines 157 - 162, The current no-op logic treats an unmappable planPeriod (returned as undefined by getPlanPeriodFromStripeSubscription) as equal to any existing value, preserving stale project.planPeriod; change the planEqual computation so undefined is not considered equal (e.g., set planEqual = planPeriod !== undefined && planPeriod === project.planPeriod) so that noOp (which uses planEqual and sameInstant(subscriptionCanceledAt, project.subscriptionCanceledAt) and sameInstant(billingCycleEndsAt, project.billingCycleEndsAt)) will correctly detect and surface changes where planPeriod became unmappable/undefined.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/app/api/workspaces/`[idOrSlug]/billing/upgrade/route.ts:
- Around line 103-105: The current code in route.ts reads existingStore and then
writes back workspace.store as { ...existingStore, trialCheckoutExperiment: ...
}, which can clobber concurrent writes; instead perform an atomic JSON patch
update (e.g., use Postgres jsonb_set or Prisma raw SQL) to set the
trialCheckoutExperiment key only, or move this data to a separate column/table,
so you don't overwrite the whole JSON blob—locate the write that updates
workspace.store (uses existingStore and trialCheckoutExperiment) and replace it
with an atomic update (or schema change) that targets the JSON path rather than
rewriting the entire store.
In `@apps/web/ui/modals/upgraded-modal.tsx`:
- Around line 34-43: The modal currently uses stale workspace state (trialEndsAt
via useWorkspace) to decide showDotLinkClaimUi, which allows the claim UI to
appear after Stripe returns with ?upgraded=true before
checkout-session-completed updates trialEndsAt; change the gate to require a
synchronous checkout-success signal instead of relying solely on trialEndsAt:
add a transient client-side flag produced by the checkout return path (e.g.,
read ?upgraded=true or a dedicated checkoutSuccess flag set by the redirect
handler) and include it in the showDotLinkClaimUi condition so that when
checkoutSuccess is present the dot-link claim UI is suppressed until the
workspace store (useWorkspace / trialEndsAt) is refreshed or
checkout-session-completed explicitly confirms the new state; reference
useWorkspace, trialEndsAt, isWorkspaceBillingTrialActive, showDotLinkClaimUi and
checkout-session-completed when implementing this change.
---
Outside diff comments:
In `@apps/web/ui/workspaces/upgrade-plan-button.tsx`:
- Around line 85-99: In upgrade-plan-button.tsx, don’t call plausible or perform
redirects before verifying the fetch response: check res.ok first, if false
parse the error body (await res.json() or text()) and surface the message to the
user (e.g., show a toast/error UI) and return; only after a successful response
parse the session/portal data, emit the plausible event, and then call
getStripe().redirectToCheckout({ sessionId }) or router.push(url) depending on
stripeId/currentPlan to avoid logging failures as successes. Ensure you
reference res.ok, res.json(), plausible, getStripe, stripeId, currentPlan, and
router.push in the fix.
---
Nitpick comments:
In `@apps/web/scripts/migrations/backfill-workspace-stripe-billing-fields.ts`:
- Around line 157-162: The current no-op logic treats an unmappable planPeriod
(returned as undefined by getPlanPeriodFromStripeSubscription) as equal to any
existing value, preserving stale project.planPeriod; change the planEqual
computation so undefined is not considered equal (e.g., set planEqual =
planPeriod !== undefined && planPeriod === project.planPeriod) so that noOp
(which uses planEqual and sameInstant(subscriptionCanceledAt,
project.subscriptionCanceledAt) and sameInstant(billingCycleEndsAt,
project.billingCycleEndsAt)) will correctly detect and surface changes where
planPeriod became unmappable/undefined.
In `@apps/web/ui/modals/approve-partner-application-modal.tsx`:
- Around line 55-63: The onError handler in
approve-partner-application-modal.tsx is matching English substrings on
error.serverError to decide whether to call
openTrialLimitModal("partnerEnrollments"); instead, read a stable error
identifier from the server error (e.g., error.serverError.code or error.code
emitted by throwIfTrialProgramEnrollmentLimitExceeded) and branch only on that
specific code (for example 'TRIAL_PROGRAM_ENROLLMENT_LIMIT_EXCEEDED'), calling
openTrialLimitModal("partnerEnrollments") when it matches; otherwise fall back
to toast.error(msg || "Failed to approve partner."). Ensure you update the
onError callback (the onError function) to check the server error code rather
than using message.includes and keep existing toast.error fallback.
In `@packages/utils/src/constants/pricing/trial-limits.ts`:
- Around line 46-59: The function getTrialLimitResourceForOverageBanner
currently defaults to returning "payouts" when no exceeded* flags are true,
which can mislead UI; update getTrialLimitResourceForOverageBanner to explicitly
handle the no-overage case by returning undefined (or null) or by throwing an
error for an invalid state, and update callers to handle that new return (or
catch the thrown error). Locate the function by name and replace the final
fallback return "payouts" with a clear invalid-state handling strategy (e.g.,
return undefined or throw new Error("no overage resource")), and adjust any code
that calls getTrialLimitResourceForOverageBanner to respect the new contract.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c14019b4-31cc-4ec0-bd92-f13c6e6f7731
📒 Files selected for processing (22)
apps/web/app/(ee)/api/cron/trial-emails/route.tsapps/web/app/(ee)/api/stripe/webhook/checkout-session-completed.tsapps/web/app/(ee)/api/stripe/webhook/customer-subscription-deleted.tsapps/web/app/(ee)/api/stripe/webhook/utils/update-workspace-plan.tsapps/web/app/api/workspaces/[idOrSlug]/billing/upgrade/route.tsapps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/settings/billing/plan-usage.tsxapps/web/lib/actions/partners/accept-program-invite.tsapps/web/lib/actions/partners/bulk-approve-partners.tsapps/web/lib/api/campaigns/pause-campaigns-on-plan-downgrade.tsapps/web/lib/partners/approve-partner-enrollment.tsapps/web/lib/partners/throw-if-trial-program-enrollment-exceeded.tsapps/web/lib/stripe/workspace-subscription-fields.tsapps/web/playwright/workspaces/billing-mocks.tsapps/web/scripts/migrations/backfill-workspace-stripe-billing-fields.tsapps/web/ui/modals/approve-partner-application-modal.tsxapps/web/ui/modals/bulk-approve-partners-modal.tsxapps/web/ui/modals/plan-change-confirmation-modal.tsxapps/web/ui/modals/start-paid-plan-modal.tsxapps/web/ui/modals/trial-limit-activate-modal.tsxapps/web/ui/modals/upgraded-modal.tsxapps/web/ui/workspaces/upgrade-plan-button.tsxpackages/utils/src/constants/pricing/trial-limits.ts
✅ Files skipped from review due to trivial changes (3)
- apps/web/lib/api/campaigns/pause-campaigns-on-plan-downgrade.ts
- apps/web/app/(ee)/api/stripe/webhook/checkout-session-completed.ts
- apps/web/ui/modals/trial-limit-activate-modal.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/web/lib/actions/partners/accept-program-invite.ts
- apps/web/lib/actions/partners/bulk-approve-partners.ts
- apps/web/lib/partners/throw-if-trial-program-enrollment-exceeded.ts
- apps/web/ui/modals/bulk-approve-partners-modal.tsx
- apps/web/ui/modals/plan-change-confirmation-modal.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/ui/workspaces/upgrade-plan-button.tsx`:
- Around line 123-126: The component repeatedly calls
isWorkspaceBillingTrialActive(trialEndsAt) during render causing possible
inconsistent behavior at time boundaries; compute a local const (e.g., const
isTrialActive = isWorkspaceBillingTrialActive(trialEndsAt)) once at the top of
the component (in upgrade-plan-button.tsx) and replace all subsequent calls (the
checks that currently call isWorkspaceBillingTrialActive in the conditional
where you call setShowStartPaidPlanModal, and the other uses later in the
render) with that single isTrialActive variable so all branches use the same
time-evaluated value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4398b00b-1782-4b0c-bfe9-8e72c2bf2e35
📒 Files selected for processing (3)
apps/web/app/app.dub.co/(onboarding)/onboarding/(steps)/products/product-selector.tsxapps/web/playwright.config.tsapps/web/ui/workspaces/upgrade-plan-button.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/playwright.config.ts
Collaborator
|
@coderabbitai full review please |
Contributor
|
✅ Actions performedFull review triggered. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/lib/auth/workspace.ts (1)
187-195:⚠️ Potential issue | 🟠 MajorTrial-aware rate limiting still skips non-restricted API keys.
These fields are only selected for restricted tokens, but the limit lookup now reads
token.projectfor every API key. Any still-supported non-restricted/personal key therefore falls back to"free"here and never gets the workspace trial bucket. Please fetch the workspace billing context for that path too, or move the limit lookup until after the workspace is resolved.Also applies to: 240-243
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/auth/workspace.ts` around lines 187 - 195, The rate-limit lookup uses token.project for every API key but the code only selects project (plan and trialEndsAt) when isRestrictedToken is true, causing non-restricted tokens to default to "free"; update the query to always fetch the workspace billing context (project.plan and project.trialEndsAt) regardless of isRestrictedToken or move the limit lookup that reads token.project to after workspace resolution so token.project is guaranteed populated; reference the isRestrictedToken branching and token.project usage and ensure project.select { plan, trialEndsAt } is included for both code paths (also apply same fix to the other occurrence around lines 240-243).
♻️ Duplicate comments (3)
apps/web/app/api/workspaces/[idOrSlug]/billing/upgrade/route.ts (2)
93-101:⚠️ Potential issue | 🟠 MajorFail closed if the feature-flag lookup is unavailable.
getFeatureFlags()still sits directly on the checkout path with no fallback. If Edge Config throws or times out, the route 500s before creating the Stripe session. Wrap this in a safe fallback and default trial checkout to disabled so upgrades continue normally.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/api/workspaces/`[idOrSlug]/billing/upgrade/route.ts around lines 93 - 101, The call to getFeatureFlags can throw and must not break the checkout flow; wrap the getFeatureFlags(...) invocation in a try/catch (or use a safe helper like safeGetFeatureFlags) and, on any error or timeout, use a fallback empty/disabled flags object so shouldEnableStripeCheckoutTrial(flags, workspace.id) returns false; keep using getTrialAbVariant(workspace.id) as-is but ensure checkoutTrialEnabled defaults to false when feature lookup failed so session creation continues normally.
103-105:⚠️ Potential issue | 🟠 MajorDon't rewrite
workspace.storefrom a stale snapshot.
existingStoreis captured before the Stripe call, then the whole JSON blob is written back afterward. Any concurrent write toworkspace.storein between is lost. PersisttrialCheckoutExperimentwith an atomic JSON patch or a separate field/table instead of{ ...existingStore, ... }.Also applies to: 162-176
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/api/workspaces/`[idOrSlug]/billing/upgrade/route.ts around lines 103 - 105, The code captures workspace.store into existingStore before the Stripe call and then writes back the whole blob, risking lost concurrent updates; change the update to persist only the trialCheckoutExperiment key (or store it in a separate column/table) instead of writing { ...existingStore, ... }; locate where existingStore and trialCheckoutExperiment are used in the route handler and replace the full-object write with an atomic JSON patch (e.g., a DB jsonb_set/update that sets ["trialCheckoutExperiment"] or a targeted update method) or persist trialCheckoutExperiment to its own field/table so you never overwrite workspace.store from a stale snapshot.apps/web/ui/modals/upgraded-modal.tsx (1)
46-68:⚠️ Potential issue | 🟠 MajorOne revalidation still isn't a reliable checkout-sync gate.
checkoutWorkspaceSyncedflips totrueinfinally(), even if the refresh fails or returns pre-webhook workspace data. In that windowshowDotLinkClaimUican still become true for trial checkouts. Keep the claim UI suppressed until the workspace state explicitly reflects the post-checkout state, not just until onemutate()finishes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/ui/modals/upgraded-modal.tsx` around lines 46 - 68, The current logic sets checkoutWorkspaceSynced to true in the mutateWorkspaceData().finally() block which can flip the UI before the workspace actually reflects post-checkout state; instead, call mutateWorkspaceData() and only set checkoutWorkspaceSynced when the mutation/refresh returns workspace data that explicitly indicates the post-checkout state (for example by verifying the updated subscription/trial status or dotLinkClaimed flag) — use the promise resolution (.then) to inspect the returned workspace payload, keep the cancelled guard, and fall back to a retry/poll or timeout if the refreshed workspace does not yet show the expected post-checkout properties; ensure showDotLinkClaimUi remains gated by checkoutWorkspaceSynced and the explicit state checks (isWorkspaceBillingTrialActive(trialEndsAt), dotLinkClaimed) so the claim UI is suppressed until the workspace clearly reflects the checkout.
🧹 Nitpick comments (5)
apps/web/lib/api/campaigns/pause-campaigns-on-plan-downgrade.ts (1)
56-76: Consider wrappingscheduleTransactionalCampaigncalls to prevent one failure from blocking others.If
scheduleTransactionalCampaignthrows for one campaign, the remaining transactional campaigns won't be processed. This could leave the system in a partially-processed state during a plan downgrade.♻️ Proposed fix: wrap in try-catch or use Promise.allSettled
+ const scheduleResults = []; for (const campaign of transactionalCampaigns) { const updatedCampaign = await prisma.$transaction(async (tx) => { if (campaign.workflowId) { await tx.workflow.update({ where: { id: campaign.workflowId }, data: { disabledAt: new Date() }, }); } return tx.campaign.update({ where: { id: campaign.id }, data: { status: CampaignStatus.paused }, include: { workflow: true }, }); }); - await scheduleTransactionalCampaign({ - campaign, - updatedCampaign, - }); + scheduleResults.push( + scheduleTransactionalCampaign({ + campaign, + updatedCampaign, + }).catch((error) => { + console.warn( + `Failed to reschedule transactional campaign ${campaign.id}:`, + error, + ); + }), + ); } + await Promise.allSettled(scheduleResults);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/api/campaigns/pause-campaigns-on-plan-downgrade.ts` around lines 56 - 76, The loop that pauses campaigns uses prisma.$transaction to update each campaign but calls scheduleTransactionalCampaign(campaign, updatedCampaign) without error handling, so a thrown error aborts processing remaining transactionalCampaigns; wrap the scheduleTransactionalCampaign call in a try-catch (inside the for loop) or collect promises and use Promise.allSettled to ensure each campaign is attempted independently, log failures with the campaign id and error, and continue processing the rest so one failure doesn't block others.apps/web/lib/email/render-trial-email.tsx (1)
38-40: Throw on unexpected email types instead of returning them.The
neverassignment gives you compile-time exhaustiveness, but if an out-of-band value ever reaches this helper, the current default branch returns that raw value as the email body. Failing loudly here is safer.Possible fix
default: { const _exhaustive: never = type; - return _exhaustive; + throw new Error(`Unhandled trial email type: ${_exhaustive}`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/email/render-trial-email.tsx` around lines 38 - 40, The default branch in the switch that currently does "const _exhaustive: never = type; return _exhaustive;" should instead both preserve the never assignment for compile-time exhaustiveness and throw at runtime so unexpected email types fail loudly; update the default case in the helper (the switch over type) to keep "const _exhaustive: never = type;" and then "throw new Error(`Unexpected email type: ${String(_exhaustive)}`);" (or call an assertUnreachable helper) so an out-of-band value does not get returned as the email body.apps/web/ui/modals/bulk-approve-partners-modal.tsx (1)
47-62: Avoid keying the trial upsell flow off error copy.Matching
"free trial"/"enrolled partners"here is brittle. If the server-side message changes, this path silently regresses to the generic toast instead of opening the upgrade modal. Prefer a stable error code or discriminant frombulkApprovePartnersActionand branch on that instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/ui/modals/bulk-approve-partners-modal.tsx` around lines 47 - 62, The code currently keys the trial upsell flow on brittle error text checks on serverMsg; instead change the branch to inspect a stable discriminant (e.g., an error.code or error.type) returned by bulkApprovePartnersAction and call openTrialLimitModal("partnerEnrollments") when that discriminant equals the trial-limit value (for example "TRIAL_LIMIT" or "PARTNER_ENROLLMENT_LIMIT"); update bulkApprovePartnersAction to return that structured error field if it doesn't already, then replace the serverMsg.includes checks in the component (around serverMsg, openTrialLimitModal, and toast.error usage) with a check like if (error?.code === "TRIAL_LIMIT") { openTrialLimitModal(...); return; } and fall back to toast.error(message || "An error occurred") for other errors.apps/web/lib/email/trial-email-schedule.ts (1)
61-63: Tightensenttyping to valid trial-email values.
sent: Set<string>allows invalid values and weakens compile-time guarantees. UseSet<TrialEmailType>here.Proposed typing change
}: { trialEndsAt: Date; - sent: Set<string>; + sent: Set<TrialEmailType>; now: Date; }): TrialEmailType[] {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/email/trial-email-schedule.ts` around lines 61 - 63, The `sent` property is typed too loosely as Set<string>; change its type to Set<TrialEmailType> to enforce valid trial-email values (update the declaration where `trialEndsAt`, `sent`, `now` are defined), and ensure `TrialEmailType` is imported or declared in that module so all places constructing or iterating over `sent` use the stronger type (e.g., when adding/checking values, cast/validate to TrialEmailType if needed).apps/web/ui/modals/approve-partner-application-modal.tsx (1)
55-64: Avoid keying the trial CTA off error message text.This path is coupled to human-readable substrings, so a wording change on the server will silently downgrade back to a toast. Prefer a stable error code/shape from
approvePartnerAction/throwIfTrialProgramEnrollmentLimitExceededso the trial-limit modal stays wired independently of copy changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/ui/modals/approve-partner-application-modal.tsx` around lines 55 - 64, The onError handler in the approvePartnerAction currently detects trial-limit cases by string-matching serverError text, which is brittle; update it to check a stable error shape or code (e.g., an error.code or error.type set by throwIfTrialProgramEnrollmentLimitExceeded) instead of msg.includes(...). Locate the onError callback in approve-partner-application-modal.tsx (the approvePartnerAction onError) and change the conditional to look for the agreed-upon sentinel (for example error.code === "TRIAL_ENROLLMENT_LIMIT_EXCEEDED" or error.type === "TrialEnrollmentLimitExceeded") and call openTrialLimitModal("partnerEnrollments") when present; otherwise fall back to toast.error with a safe message. Ensure approvePartnerAction / throwIfTrialProgramEnrollmentLimitExceeded return that code/shape on error so the check is reliable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/app/`(ee)/api/stripe/webhook/utils/stripe-plan-period.ts:
- Around line 7-19: The current mapping in stripe-plan-period.ts uses interval
and interval_count from subscription.items.data[0]?.price?.recurring but treats
any non-3 "month" as monthly and any "year" as yearly; update the mapping logic
in the function that returns PlanPeriod so you explicitly guard interval_count:
for interval === "month" return PlanPeriod.monthly only when interval_count ===
1 and PlanPeriod.quarterly when interval_count === 3; for interval === "year"
return PlanPeriod.yearly only when interval_count === 1; for interval === "week"
keep PlanPeriod.biweekly only when interval_count === 2; otherwise return
undefined (do not default to monthly/yearly). Ensure you check
subscription.items.data[0]?.price?.recurring exists before using interval_count
and reference PlanPeriod exactly as used in the file.
In `@apps/web/app/`(ee)/api/stripe/webhook/utils/update-workspace-plan.ts:
- Around line 97-113: The code unconditionally sets paymentFailedAt: null which
clears the payment-failed UI on any subscription webhook; change this to only
clear paymentFailedAt when the subscription has actually recovered (e.g.,
subscription.status indicates paid/active). Remove the unconditional
paymentFailedAt: null and instead conditionally include it in the update object
(for example using ...(subscriptionStatusIsRecovered && { paymentFailedAt: null
})) so paymentFailedAt is preserved for past_due/unpaid events; use the existing
symbols (subscription/status, paymentFailedAt, trialEndsAt, cancellationFields,
planPeriod) to locate and implement the change.
- Around line 159-171: The defaultDomains.dublink toggle runs even if the
project plan write fails because you used Promise.allSettled; change the flow so
the dublink update (prisma.defaultDomains.updateMany) only runs after confirming
prisma.project.update succeeded: perform or await the project update (or inspect
the Promise.allSettled result for the prisma.project.update entry) and only when
its status is "fulfilled" and the workspace/newPlan persisted, then call
defaultDomains.updateMany to set dublink=true; keep the existing condition
(subscription?.status === "active" && newPlanName !== "free") but gate the
domain update behind the successful project write to avoid enabling entitlements
on failed plan changes.
In `@apps/web/app/api/workspaces/`[idOrSlug]/billing/end-trial/route.ts:
- Around line 13-33: The current flow throws if no trialing subscription is
found, causing retries to surface as errors; make the end-trial action
idempotent by treating an already-ended trial as success: after listing
trialingSubscription (from stripe.subscriptions.list using workspace.stripeId),
if trialingSubscription is missing, call stripe.subscriptions.list again (or
reuse the same call) to check for an active subscription for the same customer
(status "active" or other non-trial statuses you consider valid) and return
NextResponse.json({ ok: true }) if one exists; only throw the DubApiError when
neither a trialing nor an active/non-trial subscription exists. Ensure you
update logic around trialingSubscription and the stripe.subscriptions.update
call to reflect this conditional flow.
In `@apps/web/lib/email/run-trial-email-cron.ts`:
- Around line 29-45: The current idempotency key for sendBatchEmail depends on
startingAfter and unstable chunking order which can assign the same recipient to
different keys causing duplicates; to fix, ensure recipients are
deterministically ordered (e.g., sort by lowercased email) before
deduping/chunking and compute the idempotency key from that stable ordering (for
example join of sorted emails plus workspace id/email type) in the places that
call sendBatchEmail and create sentEmail records (refer to dedupeRecipients,
sendBatchEmail invocation, and sentEmail.create) so retries use the same
idempotency key for the same recipient set.
In `@apps/web/lib/email/send-advanced-downgrade-notice-email.ts`:
- Around line 22-54: The current read-then-send-then-write flow using
prisma.sentEmail.findFirst + sendEmail + prisma.sentEmail.create is racy; make
the dedupe atomic by reserving the (projectId, type) key before calling
sendEmail: add a unique constraint on (projectId, type) for sentEmail, attempt
to create a record with a provisional status (e.g., "pending") via
prisma.sentEmail.create and treat a unique-constraint error (Prisma P2002) as
“already handled” and return, then perform sendEmail only if create succeeded
and finally update that record to "sent" (or set sentAt) after successful send;
ensure you handle send failures by cleaning up or marking the record failed so
retries can proceed. Reference symbols: prisma.sentEmail, dedupeType, projectId,
sendEmail, AdvancedPlanDowngradeNotice.
In `@apps/web/lib/email/trial-email-schedule.ts`:
- Around line 68-109: The current exact-equality checks (daysSinceStart === ...)
in trial-email-schedule.ts can miss emails if the job is delayed and can send
“ends today” late after the trial day; update the logic around
differenceInCalendarDaysUTC, tryAdd, TRIAL_EMAIL_TYPE and
TRIAL_EMAIL_DAYS_FROM_START so each scheduled send uses a catch-up window and an
explicit expiry guard: for start/feature emails change the condition to treat
any daysSinceStart >= expected && daysSinceStart < expected + GRACE_DAYS
(introduce a small GRACE_DAYS constant like 1 or 2) and still skip if
sent.has(type); for countdown emails compute daysUntilEnd as
differenceInCalendarDaysUTC(trialEndsAt, now) and only allow sends when
daysUntilEnd === N and also ensure trialEndsAt is not already past a cutoff
(e.g., trialEndsAt >= endOfDayUTC(now) or daysUntilEnd >= 0) to prevent late
“ends today” sends; keep using tryAdd(...) to respect sent/due deduping.
In `@apps/web/lib/zod/schemas/workspaces.ts`:
- Around line 39-44: The WorkspaceSchema's planPeriod enum only allows "monthly"
and "yearly" but the persisted Project.planPeriod can also be "quarterly" and
"biweekly"; update the planPeriod validation (the z.enum used for planPeriod in
apps/web/lib/zod/schemas/workspaces.ts) to include "quarterly" and "biweekly"
while preserving .nullish() and the existing description so schema parsing
accepts all stored values.
---
Outside diff comments:
In `@apps/web/lib/auth/workspace.ts`:
- Around line 187-195: The rate-limit lookup uses token.project for every API
key but the code only selects project (plan and trialEndsAt) when
isRestrictedToken is true, causing non-restricted tokens to default to "free";
update the query to always fetch the workspace billing context (project.plan and
project.trialEndsAt) regardless of isRestrictedToken or move the limit lookup
that reads token.project to after workspace resolution so token.project is
guaranteed populated; reference the isRestrictedToken branching and
token.project usage and ensure project.select { plan, trialEndsAt } is included
for both code paths (also apply same fix to the other occurrence around lines
240-243).
---
Duplicate comments:
In `@apps/web/app/api/workspaces/`[idOrSlug]/billing/upgrade/route.ts:
- Around line 93-101: The call to getFeatureFlags can throw and must not break
the checkout flow; wrap the getFeatureFlags(...) invocation in a try/catch (or
use a safe helper like safeGetFeatureFlags) and, on any error or timeout, use a
fallback empty/disabled flags object so shouldEnableStripeCheckoutTrial(flags,
workspace.id) returns false; keep using getTrialAbVariant(workspace.id) as-is
but ensure checkoutTrialEnabled defaults to false when feature lookup failed so
session creation continues normally.
- Around line 103-105: The code captures workspace.store into existingStore
before the Stripe call and then writes back the whole blob, risking lost
concurrent updates; change the update to persist only the
trialCheckoutExperiment key (or store it in a separate column/table) instead of
writing { ...existingStore, ... }; locate where existingStore and
trialCheckoutExperiment are used in the route handler and replace the
full-object write with an atomic JSON patch (e.g., a DB jsonb_set/update that
sets ["trialCheckoutExperiment"] or a targeted update method) or persist
trialCheckoutExperiment to its own field/table so you never overwrite
workspace.store from a stale snapshot.
In `@apps/web/ui/modals/upgraded-modal.tsx`:
- Around line 46-68: The current logic sets checkoutWorkspaceSynced to true in
the mutateWorkspaceData().finally() block which can flip the UI before the
workspace actually reflects post-checkout state; instead, call
mutateWorkspaceData() and only set checkoutWorkspaceSynced when the
mutation/refresh returns workspace data that explicitly indicates the
post-checkout state (for example by verifying the updated subscription/trial
status or dotLinkClaimed flag) — use the promise resolution (.then) to inspect
the returned workspace payload, keep the cancelled guard, and fall back to a
retry/poll or timeout if the refreshed workspace does not yet show the expected
post-checkout properties; ensure showDotLinkClaimUi remains gated by
checkoutWorkspaceSynced and the explicit state checks
(isWorkspaceBillingTrialActive(trialEndsAt), dotLinkClaimed) so the claim UI is
suppressed until the workspace clearly reflects the checkout.
---
Nitpick comments:
In `@apps/web/lib/api/campaigns/pause-campaigns-on-plan-downgrade.ts`:
- Around line 56-76: The loop that pauses campaigns uses prisma.$transaction to
update each campaign but calls scheduleTransactionalCampaign(campaign,
updatedCampaign) without error handling, so a thrown error aborts processing
remaining transactionalCampaigns; wrap the scheduleTransactionalCampaign call in
a try-catch (inside the for loop) or collect promises and use Promise.allSettled
to ensure each campaign is attempted independently, log failures with the
campaign id and error, and continue processing the rest so one failure doesn't
block others.
In `@apps/web/lib/email/render-trial-email.tsx`:
- Around line 38-40: The default branch in the switch that currently does "const
_exhaustive: never = type; return _exhaustive;" should instead both preserve the
never assignment for compile-time exhaustiveness and throw at runtime so
unexpected email types fail loudly; update the default case in the helper (the
switch over type) to keep "const _exhaustive: never = type;" and then "throw new
Error(`Unexpected email type: ${String(_exhaustive)}`);" (or call an
assertUnreachable helper) so an out-of-band value does not get returned as the
email body.
In `@apps/web/lib/email/trial-email-schedule.ts`:
- Around line 61-63: The `sent` property is typed too loosely as Set<string>;
change its type to Set<TrialEmailType> to enforce valid trial-email values
(update the declaration where `trialEndsAt`, `sent`, `now` are defined), and
ensure `TrialEmailType` is imported or declared in that module so all places
constructing or iterating over `sent` use the stronger type (e.g., when
adding/checking values, cast/validate to TrialEmailType if needed).
In `@apps/web/ui/modals/approve-partner-application-modal.tsx`:
- Around line 55-64: The onError handler in the approvePartnerAction currently
detects trial-limit cases by string-matching serverError text, which is brittle;
update it to check a stable error shape or code (e.g., an error.code or
error.type set by throwIfTrialProgramEnrollmentLimitExceeded) instead of
msg.includes(...). Locate the onError callback in
approve-partner-application-modal.tsx (the approvePartnerAction onError) and
change the conditional to look for the agreed-upon sentinel (for example
error.code === "TRIAL_ENROLLMENT_LIMIT_EXCEEDED" or error.type ===
"TrialEnrollmentLimitExceeded") and call
openTrialLimitModal("partnerEnrollments") when present; otherwise fall back to
toast.error with a safe message. Ensure approvePartnerAction /
throwIfTrialProgramEnrollmentLimitExceeded return that code/shape on error so
the check is reliable.
In `@apps/web/ui/modals/bulk-approve-partners-modal.tsx`:
- Around line 47-62: The code currently keys the trial upsell flow on brittle
error text checks on serverMsg; instead change the branch to inspect a stable
discriminant (e.g., an error.code or error.type) returned by
bulkApprovePartnersAction and call openTrialLimitModal("partnerEnrollments")
when that discriminant equals the trial-limit value (for example "TRIAL_LIMIT"
or "PARTNER_ENROLLMENT_LIMIT"); update bulkApprovePartnersAction to return that
structured error field if it doesn't already, then replace the
serverMsg.includes checks in the component (around serverMsg,
openTrialLimitModal, and toast.error usage) with a check like if (error?.code
=== "TRIAL_LIMIT") { openTrialLimitModal(...); return; } and fall back to
toast.error(message || "An error occurred") for other errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ac429a5c-116c-4142-8a84-44a72963f741
📒 Files selected for processing (74)
apps/web/app/(ee)/api/cron/trial-emails/route.tsapps/web/app/(ee)/api/cron/welcome-user/route.tsapps/web/app/(ee)/api/stripe/webhook/checkout-session-completed.tsapps/web/app/(ee)/api/stripe/webhook/customer-subscription-deleted.tsapps/web/app/(ee)/api/stripe/webhook/customer-subscription-updated.tsapps/web/app/(ee)/api/stripe/webhook/utils/stripe-plan-period.tsapps/web/app/(ee)/api/stripe/webhook/utils/update-workspace-plan.tsapps/web/app/api/workspaces/[idOrSlug]/billing/end-trial/route.tsapps/web/app/api/workspaces/[idOrSlug]/billing/upgrade/route.tsapps/web/app/app.dub.co/(auth)/side-panel.tsxapps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/settings/billing/plan-usage.tsxapps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/settings/billing/upgrade/page-client.tsxapps/web/app/app.dub.co/(dashboard)/[slug]/links/page-client.tsxapps/web/app/app.dub.co/(onboarding)/onboarding/(steps)/plan/plan-selector.tsxapps/web/app/app.dub.co/(onboarding)/onboarding/(steps)/products/product-selector.tsxapps/web/lib/actions/partners/accept-program-invite.tsapps/web/lib/actions/partners/bulk-approve-partners.tsapps/web/lib/api/campaigns/pause-campaigns-on-plan-downgrade.tsapps/web/lib/api/domains/claim-dot-link-domain.tsapps/web/lib/api/get-ratelimit-for-plan.tsapps/web/lib/api/partners/strip-advanced-reward-modifiers.tsapps/web/lib/auth/token-cache.tsapps/web/lib/auth/workspace.tsapps/web/lib/billing/trial-checkout-experiment.tsapps/web/lib/edge-config/get-feature-flags.tsapps/web/lib/email/render-trial-email.tsxapps/web/lib/email/run-trial-email-cron.tsapps/web/lib/email/send-advanced-downgrade-notice-email.tsapps/web/lib/email/trial-email-schedule.tsapps/web/lib/partners/approve-partner-enrollment.tsapps/web/lib/partners/throw-if-trial-program-enrollment-exceeded.tsapps/web/lib/plans/has-advanced-features.tsapps/web/lib/stripe/workspace-subscription-fields.tsapps/web/lib/types.tsapps/web/lib/zod/schemas/workspaces.tsapps/web/playwright.config.tsapps/web/playwright/workspaces/auth.setup.tsapps/web/playwright/workspaces/billing-mocks.tsapps/web/playwright/workspaces/billing-trial.spec.tsapps/web/scripts/dev/seed.tsapps/web/scripts/migrations/backfill-workspace-stripe-billing-fields.tsapps/web/tests/email/run-trial-email-cron.test.tsapps/web/tests/email/trial-email-schedule.test.tsapps/web/tests/plans/has-advanced-features.test.tsapps/web/ui/domains/free-dot-link-banner.tsxapps/web/ui/layout/sidebar/app-sidebar-nav.tsxapps/web/ui/layout/sidebar/sidebar-usage.tsxapps/web/ui/layout/upgrade-banner.tsxapps/web/ui/modals/approve-partner-application-modal.tsxapps/web/ui/modals/bulk-approve-partners-modal.tsxapps/web/ui/modals/link-builder/index.tsxapps/web/ui/modals/plan-change-confirmation-modal.tsxapps/web/ui/modals/start-paid-plan-modal.tsxapps/web/ui/modals/trial-limit-activate-modal.tsxapps/web/ui/modals/upgraded-modal.tsxapps/web/ui/partners/overview/exceeded-events-limit.tsxapps/web/ui/partners/partner-network/network-partner-sheet.tsxapps/web/ui/workspaces/invite-teammates-form.tsxapps/web/ui/workspaces/upgrade-plan-button.tsxapps/web/ui/workspaces/workspace-exceeded-events.tsxapps/web/vercel.jsonpackages/email/src/templates/advanced-plan-downgrade-notice.tsxpackages/email/src/templates/trial/trial-3-days-remaining.tsxpackages/email/src/templates/trial/trial-7-days-remaining.tsxpackages/email/src/templates/trial/trial-ends-today.tsxpackages/email/src/templates/trial/trial-links-focus.tsxpackages/email/src/templates/trial/trial-partner-focus.tsxpackages/email/src/templates/trial/trial-social-proof.tsxpackages/email/src/templates/trial/trial-started.tsxpackages/email/src/types/trial-marketing-email.tspackages/prisma/client.tspackages/prisma/schema/workspace.prismapackages/utils/src/constants/index.tspackages/utils/src/constants/pricing/trial-limits.ts
👮 Files not reviewed due to content moderation or server errors (8)
- apps/web/ui/partners/partner-network/network-partner-sheet.tsx
- apps/web/tests/email/trial-email-schedule.test.ts
- apps/web/tests/email/run-trial-email-cron.test.ts
- apps/web/app/(ee)/api/stripe/webhook/customer-subscription-deleted.ts
- apps/web/app/(ee)/api/stripe/webhook/checkout-session-completed.ts
- apps/web/playwright/workspaces/billing-trial.spec.ts
- apps/web/app/(ee)/api/cron/trial-emails/route.ts
- apps/web/ui/workspaces/upgrade-plan-button.tsx
Comment on lines
+7
to
+19
| const { interval, interval_count } = | ||
| subscription.items.data[0]?.price?.recurring ?? {}; | ||
| if (interval === "month") { | ||
| if (interval_count === 3) { | ||
| return PlanPeriod.quarterly; | ||
| } | ||
| return PlanPeriod.monthly; | ||
| } | ||
| if (interval === "year") { | ||
| return PlanPeriod.yearly; | ||
| } | ||
| if (interval === "week" && interval_count === 2) { | ||
| return PlanPeriod.biweekly; |
Contributor
There was a problem hiding this comment.
Guard interval_count before mapping month/year plans.
Right now any monthly cadence other than 3 months is stored as monthly, and any yearly cadence is stored as yearly. That misclassifies unsupported Stripe prices like 6-month or 2-year intervals instead of returning undefined as intended.
Possible fix
- if (interval === "month") {
- if (interval_count === 3) {
- return PlanPeriod.quarterly;
- }
- return PlanPeriod.monthly;
- }
- if (interval === "year") {
- return PlanPeriod.yearly;
- }
+ if (interval === "month" && interval_count === 3) {
+ return PlanPeriod.quarterly;
+ }
+ if (interval === "month" && interval_count === 1) {
+ return PlanPeriod.monthly;
+ }
+ if (interval === "year" && interval_count === 1) {
+ return PlanPeriod.yearly;
+ }
if (interval === "week" && interval_count === 2) {
return PlanPeriod.biweekly;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/app/`(ee)/api/stripe/webhook/utils/stripe-plan-period.ts around
lines 7 - 19, The current mapping in stripe-plan-period.ts uses interval and
interval_count from subscription.items.data[0]?.price?.recurring but treats any
non-3 "month" as monthly and any "year" as yearly; update the mapping logic in
the function that returns PlanPeriod so you explicitly guard interval_count: for
interval === "month" return PlanPeriod.monthly only when interval_count === 1
and PlanPeriod.quarterly when interval_count === 3; for interval === "year"
return PlanPeriod.yearly only when interval_count === 1; for interval === "week"
keep PlanPeriod.biweekly only when interval_count === 2; otherwise return
undefined (do not default to monthly/yearly). Ensure you check
subscription.items.data[0]?.price?.recurring exists before using interval_count
and reference PlanPeriod exactly as used in the file.
Comment on lines
97
to
113
| plan: newPlanName, | ||
| planTier: newPlanTier, | ||
| usageLimit: newPlan.limits.clicks, | ||
| linksLimit: newPlan.limits.links, | ||
| payoutsLimit: newPlan.limits.payouts, | ||
| domainsLimit: newPlan.limits.domains, | ||
| aiLimit: newPlan.limits.ai, | ||
| tagsLimit: newPlan.limits.tags, | ||
| foldersLimit: newPlan.limits.folders, | ||
| groupsLimit: newPlan.limits.groups, | ||
| networkInvitesLimit: newPlan.limits.networkInvites, | ||
| usersLimit: newPlan.limits.users, | ||
| usageLimit: limits.clicks, | ||
| linksLimit: limits.links, | ||
| payoutsLimit: limits.payouts, | ||
| domainsLimit: limits.domains, | ||
| aiLimit: limits.ai, | ||
| tagsLimit: limits.tags, | ||
| foldersLimit: limits.folders, | ||
| groupsLimit: limits.groups, | ||
| networkInvitesLimit: limits.networkInvites, | ||
| usersLimit: limits.users, | ||
| paymentFailedAt: null, | ||
| ...(trialEndsAt !== undefined && { trialEndsAt }), | ||
| ...cancellationFields, | ||
| ...(planPeriod !== undefined && { planPeriod }), | ||
| }, |
Contributor
There was a problem hiding this comment.
Preserve paymentFailedAt until billing actually recovers.
This branch now runs whenever a webhook passes a subscription, so past_due/unpaid updates will also null out paymentFailedAt. That clears the payment-failed UI even though the customer is still delinquent.
Proposed fix
data: {
plan: newPlanName,
planTier: newPlanTier,
usageLimit: limits.clicks,
linksLimit: limits.links,
payoutsLimit: limits.payouts,
domainsLimit: limits.domains,
aiLimit: limits.ai,
tagsLimit: limits.tags,
foldersLimit: limits.folders,
groupsLimit: limits.groups,
networkInvitesLimit: limits.networkInvites,
usersLimit: limits.users,
- paymentFailedAt: null,
+ ...(!subscription ||
+ !["past_due", "unpaid"].includes(subscription.status)
+ ? { paymentFailedAt: null }
+ : {}),
...(trialEndsAt !== undefined && { trialEndsAt }),
...cancellationFields,
...(planPeriod !== undefined && { planPeriod }),
},📝 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.
Suggested change
| plan: newPlanName, | |
| planTier: newPlanTier, | |
| usageLimit: newPlan.limits.clicks, | |
| linksLimit: newPlan.limits.links, | |
| payoutsLimit: newPlan.limits.payouts, | |
| domainsLimit: newPlan.limits.domains, | |
| aiLimit: newPlan.limits.ai, | |
| tagsLimit: newPlan.limits.tags, | |
| foldersLimit: newPlan.limits.folders, | |
| groupsLimit: newPlan.limits.groups, | |
| networkInvitesLimit: newPlan.limits.networkInvites, | |
| usersLimit: newPlan.limits.users, | |
| usageLimit: limits.clicks, | |
| linksLimit: limits.links, | |
| payoutsLimit: limits.payouts, | |
| domainsLimit: limits.domains, | |
| aiLimit: limits.ai, | |
| tagsLimit: limits.tags, | |
| foldersLimit: limits.folders, | |
| groupsLimit: limits.groups, | |
| networkInvitesLimit: limits.networkInvites, | |
| usersLimit: limits.users, | |
| paymentFailedAt: null, | |
| ...(trialEndsAt !== undefined && { trialEndsAt }), | |
| ...cancellationFields, | |
| ...(planPeriod !== undefined && { planPeriod }), | |
| }, | |
| plan: newPlanName, | |
| planTier: newPlanTier, | |
| usageLimit: limits.clicks, | |
| linksLimit: limits.links, | |
| payoutsLimit: limits.payouts, | |
| domainsLimit: limits.domains, | |
| aiLimit: limits.ai, | |
| tagsLimit: limits.tags, | |
| foldersLimit: limits.folders, | |
| groupsLimit: limits.groups, | |
| networkInvitesLimit: limits.networkInvites, | |
| usersLimit: limits.users, | |
| ...(!subscription || | |
| !["past_due", "unpaid"].includes(subscription.status) | |
| ? { paymentFailedAt: null } | |
| : {}), | |
| ...(trialEndsAt !== undefined && { trialEndsAt }), | |
| ...cancellationFields, | |
| ...(planPeriod !== undefined && { planPeriod }), | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/app/`(ee)/api/stripe/webhook/utils/update-workspace-plan.ts around
lines 97 - 113, The code unconditionally sets paymentFailedAt: null which clears
the payment-failed UI on any subscription webhook; change this to only clear
paymentFailedAt when the subscription has actually recovered (e.g.,
subscription.status indicates paid/active). Remove the unconditional
paymentFailedAt: null and instead conditionally include it in the update object
(for example using ...(subscriptionStatusIsRecovered && { paymentFailedAt: null
})) so paymentFailedAt is preserved for past_due/unpaid events; use the existing
symbols (subscription/status, paymentFailedAt, trialEndsAt, cancellationFields,
planPeriod) to locate and implement the change.
Comment on lines
+159
to
+171
| // Checkout skips enabling dub.link during Stripe billing trial; turn it on when the | ||
| // subscription becomes active (e.g. trialing → active). | ||
| if (subscription?.status === "active" && newPlanName !== "free") { | ||
| await prisma.defaultDomains.updateMany({ | ||
| where: { | ||
| projectId: workspace.id, | ||
| dublink: false, | ||
| }, | ||
| data: { | ||
| dublink: true, | ||
| }, | ||
| }); | ||
| } |
Contributor
There was a problem hiding this comment.
Only re-enable dub.link after the plan write succeeds.
Promise.allSettled allows prisma.project.update(...) to fail while this block still flips dublink back on. That can leave domain entitlements enabled even though the workspace never reached the new paid state.
Proposed fix
- if (subscription?.status === "active" && newPlanName !== "free") {
+ if (
+ updatedWorkspace.status === "fulfilled" &&
+ subscription?.status === "active" &&
+ newPlanName !== "free"
+ ) {
await prisma.defaultDomains.updateMany({
where: {
projectId: workspace.id,
dublink: false,
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/app/`(ee)/api/stripe/webhook/utils/update-workspace-plan.ts around
lines 159 - 171, The defaultDomains.dublink toggle runs even if the project plan
write fails because you used Promise.allSettled; change the flow so the dublink
update (prisma.defaultDomains.updateMany) only runs after confirming
prisma.project.update succeeded: perform or await the project update (or inspect
the Promise.allSettled result for the prisma.project.update entry) and only when
its status is "fulfilled" and the workspace/newPlan persisted, then call
defaultDomains.updateMany to set dublink=true; keep the existing condition
(subscription?.status === "active" && newPlanName !== "free") but gate the
domain update behind the successful project write to avoid enabling entitlements
on failed plan changes.
Comment on lines
+13
to
+33
| try { | ||
| const trialingSubscription = await stripe.subscriptions | ||
| .list({ | ||
| customer: workspace.stripeId, | ||
| status: "trialing", | ||
| limit: 1, | ||
| }) | ||
| .then((res) => res.data[0]); | ||
|
|
||
| if (!trialingSubscription) { | ||
| throw new DubApiError({ | ||
| code: "bad_request", | ||
| message: "No trialing subscription found for this workspace.", | ||
| }); | ||
| } | ||
|
|
||
| await stripe.subscriptions.update(trialingSubscription.id, { | ||
| trial_end: "now", | ||
| }); | ||
|
|
||
| return NextResponse.json({ ok: true }); |
Contributor
There was a problem hiding this comment.
Make the end-trial action idempotent.
After the first successful update, the subscription is no longer trialing, so any retry returns "No trialing subscription found" even though the trial already ended. For a billing mutation, that turns harmless client retries into false failures. Treat the "already ended" state as success, e.g. by accepting an active subscription for the same customer.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/app/api/workspaces/`[idOrSlug]/billing/end-trial/route.ts around
lines 13 - 33, The current flow throws if no trialing subscription is found,
causing retries to surface as errors; make the end-trial action idempotent by
treating an already-ended trial as success: after listing trialingSubscription
(from stripe.subscriptions.list using workspace.stripeId), if
trialingSubscription is missing, call stripe.subscriptions.list again (or reuse
the same call) to check for an active subscription for the same customer (status
"active" or other non-trial statuses you consider valid) and return
NextResponse.json({ ok: true }) if one exists; only throw the DubApiError when
neither a trialing nor an active/non-trial subscription exists. Ensure you
update logic around trialingSubscription and the stripe.subscriptions.update
call to reflect this conditional flow.
Comment on lines
+29
to
+45
| function dedupeRecipients( | ||
| users: { user: { email: string | null; name: string | null } }[], | ||
| ): { email: string; name: string | null }[] { | ||
| const byEmail = new Map<string, { email: string; name: string | null }>(); | ||
|
|
||
| for (const { user } of users) { | ||
| if (!user.email) { | ||
| continue; | ||
| } | ||
|
|
||
| const key = user.email.toLowerCase(); | ||
| if (!byEmail.has(key)) { | ||
| byEmail.set(key, { email: user.email, name: user.name }); | ||
| } | ||
| } | ||
|
|
||
| return [...byEmail.values()]; |
Contributor
There was a problem hiding this comment.
Use a stable idempotency key for each recipient batch.
If sendBatchEmail succeeds and sentEmail.create then fails, this workspace will be retried later. Because the key currently depends on startingAfter, and recipients are not sorted before chunking, the same recipients can move to a different batch/key and get duplicate trial emails.
Proposed fix
+import { createHash } from "node:crypto";
+
function dedupeRecipients(
users: { user: { email: string | null; name: string | null } }[],
): { email: string; name: string | null }[] {
const byEmail = new Map<string, { email: string; name: string | null }>();
@@
- return [...byEmail.values()];
+ return [...byEmail.values()].sort((a, b) =>
+ a.email.localeCompare(b.email),
+ );
}
@@
- const pageKey = startingAfter ?? "start";
- const idempotencyKey = `trial-emails/${workspace.id}/${type}/${pageKey}/c${i}`;
+ const batchKey = createHash("sha256")
+ .update(batch.map(({ to }) => to.toLowerCase()).join(","))
+ .digest("hex");
+ const idempotencyKey = `trial-emails/${workspace.id}/${type}/${batchKey}`;Also applies to: 149-190
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/lib/email/run-trial-email-cron.ts` around lines 29 - 45, The current
idempotency key for sendBatchEmail depends on startingAfter and unstable
chunking order which can assign the same recipient to different keys causing
duplicates; to fix, ensure recipients are deterministically ordered (e.g., sort
by lowercased email) before deduping/chunking and compute the idempotency key
from that stable ordering (for example join of sorted emails plus workspace
id/email type) in the places that call sendBatchEmail and create sentEmail
records (refer to dedupeRecipients, sendBatchEmail invocation, and
sentEmail.create) so retries use the same idempotency key for the same recipient
set.
Comment on lines
+22
to
+54
| const existing = await prisma.sentEmail.findFirst({ | ||
| where: { | ||
| projectId, | ||
| type: dedupeType, | ||
| }, | ||
| }); | ||
|
|
||
| if (existing) { | ||
| return; | ||
| } | ||
|
|
||
| await sendEmail({ | ||
| to: ownerEmail, | ||
| subject: "Your Advanced plan features have been removed", | ||
| react: AdvancedPlanDowngradeNotice({ | ||
| email: ownerEmail, | ||
| workspace: { | ||
| name: workspaceName, | ||
| slug: workspaceSlug, | ||
| }, | ||
| }), | ||
| variant: "notifications", | ||
| headers: { | ||
| "Idempotency-Key": `${dedupeType}:${projectId}`, | ||
| }, | ||
| }); | ||
|
|
||
| await prisma.sentEmail.create({ | ||
| data: { | ||
| projectId, | ||
| type: dedupeType, | ||
| }, | ||
| }); |
Contributor
There was a problem hiding this comment.
Make the dedupe/send flow atomic.
Lines 22-54 do a read-then-send-then-write sequence. Two concurrent webhook deliveries can both pass findFirst() and attempt the same send before either create() lands, so this can still duplicate the notice. Reserve the (projectId, type) key atomically before the external call, or persist a pending/send status behind a unique constraint so only the winner sends.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/lib/email/send-advanced-downgrade-notice-email.ts` around lines 22 -
54, The current read-then-send-then-write flow using prisma.sentEmail.findFirst
+ sendEmail + prisma.sentEmail.create is racy; make the dedupe atomic by
reserving the (projectId, type) key before calling sendEmail: add a unique
constraint on (projectId, type) for sentEmail, attempt to create a record with a
provisional status (e.g., "pending") via prisma.sentEmail.create and treat a
unique-constraint error (Prisma P2002) as “already handled” and return, then
perform sendEmail only if create succeeded and finally update that record to
"sent" (or set sentAt) after successful send; ensure you handle send failures by
cleaning up or marking the record failed so retries can proceed. Reference
symbols: prisma.sentEmail, dedupeType, projectId, sendEmail,
AdvancedPlanDowngradeNotice.
Comment on lines
+68
to
+109
| const daysSinceStart = differenceInCalendarDaysUTC(now, trialStart); | ||
|
|
||
| const daysUntilEnd = differenceInCalendarDaysUTC(trialEndsAt, now); | ||
|
|
||
| const tryAdd = (type: TrialEmailType) => { | ||
| if (!sent.has(type) && !due.includes(type)) { | ||
| due.push(type); | ||
| } | ||
| }; | ||
|
|
||
| if ( | ||
| daysSinceStart === TRIAL_EMAIL_DAYS_FROM_START[TRIAL_EMAIL_TYPE.STARTED] | ||
| ) { | ||
| tryAdd(TRIAL_EMAIL_TYPE.STARTED); | ||
| } | ||
| if ( | ||
| daysSinceStart === TRIAL_EMAIL_DAYS_FROM_START[TRIAL_EMAIL_TYPE.LINKS_FOCUS] | ||
| ) { | ||
| tryAdd(TRIAL_EMAIL_TYPE.LINKS_FOCUS); | ||
| } | ||
| if ( | ||
| daysSinceStart === | ||
| TRIAL_EMAIL_DAYS_FROM_START[TRIAL_EMAIL_TYPE.PARTNER_FOCUS] | ||
| ) { | ||
| tryAdd(TRIAL_EMAIL_TYPE.PARTNER_FOCUS); | ||
| } | ||
| if ( | ||
| daysSinceStart === | ||
| TRIAL_EMAIL_DAYS_FROM_START[TRIAL_EMAIL_TYPE.SOCIAL_PROOF] | ||
| ) { | ||
| tryAdd(TRIAL_EMAIL_TYPE.SOCIAL_PROOF); | ||
| } | ||
|
|
||
| if (daysUntilEnd === 7) { | ||
| tryAdd(TRIAL_EMAIL_TYPE.SEVEN_DAYS_REMAINING); | ||
| } | ||
| if (daysUntilEnd === 3) { | ||
| tryAdd(TRIAL_EMAIL_TYPE.THREE_DAYS_REMAINING); | ||
| } | ||
| if (daysUntilEnd === 0) { | ||
| tryAdd(TRIAL_EMAIL_TYPE.ENDS_TODAY); | ||
| } |
Contributor
There was a problem hiding this comment.
Prevent missed or late trial emails due to exact-day matching.
Using only === day checks means a delayed/missed cron run drops that email forever. Also, daysUntilEnd === 0 can still be true after trialEndsAt has already passed on the same UTC date, causing a late “ends today” send. Add an explicit expiry guard and catch-up semantics.
Proposed reliability fix
export function getDueTrialEmailTypes({
trialEndsAt,
sent,
now,
}: {
trialEndsAt: Date;
sent: Set<string>;
now: Date;
}): TrialEmailType[] {
const due: TrialEmailType[] = [];
+ if (now > trialEndsAt) {
+ return due;
+ }
+
const trialStart = getTrialStartDate(trialEndsAt);
const daysSinceStart = differenceInCalendarDaysUTC(now, trialStart);
const daysUntilEnd = differenceInCalendarDaysUTC(trialEndsAt, now);
@@
- if (
- daysSinceStart === TRIAL_EMAIL_DAYS_FROM_START[TRIAL_EMAIL_TYPE.STARTED]
- ) {
+ if (daysSinceStart >= TRIAL_EMAIL_DAYS_FROM_START[TRIAL_EMAIL_TYPE.STARTED]) {
tryAdd(TRIAL_EMAIL_TYPE.STARTED);
}
@@
- if (
- daysSinceStart === TRIAL_EMAIL_DAYS_FROM_START[TRIAL_EMAIL_TYPE.LINKS_FOCUS]
- ) {
+ if (daysSinceStart >= TRIAL_EMAIL_DAYS_FROM_START[TRIAL_EMAIL_TYPE.LINKS_FOCUS]) {
tryAdd(TRIAL_EMAIL_TYPE.LINKS_FOCUS);
}
@@
- if (
- daysSinceStart ===
- TRIAL_EMAIL_DAYS_FROM_START[TRIAL_EMAIL_TYPE.PARTNER_FOCUS]
- ) {
+ if (daysSinceStart >= TRIAL_EMAIL_DAYS_FROM_START[TRIAL_EMAIL_TYPE.PARTNER_FOCUS]) {
tryAdd(TRIAL_EMAIL_TYPE.PARTNER_FOCUS);
}
@@
- if (
- daysSinceStart ===
- TRIAL_EMAIL_DAYS_FROM_START[TRIAL_EMAIL_TYPE.SOCIAL_PROOF]
- ) {
+ if (daysSinceStart >= TRIAL_EMAIL_DAYS_FROM_START[TRIAL_EMAIL_TYPE.SOCIAL_PROOF]) {
tryAdd(TRIAL_EMAIL_TYPE.SOCIAL_PROOF);
}
- if (daysUntilEnd === 7) {
+ if (daysUntilEnd <= 7) {
tryAdd(TRIAL_EMAIL_TYPE.SEVEN_DAYS_REMAINING);
}
- if (daysUntilEnd === 3) {
+ if (daysUntilEnd <= 3) {
tryAdd(TRIAL_EMAIL_TYPE.THREE_DAYS_REMAINING);
}
if (daysUntilEnd === 0) {
tryAdd(TRIAL_EMAIL_TYPE.ENDS_TODAY);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/lib/email/trial-email-schedule.ts` around lines 68 - 109, The
current exact-equality checks (daysSinceStart === ...) in
trial-email-schedule.ts can miss emails if the job is delayed and can send “ends
today” late after the trial day; update the logic around
differenceInCalendarDaysUTC, tryAdd, TRIAL_EMAIL_TYPE and
TRIAL_EMAIL_DAYS_FROM_START so each scheduled send uses a catch-up window and an
explicit expiry guard: for start/feature emails change the condition to treat
any daysSinceStart >= expected && daysSinceStart < expected + GRACE_DAYS
(introduce a small GRACE_DAYS constant like 1 or 2) and still skip if
sent.has(type); for countdown emails compute daysUntilEnd as
differenceInCalendarDaysUTC(trialEndsAt, now) and only allow sends when
daysUntilEnd === N and also ensure trialEndsAt is not already past a cutoff
(e.g., trialEndsAt >= endOfDayUTC(now) or daysUntilEnd >= 0) to prevent late
“ends today” sends; keep using tryAdd(...) to respect sent/due deduping.
Comment on lines
+39
to
+44
| planPeriod: z | ||
| .enum(["monthly", "yearly"]) | ||
| .nullish() | ||
| .describe( | ||
| "Billing cadence for the Stripe subscription (monthly vs yearly), when applicable.", | ||
| ), |
Contributor
There was a problem hiding this comment.
Expand planPeriod validation to include all persisted enum values.
WorkspaceSchema currently allows only monthly | yearly, but Project.planPeriod now supports quarterly and biweekly too. This can break schema parsing for valid DB rows (Line 39 onward).
🔧 Proposed fix
- planPeriod: z
- .enum(["monthly", "yearly"])
+ planPeriod: z
+ .enum(["monthly", "yearly", "quarterly", "biweekly"])
.nullish()📝 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.
Suggested change
| planPeriod: z | |
| .enum(["monthly", "yearly"]) | |
| .nullish() | |
| .describe( | |
| "Billing cadence for the Stripe subscription (monthly vs yearly), when applicable.", | |
| ), | |
| planPeriod: z | |
| .enum(["monthly", "yearly", "quarterly", "biweekly"]) | |
| .nullish() | |
| .describe( | |
| "Billing cadence for the Stripe subscription (monthly vs yearly), when applicable.", | |
| ), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/lib/zod/schemas/workspaces.ts` around lines 39 - 44, The
WorkspaceSchema's planPeriod enum only allows "monthly" and "yearly" but the
persisted Project.planPeriod can also be "quarterly" and "biweekly"; update the
planPeriod validation (the z.enum used for planPeriod in
apps/web/lib/zod/schemas/workspaces.ts) to include "quarterly" and "biweekly"
while preserving .nullish() and the existing description so schema parsing
accepts all stored values.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation