feat: adds user plan info in useHasPaidPlan for intercom
#23790
Conversation
WalkthroughAdds a billing plan determination flow and propagates it to client and Intercom. Introduces BillingPlan enum, platform plan map, and enterprise slug env vars. Implements BillingPlanService.getUserPlanByMemberships using membership data (including platform and metadata parsing). Adds MembershipRepository.findAllMembershipsByUserIdForBilling to fetch required fields. Updates hasTeamPlan handler to return { hasTeamPlan, plan }. Extends useHasPaidPlan hook to return plan. Passes Plan to Intercom in both web Intercom hook and support conversation API when creating contacts. Updates turbo.json to expose ENTERPRISE_SLUGS and PLATFORM_ENTERPRISE_SLUGS. Minor non-functional formatting changes. Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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. Comment |
dosubot
bot
added
billing
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
| @@ -0,0 +1,46 @@ | |||
| import { BillingPlans, ENTERPRISE_SLUGS, PLATFORM_ENTERPRISE_SLUGS } from "@calcom/ee/billing/constants"; | |||
There was a problem hiding this comment.
There should be tests added around this logic to verify its correctness
|
|
||
| if (memberships.length === 0) return BillingPlans.INDIVIDUALS; | ||
|
|
||
| for (const { team, user } of memberships) { |
There was a problem hiding this comment.
Please move this logic to a different class that takes in the data it uses. It should live in a folder inside of /billing called /domain. That way you can easily write unit tests for it.
There was a problem hiding this comment.
The service is ok to keep and have it read data from the repository, but then it should pass the data to a domain-level class/function for the logic-only piece
…wngrade/create of teams and orgs
…om/cal.com into feat/user_plan_for_intercom
E2E results are ready! |
…lows - Add unit tests for BillingPlanService.getUserPlanByMemberships() covering all plan determination scenarios - Add tests for team creation handler verifying TEAMS vs ORGANIZATIONS plan assignment - Add tests for hasTeamPlan handler integration with BillingPlanService - Add tests for MembershipRepository.findAllMembershipsByUserIdForBilling() data fetching - Add tests for InternalTeamBilling upgrade/downgrade flows with proper mocking - All tests follow existing vitest patterns with proper Prisma and service mocking - Covers both self-serve and platform billing scenarios with comprehensive edge cases Co-Authored-By: [email protected] <[email protected]>
…am/org flows" This reverts commit 58e511f.
dosubot
bot
added
platform
| static async findAllMembershipsByUserIdForBilling({ userId }: { userId: number }) { | ||
| return await prisma.membership.findMany({ |
There was a problem hiding this comment.
no need to be static and no need the await here.
packages/trpc/server/routers/viewer/teams/hasTeamPlan.handler.ts
Outdated
Show resolved
Hide resolved
packages/prisma/schema.prisma
Outdated
| WEBAPP @map("webapp") | ||
| } | ||
|
|
||
| enum Plans { |
There was a problem hiding this comment.
Consistency is key, Plans should be "Plan" and all enum keys should be singular.
There was a problem hiding this comment.
fixed for enum Name. But enum keys are plural in intercom, so for proper mapping and worfklows in intercom, kept it plural here as well.
|
@Amit91848 we were chatting and decided that we are going to involve @joeauyeung in a bigger overhaul of billing instead of just tacking this on quickly. For intercom, let’s build a daily sync using the query I provided (adding filters for dates) |
|
Chatting with @Amit91848 some more FYI @joeauyeung and we think a good middleground is to use the runtime code built here without adding the new plan column to the DB so we can still properly kick off intercom sessions based on plan. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
turbo.json (1)
25-36: Deduplicate repeated env keys in globalEnvThere are duplicates that add noise and confusion:
- CALCOM_CREDENTIAL_SYNC_SECRET (Line 26 and Line 35)
- INTERCOM_SECRET (Line 90 and Line 91)
- ORG_MONTHLY_CREDITS (Line 124 and Line 193)
- VAPID_PRIVATE_KEY (Line 254 and Line 260)
Recommend removing duplicates in this PR or a follow-up.
Also applies to: 90-92, 124-124, 193-193, 254-261
packages/features/ee/billing/domain/billing-plans.ts (3)
11-12: Remove unnecessary async and annotate return typeMethod has no awaits. Make it synchronous and explicitly typed for clarity.
-export class BillingPlanService { - async getUserPlanByMemberships( +export class BillingPlanService { + getUserPlanByMemberships( memberships: { team: { isOrganization: boolean; isPlatform: boolean; slug: string | null; metadata: JsonValue; parent: { isOrganization: boolean; slug: string | null; isPlatform: boolean; metadata: JsonValue; } | null; platformBilling: { plan: string; } | null; }; user: { isPlatformManaged: boolean; }; }[] - ) { + ): BillingPlan {Also applies to: 32-33
35-76: Membership order affects outcome; choose highest-precedence plan insteadCurrent early returns make the result depend on array order. Prefer computing the best plan across all memberships (e.g., PLATFORM_ENTERPRISE > ENTERPRISE > PLATFORM_SCALE > ORGANIZATIONS > TEAMS > PLATFORM_ESSENTIALS > PLATFORM_STARTER > INDIVIDUALS > UNKNOWN).
If you want, I can provide a concrete refactor that accumulates a candidate per membership using a rank map and returns the highest-ranked plan.
33-33: Fallback on no match: UNKNOWN vs INDIVIDUALSYou return INDIVIDUALS when memberships is empty, but UNKNOWN when memberships exist yet don’t match. Consider consistently defaulting to INDIVIDUALS, or document the distinction.
Also applies to: 77-78
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
apps/web/app/api/support/conversation/route.ts(3 hunks)packages/features/ee/billing/constants.ts(1 hunks)packages/features/ee/billing/domain/billing-plans.ts(1 hunks)packages/lib/server/repository/membership.ts(2 hunks)packages/trpc/server/routers/viewer/organizations/publish.handler.ts(1 hunks)packages/trpc/server/routers/viewer/teams/hasTeamPlan.handler.ts(2 hunks)turbo.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/trpc/server/routers/viewer/organizations/publish.handler.ts
- packages/lib/server/repository/membership.ts
- apps/web/app/api/support/conversation/route.ts
- packages/features/ee/billing/constants.ts
- packages/trpc/server/routers/viewer/teams/hasTeamPlan.handler.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/features/ee/billing/domain/billing-plans.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/ee/billing/domain/billing-plans.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/ee/billing/domain/billing-plans.ts
🧬 Code graph analysis (1)
packages/features/ee/billing/domain/billing-plans.ts (2)
packages/features/ee/billing/constants.ts (3)
PLATFORM_ENTERPRISE_SLUGS(31-31)PLATFORM_PLANS_MAP(23-29)ENTERPRISE_SLUGS(32-32)packages/prisma/zod-utils.ts (1)
teamMetadataStrictSchema(389-405)
🔇 Additional comments (2)
turbo.json (1)
284-287: LGTM: exposing enterprise slug envs to TurboThese belong in globalEnv so changes invalidate cache appropriately. Please also add them to .env.example and developer docs.
packages/features/ee/billing/domain/billing-plans.ts (1)
36-41: Confirm platform gating via user.isPlatformManagedUsing user.isPlatformManaged to enter the platform path even when team.isPlatform is false can classify non-platform teams as platform based on the user flag. Verify this is intentional.
| return PLATFORM_PLANS_MAP[team.platformBilling.plan] ?? team.platformBilling.plan; | ||
| } else { |
There was a problem hiding this comment.
Keep return type strictly BillingPlan (don’t return raw strings)
Fallback currently returns a string which breaks type guarantees and can leak unknown values downstream.
Apply this diff:
- return PLATFORM_PLANS_MAP[team.platformBilling.plan] ?? team.platformBilling.plan;
+ const planKey = team.platformBilling.plan?.toUpperCase?.() ?? "";
+ return PLATFORM_PLANS_MAP[planKey] ?? BillingPlan.UNKNOWN;📝 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.
| return PLATFORM_PLANS_MAP[team.platformBilling.plan] ?? team.platformBilling.plan; | |
| } else { | |
| const planKey = team.platformBilling.plan?.toUpperCase?.() ?? ""; | |
| return PLATFORM_PLANS_MAP[planKey] ?? BillingPlan.UNKNOWN; | |
| } else { |
🤖 Prompt for AI Agents
In packages/features/ee/billing/domain/billing-plans.ts around lines 40-41, the
fallback currently returns the raw string team.platformBilling.plan which breaks
the function's BillingPlan return type; replace the raw-string fallback with a
strict BillingPlan value by adding a small type-guard isBillingPlan(value: any):
value is BillingPlan that checks Object.values(BillingPlan).includes(value),
then return PLATFORM_PLANS_MAP[team.platformBilling.plan] if defined, otherwise
if isBillingPlan(team.platformBilling.plan) return team.platformBilling.plan,
else throw a descriptive error (or return an explicit safe BillingPlan default
if your codebase prefers) so the function never returns an untyped string.
What does this PR do?
Video Demo (if applicable):
Screen.Recording.2025-09-12.at.4.58.52.PM.mov
Mandatory Tasks (DO NOT REMOVE)