fix: platform billing portal #23975
fix: platform billing portal #23975
Conversation
WalkthroughThe UI billing view now fetches user attributes earlier and appends a Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
keithwillcode
added
core
dosubot
bot
added
billing
|
@coderabbitai update PR description |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainI'll help you update the PR description. Let me first examine the changes in this PR to understand what it actually does. 🏁 Script executed: Length of output: 7111 Let me get more information about the specific changes and any related issues or context. 🏁 Script executed: Length of output: 430 Now let me update the PR description based on the code changes I've analyzed. I can see this PR fixes an issue with platform billing portal functionality. 🏁 Script executed: Length of output: 80 @Ryukemeister I've successfully updated the PR description! The new description provides a comprehensive overview of what this PR accomplishes: 🔧 Main Fix: Platform billing portal functionality that properly handles billing portal URLs for platform teams vs regular teams 📝 Key Changes:
✅ Testing Guidelines: Added specific test cases for both platform and regular teams to ensure the fix works correctly without breaking existing functionality The description now accurately reflects the technical changes made in the code and provides clear testing instructions for reviewers and QA. |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/app-store/stripepayment/api/portal.ts (1)
70-77: Authorization check missing: ensure requester is an Admin/Owner of the team
getTeamByIdIfUserIsAdmindoes not enforce membership; it only filtersmembers. Without checkingteam.members.length, any authenticated user can pass ateamIdand get a billing portal link for another team. Add an explicit check and return 403 on failure.const teamRepository = new TeamRepository(prisma); const team = await teamRepository.getTeamByIdIfUserIsAdmin({ teamId, userId, }); if (!team) return res.status(404).json({ message: "Team not found" }); + if (!team.members || team.members.length === 0) { + return res.status(403).json({ message: "Forbidden" }); + }
🧹 Nitpick comments (7)
packages/lib/server/repository/team.ts (1)
388-401: Limit selected membership fields to the minimum neededCurrently, the
membersrelation is selected without aselect, which can return more data than necessary. Add a minimalselectto avoid over-fetching.select: { id: true, metadata: true, + // Only need to know if the user is an admin/owner; avoid fetching full membership rows members: { where: { userId, role: { in: [MembershipRole.ADMIN, MembershipRole.OWNER], }, }, + select: { id: true }, }, + isPlatform: true, + platformBilling: true, },packages/app-store/stripepayment/api/portal.ts (2)
30-42: Make validators synchronous and unify error messagesThese helpers don’t await anything; making them sync simplifies control flow and stack traces. Keep error messages consistent.
-const getValidatedTeamSubscriptionId = async (metadata: Prisma.JsonValue) => { +const getValidatedTeamSubscriptionId = (metadata: Prisma.JsonValue) => { const teamMetadataParsed = teamMetadataSchema.safeParse(metadata); if (!teamMetadataParsed.success) { throw new Error("Invalid team metadata"); } if (!teamMetadataParsed.data?.subscriptionId) { - throw new Error("Subscription Id not found for team"); + throw new Error("Subscription ID not found for team"); } return teamMetadataParsed.data.subscriptionId; }; -const getValidatedTeamSubscriptionIdForPlatform = async (subscriptionId?: string | null) => { +const getValidatedTeamSubscriptionIdForPlatform = (subscriptionId?: string | null) => { if (!subscriptionId) { - throw new Error("Subscription Id not found for team"); + throw new Error("Subscription ID not found for team"); } return subscriptionId; };Also applies to: 44-50
59-61: Defensively parse teamIdAvoid treating NaN as a falsy team ID. Use base 10 and coerce invalid values to
null.-const teamId = req.query.teamId ? parseInt(req.query.teamId as string) : null; +const teamIdRaw = Array.isArray(req.query.teamId) ? req.query.teamId[0] : req.query.teamId; +const parsedTeamId = teamIdRaw ? Number.parseInt(teamIdRaw, 10) : NaN; +const teamId = Number.isNaN(parsedTeamId) ? null : parsedTeamId;apps/web/modules/settings/platform/billing/billing-view.tsx (4)
33-39: Build portal URL with URLSearchParams and only include teamId when availablePrevents
teamId=undefinedand handles encoding forreturnTo.-const { isUserLoading, isUserBillingDataLoading, isPlatformUser, userBillingData, isPaidUser, userOrgId } = - useGetUserAttributes(); +const { + isUserLoading, + isUserBillingDataLoading, + isPlatformUser, + userBillingData, + isPaidUser, + userOrgId, +} = useGetUserAttributes(); const returnTo = pathname; -const teamId = `teamId=${userOrgId}`; -const billingHref = `/api/integrations/stripepayment/portal?returnTo=${WEBAPP_URL}${returnTo}&${teamId}`; +const qp = new URLSearchParams({ returnTo: `${WEBAPP_URL}${returnTo}` }); +if (userOrgId) qp.set("teamId", String(userOrgId)); +const billingHref = `/api/integrations/stripepayment/portal?${qp.toString()}`;
96-99: Add rel to target=_blank linksAvoid reverse tabnabbing and follow best practices.
-<Button color="primary" href={billingHref} target="_blank" EndIcon="external-link"> +<Button color="primary" href={billingHref} target="_blank" rel="noopener noreferrer" EndIcon="external-link">
104-111: Localize UI strings with t() keys (no raw strings in TSX)Replace hard-coded text with
t()keys per guidelines.-<CtaRow - title="Change plan" - description={t("Want to change your existing plan or check out other plans?")}> - <Button href="/settings/platform/plans" color="secondary"> - Plans - </Button> -</CtaRow> +<CtaRow + title={t("change_plan")} + description={t("change_plan_description")}> + <Button href="/settings/platform/plans" color="secondary"> + {t("plans")} + </Button> +</CtaRow>
113-121: Localize section titleBring "Cancel subscription" under i18n for consistency.
-<CtaRow title="Cancel subscription" description={t("Cancel your existing platform subscription")}> +<CtaRow title={t("cancel_subscription")} description={t("cancel_subscription_platform_description")}>
📜 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 (3)
apps/web/modules/settings/platform/billing/billing-view.tsx(1 hunks)packages/app-store/stripepayment/api/portal.ts(3 hunks)packages/lib/server/repository/team.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.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/lib/server/repository/team.tspackages/app-store/stripepayment/api/portal.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/lib/server/repository/team.tspackages/app-store/stripepayment/api/portal.tsapps/web/modules/settings/platform/billing/billing-view.tsx
**/*.{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/lib/server/repository/team.tspackages/app-store/stripepayment/api/portal.tsapps/web/modules/settings/platform/billing/billing-view.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
apps/web/modules/settings/platform/billing/billing-view.tsx
🧬 Code graph analysis (2)
packages/app-store/stripepayment/api/portal.ts (1)
packages/app-store/stripepayment/lib/subscriptions.ts (1)
getSubscriptionFromId(31-33)
apps/web/modules/settings/platform/billing/billing-view.tsx (2)
apps/web/components/settings/platform/hooks/useGetUserAttributes.ts (1)
useGetUserAttributes(5-30)packages/lib/constants.ts (1)
WEBAPP_URL(12-18)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Install dependencies / Yarn install & cache
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
|
||
| if (!teamMetadataParsed.data?.subscriptionId) { | ||
| return res.status(400).json({ message: "subscriptionId not found for team" }); | ||
| if (team.isPlatform) { |
There was a problem hiding this comment.
@joeauyeung the reason I have two separate functions for fetching subscription id is because in web app the metadata is where all the billing details are stored. however in platform we have a dedicated PlatformBilling table where we store all the billing info hence two separate functions, in case in future if we wanna change some code logic its much this way
E2E results are ready! |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/app-store/stripepayment/lib/services/team/TeamBillingPortalService.ts (4)
22-30: Drop unnecessary async and add explicit return type.
These helpers are synchronous; remove async to reduce Promise churn.Apply:
- async getValidatedTeamSubscriptionId(metadata: Prisma.JsonValue) { + getValidatedTeamSubscriptionId(metadata: Prisma.JsonValue): string | null { const teamMetadataParsed = teamMetadataSchema.safeParse(metadata); if (!teamMetadataParsed.success || !teamMetadataParsed.data?.subscriptionId) { return null; } return teamMetadataParsed.data.subscriptionId; }
32-38: Same here: make it sync and simplify.Apply:
- async getValidatedTeamSubscriptionIdForPlatform(subscriptionId?: string | null) { - if (!subscriptionId) { - return null; - } - - return subscriptionId; - } + getValidatedTeamSubscriptionIdForPlatform(subscriptionId?: string | null): string | null { + return subscriptionId ?? null; + }
46-60: Avoid sentinel empty string; add logs and remove unnecessary awaits.
Use a typed variable, early returns with context logs, and call the now‑sync helpers.Apply:
- let teamSubscriptionId = ""; + let teamSubscriptionId: string; - if (team.isPlatform) { - const subscriptionId = await this.getValidatedTeamSubscriptionIdForPlatform( - team.platformBilling?.subscriptionId - ); - - if (!subscriptionId) return null; - teamSubscriptionId = subscriptionId; - } else { - const subscriptionId = await this.getValidatedTeamSubscriptionId(team.metadata); - - if (!subscriptionId) return null; - teamSubscriptionId = subscriptionId; - } + if (team.isPlatform) { + const subscriptionId = this.getValidatedTeamSubscriptionIdForPlatform( + team.platformBilling?.subscriptionId + ); + if (!subscriptionId) { + log.warn("Missing subscriptionId for platform team", { teamId }); + return null; + } + teamSubscriptionId = subscriptionId; + } else { + const subscriptionId = this.getValidatedTeamSubscriptionId(team.metadata); + if (!subscriptionId) { + log.warn("Missing subscriptionId in team metadata", { teamId }); + return null; + } + teamSubscriptionId = subscriptionId; + }
63-74: Handle Stripe customer being string or expanded object.
Prevent returning a non-string at runtime if customer is expanded.Apply:
- if (!subscription?.customer) { - log.warn("Subscription found but no customer ID", { - teamId, - teamSubscriptionId, - }); - return null; - } - - return subscription.customer as string; + const customer = subscription?.customer; + if (!customer) { + log.warn("Subscription found but no customer ID", { + teamId, + teamSubscriptionId, + }); + return null; + } + if (typeof customer === "string") { + return customer; + } + if (typeof customer === "object" && customer !== null && "id" in customer && typeof (customer as any).id === "string") { + return (customer as any).id; + } + log.warn("Customer present but not a string or object with id", { teamId, teamSubscriptionId }); + return null;
📜 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 (2)
packages/app-store/stripepayment/lib/services/team/TeamBillingPortalService.ts(2 hunks)packages/lib/server/repository/team.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/lib/server/repository/team.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*Service.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Service files must include
Servicesuffix, use PascalCase matching exported class, and avoid generic names (e.g.,MembershipService.ts)
Files:
packages/app-store/stripepayment/lib/services/team/TeamBillingPortalService.ts
**/*.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/app-store/stripepayment/lib/services/team/TeamBillingPortalService.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/app-store/stripepayment/lib/services/team/TeamBillingPortalService.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/app-store/stripepayment/lib/services/team/TeamBillingPortalService.ts
🧠 Learnings (1)
📚 Learning: 2025-09-09T03:29:43.025Z
Learnt from: emrysal
PR: calcom/cal.com#23692
File: packages/lib/server/service/InsightsBookingBaseService.ts:16-16
Timestamp: 2025-09-09T03:29:43.025Z
Learning: In the Cal.com codebase, readonlyPrisma is still an instance of PrismaClient, making type changes from `typeof readonlyPrisma` to `PrismaClient` less critical since they are fundamentally compatible types.
Applied to files:
packages/app-store/stripepayment/lib/services/team/TeamBillingPortalService.ts
🧬 Code graph analysis (1)
packages/app-store/stripepayment/lib/services/team/TeamBillingPortalService.ts (1)
packages/app-store/stripepayment/lib/subscriptions.ts (1)
getSubscriptionFromId(31-33)
🔇 Additional comments (2)
packages/app-store/stripepayment/lib/services/team/TeamBillingPortalService.ts (2)
2-2: Type-only Prisma import — good.
48-51: TeamRepository.findById already selects isPlatform and platformBilling.subscriptionId
teamSelect contains isPlatform and findById uses select: { ...teamSelect, platformBilling: true } (platformBilling:true includes subscriptionId).
joeauyeung
left a comment
joeauyeung
left a comment
There was a problem hiding this comment.
Overall LGTM. I just have a small nit
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/app-store/stripepayment/lib/services/team/TeamBillingPortalService.ts (2)
22-30: Consider simplifying the validation logic.The method correctly validates team metadata, but the logic can be streamlined for better readability.
Apply this diff to simplify the validation:
async getValidatedTeamSubscriptionId(metadata: Prisma.JsonValue) { const teamMetadataParsed = teamMetadataSchema.safeParse(metadata); - - if (!teamMetadataParsed.success || !teamMetadataParsed.data?.subscriptionId) { - return null; - } - - return teamMetadataParsed.data.subscriptionId; + return teamMetadataParsed.success ? teamMetadataParsed.data?.subscriptionId || null : null; }
32-38: Simplify platform subscription validation.The method logic is correct but overly verbose for a simple null check and passthrough.
Apply this diff to simplify the method:
async getValidatedTeamSubscriptionIdForPlatform(subscriptionId?: string | null) { - if (!subscriptionId) { - return null; - } - - return subscriptionId; + return subscriptionId || null; }
📜 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 (2)
packages/app-store/stripepayment/lib/services/team/TeamBillingPortalService.ts(2 hunks)packages/lib/server/repository/team.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/lib/server/repository/team.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*Service.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Service files must include
Servicesuffix, use PascalCase matching exported class, and avoid generic names (e.g.,MembershipService.ts)
Files:
packages/app-store/stripepayment/lib/services/team/TeamBillingPortalService.ts
**/*.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/app-store/stripepayment/lib/services/team/TeamBillingPortalService.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/app-store/stripepayment/lib/services/team/TeamBillingPortalService.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/app-store/stripepayment/lib/services/team/TeamBillingPortalService.ts
🧠 Learnings (1)
📚 Learning: 2025-09-09T03:29:43.025Z
Learnt from: emrysal
PR: calcom/cal.com#23692
File: packages/lib/server/service/InsightsBookingBaseService.ts:16-16
Timestamp: 2025-09-09T03:29:43.025Z
Learning: In the Cal.com codebase, readonlyPrisma is still an instance of PrismaClient, making type changes from `typeof readonlyPrisma` to `PrismaClient` less critical since they are fundamentally compatible types.
Applied to files:
packages/app-store/stripepayment/lib/services/team/TeamBillingPortalService.ts
🧬 Code graph analysis (1)
packages/app-store/stripepayment/lib/services/team/TeamBillingPortalService.ts (1)
packages/app-store/stripepayment/lib/subscriptions.ts (1)
getSubscriptionFromId(31-33)
🔇 Additional comments (4)
packages/app-store/stripepayment/lib/services/team/TeamBillingPortalService.ts (4)
2-2: LGTM! Proper type import added.The Prisma type import is correctly added to support the new validation method parameter types.
46-60: LGTM! Platform vs regular team branching logic implemented correctly.The conditional logic properly handles both platform and regular teams:
- Platform teams use
platformBilling.subscriptionId- Regular teams use metadata-based subscription ID
- Both paths validate the subscription ID appropriately before proceeding
The unified
teamSubscriptionIdvariable approach eliminates code duplication in subsequent logic.
63-68: LGTM! Consistent use of unified subscription ID variable.The code correctly uses the
teamSubscriptionIdvariable instead of directly accessing parsed metadata, maintaining consistency with the new branching logic.
43-43: Verified findByIdIncludePlatformBilling exists Confirmed thatpackages/lib/server/repository/team.tsdefinesfindByIdIncludePlatformBillingselectingplatformBilling, so no further changes are needed.
What does this PR do?
This PR fixes the platform billing portal functionality by properly handling billing portal URLs for platform teams vs regular teams.
Changes Made:
teamIdparameter for platform teamsisPlatform,platformBilling) to support platform-specific billing logicKey Fixes:
platformBilling.subscriptionIdinstead of team metadatateamIdparameter to the billing portal URLHow should this be tested?
Prerequisites:
Test Cases:
Platform Team Billing Portal:
Regular Team Billing Portal:
Expected Results:
Mandatory Tasks
Checklist