fix: Skip trial button visibility #23034
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Walkthrough
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes ✨ 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
Status, Documentation and Community
|
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (08/12/25)1 reviewer was added to this PR based on Keith Williams's automation. "Add ready-for-e2e label" took an action on this PR • (08/26/25)1 label was added to this PR based on Keith Williams's automation. |
![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
🔭 Outside diff range comments (1)
packages/trpc/server/routers/viewer/teams/hasActiveTeamPlan.handler.ts (1)
41-41: Applyselectpattern to platformBilling queryFollowing the coding guideline to only select necessary data in Prisma queries, limit the selection to only the
planfield.-const platformBilling = await prisma.platformBilling.findUnique({ where: { id: team.id } }); +const platformBilling = await prisma.platformBilling.findUnique({ + where: { id: team.id }, + select: { plan: true } +});
🧹 Nitpick comments (3)
packages/trpc/server/routers/viewer/teams/hasActiveTeamPlan.handler.ts (3)
1-1: Consider using more specific Prisma type importsInstead of importing the entire
Prismanamespace, consider importing only the specific type needed (MembershipWhereInput) for better tree-shaking and clarity.-import type { Prisma } from "@prisma/client"; +import type { MembershipWhereInput } from "@prisma/client";Then update line 20:
-const whereClause: Prisma.MembershipWhereInput = { userId: ctx.user.id, accepted: true }; +const whereClause: MembershipWhereInput = { userId: ctx.user.id, accepted: true };
33-33: Consider adding null safety for team mappingWhile the Prisma query should return teams, adding a defensive check ensures robustness in case of data inconsistencies.
-const teams = memberships.map((membership) => membership.team); +const teams = memberships.map((membership) => membership.team).filter(Boolean);
39-55: Consider extracting team billing logic into a separate functionThe nested loop with multiple return statements and complex billing logic could benefit from extraction into a separate function for better readability and testability.
Consider refactoring this logic into a helper function:
async function checkTeamBillingStatus(team: typeof teams[0]) { if (team.isPlatform && team.isOrganization) { const platformBilling = await prisma.platformBilling.findUnique({ where: { id: team.id }, select: { plan: true } }); if (platformBilling && platformBilling.plan !== "none" && platformBilling.plan !== "FREE") { return { isActive: true, isTrial: false }; } } const teamBillingService = new InternalTeamBilling(team); const subscriptionStatus = await teamBillingService.getSubscriptionStatus(); if (subscriptionStatus === "active" || subscriptionStatus === "past_due") { return { isActive: true, isTrial: false }; } if (subscriptionStatus === "trialing") { return { isActive: false, isTrial: true }; } return null; }Then simplify the main loop:
for (const team of teams) { const status = await checkTeamBillingStatus(team); if (status) { if (status.isActive) return status; isTrial = status.isTrial || isTrial; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/features/shell/useBottomNavItems.ts(2 hunks)packages/lib/hooks/useHasPaidPlan.ts(1 hunks)packages/trpc/server/routers/viewer/teams/_router.tsx(2 hunks)packages/trpc/server/routers/viewer/teams/hasActiveTeamPlan.handler.ts(1 hunks)packages/trpc/server/routers/viewer/teams/hasActiveTeamPlan.schema.ts(1 hunks)packages/trpc/server/routers/viewer/teams/skipTeamTrials.handler.ts(1 hunks)
🧰 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/lib/hooks/useHasPaidPlan.tspackages/trpc/server/routers/viewer/teams/skipTeamTrials.handler.tspackages/features/shell/useBottomNavItems.tspackages/trpc/server/routers/viewer/teams/hasActiveTeamPlan.handler.tspackages/trpc/server/routers/viewer/teams/hasActiveTeamPlan.schema.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/hooks/useHasPaidPlan.tspackages/trpc/server/routers/viewer/teams/skipTeamTrials.handler.tspackages/trpc/server/routers/viewer/teams/_router.tsxpackages/features/shell/useBottomNavItems.tspackages/trpc/server/routers/viewer/teams/hasActiveTeamPlan.handler.tspackages/trpc/server/routers/viewer/teams/hasActiveTeamPlan.schema.ts
**/*.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:
packages/trpc/server/routers/viewer/teams/_router.tsx
🧬 Code Graph Analysis (4)
packages/lib/hooks/useHasPaidPlan.ts (2)
packages/lib/constants.ts (1)
IS_SELF_HOSTED(59-59)apps/web/app/_trpc/trpc.ts (1)
trpc(7-7)
packages/trpc/server/routers/viewer/teams/_router.tsx (1)
packages/trpc/server/routers/viewer/teams/hasActiveTeamPlan.schema.ts (1)
ZHasActiveTeamPlanInputSchema(3-7)
packages/features/shell/useBottomNavItems.ts (1)
packages/lib/hooks/useHasPaidPlan.ts (1)
useHasActiveTeamPlanAsOwner(49-55)
packages/trpc/server/routers/viewer/teams/hasActiveTeamPlan.handler.ts (2)
packages/trpc/server/routers/viewer/teams/hasActiveTeamPlan.schema.ts (1)
THasActiveTeamPlanInputSchema(9-9)packages/lib/constants.ts (1)
IS_SELF_HOSTED(59-59)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (8)
packages/trpc/server/routers/viewer/teams/skipTeamTrials.handler.ts (1)
32-43: LGTM! Membership-based query aligns with ownership filtering pattern.The change from a nested Team query to a direct Membership query with explicit filtering is consistent with the PR's pattern of using membership-based ownership checks. The logic correctly filters for accepted OWNER memberships and maps to teams.
packages/trpc/server/routers/viewer/teams/hasActiveTeamPlan.schema.ts (1)
1-9: LGTM! Well-structured input schema with appropriate typing.The schema correctly defines an optional
ownerOnlyboolean parameter wrapped in an optional object, providing flexibility for the API consumers. The type inference and exports are properly structured.packages/features/shell/useBottomNavItems.ts (1)
23-23: LGTM! Trial detection now correctly uses owner-only filtering.The change ensures that the skip trial button only appears when the user owns teams in trial status, rather than being a member of any team in trial status. This addresses the core issue described in the PR.
packages/lib/hooks/useHasPaidPlan.ts (1)
49-55: LGTM! New hook correctly implements owner-only team plan checking.The new hook properly mirrors the existing
useHasActiveTeamPlanpattern while adding theownerOnly: trueparameter to the TRPC query. The self-hosted short-circuit and return structure are consistent with the existing hook.packages/trpc/server/routers/viewer/teams/_router.tsx (1)
16-16: LGTM! Input validation properly added to hasActiveTeamPlan route.The import of
ZHasActiveTeamPlanInputSchemaand its application to thehasActiveTeamPlanroute ensures proper validation of theownerOnlyparameter. The route structure remains consistent with other routes in the router.Also applies to: 191-194
packages/trpc/server/routers/viewer/teams/hasActiveTeamPlan.handler.ts (3)
26-31: Good use ofselectoverincludefor Prisma queryThe query correctly uses
selectto fetch only the necessary data (team), following the Prisma best practices outlined in the coding guidelines.
17-58: Overall implementation looks good with owner-only filteringThe handler successfully implements the owner-only filtering feature for team plan checking, aligning well with the PR objectives to fix "Skip trial button visibility". The membership-based approach is consistent with other handlers in the codebase and properly handles both platform and regular team billing scenarios.
22-24: No action needed:ownerOnlyis defined in the input schemaVerified that
ZHasActiveTeamPlanInputSchemainpackages/trpc/server/routers/viewer/teams/hasActiveTeamPlan.schema.tsdeclaresownerOnlyas an optional boolean. The handler’s check oninput?.ownerOnlyis valid.
…com/cal.com into skip_trial_button_visibility
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/trpc/server/routers/viewer/workflows/update.handler.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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/trpc/server/routers/viewer/workflows/update.handler.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/trpc/server/routers/viewer/workflows/update.handler.ts
🧬 Code Graph Analysis (1)
packages/trpc/server/routers/viewer/workflows/update.handler.ts (1)
packages/trpc/server/routers/viewer/teams/hasActiveTeamPlan.handler.ts (1)
hasActiveTeamPlanHandler(17-58)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Detect changes
joeauyeung
left a comment
joeauyeung
left a comment
There was a problem hiding this comment.
Since this is a non urgent fix and we're making changes to the prisma queries. Let's take the time and move the queries to a repository.
| }, | ||
| }); | ||
|
|
||
| const ownedTeams = memberships.map((membership) => membership.team); |
There was a problem hiding this comment.
NIT: Personally I'm not a huge fan of iterating over this array here and on line 45.
dosubot
bot
added
the
teams
E2E results are ready! |
What does this PR do?
Make sure to show the skip trial button to the correct team owner