Conversation
WalkthroughAdds plan-limit infrastructure (constants, LimitService, limits API), UI gating with an upgrade modal and Zustand store, moves contact-book and team logic into services, centralizes usage and billing changes to support multiple Stripe price IDs (DB migration + env vars), and integrates limit checks across relevant UIs. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant UI as Client UI (Dialog/Button)
participant LS as limitsRouter.get
participant S as LimitService
participant DB as Prisma DB
participant M as UpgradeModal
U->>UI: Attempt action (Add Domain / Contact Book / Invite)
UI->>LS: limits.get({ type })
LS->>S: check*Limit(teamId)
S->>DB: Read team/related counts or usage
DB-->>S: Counts / plan / usage
S-->>LS: { isLimitReached, limit, reason? }
LS-->>UI: Limit result
alt Limit reached
UI->>M: openModal(reason)
M-->>U: Show upgrade prompt
else Under limit
UI->>UI: Proceed with mutation / open dialog
end
sequenceDiagram
participant C as Client (Admin)
participant P as payments.createCheckoutSessionForTeam
participant Stripe as Stripe
participant Sync as syncStripeData
participant DB as Prisma DB
C->>P: createCheckoutSessionForTeam(teamId)
P->>Stripe: create checkout session (line items include STRIPE_BASIC_USAGE_PRICE_ID)
Stripe-->>P: session, subscription id
Stripe->>Sync: webhook / sync call (subscription object)
Sync->>Stripe: read subscription.items -> price ids
Sync->>DB: upsert Subscription (priceIds = [...]) & update Team (plan from priceIds)
DB-->>Sync: confirm write
Sync-->>Stripe: ack
P-->>C: checkout session URL
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/web/src/lib/usage.ts (3)
10-19: Credits vs pricing mismatch (2.5 vs documented 4 transactional emails per unit).Comments say “4 transaction emails = 1 unit” but with UNIT_PRICE=0.001 and transactional price=0.0004, the conversion is 2.5 emails per unit. This inconsistency will ripple into quota math and cost displays.
Pick one model and align all helpers/docs:
- If you want pricing-driven units: 1 transactional email = 0.4 units. Update comments and any “credits” logic accordingly.
- If you want a fixed 4:1 credit model, set transactional price to 0.00025 or keep 0.0004 but decouple credits from price for quota math.
Apply this small doc correction if you adopt pricing-driven units:
- * 1 marketing email = 1 unit - * 4 transaction emails = 1 unit + * 1 marketing email = 1 unit + * 1 transactional email = 0.4 units (at $0.0004/email and $0.001 per unit)
59-67: Fix typo in exported API name and preserve backward compatibility.The exported name
getUsageUinitsis misspelled. This will cause foot-guns for callers and breaks discoverability.Apply this diff to introduce the correctly spelled API while keeping the old name as a deprecated alias:
-export function getUsageUinits( - marketingUsage: number, - transactionUsage: number -) { - return ( - marketingUsage + - Math.floor(transactionUsage / TRANSACTIONAL_UNIT_CONVERSION) - ); -} +// NOTE: Keep integer flooring only if your downstream collector (e.g., Stripe) requires integers. +export function getUsageUnits( + marketingUsage: number, + transactionUsage: number +) { + return ( + marketingUsage + + Math.floor(transactionUsage / TRANSACTIONAL_UNIT_CONVERSION) + ); +} + +/** + * @deprecated Typo in name. Use getUsageUnits instead. + */ +export const getUsageUinits = getUsageUnits;
69-76: Avoid under-billing: compute cost directly by per-email rates (don’t floor units per call).
getCostfloors transactional units, yielding $0 for 1–2 transactional emails and under-billing for small batches. Compute cost directly via the per-type price map.Apply:
-export function getCost(usage: number, type: EmailUsageType) { - const calculatedUsage = - type === EmailUsageType.MARKETING - ? usage - : Math.floor(usage / TRANSACTIONAL_UNIT_CONVERSION); - - return calculatedUsage * UNIT_PRICE; -} +export function getCost(usage: number, type: EmailUsageType) { + // Price per email; aggregate before calling if you want whole-period rounding. + return usage * USAGE_UNIT_PRICE[type]; +}apps/web/src/server/api/routers/contacts.ts (1)
71-116: Bug: ctx.db is not present here; and contactBookId should come from ctx, not input.
contactBookProcedure’s ctx containscontactBook, notdb; and your input schema no longer includescontactBookId. This will throw at runtime and/or fail type checks.Apply this diff, and add the import noted below:
- ).query(async ({ ctx: { db }, input }) => { + ).query(async ({ ctx: { contactBook }, input }) => { const page = input.page || 1; const limit = 30; const offset = (page - 1) * limit; const whereConditions: Prisma.ContactFindManyArgs["where"] = { - contactBookId: input.contactBookId, + contactBookId: contactBook.id, ...(input.subscribed !== undefined ? { subscribed: input.subscribed } : {}), ...(input.search ? { OR: [ { email: { contains: input.search, mode: "insensitive" } }, { firstName: { contains: input.search, mode: "insensitive" } }, { lastName: { contains: input.search, mode: "insensitive" } }, ], } : {}), }; - const countP = db.contact.count({ where: whereConditions }); + const countP = db.contact.count({ where: whereConditions }); - const contactsP = db.contact.findMany({ + const contactsP = db.contact.findMany({ where: whereConditions, select: { id: true, email: true, firstName: true, lastName: true, subscribed: true, createdAt: true, contactBookId: true, unsubscribeReason: true, }, orderBy: { createdAt: "desc", }, skip: offset, take: limit, }); const [contacts, count] = await Promise.all([contactsP, countP]); return { contacts, totalPage: Math.ceil(count / limit) }; }),Add this import at the top of the file (outside the diffed range):
import { db } from "~/server/db";
🧹 Nitpick comments (21)
apps/web/src/components/payments/PlanDetails.tsx (3)
14-16: Show a lightweight loading state instead of returning nullReturning null can cause layout jumps. You already import Spinner; render it (or a skeleton) while loading.
- if (subscriptionQuery.isLoading || !currentTeam) { - return null; - } + if (subscriptionQuery.isLoading || !currentTeam) { + return ( + <div className="flex items-center h-5"> + <Spinner /> + </div> + ); + }
31-33: Harden date formatting against string-serialized datesDepending on tRPC transformer, cancelAtPeriodEnd may arrive as a string. Wrapping with new Date makes this safe either way.
- Cancels {format(subscriptionQuery.data.cancelAtPeriodEnd, "MMM dd")} + Cancels {format(new Date(subscriptionQuery.data.cancelAtPeriodEnd), "MMM dd")}
37-41: Prefer using the perk string as the React key instead of the indexKeys should be stable; the string value is unique and avoids potential reordering issues.
- {perks.map((perk, index) => ( - <li key={index} className="flex items-center gap-2"> + {perks.map((perk) => ( + <li key={perk} className="flex items-center gap-2">apps/web/prisma/schema.prisma (1)
106-130: Consider indexes for frequent checks on plan/sending-disabledIf you’ll frequently gate actions based on team.plan or team.sendingDisabled, simple indexes can help.
model Team { // ...fields... suppressionList SuppressionList[] } +@@index([plan]) +@@index([sendingDisabled])apps/web/src/lib/usage.ts (3)
26-28: Type PLAN_CREDIT_UNITS to surface missing plan entries at compile-time.If additional Plan values exist (FREE/PRO/ENTERPRISE), this map currently has only BASIC.
-export const PLAN_CREDIT_UNITS = { - [Plan.BASIC]: 10_000, -}; +export const PLAN_CREDIT_UNITS: Partial<Record<Plan, number>> = { + [Plan.BASIC]: 10_000, +};If you intend this to be exhaustive, drop Partial<> and add all keys.
31-39: Normalize to UTC to avoid off-by-one day issues around DST/timezones.Both helpers use local time, but the “DailyEmailUsage"."date" appears to be a UTC date (YYYY-MM-DD). Prefer UTC midnight.
-export function getUsageTimestamp() { - const yesterday = new Date(); - yesterday.setDate(yesterday.getDate() - 1); - return Math.floor(yesterday.getTime() / 1000); -} +export function getUsageTimestamp() { + const now = new Date(); + const utcYesterday = new Date( + Date.UTC(now.getUTCFullYear(), now.getUTCMonth(), now.getUTCDate() - 1) + ); + return Math.floor(utcYesterday.getTime() / 1000); +} -export function getUsageDate(): string { - const yesterday = new Date(); - yesterday.setDate(yesterday.getDate() - 1); - const isoString = yesterday.toISOString(); - return isoString.split("T")[0] as string; -} +export function getUsageDate(): string { + const now = new Date(); + const utcYesterday = new Date( + Date.UTC(now.getUTCFullYear(), now.getUTCMonth(), now.getUTCDate() - 1) + ); + return utcYesterday.toISOString().slice(0, 10); +}Also applies to: 45-50
104-108: Use UTC dates for boundaries to match “date” column semantics.
format()uses local TZ; use ISO (UTC) to avoid fencepost errors on month/day cutovers.- const isoStartDate = subscription?.currentPeriodStart - ? format(subscription.currentPeriodStart, "yyyy-MM-dd") - : format(new Date(), "yyyy-MM-01"); // First day of current month - const today = format(new Date(), "yyyy-MM-dd"); + const now = new Date(); + const isoStartDate = subscription?.currentPeriodStart + ? subscription.currentPeriodStart.toISOString().slice(0, 10) + : new Date(Date.UTC(now.getUTCFullYear(), now.getUTCMonth(), 1)) + .toISOString() + .slice(0, 10); // First day of current month (UTC) + const today = new Date().toISOString().slice(0, 10); // UTC YYYY-MM-DDupdated-billing.md (3)
94-102: Path/name consistency: LimitService file casing differs.Doc references
apps/web/src/server/service/limit-service.ts, but the implementation in this PR isLimitService.ts. Align the docs to the actual path to avoid confusion.
261-279: Webhook section is high-level; add failure paths and idempotency notes.Stripe webhooks need: signature verification, idempotency (event replay), and handling of out-of-order events. Add explicit bullets for these to guide implementation.
Happy to draft a concrete checklist and example handler skeleton if useful.
1-6: Minor editorial cleanups for readability.Headings occasionally run together after code blocks; add blank lines before/after fenced blocks and between sections to improve rendering.
apps/web/src/server/api/routers/team.ts (1)
31-37: Validate email format at the router boundary.Tighten input with
z.string().email()to catch typos before hitting the service.- z.object({ - email: z.string(), + z.object({ + email: z.string().email(), role: z.enum(["MEMBER", "ADMIN"]), sendEmail: z.boolean().default(true), })apps/web/src/server/service/contact-book-service.ts (1)
20-29: Enforce non-empty names and trim input before create.Avoid creating blank/whitespace-only books.
-export async function createContactBook(teamId: number, name: string) { +export async function createContactBook(teamId: number, name: string) { + const trimmed = name.trim(); + if (!trimmed) { + throw new UnsendApiError({ code: "BAD_REQUEST", message: "Name is required" }); + } const { isLimitReached, reason } = await LimitService.checkContactBookLimit(teamId); if (isLimitReached) { throw new UnsendApiError({ code: "FORBIDDEN", message: reason ?? "Contact book limit reached", }); } - return db.contactBook.create({ + return db.contactBook.create({ data: { - name, + name: trimmed, teamId, properties: {}, }, }); }apps/web/src/server/service/LimitService.ts (3)
22-24: Use consistent error types (avoid raw Error) for better client mappingThrowing new Error loses status/code. Prefer TRPCError so callers/UI can handle uniformly.
- if (!team) { - throw new Error("Team not found"); - } + if (!team) { + throw new TRPCError({ code: "NOT_FOUND", message: "Team not found" }); + }Additionally add this import at the top of the file:
import { TRPCError } from "@trpc/server";Also applies to: 56-58, 86-88, 113-115
1-3: Nit: Align imports with repo guidelines (group & alphabetize)Group external first (none here), then internal; alphabetize within groups. Also include TRPCError if adopting the above change.
-import { PLAN_LIMITS } from "~/lib/constants/plans"; -import { db } from "../db"; -import { getThisMonthUsage } from "~/lib/usage"; +import { TRPCError } from "@trpc/server"; +import { db } from "../db"; +import { PLAN_LIMITS } from "~/lib/constants/plans"; +import { getThisMonthUsage } from "~/lib/usage";
6-38: Race window between "check" and "create" in callersThese methods are intended to gate creates. Without a transaction/lock in the caller, two concurrent requests can both pass the check and push the resource over the limit. Consider:
- Running the check and the subsequent create within a single db.$transaction and verifying the count again before commit.
- Enforcing limits at the mutation point (e.g., in the create method) rather than only in the controller.
Would you like me to propose a transaction pattern for Prisma here?
Also applies to: 40-72, 74-102, 104-148
apps/web/src/server/service/team-service.ts (6)
1-9: Nit: Reorder imports per guidelines and add Zod for input validation
- Group external imports first and alphabetize.
- Then internal imports, alphabetized.
- Add zod for email validation below.
-import { TRPCError } from "@trpc/server"; -import { env } from "~/env"; -import { db } from "~/server/db"; -import { sendTeamInviteEmail } from "~/server/mailer"; -import { logger } from "~/server/logger/log"; -import type { Team, TeamInvite } from "@prisma/client"; -import { LimitService } from "./LimitService"; -import { UnsendApiError } from "../public-api/api-error"; +import type { Team, TeamInvite } from "@prisma/client"; +import { TRPCError } from "@trpc/server"; +import { z } from "zod"; + +import { env } from "~/env"; +import { db } from "~/server/db"; +import { logger } from "~/server/logger/log"; +import { sendTeamInviteEmail } from "~/server/mailer"; +import { LimitService } from "./LimitService"; +// Removed: UnsendApiError (use TRPCError for consistency)
25-28: Clarify createTeam behavior when a team already exists (return existing or throw)Returning undefined is ambiguous and easy to mishandle. Prefer explicit behavior: either return the existing team (idempotent) or throw a CONFLICT.
Option A (idempotent):
- if (teams.length > 0) { - logger.info({ userId }, "User already has a team"); - return; - } + if (teams.length > 0) { + logger.info({ userId }, "User already has a team"); + return teams[0]; + }Option B (strict):
- if (teams.length > 0) { - logger.info({ userId }, "User already has a team"); - return; - } + if (teams.length > 0) { + throw new TRPCError({ + code: "CONFLICT", + message: "User already has a team", + }); + }
30-38: Self-hosted guard can still race; consider a unique constraint or transactionTwo concurrent createTeam calls can both pass the findFirst() check. In self-hosted mode, consider a transaction that rechecks before insert or a db-level uniqueness (e.g., a singleton sentinel row) to prevent multiple teams.
Would you like a migration pattern to enforce a “single team” constraint in self-hosted?
123-129: Differentiate “already in this team” vs “in another team” and message accordinglyCurrent check blocks inviting users who are already in any team. If the user is already in this team, return a clearer error; if in another team, keep current behavior. Also reduces confusion for admins.
- if (user && user.teamUsers.length > 0) { - throw new TRPCError({ - code: "BAD_REQUEST", - message: "User already part of a team", - }); - } + if (user) { + const inThisTeam = user.teamUsers.some((tu) => tu.teamId === teamId); + if (inThisTeam) { + throw new TRPCError({ + code: "BAD_REQUEST", + message: "User is already a member of this team", + }); + } + if (user.teamUsers.length > 0) { + throw new TRPCError({ + code: "BAD_REQUEST", + message: "User is already part of another team", + }); + } + }
214-219: Authorization trust on parameters; derive requester role server-sidedeleteTeamUser trusts requestorRole/requestorId arguments. Prefer deriving the requester’s membership/role from the session or DB to avoid accidental misuse of this internal API.
If the router already verifies this, consider narrowing this method’s signature to not accept requestorRole and to fetch role internally.
260-263: Add NEXTAUTH_URL guard and handle mailer errors on resendMirror the guard you added in createTeamInvite and wrap send in try/catch to produce a stable error shape.
- const teamUrl = `${env.NEXTAUTH_URL}/join-team?inviteId=${invite.id}`; - - await sendTeamInviteEmail(invite.email, teamUrl, teamName); + if (!env.NEXTAUTH_URL) { + throw new TRPCError({ + code: "INTERNAL_SERVER_ERROR", + message: "Missing NEXTAUTH_URL configuration", + }); + } + const teamUrl = `${env.NEXTAUTH_URL}/join-team?inviteId=${invite.id}`; + try { + await sendTeamInviteEmail(invite.email, teamUrl, teamName); + } catch { + throw new TRPCError({ code: "INTERNAL_SERVER_ERROR", message: "Failed to resend invite email" }); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (12)
apps/web/prisma/schema.prisma(1 hunks)apps/web/src/components/payments/PlanDetails.tsx(1 hunks)apps/web/src/lib/constants/plans.ts(1 hunks)apps/web/src/lib/usage.ts(2 hunks)apps/web/src/server/api/routers/billing.ts(2 hunks)apps/web/src/server/api/routers/contacts.ts(3 hunks)apps/web/src/server/api/routers/team.ts(3 hunks)apps/web/src/server/service/LimitService.ts(1 hunks)apps/web/src/server/service/contact-book-service.ts(1 hunks)apps/web/src/server/service/domain-service.ts(6 hunks)apps/web/src/server/service/team-service.ts(1 hunks)updated-billing.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
{apps,packages}/**/*.{js,jsx,ts,tsx,css,scss,md,mdx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Prettier with the Tailwind plugin for code formatting
Files:
apps/web/src/components/payments/PlanDetails.tsxapps/web/src/server/service/LimitService.tsapps/web/src/server/api/routers/team.tsapps/web/src/lib/constants/plans.tsapps/web/src/server/api/routers/billing.tsapps/web/src/server/service/contact-book-service.tsapps/web/src/server/service/domain-service.tsapps/web/src/server/service/team-service.tsapps/web/src/server/api/routers/contacts.tsapps/web/src/lib/usage.ts
{apps,packages}/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
{apps,packages}/**/*.{js,jsx,ts,tsx}: Group imports by source (internal/external) and alphabetize them
Use camelCase for variables and functions, PascalCase for components and classes
Use try/catch with specific error types for error handling
Files:
apps/web/src/components/payments/PlanDetails.tsxapps/web/src/server/service/LimitService.tsapps/web/src/server/api/routers/team.tsapps/web/src/lib/constants/plans.tsapps/web/src/server/api/routers/billing.tsapps/web/src/server/service/contact-book-service.tsapps/web/src/server/service/domain-service.tsapps/web/src/server/service/team-service.tsapps/web/src/server/api/routers/contacts.tsapps/web/src/lib/usage.ts
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
{apps,packages}/**/*.{ts,tsx}: Use strong typing in TypeScript, avoidany, and use Zod for validation
Follow Vercel style guides with strict TypeScript
Files:
apps/web/src/components/payments/PlanDetails.tsxapps/web/src/server/service/LimitService.tsapps/web/src/server/api/routers/team.tsapps/web/src/lib/constants/plans.tsapps/web/src/server/api/routers/billing.tsapps/web/src/server/service/contact-book-service.tsapps/web/src/server/service/domain-service.tsapps/web/src/server/service/team-service.tsapps/web/src/server/api/routers/contacts.tsapps/web/src/lib/usage.ts
{apps,packages}/**/*.{jsx,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
{apps,packages}/**/*.{jsx,tsx}: Use functional React components with hooks and group related hooks together
In React components, structure code with props at the top, hooks next, helper functions, then JSX
Files:
apps/web/src/components/payments/PlanDetails.tsx
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use tRPC for internal API endpoints
Files:
apps/web/src/components/payments/PlanDetails.tsxapps/web/src/server/service/LimitService.tsapps/web/src/server/api/routers/team.tsapps/web/src/lib/constants/plans.tsapps/web/src/server/api/routers/billing.tsapps/web/src/server/service/contact-book-service.tsapps/web/src/server/service/domain-service.tsapps/web/src/server/service/team-service.tsapps/web/src/server/api/routers/contacts.tsapps/web/src/lib/usage.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Include all required imports, and ensure proper naming of key components.
Files:
apps/web/src/components/payments/PlanDetails.tsxapps/web/src/server/service/LimitService.tsapps/web/src/server/api/routers/team.tsapps/web/src/lib/constants/plans.tsapps/web/src/server/api/routers/billing.tsapps/web/src/server/service/contact-book-service.tsapps/web/src/server/service/domain-service.tsapps/web/src/server/service/team-service.tsapps/web/src/server/api/routers/contacts.tsapps/web/src/lib/usage.ts
🧬 Code graph analysis (8)
apps/web/src/server/service/LimitService.ts (3)
apps/web/src/server/db.ts (1)
db(20-20)apps/web/src/lib/constants/plans.ts (1)
PLAN_LIMITS(3-27)apps/web/src/lib/usage.ts (1)
getThisMonthUsage(85-136)
apps/web/src/server/api/routers/team.ts (2)
apps/web/src/server/api/trpc.ts (3)
protectedProcedure(101-112)teamProcedure(114-139)teamAdminProcedure(141-150)apps/web/src/server/service/team-service.ts (1)
TeamService(10-293)
apps/web/src/server/api/routers/billing.ts (1)
apps/web/src/lib/usage.ts (1)
getThisMonthUsage(85-136)
apps/web/src/server/service/contact-book-service.ts (3)
apps/web/src/server/db.ts (1)
db(20-20)apps/web/src/server/service/LimitService.ts (1)
LimitService(5-148)apps/web/src/server/public-api/api-error.ts (1)
UnsendApiError(62-75)
apps/web/src/server/service/domain-service.ts (2)
apps/web/src/server/service/LimitService.ts (1)
LimitService(5-148)apps/web/src/server/public-api/api-error.ts (1)
UnsendApiError(62-75)
apps/web/src/server/service/team-service.ts (5)
apps/web/src/server/db.ts (1)
db(20-20)apps/web/src/server/logger/log.ts (1)
logger(31-63)apps/web/src/server/service/LimitService.ts (1)
LimitService(5-148)apps/web/src/server/public-api/api-error.ts (1)
UnsendApiError(62-75)apps/web/src/server/mailer.ts (1)
sendTeamInviteEmail(46-70)
apps/web/src/server/api/routers/contacts.ts (1)
apps/web/src/server/api/trpc.ts (3)
createTRPCRouter(82-82)teamProcedure(114-139)contactBookProcedure(191-209)
apps/web/src/lib/usage.ts (1)
apps/web/src/server/db.ts (1)
db(20-20)
🪛 LanguageTool
updated-billing.md
[grammar] ~3-~3: There might be a mistake here.
Context: ...lling Flow Enhancement Plan ## Overview Transform Unsend from a usage-only billi...
(QB_NEW_EN)
[grammar] ~8-~8: There might be a mistake here.
Context: ...ent State Analysis ### ✅ What's Working - Stripe Integration: Full integration w...
(QB_NEW_EN)
[grammar] ~9-~9: There might be a mistake here.
Context: ...ustomer creation, checkout, and webhooks - Database Schema: Solid foundation with...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ...Subscription, and DailyEmailUsage tables - Usage Tracking: Daily email usage trac...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: ...racked by type (transactional/marketing) - Payment Flows: Upgrade flow with Strip...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ... with Stripe checkout and billing portal - Plan Definitions: FREE and BASIC plans...
(QB_NEW_EN)
[grammar] ~16-~16: There might be a mistake here.
Context: ...ture limits ### 🔴 Critical Issues - No Limit Enforcement: FREE plan limits ...
(QB_NEW_EN)
[grammar] ~17-~17: There might be a mistake here.
Context: ...d but not enforced in email sending - Usage-Only Pricing: Missing subscriptio...
(QB_NEW_EN)
[grammar] ~18-~18: There might be a mistake here.
Context: ...g - only usage-based pricing exists - Missing Upgrade Prompts: No generic upg...
(QB_NEW_EN)
[grammar] ~19-~19: There might be a mistake here.
Context: ...ook, and team member limits not enforced ## Phase 1: Database & Plan Structure Update...
(QB_NEW_EN)
[grammar] ~338-~338: There might be a mistake here.
Context: ...Testing & Validation ### 7.1 Unit Tests - Limit checking service tests - Email ser...
(QB_NEW_EN)
[grammar] ~339-~339: There might be a mistake here.
Context: ...nit Tests - Limit checking service tests - Email service limit enforcement tests ...
(QB_NEW_EN)
[grammar] ~340-~340: There might be a mistake here.
Context: ... - Email service limit enforcement tests - Usage calculation tests - Billing servic...
(QB_NEW_EN)
[grammar] ~341-~341: There might be a mistake here.
Context: ...cement tests - Usage calculation tests - Billing service tests ### 7.2 Integrati...
(QB_NEW_EN)
[grammar] ~344-~344: There might be a mistake here.
Context: ...service tests ### 7.2 Integration Tests - Stripe webhook handling tests - End-to-e...
(QB_NEW_EN)
[grammar] ~345-~345: There might be a mistake here.
Context: ...on Tests - Stripe webhook handling tests - End-to-end upgrade flow tests - Limit en...
(QB_NEW_EN)
[grammar] ~346-~346: There might be a mistake here.
Context: ...ng tests - End-to-end upgrade flow tests - Limit enforcement in API tests ### 7.3 ...
(QB_NEW_EN)
[grammar] ~349-~349: There might be a mistake here.
Context: ... tests ### 7.3 Manual Testing Scenarios - Free user hitting email limits - Free us...
(QB_NEW_EN)
[grammar] ~350-~350: There might be a mistake here.
Context: ...enarios - Free user hitting email limits - Free user trying to create 2nd domain - ...
(QB_NEW_EN)
[grammar] ~351-~351: There might be a mistake here.
Context: ... - Free user trying to create 2nd domain - Upgrade flow from FREE to BASIC - Usage ...
(QB_NEW_EN)
[grammar] ~352-~352: There might be a mistake here.
Context: ...domain - Upgrade flow from FREE to BASIC - Usage billing calculation accuracy - Upg...
(QB_NEW_EN)
[grammar] ~353-~353: There might be a mistake here.
Context: ...SIC - Usage billing calculation accuracy - Upgrade modal triggering correctly ## I...
(QB_NEW_EN)
[grammar] ~358-~358: There might be a mistake here.
Context: ... Database updates and plan configuration 2. Phase 2: Limit enforcement in email se...
(QB_NEW_EN)
[grammar] ~359-~359: There might be a mistake here.
Context: ...orcement in email service (critical fix) 3. Phase 3: Generic upgrade modal system ...
(QB_NEW_EN)
[grammar] ~360-~360: There might be a mistake here.
Context: ...Phase 3*: Generic upgrade modal system 4. Phase 4: Integration points and error ...
(QB_NEW_EN)
[grammar] ~361-~361: There might be a mistake here.
Context: ...*: Integration points and error handling 5. Phase 5: Subscription billing implemen...
(QB_NEW_EN)
[grammar] ~362-~362: There might be a mistake here.
Context: ...5**: Subscription billing implementation 6. Phase 6: UI/UX improvements 7. **Phase...
(QB_NEW_EN)
[grammar] ~363-~363: There might be a mistake here.
Context: ...ation 6. Phase 6: UI/UX improvements 7. Phase 7: Testing and validation ## St...
(QB_NEW_EN)
[grammar] ~380-~380: There might be a mistake here.
Context: ...RICE_ID` 3. Products Configuration: - Combined subscription + usage products ...
(QB_NEW_EN)
[grammar] ~381-~381: There might be a mistake here.
Context: ...- Combined subscription + usage products - Usage reporting configuration - Billi...
(QB_NEW_EN)
[grammar] ~382-~382: There might be a mistake here.
Context: ...ducts - Usage reporting configuration - Billing thresholds setup This plan addr...
(QB_NEW_EN)
🔇 Additional comments (17)
apps/web/src/components/payments/PlanDetails.tsx (1)
9-9: Formatting-only change — LGTMNo functional impact.
apps/web/prisma/schema.prisma (1)
99-104: Enum addition looks goodClear, bounded reasons to track why sending is disabled.
apps/web/src/server/service/domain-service.ts (1)
9-9: Good call moving limit enforcement into a shared serviceCentralizing checks in LimitService keeps domain creation lean and consistent.
apps/web/src/server/api/routers/billing.ts (1)
28-30: Nice consolidation — usage logic now lives in a single helperThis reduces duplication and keeps router slim. No further issues spotted.
apps/web/src/lib/usage.ts (2)
109-130: LGTM on aggregation queries and parameterization.Queries are parameterized via Prisma’s template tag, grouped correctly, and typed. This should be performant with appropriate indexes on (teamId, date, type).
132-136: Return shape is minimal and predictable.Downstream reducers can safely sum over empty arrays; no need to prefill missing types.
apps/web/src/server/api/routers/team.ts (2)
12-16: Service delegation LGTM.Routing is thin and consistent; business logic is centralized in TeamService, aligning with the new architecture.
63-72: Delete user endpoint context threading looks correct.Passing both the requestor’s role and id gives the service enough info to enforce “last admin” and self-delete rules.
apps/web/src/server/service/contact-book-service.ts (3)
41-58: Good use of Promise.all for parallelism.Concurrent counts + recent campaigns reduce round-trips and improve latency.
6-18: Query includes counts and optional search; looks solid.The “insensitive” search mode is appropriate; count is scoped correctly.
81-83: Cascading delete is already configured in the Prisma schema• In schema.prisma, the Contact model’s relation to ContactBook includes
onDelete: Cascade(around line 319), ensuring that deleting a ContactBook will automatically remove its Contacts.
• No additional cleanup path is needed.apps/web/src/server/api/routers/contacts.ts (5)
15-17: LGTM – service extraction for getContactBooks.Keeps router thin and consistent with the new service pattern.
25-28: LGTM – createContactBook delegates and benefits from centralized limit checks.This ensures consistent enforcement across entry points.
31-41: LGTM – details aggregation is now centralized.Cleaner router; less duplication.
53-55: LGTM – update path is concise.Service handles the Prisma update directly.
58-61: LGTM – delete delegates cleanly.Consistent with the service-first approach.
apps/web/src/server/service/team-service.ts (1)
91-112: Good call: limit enforcement before creating invitesPre-checking member limits before inviting helps prevent oversubscription. With the unlimited-fix in LimitService, this flow will behave correctly across plans.
| const { isLimitReached, reason } = | ||
| await LimitService.checkDomainLimit(teamId); | ||
|
|
||
| if (isLimitReached) { | ||
| throw new UnsendApiError({ | ||
| code: "FORBIDDEN", | ||
| message: reason ?? "Domain limit reached", | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Bug spillover: With current LimitService, paid plans will be blocked here
Because PLAN_LIMITS uses -1 for “unlimited” and LimitService compares with >=, BASIC will always hit this path and throw FORBIDDEN on domain creation.
Fixing LimitService as suggested in plans.ts will resolve this without further changes here.
🤖 Prompt for AI Agents
In apps/web/src/server/service/domain-service.ts around lines 78 to 86, the
check here is being tripped because LimitService uses -1 to indicate “unlimited”
but compares using >= which causes paid plans to be treated as limit-reached; do
not change this throw site — instead update LimitService.checkDomainLimit (in
plans.ts/LimitService) so that when a plan limit is -1 it is treated as
unlimited (always return isLimitReached: false) or only perform the >=
comparison for non-negative limits, ensuring paid/unlimited plans no longer
cause this FORBIDDEN throw.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (13)
apps/web/src/store/upgradeModalStore.ts (3)
3-10: Export the store interface for wider type reuseExporting the interface helps consumers type selectors/mocks in tests.
-interface UpgradeModalStore { +export interface UpgradeModalStore { isOpen: boolean; reason?: string; action: { openModal: (reason?: string) => void; closeModal: () => void; }; }
4-8: Consider a structured reason type to decouple codes from display stringsIf callers pass reason codes (e.g.,
FREE_LIMIT_REACHED), the UI currently renders the raw code. A structured reason avoids leaking codes into UX and aligns with the new Prisma enum.Apply:
- reason?: string; + // Prefer message (for UX) and optional code (for analytics/logic) + reason?: { message: string; code?: "FREE_LIMIT_REACHED" | "BILLING_ISSUE" | "SPAM" };And update actions accordingly:
- openModal: (reason?: string) => set({ isOpen: true, reason }), + openModal: (reason?: { message: string; code?: "FREE_LIMIT_REACHED" | "BILLING_ISSUE" | "SPAM" }) => + set({ isOpen: true, reason }),Note: this requires a small change in
UpgradeModal.tsxto renderreason?.message.
12-19: Optional: add lightweight selectors to minimize re-rendersUsing selectors helps components subscribe only to the slices they need.
// Add alongside the existing export export const useUpgradeModalIsOpen = () => useUpgradeModalStore((s) => s.isOpen); export const useUpgradeModalReason = () => useUpgradeModalStore((s) => s.reason); export const useUpgradeModalActions = () => useUpgradeModalStore((s) => s.action);apps/web/src/app/(dashboard)/dasboard-layout.tsx (2)
3-7: Group and consolidate imports per guidelinesCombine duplicated module imports and group external/internal imports for clarity.
-import { AppSidebar } from "~/components/AppSideBar"; -import { SidebarInset, SidebarTrigger } from "@unsend/ui/src/sidebar"; -import { SidebarProvider } from "@unsend/ui/src/sidebar"; -import { useIsMobile } from "@unsend/ui/src/hooks/use-mobile"; +import { SidebarInset, SidebarProvider, SidebarTrigger } from "@unsend/ui/src/sidebar"; +import { useIsMobile } from "@unsend/ui/src/hooks/use-mobile"; +import { AppSidebar } from "~/components/AppSideBar"; import { UpgradeModal } from "~/components/payments/UpgradeModal";
1-1: Nit: consider correcting the filename to 'dashboard-layout.tsx'Avoids the lingering typo in paths/imports and improves discoverability. Defer if other tooling or routes rely on the current name.
apps/web/src/app/(dashboard)/domains/add-domain.tsx (4)
80-84: Refetch limits at submit time to avoid stale quota checksRelying on cached
limitsQuery.datacan be stale under concurrent team activity. Refetch before deciding to open the upgrade modal.- if (limitsQuery.data?.isLimitReached) { - openModal(limitsQuery.data.reason); - return; - } + // Always re-check limits at submit time to avoid stale client cache. + const { data: latest } = await limitsQuery.refetch(); + if (latest?.isLimitReached) { + openModal(latest.reason); + return; + }
183-185: Prevent double-submit while limits are refetchingIf you refetch limits on submit, also disable the button during
isFetchingto prevent back-to-back submissions.- disabled={ - addDomainMutation.isPending || limitsQuery.isLoading - } + disabled={ + addDomainMutation.isPending || + limitsQuery.isLoading || + limitsQuery.isFetching + }
53-53: Scope the limit check to when the dialog is open (fewer background requests)This avoids fetching limits on every page render and keeps the data fresh right when the user intends to add a domain.
- const limitsQuery = api.limits.get.useQuery({ type: "DOMAIN" }); + const limitsQuery = api.limits.get.useQuery( + { type: "DOMAIN" as const }, + { enabled: open, staleTime: 60_000 } + );
71-78: Either inline the validation or use the normalized value
domainis only used for a truthy-check. Either inline the call or intentionally use the normalized value for submission (if backend expects apex).Option A (inline the check, keep submitting
values.domainas-is):- const domain = tldts.getDomain(values.domain); - if (!domain) { + if (!tldts.getDomain(values.domain)) { domainForm.setError("domain", { message: "Invalid domain", }); return; }If the backend expects apex domains, switch to using the normalized
domainin the payload instead (confirm desired behavior before changing).apps/web/src/server/api/routers/limits.ts (2)
5-10: Export the enum (and type) to prevent FE/BE drift on limit kindsExposing a type alias allows reuse across server modules and, if you later move it to a shared package, the client can consume it without string literals.
-const LimitTypeEnum = z.enum([ +export const LimitTypeEnum = z.enum([ "CONTACT_BOOK", "DOMAIN", "TEAM_MEMBER", "EMAIL", ]); +export type LimitType = z.infer<typeof LimitTypeEnum>;
19-33: Use a typed handler map for exhaustiveness and less branchingThis removes the unreachable default, enforces coverage at compile time, and is more maintainable if new limit types are added.
- .query(async ({ ctx, input }) => { - switch (input.type) { - case "CONTACT_BOOK": - return LimitService.checkContactBookLimit(ctx.team.id); - case "DOMAIN": - return LimitService.checkDomainLimit(ctx.team.id); - case "TEAM_MEMBER": - return LimitService.checkTeamMemberLimit(ctx.team.id); - case "EMAIL": - return LimitService.checkEmailLimit(ctx.team.id); - default: - // exhaustive guard - throw new Error("Unsupported limit type"); - } - }), + .query(async ({ ctx, input }) => { + const handlers = { + CONTACT_BOOK: LimitService.checkContactBookLimit, + DOMAIN: LimitService.checkDomainLimit, + TEAM_MEMBER: LimitService.checkTeamMemberLimit, + EMAIL: LimitService.checkEmailLimit, + } as const satisfies Record< + z.infer<typeof LimitTypeEnum>, + (teamId: number) => Promise<{ isLimitReached: boolean; limit: number; reason?: string }> + >; + return handlers[input.type](ctx.team.id); + }),apps/web/src/components/payments/UpgradeModal.tsx (2)
28-33: Avoid hard-coding “Basic” as the upgrade targetIf plans evolve, drive the title and perks from a “next plan” derived from the user’s current plan or from the server. Keeps UX consistent with billing logic.
40-45: Use a stable key instead of array indexPerks are static strings—using the string as the key avoids issues if the list changes or reorders.
- {basicPlanPerks.map((perk, index) => ( - <li key={index} className="flex items-start gap-2"> + {basicPlanPerks.map((perk) => ( + <li key={perk} className="flex items-start gap-2">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
apps/web/package.json(2 hunks)apps/web/prisma/schema.prisma(1 hunks)apps/web/src/app/(dashboard)/dasboard-layout.tsx(2 hunks)apps/web/src/app/(dashboard)/domains/add-domain.tsx(4 hunks)apps/web/src/components/payments/UpgradeModal.tsx(1 hunks)apps/web/src/server/api/root.ts(2 hunks)apps/web/src/server/api/routers/limits.ts(1 hunks)apps/web/src/server/service/LimitService.ts(1 hunks)apps/web/src/store/upgradeModalStore.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/src/server/service/LimitService.ts
🧰 Additional context used
📓 Path-based instructions (6)
{apps,packages}/**/*.{js,jsx,ts,tsx,css,scss,md,mdx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Prettier with the Tailwind plugin for code formatting
Files:
apps/web/src/server/api/routers/limits.tsapps/web/src/app/(dashboard)/dasboard-layout.tsxapps/web/src/app/(dashboard)/domains/add-domain.tsxapps/web/src/server/api/root.tsapps/web/src/components/payments/UpgradeModal.tsxapps/web/src/store/upgradeModalStore.ts
{apps,packages}/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
{apps,packages}/**/*.{js,jsx,ts,tsx}: Group imports by source (internal/external) and alphabetize them
Use camelCase for variables and functions, PascalCase for components and classes
Use try/catch with specific error types for error handling
Files:
apps/web/src/server/api/routers/limits.tsapps/web/src/app/(dashboard)/dasboard-layout.tsxapps/web/src/app/(dashboard)/domains/add-domain.tsxapps/web/src/server/api/root.tsapps/web/src/components/payments/UpgradeModal.tsxapps/web/src/store/upgradeModalStore.ts
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
{apps,packages}/**/*.{ts,tsx}: Use strong typing in TypeScript, avoidany, and use Zod for validation
Follow Vercel style guides with strict TypeScript
Files:
apps/web/src/server/api/routers/limits.tsapps/web/src/app/(dashboard)/dasboard-layout.tsxapps/web/src/app/(dashboard)/domains/add-domain.tsxapps/web/src/server/api/root.tsapps/web/src/components/payments/UpgradeModal.tsxapps/web/src/store/upgradeModalStore.ts
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use tRPC for internal API endpoints
Files:
apps/web/src/server/api/routers/limits.tsapps/web/src/app/(dashboard)/dasboard-layout.tsxapps/web/src/app/(dashboard)/domains/add-domain.tsxapps/web/src/server/api/root.tsapps/web/src/components/payments/UpgradeModal.tsxapps/web/src/store/upgradeModalStore.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Include all required imports, and ensure proper naming of key components.
Files:
apps/web/src/server/api/routers/limits.tsapps/web/src/app/(dashboard)/dasboard-layout.tsxapps/web/src/app/(dashboard)/domains/add-domain.tsxapps/web/src/server/api/root.tsapps/web/src/components/payments/UpgradeModal.tsxapps/web/src/store/upgradeModalStore.ts
{apps,packages}/**/*.{jsx,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
{apps,packages}/**/*.{jsx,tsx}: Use functional React components with hooks and group related hooks together
In React components, structure code with props at the top, hooks next, helper functions, then JSX
Files:
apps/web/src/app/(dashboard)/dasboard-layout.tsxapps/web/src/app/(dashboard)/domains/add-domain.tsxapps/web/src/components/payments/UpgradeModal.tsx
🧬 Code graph analysis (5)
apps/web/src/server/api/routers/limits.ts (2)
apps/web/src/server/api/trpc.ts (2)
createTRPCRouter(82-82)teamProcedure(114-139)apps/web/src/server/service/LimitService.ts (1)
LimitService(10-160)
apps/web/src/app/(dashboard)/dasboard-layout.tsx (1)
apps/web/src/components/payments/UpgradeModal.tsx (1)
UpgradeModal(15-54)
apps/web/src/app/(dashboard)/domains/add-domain.tsx (1)
apps/web/src/store/upgradeModalStore.ts (1)
useUpgradeModalStore(12-19)
apps/web/src/server/api/root.ts (1)
apps/web/src/server/api/routers/limits.ts (1)
limitsRouter(12-34)
apps/web/src/components/payments/UpgradeModal.tsx (3)
apps/web/src/store/upgradeModalStore.ts (1)
useUpgradeModalStore(12-19)apps/web/src/lib/constants/payments.ts (1)
PLAN_PERKS(1-17)apps/web/src/components/payments/UpgradeButton.tsx (1)
UpgradeButton(5-24)
🔇 Additional comments (7)
apps/web/prisma/schema.prisma (1)
99-104: LGTM — new enum fits the limits/billing flow
SendingDisabledReasonlooks well-scoped and future-proofs how we communicate why sending is disabled. No issues with naming or value set from a data-modeling standpoint.apps/web/package.json (2)
47-47: Zustand addition and deps reshuffle look fine
- Adding
zustandfor UI state is appropriate given the new Upgrade modal.- Reordering
jsx-emailand the trailing comma afterzodare harmless.Also applies to: 70-71
70-71: Zustand import style is consistent with version 5.xThe project declares
"zustand": "^5.0.8"in apps/web/package.json, and the only import found—import { create } from "zustand";in apps/web/src/store/upgradeModalStore.ts—matches the named‐import pattern introduced in v5. No default imports are present, so everything is aligned. No changes needed.apps/web/src/store/upgradeModalStore.ts (1)
1-19: Solid, minimal store for modal state
- Simple boolean gate + optional reason string is sufficient.
- Action methods are stable references and safe for component usage.
apps/web/src/app/(dashboard)/dasboard-layout.tsx (1)
7-7: Mounting UpgradeModal at layout level is a good callKeeps the modal accessible app-wide without prop drilling. No SSR/hydration concerns since initial
isOpenis false.Also applies to: 25-25
apps/web/src/server/api/root.ts (2)
14-14: limits router wiring looks correctRoute is properly registered in
appRouterand included in the exportedAppRoutertype.Also applies to: 34-35
14-14: Sanity check passed: router uses authenticatedteamProcedureandLimitTypeEnumis exhaustive
limitsRouter.getis defined withteamProcedure(imported from~/server/api/trpc), ensuring it’s only callable by an authenticated team context (lines 2–3, 12–14).LimitTypeEnumdeclares exactly four variants (CONTACT_BOOK,DOMAIN,TEAM_MEMBER,defaultguard to catch any unsupported value (lines 5–10, 20–31).- Client code (e.g. in
add-domain.tsx) invokesapi.limits.get.useQuery({ type: "DOMAIN" }), which aligns with the enum and is statically type‐checked by tRPC/Zod.No further action needed.
- Add limit checking and upgrade modal integration to add-contact-book component - Add limit checking and upgrade modal integration to invite-team-member component - Prevent actions when plan limits are reached and guide users to upgrade - Maintain consistent UX pattern across all components with limits
- Add proper LimitReason enum for type safety - Refactor LimitService to limit-service following naming conventions - Extract usage utilities to dedicated usage-service module - Improve upgrade modal with specific limit messages - Update domain add component to use LimitReason enum - Standardize limit checking across all services - Remove usage functions from lib/usage.ts and consolidate in service layer
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/lib/usage.ts (1)
3-6: Units math contradicts doc; transactional emails per unit is 2.5, not 4 (billing under/over-charge risk).Docs say “4 transaction emails = 1 unit” but 0.001 / 0.0004 = 2.5 and getCost floors fractional units, undercharging. Align constants and remove lossy flooring.
Apply either of the following. Option A minimally fixes prices; Option B makes prices derive from an explicit conversion source of truth.
Option A (quick fix: correct price for transactional):
export const USAGE_UNIT_PRICE: Record<EmailUsageType, number> = { [EmailUsageType.MARKETING]: 0.001, - [EmailUsageType.TRANSACTIONAL]: 0.0004, + [EmailUsageType.TRANSACTIONAL]: 0.00025, }; export const UNIT_PRICE = 0.001; export const TRANSACTIONAL_UNIT_CONVERSION = UNIT_PRICE / USAGE_UNIT_PRICE[EmailUsageType.TRANSACTIONAL]; -export function getCost(usage: number, type: EmailUsageType) { - const calculatedUsage = - type === EmailUsageType.MARKETING - ? usage - : Math.floor(usage / TRANSACTIONAL_UNIT_CONVERSION); - - return calculatedUsage * UNIT_PRICE; -} +export function getCost(usage: number, type: EmailUsageType) { + // Bill precisely by email count and per-email price; no lossy flooring + return usage * USAGE_UNIT_PRICE[type]; +}Option B (preferred: single source of truth for conversion):
export const UNIT_PRICE = 0.001; +export const TRANSACTIONAL_EMAILS_PER_UNIT = 4 as const; -export const TRANSACTIONAL_UNIT_CONVERSION = - UNIT_PRICE / USAGE_UNIT_PRICE[EmailUsageType.TRANSACTIONAL]; +export const TRANSACTIONAL_UNIT_CONVERSION = TRANSACTIONAL_EMAILS_PER_UNIT; +export const USAGE_UNIT_PRICE: Record<EmailUsageType, number> = { + [EmailUsageType.MARKETING]: UNIT_PRICE, + [EmailUsageType.TRANSACTIONAL]: + UNIT_PRICE / TRANSACTIONAL_EMAILS_PER_UNIT, +}; + export function getCost(usage: number, type: EmailUsageType) { - const calculatedUsage = - type === EmailUsageType.MARKETING - ? usage - : Math.floor(usage / TRANSACTIONAL_UNIT_CONVERSION); - - return calculatedUsage * UNIT_PRICE; + return usage * USAGE_UNIT_PRICE[type]; }Also applies to: 8-16, 67-74
♻️ Duplicate comments (2)
apps/web/src/server/api/routers/billing.ts (1)
33-36: Deterministic subscription selection (filter by active/trialing; order by latest period).Ordering by status asc is brittle and can return a canceled/inactive row.
- const subscription = await db.subscription.findFirst({ - where: { teamId: ctx.team.id }, - orderBy: { status: "asc" }, - }); + const subscription = await db.subscription.findFirst({ + where: { + teamId: ctx.team.id, + status: { in: ["active", "trialing"] }, + }, + orderBy: { currentPeriodStart: "desc" }, + });Run this to confirm allowed statuses in your schema and handlers:
#!/bin/bash rg -nP --glob 'apps/web/**' -C2 '\bstatus\s*:\s*["'\''](active|trialing|canceled|incomplete|past_due|unpaid)["'\'']'apps/web/src/server/service/usage-service.ts (1)
24-29: Deterministic subscription lookup (active/trialing + latest currentPeriodStart).Same issue as router. Use a stable filter and sort or you may pick an inactive period.
- if (isPaidPlan) { - subscription = await db.subscription.findFirst({ - where: { teamId: team.id }, - orderBy: { status: "asc" }, - }); - } + if (isPaidPlan) { + subscription = await db.subscription.findFirst({ + where: { + teamId: team.id, + status: { in: ["active", "trialing"] }, + }, + orderBy: { currentPeriodStart: "desc" }, + }); + }
🧹 Nitpick comments (25)
apps/web/src/lib/usage.ts (3)
57-65: Typo in exported API: getUsageUinits → getUsageUnits (keep back-compat alias).Avoid propagating misspelling across codebase. Add a typed alias to prevent breakage.
-export function getUsageUinits( +export function getUsageUnits( marketingUsage: number, transactionUsage: number ) { return ( marketingUsage + Math.floor(transactionUsage / TRANSACTIONAL_UNIT_CONVERSION) ); } + +// Back-compat; consider deprecating in next minor +export const getUsageUinits = getUsageUnits;
33-37: Normalize to UTC to avoid off-by-one around midnight.Local timezone math can drift from DB “YYYY-MM-DD” values. Use UTC ISO dates for consistency.
export function getUsageTimestamp() { - const yesterday = new Date(); - yesterday.setDate(yesterday.getDate() - 1); - return Math.floor(yesterday.getTime() / 1000); + const now = new Date(); + const utcMidnight = Date.UTC( + now.getUTCFullYear(), + now.getUTCMonth(), + now.getUTCDate() - 1, + ); + return Math.floor(utcMidnight / 1000); } export function getUsageDate(): string { - const yesterday = new Date(); - yesterday.setDate(yesterday.getDate() - 1); - const isoString = yesterday.toISOString(); - return isoString.split("T")[0] as string; + const now = new Date(); + const y = new Date(Date.UTC( + now.getUTCFullYear(), + now.getUTCMonth(), + now.getUTCDate() - 1, + )); + return y.toISOString().slice(0, 10); }Also applies to: 43-48
24-26: PLAN_CREDIT_UNITS only defines BASIC; verify callers don’t index FREE (undefined).If any code indexes FREE here, it will read undefined. If FREE intentionally has 0, add it explicitly.
I can add a safe default or migrate callers; want me to scan usages and patch?
apps/web/src/app/(dashboard)/domains/add-domain.tsx (2)
104-111: Also gate when limits are loading to avoid brief dialog open before modal.Optional: if limits are still loading, avoid opening the dialog and either show a spinner or refetch first.
function onOpenChange(_open: boolean) { - if (_open && limitsQuery.data?.isLimitReached) { + if (_open && (limitsQuery.isLoading || limitsQuery.data?.isLimitReached)) { openModal(limitsQuery.data.reason); return; } setOpen(_open); }
54-57: Ensure limits freshness before critical actions.Consider passing staleTime: 0 and refetchOnWindowFocus: true to keep limits current, especially if users upgrade in another tab.
-const limitsQuery = api.limits.get.useQuery({ type: LimitReason.DOMAIN }); +const limitsQuery = api.limits.get.useQuery( + { type: LimitReason.DOMAIN }, + { staleTime: 0, refetchOnWindowFocus: true } +);apps/web/src/server/api/routers/billing.ts (1)
1-4: Trim unused imports to satisfy strict TS and lints.Imports of DailyEmailUsage, EmailUsageType, Subscription, format, sub appear unused after refactor.
-import { DailyEmailUsage, EmailUsageType, Subscription } from "@prisma/client"; +import { Subscription } from "@prisma/client"; -import { format, sub } from "date-fns"; +// date-fns no longer used hereapps/web/src/server/service/usage-service.ts (3)
22-23: Prefer enum over string literal for plan comparison.Use Prisma’s Plan enum for type safety.
-import { EmailUsageType, Subscription } from "@prisma/client"; +import { EmailUsageType, Subscription, Plan } from "@prisma/client"; @@ - const isPaidPlan = team.plan !== "FREE"; + const isPaidPlan = team.plan !== Plan.FREE;
31-35: Normalize dates to UTC strings to match “YYYY-MM-DD” semantics.Avoid local-time skew when generating ISO dates.
- const isoStartDate = subscription?.currentPeriodStart - ? format(subscription.currentPeriodStart, "yyyy-MM-dd") - : format(new Date(), "yyyy-MM-01"); // First day of current month - const today = format(new Date(), "yyyy-MM-dd"); + const isoStartDate = subscription?.currentPeriodStart + ? new Date(subscription.currentPeriodStart).toISOString().slice(0, 10) + : new Date(Date.UTC( + new Date().getUTCFullYear(), + new Date().getUTCMonth(), + 1 + )).toISOString().slice(0, 10); // First day of current month (UTC) + const today = new Date().toISOString().slice(0, 10);
5-11: JSDoc params don’t match signature (no db/subscription params).Update or remove misleading docs.
/** * Gets the monthly and daily usage for a team - * @param teamId - The team ID to get usage for - * @param db - Prisma database client - * @param subscription - Optional subscription to determine billing period start - * @returns Object containing month and day usage arrays + * @param teamId The team ID + * @returns { month: Array<{ type: EmailUsageType; sent: number }>, day: Array<{ type: EmailUsageType; sent: number }> } */apps/web/src/server/service/limit-service.ts (2)
86-91: Avoid loading all teamUsers; use _count for scalability.Counting via length forces fetching full rows. Use Prisma’s _count for O(1) count and less memory.
- const team = await db.team.findUnique({ - where: { id: teamId }, - include: { - teamUsers: true, - }, - }); + const team = await db.team.findUnique({ + where: { id: teamId }, + include: { + _count: { select: { teamUsers: true } }, + }, + }); @@ - if (isLimitExceeded(team.teamUsers.length, limit)) { + if (isLimitExceeded(team._count.teamUsers, limit)) { return { isLimitReached: true, limit, reason: LimitReason.TEAM_MEMBER, }; }Also applies to: 97-104
112-159: Email limits sum raw sends across types; confirm intended semantics vs “credits/units”.FREE plan limits are named “emailsPerDay/Month”. If you intend “credits” (e.g., transactional weighting), you should weight types before comparison.
If weighting is intended:
- const monthlyUsage = usage.month.reduce( - (acc, curr) => acc + curr.sent, - 0, - ); - const dailyUsage = usage.day.reduce((acc, curr) => acc + curr.sent, 0); + const weight = (t: typeof usage.month[number]["type"]) => + t === "TRANSACTIONAL" ? 1 / TRANSACTIONAL_UNIT_CONVERSION : 1; // or derive from USAGE_UNIT_PRICE + const monthlyUsage = usage.month.reduce( + (acc, { type, sent }) => acc + sent * weight(type), + 0 + ); + const dailyUsage = usage.day.reduce( + (acc, { type, sent }) => acc + sent * weight(type), + 0 + );Otherwise, consider renaming limits/constants/docs to “emails” to avoid confusion with “credits”.
apps/web/src/app/(dashboard)/settings/team/invite-team-member.tsx (7)
55-59: Avoid unnecessary network calls when the user isn’t an adminGate the limits query so it doesn’t fire for non-admins (the component returns
nullfor them anyway).- const limitsQuery = api.limits.get.useQuery({ - type: LimitReason.TEAM_MEMBER, - }); + const limitsQuery = api.limits.get.useQuery( + { type: LimitReason.TEAM_MEMBER }, + { enabled: currentIsAdmin }, + );Optional: after successful invite creation, also invalidate this limits query so the UI reflects updated usage (see notes below).
75-79: Fail gracefully when the limits query errors and provide a fallback reasonRight now we only block on “limit reached.” If the limits query errors, actions proceed and can bypass the intended gating. Also guard
reasonwith a fallback.- if (limitsQuery.data?.isLimitReached) { - openModal(limitsQuery.data.reason); - return; - } + if (limitsQuery.isError) { + toast.error("Unable to check your plan limits. Please try again shortly."); + return; + } + if (limitsQuery.data?.isLimitReached) { + openModal(limitsQuery.data?.reason ?? LimitReason.TEAM_MEMBER); + return; + }Follow-up (optional, no diff shown here): after a successful invite, call
void utils.limits.get.invalidate({ type: LimitReason.TEAM_MEMBER });
so the on-screen gating reflects the latest count.Please verify that
openModalindeed expects aLimitReason(or a compatible type) and will handle the fallback correctly.
102-106: Mirror the same error handling in Copy LinkKeep gating behavior consistent between “Send Invitation” and “Copy Invitation.”
- if (limitsQuery.data?.isLimitReached) { - openModal(limitsQuery.data.reason); - return; - } + if (limitsQuery.isError) { + toast.error("Unable to check your plan limits. Please try again shortly."); + return; + } + if (limitsQuery.data?.isLimitReached) { + openModal(limitsQuery.data?.reason ?? LimitReason.TEAM_MEMBER); + return; + }
114-121: Handle clipboard failures and avoid optimistic success toastThe current code doesn’t await or handle failures from
navigator.clipboard.writeText. Prefer explicit handling to give the user clear feedback.- onSuccess: (invite) => { - void utils.team.getTeamInvites.invalidate(); - navigator.clipboard.writeText( - `${location.origin}/join-team?inviteId=${invite.id}`, - ); - form.reset(); - setOpen(false); - toast.success("Invitation link copied to clipboard"); - }, + onSuccess: (invite) => { + void utils.team.getTeamInvites.invalidate(); + void navigator.clipboard + .writeText( + new URL( + `/join-team?inviteId=${invite.id}`, + window.location.origin, + ).toString(), + ) + .then(() => { + form.reset(); + setOpen(false); + toast.success("Invitation link copied to clipboard"); + }) + .catch((err) => { + console.error(err); + toast.error("Failed to copy invitation link to clipboard"); + }); + },Optional: also invalidate limits after success
void utils.limits.get.invalidate({ type: LimitReason.TEAM_MEMBER });
to keep the gating state fresh.
131-138: Guard dialog opening on limits query errors and use a fallback reasonBlock opening the dialog if we cannot determine limits; pass a safe fallback for
reason.function onOpenChange(_open: boolean) { - if (_open && limitsQuery.data?.isLimitReached) { - openModal(limitsQuery.data.reason); + if (_open && limitsQuery.isError) { + toast.error("Unable to check your plan limits. Please try again shortly."); + return; + } + if (_open && limitsQuery.data?.isLimitReached) { + openModal(limitsQuery.data?.reason ?? LimitReason.TEAM_MEMBER); return; } setOpen(_open); }
145-148: Simplify Dialog’s onOpenChange handlerShadcn’s Dialog won’t call
onOpenChangeredundantly. The equality check is unnecessary; pass the handler directly.- <Dialog - open={open} - onOpenChange={(_open) => (_open !== open ? onOpenChange(_open) : null)} - > + <Dialog open={open} onOpenChange={onOpenChange}>
233-246: Disable actions if the limits check errorsPrevent actions when we can’t validate limits. This keeps gating consistent and avoids accidental bypasses during transient failures.
- disabled={createInvite.isPending || limitsQuery.isLoading} + disabled={ + createInvite.isPending || + limitsQuery.isLoading || + limitsQuery.isError + }Apply the same change to “Send Invitation”:
- disabled={createInvite.isPending || limitsQuery.isLoading} + disabled={ + createInvite.isPending || + limitsQuery.isLoading || + limitsQuery.isError + }apps/web/src/app/(dashboard)/contacts/add-contact-book.tsx (7)
3-31: Group and alphabetize imports per guidelines (external vs internal).Reorder imports to follow the repo rules: external first (alphabetized), then internal packages (e.g., @unsend/ui), then app-local (~). This reduces churn and keeps diffs clean.
Apply this diff:
-import { Button } from "@unsend/ui/src/button"; -import { Input } from "@unsend/ui/src/input"; -import { - Dialog, - DialogContent, - DialogHeader, - DialogTitle, - DialogTrigger, -} from "@unsend/ui/src/dialog"; - -import { api } from "~/trpc/react"; -import { useState } from "react"; -import { Plus } from "lucide-react"; -import { toast } from "@unsend/ui/src/toaster"; -import { z } from "zod"; -import { useForm } from "react-hook-form"; -import { zodResolver } from "@hookform/resolvers/zod"; -import { - Form, - FormControl, - FormDescription, - FormField, - FormItem, - FormLabel, - FormMessage, -} from "@unsend/ui/src/form"; -import { useUpgradeModalStore } from "~/store/upgradeModalStore"; -import { LimitReason } from "~/lib/constants/plans"; +import { zodResolver } from "@hookform/resolvers/zod"; +import { Plus } from "lucide-react"; +import { useForm } from "react-hook-form"; +import { useState } from "react"; +import { z } from "zod"; + +import { Button } from "@unsend/ui/src/button"; +import { + Dialog, + DialogContent, + DialogHeader, + DialogTitle, + DialogTrigger, +} from "@unsend/ui/src/dialog"; +import { + Form, + FormControl, + FormDescription, + FormField, + FormItem, + FormLabel, + FormMessage, +} from "@unsend/ui/src/form"; +import { Input } from "@unsend/ui/src/input"; +import { toast } from "@unsend/ui/src/toaster"; + +import { LimitReason } from "~/lib/constants/plans"; +import { useUpgradeModalStore } from "~/store/upgradeModalStore"; +import { api } from "~/trpc/react";
43-47: Avoid unnecessary refetches on the limits query.Add lightweight React Query options to cut noise (focus refetch) and give data a short lifetime. Keeps UX snappy without changing behavior.
- const limitsQuery = api.limits.get.useQuery({ - type: LimitReason.CONTACT_BOOK, - }); + const limitsQuery = api.limits.get.useQuery( + { type: LimitReason.CONTACT_BOOK }, + { + staleTime: 30_000, + refetchOnWindowFocus: false, + }, + );
58-62: Handle limits API errors explicitly (decide fail-closed vs fail-open).Right now, if the limits query errors, we proceed with creation. If product intent is to “fail closed,” block and inform the user when the check can’t be performed.
- if (limitsQuery.data?.isLimitReached) { + if (limitsQuery.isError) { + toast.error("Could not verify plan limits. Please try again."); + return; + } + if (limitsQuery.data?.isLimitReached) { openModal(limitsQuery.data.reason); return; }Would you prefer fail-open instead (allow creation if the check errors)? If so, we can show a non-blocking warning toast and continue.
63-75: Surface mutation errors and open upgrade modal on server-denied requests.Add onError to give users feedback and to catch server-side limit enforcement (defense in depth). Also mark invalidate as fire-and-forget to appease linters.
createContactBookMutation.mutate( { name: values.name, }, { onSuccess: () => { - utils.contacts.getContactBooks.invalidate(); + void utils.contacts.getContactBooks.invalidate(); contactBookForm.reset(); setOpen(false); toast.success("Contact book created successfully"); }, + onError: (err: any) => { + // If the backend enforces limits and rejects, surface the upgrade path + const code = err?.data?.code; + if (code === "FORBIDDEN" || /limit/i.test(err?.message ?? "")) { + openModal(LimitReason.CONTACT_BOOK); + return; + } + toast.error(err?.message ?? "Failed to create contact book"); + }, }, );
78-86: Simplify onOpenChange; optionally ensure a fresh limits check when opening.The current guard is fine, but you can simplify and ensure we have fresh data when opening (without altering the existing gating behavior).
- function onOpenChange(_open: boolean) { - if (_open && limitsQuery.data?.isLimitReached) { - openModal(limitsQuery.data.reason); - return; - } - - setOpen(_open); - } + function onOpenChange(nextOpen: boolean) { + if (nextOpen && limitsQuery.data?.isLimitReached) { + openModal(limitsQuery.data.reason); + return; + } + // Optionally refresh once when opening and no data yet + if (nextOpen && !limitsQuery.data && !limitsQuery.isFetching) { + void limitsQuery.refetch(); + } + setOpen(nextOpen); + }
90-90: Pass the handler directly; the equality guard is unnecessary.The Dialog won’t spam onOpenChange with the same state; the extra guard adds noise.
- onOpenChange={(_open) => (_open !== open ? onOpenChange(_open) : null)} + onOpenChange={onOpenChange}
132-134: Add aria-busy for better a11y feedback while submitting.Keeps assistive tech informed during pending state.
type="submit" - disabled={ + aria-busy={createContactBookMutation.isPending} + disabled={ createContactBookMutation.isPending || limitsQuery.isLoading }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (14)
apps/web/src/app/(dashboard)/contacts/add-contact-book.tsx(5 hunks)apps/web/src/app/(dashboard)/domains/add-domain.tsx(5 hunks)apps/web/src/app/(dashboard)/settings/team/invite-team-member.tsx(8 hunks)apps/web/src/components/payments/UpgradeModal.tsx(1 hunks)apps/web/src/lib/constants/plans.ts(1 hunks)apps/web/src/lib/usage.ts(1 hunks)apps/web/src/server/api/routers/billing.ts(2 hunks)apps/web/src/server/api/routers/limits.ts(1 hunks)apps/web/src/server/service/contact-book-service.ts(1 hunks)apps/web/src/server/service/domain-service.ts(6 hunks)apps/web/src/server/service/limit-service.ts(1 hunks)apps/web/src/server/service/team-service.ts(1 hunks)apps/web/src/server/service/usage-service.ts(1 hunks)apps/web/src/store/upgradeModalStore.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/web/src/server/service/contact-book-service.ts
- apps/web/src/components/payments/UpgradeModal.tsx
- apps/web/src/server/api/routers/limits.ts
- apps/web/src/store/upgradeModalStore.ts
- apps/web/src/server/service/team-service.ts
- apps/web/src/server/service/domain-service.ts
- apps/web/src/lib/constants/plans.ts
🧰 Additional context used
📓 Path-based instructions (6)
{apps,packages}/**/*.{js,jsx,ts,tsx,css,scss,md,mdx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Prettier with the Tailwind plugin for code formatting
Files:
apps/web/src/server/service/usage-service.tsapps/web/src/lib/usage.tsapps/web/src/app/(dashboard)/contacts/add-contact-book.tsxapps/web/src/app/(dashboard)/settings/team/invite-team-member.tsxapps/web/src/server/service/limit-service.tsapps/web/src/server/api/routers/billing.tsapps/web/src/app/(dashboard)/domains/add-domain.tsx
{apps,packages}/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
{apps,packages}/**/*.{js,jsx,ts,tsx}: Group imports by source (internal/external) and alphabetize them
Use camelCase for variables and functions, PascalCase for components and classes
Use try/catch with specific error types for error handling
Files:
apps/web/src/server/service/usage-service.tsapps/web/src/lib/usage.tsapps/web/src/app/(dashboard)/contacts/add-contact-book.tsxapps/web/src/app/(dashboard)/settings/team/invite-team-member.tsxapps/web/src/server/service/limit-service.tsapps/web/src/server/api/routers/billing.tsapps/web/src/app/(dashboard)/domains/add-domain.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
{apps,packages}/**/*.{ts,tsx}: Use strong typing in TypeScript, avoidany, and use Zod for validation
Follow Vercel style guides with strict TypeScript
Files:
apps/web/src/server/service/usage-service.tsapps/web/src/lib/usage.tsapps/web/src/app/(dashboard)/contacts/add-contact-book.tsxapps/web/src/app/(dashboard)/settings/team/invite-team-member.tsxapps/web/src/server/service/limit-service.tsapps/web/src/server/api/routers/billing.tsapps/web/src/app/(dashboard)/domains/add-domain.tsx
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use tRPC for internal API endpoints
Files:
apps/web/src/server/service/usage-service.tsapps/web/src/lib/usage.tsapps/web/src/app/(dashboard)/contacts/add-contact-book.tsxapps/web/src/app/(dashboard)/settings/team/invite-team-member.tsxapps/web/src/server/service/limit-service.tsapps/web/src/server/api/routers/billing.tsapps/web/src/app/(dashboard)/domains/add-domain.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Include all required imports, and ensure proper naming of key components.
Files:
apps/web/src/server/service/usage-service.tsapps/web/src/lib/usage.tsapps/web/src/app/(dashboard)/contacts/add-contact-book.tsxapps/web/src/app/(dashboard)/settings/team/invite-team-member.tsxapps/web/src/server/service/limit-service.tsapps/web/src/server/api/routers/billing.tsapps/web/src/app/(dashboard)/domains/add-domain.tsx
{apps,packages}/**/*.{jsx,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
{apps,packages}/**/*.{jsx,tsx}: Use functional React components with hooks and group related hooks together
In React components, structure code with props at the top, hooks next, helper functions, then JSX
Files:
apps/web/src/app/(dashboard)/contacts/add-contact-book.tsxapps/web/src/app/(dashboard)/settings/team/invite-team-member.tsxapps/web/src/app/(dashboard)/domains/add-domain.tsx
🧬 Code graph analysis (5)
apps/web/src/server/service/usage-service.ts (1)
apps/web/src/server/db.ts (1)
db(20-20)
apps/web/src/app/(dashboard)/contacts/add-contact-book.tsx (1)
apps/web/src/store/upgradeModalStore.ts (1)
useUpgradeModalStore(13-20)
apps/web/src/server/service/limit-service.ts (3)
apps/web/src/server/db.ts (1)
db(20-20)apps/web/src/lib/constants/plans.ts (1)
PLAN_LIMITS(10-34)apps/web/src/server/service/usage-service.ts (1)
getThisMonthUsage(12-63)
apps/web/src/server/api/routers/billing.ts (1)
apps/web/src/server/service/usage-service.ts (1)
getThisMonthUsage(12-63)
apps/web/src/app/(dashboard)/domains/add-domain.tsx (1)
apps/web/src/store/upgradeModalStore.ts (1)
useUpgradeModalStore(13-20)
🔇 Additional comments (6)
apps/web/src/app/(dashboard)/domains/add-domain.tsx (2)
81-84: Nice guardrails: UI prevents domain creation when limit is reached and shows upgrade modal.Early-return on submit and on dialog open is clean; disabling the submit button while limits load avoids racey submits. LGTM.
Also applies to: 104-111, 193-195
71-79: Server-side enforcement must also block domain creation.UI gating is bypassable; confirm domain-service also calls LimitService.checkDomainLimit before inserting.
If not present, I can add the guard to domain-service and return a typed TRPC error for a consistent UI flow.
apps/web/src/server/api/routers/billing.ts (1)
29-30: Good centralization: delegate usage computation to usage-service.This removes duplication and keeps usage semantics in one place.
apps/web/src/server/service/usage-service.ts (1)
36-57: Sanity-check: DailyEmailUsage.date timezone.If “date” is stored in local time or as a date type with a timezone, the UTC strings above must match how “date” is written to avoid off-by-one. Confirm writer and reader align.
I can add tests or a migration note once you confirm the storage semantics.
apps/web/src/server/service/limit-service.ts (1)
10-44: Overall structure is clean and easy to extend.Consistent return shape, centralized helper, and per-feature checks make this service composable.
Also applies to: 46-79
apps/web/src/app/(dashboard)/settings/team/invite-team-member.tsx (1)
37-38: Good addition: hooks for upgrade modal + reason enumImporting
useUpgradeModalStoreandLimitReasonaligns this screen with the new centralized limits/upgrade flow. No issues.
- Add priceIds array field to Subscription table for multiple price support - Migrate existing priceId data to priceIds array while maintaining backward compatibility - Update billing logic to handle multiple subscription price items - Add STRIPE_BASIC_USAGE_PRICE_ID environment variable for usage-based pricing
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/src/server/billing/payments.ts (2)
54-66: Guard optional usage price before creating the checkout session.env.STRIPE_BASIC_USAGE_PRICE_ID is optional, but it’s passed directly into line_items. In TS, price expects a string; at runtime Stripe will reject undefined. Build line_items conditionally so the session creation is valid whether the usage price is configured or not.
Apply this diff:
const session = await stripe.checkout.sessions.create({ mode: "subscription", customer: customerId, - line_items: [ - { - price: env.STRIPE_BASIC_PRICE_ID, - quantity: 1, - }, - { - price: env.STRIPE_BASIC_USAGE_PRICE_ID, - }, - ], + line_items: (() => { + const items: Stripe.Checkout.SessionCreateParams.LineItem[] = [ + { + price: env.STRIPE_BASIC_PRICE_ID, + quantity: 1, + }, + ]; + if (env.STRIPE_BASIC_USAGE_PRICE_ID) { + items.push({ price: env.STRIPE_BASIC_USAGE_PRICE_ID }); + } + return items; + })(), success_url: `${env.NEXTAUTH_URL}/payments?success=true&session_id={CHECKOUT_SESSION_ID}`, cancel_url: `${env.NEXTAUTH_URL}/settings/billing`,If the usage price must be mandatory for BASIC, instead make STRIPE_BASIC_USAGE_PRICE_ID required in env and validate both price IDs before session creation.
148-171: Fix incorrect source for current period timestamps.current_period_start/end are properties on the Subscription, not on SubscriptionItem. Using item fields yields undefined and invalid dates.
Apply this diff:
priceId: subscription.items.data[0]?.price?.id || "", - priceIds: priceIds, - currentPeriodEnd: new Date( - subscription.items.data[0]?.current_period_end * 1000, - ), - currentPeriodStart: new Date( - subscription.items.data[0]?.current_period_start * 1000, - ), + priceIds: priceIds, + currentPeriodEnd: new Date(subscription.current_period_end * 1000), + currentPeriodStart: new Date(subscription.current_period_start * 1000),priceId: subscription.items.data[0]?.price?.id || "", - priceIds: priceIds, - currentPeriodEnd: new Date( - subscription.items.data[0]?.current_period_end * 1000, - ), - currentPeriodStart: new Date( - subscription.items.data[0]?.current_period_start * 1000, - ), + priceIds: priceIds, + currentPeriodEnd: new Date(subscription.current_period_end * 1000), + currentPeriodStart: new Date(subscription.current_period_start * 1000),
🧹 Nitpick comments (2)
apps/web/src/server/billing/payments.ts (2)
77-86: Plan resolution should also match on the usage price ID.If a subscription contains only the usage price or both prices, you likely still want plan BASIC. Consider matching against both BASIC price IDs.
Apply this diff:
-function getPlanFromPriceIds(priceIds: string[]) { - if ( - env.STRIPE_BASIC_PRICE_ID && - priceIds.includes(env.STRIPE_BASIC_PRICE_ID) - ) { - return "BASIC"; - } - - return "FREE"; -} +function getPlanFromPriceIds(priceIds: string[]) { + const basicIds = [env.STRIPE_BASIC_PRICE_ID, env.STRIPE_BASIC_USAGE_PRICE_ID] + .filter((x): x is string => Boolean(x)); + if (basicIds.some((id) => priceIds.includes(id))) { + return "BASIC"; + } + return "FREE"; +}
139-142: Deduplicate price IDs to avoid redundant entries.Multiple items referencing the same price (e.g., metered + tiered cases) can cause duplicates. Store unique IDs.
Apply this diff:
- const priceIds = subscription.items.data - .map((item) => item.price?.id) - .filter((id): id is string => Boolean(id)); + const priceIds = Array.from( + new Set( + subscription.items.data + .map((item) => item.price?.id) + .filter((id): id is string => Boolean(id)), + ), + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
apps/web/prisma/migrations/20250824131647_add_price_ids_array/migration.sql(1 hunks)apps/web/prisma/schema.prisma(2 hunks)apps/web/src/env.js(2 hunks)apps/web/src/server/billing/payments.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/prisma/schema.prisma
🧰 Additional context used
📓 Path-based instructions (5)
{apps,packages}/**/*.{js,jsx,ts,tsx,css,scss,md,mdx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Prettier with the Tailwind plugin for code formatting
Files:
apps/web/src/env.jsapps/web/src/server/billing/payments.ts
{apps,packages}/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
{apps,packages}/**/*.{js,jsx,ts,tsx}: Group imports by source (internal/external) and alphabetize them
Use camelCase for variables and functions, PascalCase for components and classes
Use try/catch with specific error types for error handling
Files:
apps/web/src/env.jsapps/web/src/server/billing/payments.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Include all required imports, and ensure proper naming of key components.
Files:
apps/web/src/env.jsapps/web/src/server/billing/payments.ts
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
{apps,packages}/**/*.{ts,tsx}: Use strong typing in TypeScript, avoidany, and use Zod for validation
Follow Vercel style guides with strict TypeScript
Files:
apps/web/src/server/billing/payments.ts
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use tRPC for internal API endpoints
Files:
apps/web/src/server/billing/payments.ts
🧬 Code graph analysis (1)
apps/web/src/server/billing/payments.ts (2)
apps/web/src/env.js (2)
env(5-121)env(5-121)apps/web/src/server/db.ts (1)
db(20-20)
🔇 Additional comments (2)
apps/web/src/env.js (1)
56-57: Env var addition is fine; confirm expected optionality vs usage in code.You’ve added STRIPE_BASIC_USAGE_PRICE_ID as optional in the schema and plumbed it into runtimeEnv. In apps/web/src/server/billing/payments.ts it’s used as a price in a checkout line item, which requires a non-undefined string. If this price is mandatory in your “BASIC” flow, consider making it required here; if not, ensure payments.ts guards it before sending to Stripe.
Would you like me to update payments.ts to include the usage price conditionally when present and keep this var optional?
Also applies to: 106-107
apps/web/src/server/billing/payments.ts (1)
183-185: LGTM: team.plan now derives from multi-price state.Using getPlanFromPriceIds(priceIds) keeps Team in sync with Stripe’s multi-item subscriptions. Good direction.
apps/web/prisma/migrations/20250824131647_add_price_ids_array/migration.sql
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/web/src/server/billing/payments.ts (3)
15-15: Fix: Stripe metadata values must be strings (TS compile-time error).
metadata: { teamId }passes a number. Stripe metadata keys and values must be strings; the SDK types enforce this. Convert to string.Apply this diff:
- const customer = await stripe.customers.create({ metadata: { teamId } }); + const customer = await stripe.customers.create({ + metadata: { teamId: String(teamId) }, + });Reference: Stripe metadata requires string values; see “Metadata limits” and docs. (support.stripe.com, docs.stripe.com)
72-76: Fix: metadata type + propagate teamId to the resulting Subscription.
metadata: { teamId }should use a string to satisfy Stripe’s types.- Strongly recommend also setting
subscription_data.metadataso the created Subscription carries theteamIdwithout relying solely on the Customer ID linkage.Apply this diff:
success_url: `${env.NEXTAUTH_URL}/payments?success=true&session_id={CHECKOUT_SESSION_ID}`, cancel_url: `${env.NEXTAUTH_URL}/settings/billing`, - metadata: { - teamId, - }, + // Store team linkage on the Checkout Session and the created Subscription. + metadata: { teamId: String(teamId) }, + subscription_data: { + metadata: { teamId: String(teamId) }, + }, client_reference_id: teamId.toString(),References:
- Metadata must be strings. (support.stripe.com)
- Mapping:
subscription_data.metadataon Checkout Session -> Subscription metadata. (docs.stripe.com)
155-160: Bug: Using item-level fields for period timestamps; must use Subscription fields.
current_period_endandcurrent_period_startare properties on the Subscription, not on Subscription Items. Persisting the item fields yieldsundefinedand wrong dates.Apply this diff in both upsert branches:
- currentPeriodEnd: new Date( - subscription.items.data[0]?.current_period_end * 1000 - ), - currentPeriodStart: new Date( - subscription.items.data[0]?.current_period_start * 1000 - ), + currentPeriodEnd: new Date(subscription.current_period_end * 1000), + currentPeriodStart: new Date(subscription.current_period_start * 1000),References:
current_period_start/current_period_endlive on the Subscription object. (javadoc.io)Also applies to: 173-177
🧹 Nitpick comments (5)
apps/web/src/server/billing/payments.ts (5)
50-54: Nit: Make the env check precise and remove redundant customerId check.By this point,
customerIdhas been created/assigned (or an exception would’ve been thrown), so!customerIdis redundant. Also, emit which price IDs are missing to simplify ops debugging.Example improvement:
- if ( - !env.STRIPE_BASIC_PRICE_ID || - !env.STRIPE_BASIC_USAGE_PRICE_ID || - !customerId - ) { - throw new Error("Stripe prices are not set"); - } + const missing = [ + !env.STRIPE_BASIC_PRICE_ID && "STRIPE_BASIC_PRICE_ID", + !env.STRIPE_BASIC_USAGE_PRICE_ID && "STRIPE_BASIC_USAGE_PRICE_ID", + ].filter(Boolean) as string[]; + if (missing.length) { + throw new Error(`Missing Stripe price IDs: ${missing.join(", ")}`); + }
81-92: Plan resolution logic is sound; consider defensive logging when envs are unset.Current logic returns FREE when none of the expected price IDs are present. To ease debugging misconfiguration in prod, consider logging a warning when both BASIC price IDs are absent and a non-empty priceIds array is present.
109-115: Nit: Name the portal session variable descriptively.
subscriptionsholds a single Billing Portal session. Rename for clarity.- const subscriptions = await stripe.billingPortal.sessions.create({ + const portalSession = await stripe.billingPortal.sessions.create({ customer: team.stripeCustomerId, return_url: `${env.NEXTAUTH_URL}`, }); - - return subscriptions.url; + return portalSession.url;Reference: Billing Portal sessions.create returns a single session with a
urlfield. (docs.stripe.com)
186-191: Consider trialing as “active” for feature gating.Teams on trial typically should be “active” for gating. If that’s your product intent, mark both
activeandtrialingas active state.- isActive: subscription.status === "active", + isActive: + subscription.status === "active" || + subscription.status === "trialing",
128-134: Pin Stripe API version to ensurestatus: "all"is always supportedTo avoid runtime errors in environments with older API defaults and to keep subscription‐listing semantics consistent, explicitly set the
apiVersionwhen constructing your Stripe client inpayments.ts.• File: apps/web/src/server/billing/payments.ts
export function getStripe(): Stripe { - return new Stripe(env.STRIPE_SECRET_KEY); + return new Stripe(env.STRIPE_SECRET_KEY, { + // Use the library’s pinned API version so `status: "all"` is supported + apiVersion: Stripe.API_VERSION, + }); }This matches your usage in
usage.ts, which already pins"2025-03-31.basil", and prevents surprises when the default API version—either on your account or baked into stripe-node—changes. [turn1search0][turn0search2]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
apps/web/prisma/schema.prisma(1 hunks)apps/web/src/env.js(2 hunks)apps/web/src/server/billing/payments.ts(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/src/env.js
- apps/web/prisma/schema.prisma
🧰 Additional context used
📓 Path-based instructions (5)
{apps,packages}/**/*.{js,jsx,ts,tsx,css,scss,md,mdx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Prettier with the Tailwind plugin for code formatting
Files:
apps/web/src/server/billing/payments.ts
{apps,packages}/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
{apps,packages}/**/*.{js,jsx,ts,tsx}: Group imports by source (internal/external) and alphabetize them
Use camelCase for variables and functions, PascalCase for components and classes
Use try/catch with specific error types for error handling
Files:
apps/web/src/server/billing/payments.ts
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
{apps,packages}/**/*.{ts,tsx}: Use strong typing in TypeScript, avoidany, and use Zod for validation
Follow Vercel style guides with strict TypeScript
Files:
apps/web/src/server/billing/payments.ts
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use tRPC for internal API endpoints
Files:
apps/web/src/server/billing/payments.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Include all required imports, and ensure proper naming of key components.
Files:
apps/web/src/server/billing/payments.ts
🧬 Code graph analysis (1)
apps/web/src/server/billing/payments.ts (2)
apps/web/src/env.js (2)
env(5-123)env(5-123)apps/web/src/server/db.ts (1)
db(20-20)
🔇 Additional comments (3)
apps/web/src/server/billing/payments.ts (3)
61-69: LGTM: Dual line-items for subscription + usage.
- Fixed price with
quantity: 1plus separate usage-based price is correct for Checkout in subscription mode.- Omitting
quantityfor the metered/usage price is appropriate.Reference: Checkout Session create parameters for line items in subscription mode. (docs.stripe.com)
145-148: LGTM: Robust extraction of all price IDs on the subscription.Mapping and type-narrowing to a
string[]is clean and safe.
154-154: LGTM: PersistingpriceIdsalongside legacypriceIdis a good transitional step.This preserves backward compatibility while enabling multi-price subscriptions.
Also applies to: 171-171
1 participant
Summary by CodeRabbit
New Features
Changes
Refactor
Chores
Data